Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/api/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ def get_account_from_bearer(
if user is None:
raise credentials_exception

if not user.is_active:
raise HTTPException(status_code=400, detail="Inactive account")
if not user.is_active:
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="account deactivated")

return user

def get_active_account(account: Account = Depends(get_account_from_bearer)) -> Account:
if not account.is_active:
raise HTTPException(status_code=400, detail="Inactive account")
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="account deactivated")
return account

def get_account_even_if_inactive(
Expand Down
219 changes: 199 additions & 20 deletions src/api/roles/client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from sqlalchemy import func, desc, asc, delete

from src import config
from src.api.dependencies import get_account_from_bearer, get_client_account, PaginationParams
from src.api.dependencies import get_account_from_bearer, get_client_account, get_active_account, PaginationParams

#models
from src.api.roles.client.domain import (
Expand Down Expand Up @@ -39,8 +39,18 @@
from src.database.coach_client_relationship.models import ClientCoachRequest, ClientCoachRelationship
from src.database.account.models import Account, Availability, Notification
from src.database.client.models import Client, ClientAvailability, FitnessGoals
from src.database.telemetry.models import HealthMetrics, ClientTelemetry
from src.database.telemetry.models import ClientTelemetry
from src.database.telemetry.models import (
HealthMetrics,
ClientTelemetry,
StepCount,
DailyMoodSurvey,
DailyWorkoutSurvey,
DailyBodyMetricsSurvey,
DailyStepsSurvey,
DailyMealSurvey,
CompletedMealActivity,
CompletedWorkout,
)
from src.database.reports.models import CoachReport, CoachReviews
from src.database.payment.models import PaymentInformation, Invoice, BillingCycle, Subscription, PricingPlan

Expand Down Expand Up @@ -189,11 +199,21 @@ def create_coach_request(coach_id: int, db = Depends(get_session), acc: Account
client = db.get(Client, acc.client_id)
coach = db.get(Coach, coach_id)

if coach is None:
raise HTTPException(404, detail="Coach not found")

existing_request = db.query(ClientCoachRequest).filter_by(
client_id=client.id, coach_id=coach.id, is_accepted=None
if coach is None:
raise HTTPException(404, detail="Coach not found")

coach_account = db.exec(
select(Account).where(
Account.coach_id == coach.id,
Account.is_active == True,
)
).first()

if coach_account is None or not coach.verified:
raise HTTPException(404, detail="Coach not available")

existing_request = db.query(ClientCoachRequest).filter_by(
client_id=client.id, coach_id=coach.id, is_accepted=None
).first()

if existing_request:
Expand All @@ -207,10 +227,9 @@ def create_coach_request(coach_id: int, db = Depends(get_session), acc: Account
db.refresh(request)

# notify the coach's account that a new request was created
coach_account = db.exec(select(Account).where(Account.coach_id == coach.id)).first()
if coach_account and coach_account.id is not None:
n = Notification(
account_id=coach_account.id,
if coach_account and coach_account.id is not None:
n = Notification(
account_id=coach_account.id,
fav_category="relationship_request_creation",
message=f"{acc.name} has requested to hire you.",
details=f"Request {request.id} from client {client.id} to coach {coach.id}.",
Expand Down Expand Up @@ -515,12 +534,24 @@ def get_review(coach_id: int, db = Depends(get_session), acc: Account = Depends(
if acc.id is None:
raise HTTPException(404, detail="Account not found")

if acc.client_id is None:
raise HTTPException(403, detail="You are not authorized to view this content")

reviews = db.query(CoachReviews).filter(CoachReviews.coach_id == coach_id).all()

return ReviewsResponse(reviews=reviews)
if acc.client_id is None:
raise HTTPException(403, detail="You are not authorized to view this content")

coach_account = db.exec(
select(Account).where(
Account.coach_id == coach_id,
Account.is_active == True,
)
).first()

if coach_account is None:
return ReviewsResponse(reviews=[])

reviews = db.exec(
select(CoachReviews).where(CoachReviews.coach_id == coach_id)
).all()

return ReviewsResponse(reviews=reviews)

@router.get("/my_coach", response_model=MyCoachResponse)
def get_my_coach(db = Depends(get_session), acc: Account = Depends(get_client_account)):
Expand All @@ -533,8 +564,9 @@ def get_my_coach(db = Depends(get_session), acc: Account = Depends(get_client_ac

coach_request = db.query(ClientCoachRequest).filter(ClientCoachRequest.client_id == acc.client_id).first()

if not coach_request.is_accepted:
raise HTTPException(403, detail="You are not authorized to see this coach until the request is accepted")
# If no request found or the request hasn't been accepted, surface as not found.
if coach_request is None or not getattr(coach_request, "is_accepted", False):
raise HTTPException(404, detail="No active coach relationship found")

relationship = db.query(ClientCoachRelationship).filter(ClientCoachRelationship.request_id == coach_request.id).first()

Expand All @@ -557,3 +589,150 @@ def get_my_coach_requests(db = Depends(get_session), acc: Account = Depends(get_
requests = db.get(ClientCoachRequest).filter(ClientCoachRequest.client_id == acc.client_id).all()

return MyCoachRequestsResponse(requests = requests)

@router.get("/coach_profile/{coach_id}")
def get_coach_profile(coach_id: int, db = Depends(get_session), acc: Account = Depends(get_client_account)):
"""
Allows a client to view a coach's profile given their ID.
Returns account basics, specialties, certifications, experiences,
pricing/payment plan, availability, and rating summary.
"""

coach = db.get(Coach, coach_id)

if coach is None:
raise HTTPException(404, detail="Coach not found")

coach_account = db.exec(
select(Account).where(Account.coach_id == coach_id)
).first()

if coach_account is None:
raise HTTPException(404, detail="Coach account not found")

if not coach_account.is_active or not coach.verified:
raise HTTPException(404, detail="Coach not available")

certifications = db.exec(
select(Certifications)
.join(CoachCertifications, CoachCertifications.certification_id == Certifications.id)
.where(CoachCertifications.coach_id == coach_id)
).all()

experiences = db.exec(
select(Experience)
.join(CoachExperience, CoachExperience.experience_id == Experience.id)
.where(CoachExperience.coach_id == coach_id)
).all()

availability = db.exec(
select(Availability).where(
Availability.coach_availability_id == coach.coach_availability
)
).all()

pricing_plan = db.exec(
select(PricingPlan).where(PricingPlan.coach_id == coach_id)
).first()

rating_summary = db.exec(
select(
func.count(CoachReviews.id).label("rating_count"),
func.avg(CoachReviews.rating).label("avg_rating"),
).where(CoachReviews.coach_id == coach_id)
).first()

return {
"base_account": {
"id": coach_account.id,
"name": coach_account.name,
"email": coach_account.email,
"is_active": coach_account.is_active,
"gender": coach_account.gender,
"bio": coach_account.bio,
"age": coach_account.age,
"pfp_url": coach_account.pfp_url,
"client_id": coach_account.client_id,
"coach_id": coach_account.coach_id,
"admin_id": coach_account.admin_id,
"created_at": coach_account.created_at,
},
"coach_account": coach,
"specialties": coach.specialties,
"certifications": certifications,
"experiences": experiences,
"pricing_plan": pricing_plan,
"availability": availability,
"rating_summary": {
"rating_count": int(rating_summary.rating_count or 0),
"avg_rating": float(rating_summary.avg_rating) if rating_summary.avg_rating is not None else None,
},
}
Comment on lines +645 to +670
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Misleading coach_account field — returns a Coach, not an Account.

In the get_coach_profile response, "base_account" holds the Account row while "coach_account" is set to the Coach model instance. The field naming is reversed from what consumers will expect and will likely cause integration bugs (e.g., a frontend reading coach_account.email would get nothing because email lives on Account). Rename it (e.g., "coach" or "coach_details") and consider a response_model so the contract is enforced.

🛠️ Suggested rename
     return {
         "base_account": {
             ...
         },
-        "coach_account": coach,
+        "coach": coach,
         "specialties": coach.specialties,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/roles/client/client.py` around lines 645 - 670, The response dict in
get_coach_profile currently returns "base_account" (Account) and "coach_account"
(Coach) which is misleading; change the key "coach_account" to a clear name like
"coach" or "coach_details" in the return value from get_coach_profile (and
update any callers of client.get_coach_profile accordingly), and add or update a
response_model/schema for get_coach_profile so the contract enforces the
types/fields (Account vs Coach) to prevent consumers from expecting account
fields under the coach key.



@router.get("/progress_pictures")
def get_progress_pictures(db = Depends(get_session), acc: Account = Depends(get_client_account)):
"""
Queries progress picture URLs for the logged-in client.
Progress pictures are stored in HealthMetrics.progress_pic_url.
"""

if acc.client_id is None:
raise HTTPException(403, detail="Client profile required")

pictures = db.exec(
select(
ClientTelemetry.date,
HealthMetrics.progress_pic_url,
)
.join(HealthMetrics, HealthMetrics.client_telemetry_id == ClientTelemetry.id)
.where(
ClientTelemetry.client_id == acc.client_id,
HealthMetrics.progress_pic_url.is_not(None),
)
.order_by(ClientTelemetry.date.desc())
).all()

return [
{
"date": pic.date,
"progress_pic_url": pic.progress_pic_url,
}
for pic in pictures
]


@router.get("/my_coach")
def get_my_coach(db = Depends(get_session), acc: Account = Depends(get_client_account)):
"""
Returns the active coach relationship for the logged-in client.
"""

if acc.client_id is None:
raise HTTPException(403, detail="Client profile required")

result = db.exec(
select(ClientCoachRequest, ClientCoachRelationship)
.join(ClientCoachRelationship, ClientCoachRelationship.request_id == ClientCoachRequest.id)
.where(
ClientCoachRequest.client_id == acc.client_id,
ClientCoachRequest.is_accepted == True,
ClientCoachRelationship.is_active == True,
ClientCoachRelationship.client_blocked == False,
ClientCoachRelationship.coach_blocked == False,
)
).first()

if result is None:
raise HTTPException(404, detail="No active coach relationship found")

request, relationship = result

return {
"relationship_id": relationship.id,
"request_id": request.id,
"client_id": request.client_id,
"coach_id": request.coach_id,
"created_at": relationship.created_at,
"is_active": relationship.is_active,
}
Comment on lines +705 to +738
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Duplicate /my_coach route — new handler is unreachable.

@router.get("/my_coach") is already registered at line 556 (get_my_coach returning MyCoachResponse). FastAPI dispatches to the first-matched route, so this new handler at 705 is dead code and the PR's intended supersede behavior (relationship metadata, 403/404 semantics, active+non-blocked filtering) will never run. The two implementations also disagree on response shape (MyCoachResponse vs. raw dict) and on the missing-client-id semantics (404 vs. 403), which would silently drift between docs and behavior.

Per the PR summary, the older handler was meant to be removed. Delete lines 556–578 so this new one is the only registration (or merge them into one), and ideally add a response_model to keep the API contract documented.

🛠️ Suggested cleanup

Remove the previous get_my_coach (lines 556–578):

-@router.get("/my_coach", response_model=MyCoachResponse)
-def get_my_coach(db = Depends(get_session), acc: Account = Depends(get_client_account)):
-    """
-    Returns the coach of a specific client
-    """
-
-    if acc is None:
-        raise HTTPException(404, detail="Account not found")
-    
-    coach_request = db.query(ClientCoachRequest).filter(ClientCoachRequest.client_id == acc.client_id).first()
-
-    # If no request found or the request hasn't been accepted, surface as not found.
-    if coach_request is None or not getattr(coach_request, "is_accepted", False):
-        raise HTTPException(404, detail="No active coach relationship found")
-    
-    relationship = db.query(ClientCoachRelationship).filter(ClientCoachRelationship.request_id == coach_request.id).first()
-
-    if relationship is None:
-        raise HTTPException(404, detail="Relationship not Found")
-    
-    coach = db.query(Coach).filter(Coach.id == coach_request.coach_id).first()
-
-    return MyCoachResponse(coach = coach)
🧰 Tools
🪛 Ruff (0.15.12)

[warning] 706-706: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


[warning] 706-706: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


[error] 719-719: Avoid equality comparisons to True; use ClientCoachRequest.is_accepted: for truth checks

Replace with ClientCoachRequest.is_accepted

(E712)


[error] 720-720: Avoid equality comparisons to True; use ClientCoachRelationship.is_active: for truth checks

Replace with ClientCoachRelationship.is_active

(E712)


[error] 721-721: Avoid equality comparisons to False; use not ClientCoachRelationship.client_blocked: for false checks

Replace with not ClientCoachRelationship.client_blocked

(E712)


[error] 722-722: Avoid equality comparisons to False; use not ClientCoachRelationship.coach_blocked: for false checks

Replace with not ClientCoachRelationship.coach_blocked

(E712)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/roles/client/client.py` around lines 705 - 738, There is a duplicate
route registration for `@router.get`("/my_coach") — remove the older get_my_coach
implementation that returns MyCoachResponse so the new get_my_coach (the handler
that queries ClientCoachRequest and ClientCoachRelationship) becomes the sole
route; ensure the remaining handler declares a response_model (create or reuse
MyCoachResponse or a compatible Pydantic model) and keep the intended semantics:
require Account via Depends(get_client_account), return 403 when acc.client_id
is None, return 404 when no active non-blocked relationship is found, and return
the relationship payload with the fields currently produced by the new handler.

Loading