forgejo_admin/basketball-api

Fork 1

Cleanup: interest leads endpoint QA nits from PR #187 #188

New issue

Closed

opened 2026-03-27 10:48:14 +00:00 by forgejo_admin · 1 comment

forgejo_admin commented

2026-03-27 10:48:14 +00:00

Owner

Type

Cleanup

Lineage

QA nits from PR #187 (POST /public/interest). Non-blocking, discovered scope.

Repo

forgejo_admin/basketball-api

User Story

As a developer maintaining the interest leads endpoint
I want input validation to be tight and consistent with existing patterns
So that edge cases (empty strings, whitespace, PII logging) don't cause data quality issues

Context

QA review of PR #187 flagged 5 non-blocking nits. All are validation tightening and consistency fixes — the endpoint works correctly but could be more defensive.

File Targets

Files the agent should modify:

src/basketball_api/models.py — add index=True on InterestLead.tenant_id
src/basketball_api/routes/public.py — add min_length, max_length, whitespace stripping on Pydantic fields; reduce PII logging level
tests/test_interest.py — add tests for min_length and whitespace stripping

Files the agent should NOT touch:

Alembic migration (index can be added in a new migration)
Any other routes or models

Acceptance Criteria

InterestLead.tenant_id has index=True + new Alembic migration for the index
player_name and age_grade have min_length=1 — empty strings return 422
Pydantic max_length matches DB column constraints (200, 100, 254, 30, 20)
parent_email not logged at INFO — use DEBUG or redact
player_name and age_grade stripped of leading/trailing whitespace (like parent_email already is)

Test Expectations

Existing 14 tests still pass
New test: empty player_name → 422
New test: empty age_grade → 422
New test: whitespace-only player_name → 422 or stripped
New test: leading/trailing whitespace stripped from player_name

Constraints

New Alembic migration for the tenant_id index (don't modify existing migration 025)
Keep changes minimal — this is cleanup, not a feature

Checklist

PR opened
Tests pass
No unrelated changes

Closes nits from PR #187
basketball-api#186 (parent issue)
project-westside-basketball

### Type Cleanup ### Lineage QA nits from PR #187 (POST /public/interest). Non-blocking, discovered scope. ### Repo `forgejo_admin/basketball-api` ### User Story As a developer maintaining the interest leads endpoint I want input validation to be tight and consistent with existing patterns So that edge cases (empty strings, whitespace, PII logging) don't cause data quality issues ### Context QA review of PR #187 flagged 5 non-blocking nits. All are validation tightening and consistency fixes — the endpoint works correctly but could be more defensive. ### File Targets Files the agent should modify: - `src/basketball_api/models.py` — add `index=True` on `InterestLead.tenant_id` - `src/basketball_api/routes/public.py` — add `min_length`, `max_length`, whitespace stripping on Pydantic fields; reduce PII logging level - `tests/test_interest.py` — add tests for min_length and whitespace stripping Files the agent should NOT touch: - Alembic migration (index can be added in a new migration) - Any other routes or models ### Acceptance Criteria 1. `InterestLead.tenant_id` has `index=True` + new Alembic migration for the index 2. `player_name` and `age_grade` have `min_length=1` — empty strings return 422 3. Pydantic `max_length` matches DB column constraints (200, 100, 254, 30, 20) 4. `parent_email` not logged at INFO — use DEBUG or redact 5. `player_name` and `age_grade` stripped of leading/trailing whitespace (like `parent_email` already is) ### Test Expectations - [ ] Existing 14 tests still pass - [ ] New test: empty `player_name` → 422 - [ ] New test: empty `age_grade` → 422 - [ ] New test: whitespace-only `player_name` → 422 or stripped - [ ] New test: leading/trailing whitespace stripped from `player_name` ### Constraints - New Alembic migration for the tenant_id index (don't modify existing migration 025) - Keep changes minimal — this is cleanup, not a feature ### Checklist - [ ] PR opened - [ ] Tests pass - [ ] No unrelated changes ### Related - Closes nits from PR #187 - `basketball-api#186` (parent issue) - `project-westside-basketball`

forgejo_admin referenced this issue from a commit

2026-03-27 21:07:19 +00:00

fix: interest endpoint validation nits (#188)

forgejo_admin referenced this issue from a pull request that will close it,

2026-03-27 21:08:22 +00:00

fix: interest endpoint validation nits (#188) #194

forgejo_admin referenced this issue

2026-03-27 21:09:08 +00:00

fix: interest endpoint validation nits (#188) #194

forgejo_admin referenced this issue

2026-03-27 21:11:22 +00:00

fix: interest endpoint validation nits (#188) #194

forgejo_admin

2026-03-27 21:11:23 +00:00

closed this issue
added the
status:approved
label

forgejo_admin referenced this issue from a commit

2026-03-27 21:13:10 +00:00

fix: interest endpoint validation nits (#188) (#194)

forgejo_admin commented

2026-03-27 21:32:24 +00:00

Author

Owner

Scope Review: READY (already completed)

Review note: review-462-2026-03-27

All 5 acceptance criteria verified against the current codebase — PR #194 merged successfully. Board item #462 should move from todo to done.

## Scope Review: READY (already completed) Review note: `review-462-2026-03-27` All 5 acceptance criteria verified against the current codebase — PR #194 merged successfully. Board item #462 should move from `todo` to `done`.