Skip to content
Merged
4 changes: 3 additions & 1 deletion src/api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ def refresh_payments(payload: dict = Body(...), db = Depends(get_session)):
if provided != expected:
raise HTTPException(status_code=403, detail="Invalid cron secret")

subs = db.exec(select(Subscription)).all()
subs = db.exec(
select(Subscription).where(Subscription.status == SubscriptionStatus.ACTIVE)
).all()

for s in subs:
# skip subscriptions without a pricing plan
Expand Down
44 changes: 35 additions & 9 deletions src/api/roles/shared/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from src.database.account.models import Account, Availability, Notification
from src.database.client.models import Client, FitnessGoals
from src.database.coach.models import Coach, Experience, Certifications, CoachExperience, CoachCertifications
from src.database.payment.models import PricingPlan, PaymentInformation, Subscription, BillingCycle, Invoice
from src.database.payment.models import PricingPlan, PaymentInformation, Subscription, BillingCycle, Invoice, SubscriptionStatus
from src.database.telemetry.models import (
HealthMetrics, ClientTelemetry, DailyProgressPicture,
CompletedMealActivity, CompletedWorkout,
Expand All @@ -32,7 +32,7 @@
from sqlalchemy import or_
from pydantic import BaseModel, EmailStr
from typing import Optional, List
from datetime import datetime
from datetime import date, datetime

router = APIRouter(prefix="/roles/shared/account", tags=["shared", "account"])

Expand Down Expand Up @@ -421,14 +421,8 @@ def notify_affected_accounts(
"""
Creates notification records for accounts affected by a user's deactivation.
"""
role = "account"
if deactivated_account.client_id is not None:
role = "client"
elif deactivated_account.coach_id is not None:
role = "coach"

message = f"{deactivated_account.name} has deactivated their account."
details = "Shared plans or schedules involving this user may be affected."
details = "Subscription canceled"

Comment on lines 424 to 426

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

Critical: notification details does not match the deactivation messaging contract — failing CI.

The fixed string "Subscription canceled" doesn't contain "future payments" or "stopped", but test_account_deactivate_sends_notification (line 157) and test_account_deactivate_coach_notifies_client (line 295) both assert that details includes both substrings (case-insensitive). This is the root cause of the pr-run-tests-prerequisite / test pipeline failures. The intent (per the AI summary and tests) is for affected users to be informed that future payments tied to the relationship have been stopped, which the current string fails to convey.

🐛 Proposed fix to align details with the messaging contract
-    details = "Subscription canceled"
+    details = (
+        "Any future payments tied to this coaching relationship have been stopped."
+    )

This satisfies both "future payments" and "stopped" token assertions and reads naturally to the affected client/coach.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
message = f"{deactivated_account.name} has deactivated their account."
details = f"{role.capitalize()} account {deactivated_account.id} was deactivated."
details = "Subscription canceled"
message = f"{deactivated_account.name} has deactivated their account."
details = (
"Any future payments tied to this coaching relationship have been stopped."
)
🤖 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/shared/account.py` around lines 364 - 366, The notification
`details` string currently set to "Subscription canceled" must be updated to
include both the substrings "future payments" and "stopped" (case-insensitive)
to satisfy the messaging contract and tests; locate where `message` and
`details` are set (the variables `message` and `details` in
src/api/roles/shared/account.py) and replace the fixed details text with a
phrase such as "Future payments tied to this relationship have been stopped." so
that the `test_account_deactivate_sends_notification` and
`test_account_deactivate_coach_notifies_client` assertions pass.

for affected_account in affected_accounts:
if affected_account.id is None:
Expand All @@ -445,6 +439,34 @@ def notify_affected_accounts(
)


def cancel_payments_for_request(db: Session, request: ClientCoachRequest):
subscriptions = db.exec(
select(Subscription)
.join(PricingPlan, Subscription.pricing_plan_id == PricingPlan.id)
.where(
Subscription.client_id == request.client_id,
PricingPlan.coach_id == request.coach_id,
Subscription.status == SubscriptionStatus.ACTIVE,
)
).all()

for subscription in subscriptions:
subscription.status = SubscriptionStatus.CANCELED
subscription.canceled_at = date.today()
db.add(subscription)

active_cycles = db.exec(
select(BillingCycle).where(
BillingCycle.subscription_id == subscription.id,
BillingCycle.active == True,
)
).all()

for cycle in active_cycles:
cycle.active = False
db.add(cycle)


def delete_client_coach_mappings(db: Session, account: Account):
if account.client_id is not None:
requests = db.exec(
Expand All @@ -453,6 +475,8 @@ def delete_client_coach_mappings(db: Session, account: Account):
).all()

for request in requests:
cancel_payments_for_request(db, request)

relationships = db.exec(
select(ClientCoachRelationship)
.where(ClientCoachRelationship.request_id == request.id)
Expand All @@ -470,6 +494,8 @@ def delete_client_coach_mappings(db: Session, account: Account):
).all()

for request in requests:
cancel_payments_for_request(db, request)

relationships = db.exec(
select(ClientCoachRelationship)
.where(ClientCoachRelationship.request_id == request.id)
Expand Down
2 changes: 1 addition & 1 deletion src/database/account/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,4 @@ class Notification(SQLModelLU, table=True):
message: str
details: Optional[str] = None # if they do expandable dialogs we have it built in
is_read: bool = False
created_at: date = Field(default_factory=date.today)
created_at: date = Field(default_factory=date.today)
147 changes: 144 additions & 3 deletions tests/test_shared_account_notifications.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,48 @@
from sqlmodel import select
from datetime import datetime
import os
from src.api.dependencies import create_jwt_token
from src.database.account.models import Notification, Account
from src.database.client.models import Client
from src.database.coach.models import Coach
from src.database.coach_client_relationship.models import (
ClientCoachRequest,
ClientCoachRelationship,
)
from src.database.payment.models import (
BillingCycle,
PricingInterval,
PricingPlan,
Subscription,
SubscriptionStatus,
)


def create_client_coach_relationship(db_session):
def create_client_coach_relationship(db_session, with_subscription=False):
client = db_session.exec(
select(Account).where(
Account.client_id.is_not(None),
Account.is_active == True,
)
).first()

if client is None:
client_profile = Client()
db_session.add(client_profile)
db_session.commit()
db_session.refresh(client_profile)

client = Account(
name="Notification Test Client",
email=f"notification_client_{client_profile.id}@example.com",
hashed_password="test-hash",
client_id=client_profile.id,
is_active=True,
)
db_session.add(client)
db_session.commit()
db_session.refresh(client)

assert client is not None

coach = db_session.exec(
Expand Down Expand Up @@ -64,14 +90,41 @@ def create_client_coach_relationship(db_session):
db_session.add(relationship)
db_session.commit()

if with_subscription:
pricing_plan = PricingPlan(
coach_id=coach.coach_id,
payment_interval=PricingInterval.MONTHLY,
price_cents=3000,
)
db_session.add(pricing_plan)
db_session.commit()
db_session.refresh(pricing_plan)

subscription = Subscription(
client_id=client.client_id,
pricing_plan_id=pricing_plan.id,
)
db_session.add(subscription)
db_session.commit()
db_session.refresh(subscription)

billing_cycle = BillingCycle(
active=True,
entry_date=datetime.utcnow().date(),
end_date=datetime.utcnow().date(),
subscription_id=subscription.id,
pricing_plan_id=pricing_plan.id,
)
db_session.add(billing_cycle)
db_session.commit()

return client, coach, request, relationship


def test_account_deactivate_sends_notification(
test_client,
db_session,
client_auth_header,
coach_auth_header,
):
client, coach, request, relationship = create_client_coach_relationship(db_session)

Expand Down Expand Up @@ -116,11 +169,99 @@ def test_account_deactivate_sends_notification(
assert db_session.get(ClientCoachRequest, request.id) is None


def test_account_deactivate_cancels_future_payments_for_relationship(
test_client,
db_session,
client_auth_header,
):
client, coach, request, relationship = create_client_coach_relationship(
db_session,
with_subscription=True,
)

client_auth_header = {
"Authorization": f"Bearer {create_jwt_token(client)}"
}

resp = test_client.post(
"/roles/shared/account/deactivate",
headers=client_auth_header,
)

assert resp.status_code == 200, resp.text

subscriptions = db_session.exec(
select(Subscription)
.join(PricingPlan, Subscription.pricing_plan_id == PricingPlan.id)
.where(
Subscription.client_id == client.client_id,
PricingPlan.coach_id == coach.coach_id,
)
).all()
active_cycles = db_session.exec(
select(BillingCycle)
.join(Subscription, BillingCycle.subscription_id == Subscription.id)
.join(PricingPlan, Subscription.pricing_plan_id == PricingPlan.id)
.where(
Subscription.client_id == client.client_id,
PricingPlan.coach_id == coach.coach_id,
BillingCycle.active == True,
)
).all()

assert subscriptions
assert all(subscription.status == SubscriptionStatus.CANCELED for subscription in subscriptions)
assert all(subscription.canceled_at is not None for subscription in subscriptions)
assert active_cycles == []


def test_refresh_payments_skips_canceled_subscription(
test_client,
db_session,
client_auth_header,
):
client, coach, request, relationship = create_client_coach_relationship(
db_session,
with_subscription=True,
)

subscriptions = db_session.exec(
select(Subscription)
.join(PricingPlan, Subscription.pricing_plan_id == PricingPlan.id)
.where(
Subscription.client_id == client.client_id,
PricingPlan.coach_id == coach.coach_id,
)
).all()
assert len(subscriptions) == 1

subscription = subscriptions[0]
subscription.status = SubscriptionStatus.CANCELED
db_session.add(subscription)
db_session.commit()

cycles_before = db_session.exec(
select(BillingCycle).where(BillingCycle.subscription_id == subscription.id)
).all()

os.environ["CRON_SECRET"] = "test-cron-secret"
resp = test_client.post(
"/refresh_payments",
json={"cron_secret": "test-cron-secret"},
)

cycles_after = db_session.exec(
select(BillingCycle).where(BillingCycle.subscription_id == subscription.id)
).all()

assert resp.status_code == 200, resp.text
assert len(cycles_after) == len(cycles_before)


def test_account_deactivate_coach_notifies_client(
test_client,
db_session,
client_auth_header,
coach_auth_header,
):
client, coach, request, relationship = create_client_coach_relationship(db_session)

Expand Down
Loading