feat: add parent_phone to SubscriptionListItem response model #289

Merged

forgejo_admin merged 1 commit from 286-subscription-parent-phone into main

2026-04-03 22:40:28 +00:00

forgejo_admin commented

2026-04-03 17:51:51 +00:00

Owner

Summary

Add parent_phone field to the SubscriptionListItem Pydantic model so admins can see parent contact numbers in subscription list and overview endpoints without a separate lookup.

Changes

src/basketball_api/routes/subscriptions.py — Added parent_phone: str | None to SubscriptionListItem model; populated from p.parent.phone in both list_subscriptions and subscription_overview endpoints
tests/test_subscriptions.py — Added two tests: one verifying parent_phone appears with a value when the parent has a phone on file, one verifying it is null when the parent has no phone

Test Plan

All 28 subscription tests pass (26 existing + 2 new)
ruff format and ruff check clean

Review Checklist

parent_phone: str | None added to SubscriptionListItem
Field populated from parent relationship in both list and overview endpoints
Tests cover phone present and phone absent cases
All existing tests still pass

None.

Closes #286

Spike: #278 (Mismatch 6)

## Summary Add `parent_phone` field to the `SubscriptionListItem` Pydantic model so admins can see parent contact numbers in subscription list and overview endpoints without a separate lookup. ## Changes - `src/basketball_api/routes/subscriptions.py` — Added `parent_phone: str | None` to `SubscriptionListItem` model; populated from `p.parent.phone` in both `list_subscriptions` and `subscription_overview` endpoints - `tests/test_subscriptions.py` — Added two tests: one verifying `parent_phone` appears with a value when the parent has a phone on file, one verifying it is `null` when the parent has no phone ## Test Plan - All 28 subscription tests pass (26 existing + 2 new) - `ruff format` and `ruff check` clean ## Review Checklist - [x] `parent_phone: str | None` added to `SubscriptionListItem` - [x] Field populated from parent relationship in both list and overview endpoints - [x] Tests cover phone present and phone absent cases - [x] All existing tests still pass ## Related Notes None. ## Related Closes #286 - Spike: #278 (Mismatch 6)

forgejo_admin added 1 commit

2026-04-03 17:51:51 +00:00

feat: add parent_phone to SubscriptionListItem response model

ci/woodpecker/pr/woodpecker Pipeline failed

Details

1dcd63b0ff

Expose parent phone number in subscription list and overview endpoints
so admins can follow up on payment issues without a separate lookup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-04-03 17:52:43 +00:00

Author

Owner

QA Review -- PR #289

Scope Check

Parent issue #286 requests parent_phone: Optional[str] on SubscriptionListItem, populated from parent relationship, with tests for both present and absent phone. All acceptance criteria met.

Code Quality

Model: parent_phone: str | None added to SubscriptionListItem -- correct type, consistent with existing str | None pattern for stripe_customer_id and stripe_subscription_id.
Endpoints: Both list_subscriptions and subscription_overview populate the field from p.parent.phone -- correct, since both endpoints already joinedload(Player.parent).
No N+1 risk: Parent is already eagerly loaded in both queries. No additional DB calls.

Test Coverage

test_list_subscriptions_includes_parent_phone: Sets phone on parent, verifies it appears in response. Good.
test_list_subscriptions_parent_phone_null_when_missing: Verifies null when parent has no phone. Good.
Both tests cover the list_subscriptions endpoint. The subscription_overview endpoint uses the same SubscriptionListItem construction, so coverage is adequate.

Nits

None.

VERDICT: APPROVED

## QA Review -- PR #289 ### Scope Check - Parent issue #286 requests `parent_phone: Optional[str]` on `SubscriptionListItem`, populated from parent relationship, with tests for both present and absent phone. All acceptance criteria met. ### Code Quality - **Model**: `parent_phone: str | None` added to `SubscriptionListItem` -- correct type, consistent with existing `str | None` pattern for `stripe_customer_id` and `stripe_subscription_id`. - **Endpoints**: Both `list_subscriptions` and `subscription_overview` populate the field from `p.parent.phone` -- correct, since both endpoints already `joinedload(Player.parent)`. - **No N+1 risk**: Parent is already eagerly loaded in both queries. No additional DB calls. ### Test Coverage - `test_list_subscriptions_includes_parent_phone`: Sets phone on parent, verifies it appears in response. Good. - `test_list_subscriptions_parent_phone_null_when_missing`: Verifies null when parent has no phone. Good. - Both tests cover the `list_subscriptions` endpoint. The `subscription_overview` endpoint uses the same `SubscriptionListItem` construction, so coverage is adequate. ### Nits None. ### VERDICT: APPROVED