cal/paper-dynasty-database
1
0
Fork 0
Code Issues 7 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

feat: add scout opportunities and claims API (#44) #59

Merged
cal merged 2 commits from feat/scout-opportunities-claims into next-release 2026-03-05 03:45:58 +00:00
Conversation 1 Commits 2 Files Changed 5 +310
cal commented 2026-03-05 03:41:40 +00:00
Owner
Copy Link

Summary

  • Add scout_opportunity and scout_claim Peewee models with BigIntegerField timestamps and JSON card_ids storage
  • Add POST/GET/DELETE /api/v2/scout_opportunities — supports claimed (bool), expired_before (ms timestamp), and opener_team_id query filters
  • Add POST/GET/DELETE /api/v2/scout_claims — supports scout_opportunity_id and claimed_by_team_id query filters
  • Add unique constraint on (scout_opportunity_id, claimed_by_team_id) to prevent duplicate claims at DB level
  • Add PostgreSQL migration script with expires_at index for efficient cleanup queries
  • Migration already applied to dev database (paperdynasty_dev)

Closes #44

Test plan

  • Verify POST /api/v2/scout_opportunities creates a record and returns it with id
  • Verify GET /api/v2/scout_opportunities?claimed=false&expired_before=<ts> returns unclaimed expired opportunities
  • Verify POST /api/v2/scout_claims creates a claim and returns it with id
  • Verify duplicate claim (same team + opportunity) is rejected by unique constraint
  • Verify card_ids is returned as a JSON array (not a string) in GET responses
  • Deploy to dev container and test with Discord bot end-to-end

🤖 Generated with Claude Code

## Summary - Add `scout_opportunity` and `scout_claim` Peewee models with BigIntegerField timestamps and JSON card_ids storage - Add `POST/GET/DELETE /api/v2/scout_opportunities` — supports `claimed` (bool), `expired_before` (ms timestamp), and `opener_team_id` query filters - Add `POST/GET/DELETE /api/v2/scout_claims` — supports `scout_opportunity_id` and `claimed_by_team_id` query filters - Add unique constraint on `(scout_opportunity_id, claimed_by_team_id)` to prevent duplicate claims at DB level - Add PostgreSQL migration script with expires_at index for efficient cleanup queries - Migration already applied to dev database (`paperdynasty_dev`) Closes #44 ## Test plan - [ ] Verify `POST /api/v2/scout_opportunities` creates a record and returns it with `id` - [ ] Verify `GET /api/v2/scout_opportunities?claimed=false&expired_before=<ts>` returns unclaimed expired opportunities - [ ] Verify `POST /api/v2/scout_claims` creates a claim and returns it with `id` - [ ] Verify duplicate claim (same team + opportunity) is rejected by unique constraint - [ ] Verify `card_ids` is returned as a JSON array (not a string) in GET responses - [ ] Deploy to dev container and test with Discord bot end-to-end 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 2 commits 2026-03-05 03:41:41 +00:00 
Support the Discord bot's new scouting feature where players can scout
cards from other teams' opened packs. Stores opportunities with expiry
timestamps and tracks which teams claim which cards.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Creates scout_opportunity and scout_claim tables with foreign keys,
unique constraint on (opportunity, team), and expires_at index.
Already applied to dev database.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-05 03:45:31 +00:00
cal force-pushed feat/scout-opportunities-claims from 8f4e539b97 to 37439626ed 2026-03-05 03:45:41 +00:00 Compare
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-05 03:45:53 +00:00
cal merged commit 8227b57875 into next-release 2026-03-05 03:45:58 +00:00
cal deleted branch feat/scout-opportunities-claims 2026-03-05 03:45:59 +00:00
cal commented 2026-03-05 03:47:33 +00:00
Author
Owner
Copy Link

AI Code Review — REQUEST_CHANGES

Files Reviewed

  • app/db_engine.py (modified — new models)
  • app/main.py (modified — router registration)
  • app/routers_v2/scout_claims.py (added)
  • app/routers_v2/scout_opportunities.py (added)
  • migrations/2026-03-04_add_scout_opportunities_claims.sql (added)

Findings

🔴 Security — BLOCKING

[1] Bearer token logged in plaintext — both new routers repeat the exact pattern fixed by PRs #7 and #8:

logging.warning(f"Bad Token: {token}")

Appears in:

  • scout_claims.py — post_scout_claim and delete_scout_claim
  • scout_opportunities.py — post_scout_opportunity and delete_scout_opportunity

Remove {token} from the log message entirely. Just use a generic warning with no token value.

🔴 Correctness — BLOCKING

[2] raise HTTPException(status_code=200, ...) in delete handlers — PR #47 was merged specifically to fix this pattern. Both new delete handlers reintroduce it:

  • scout_claims.py: raise HTTPException(status_code=200, detail=f"Scout claim {claim_id} has been deleted")
  • scout_opportunities.py: same pattern

Replace with return {"message": f"Scout claim {claim_id} has been deleted"} (and same for opportunities).

[3] except Exception instead of except peewee.DoesNotExist — PR #48 was merged to fix broad exception catching across all routers. The new files reintroduce it in four places:

  • scout_claims.py — get_one_scout_claim (line ~41) and delete_scout_claim (line ~65)
  • scout_opportunities.py — get_one_scout_opportunity (line ~56) and delete_scout_opportunity (line ~96)

Replace with except peewee.DoesNotExist:.

🟡 Style & Conventions

[4] Unused import in scout_claims.py: ScoutOpportunity is imported from db_engine but never used in that module. Remove it.

[5] ScoutOpportunity.opener_team FK missing on_delete in Peewee model — the SQL migration correctly has ON DELETE CASCADE, but the model definition omits it:

opener_team = ForeignKeyField(Team)  # should be on_delete='CASCADE'

The Peewee model and migration are inconsistent; SQLite dev tables won't enforce the cascade.

Suggestions (non-blocking)

  • card_ids = CharField() defaults to max_length=255. For 26 cards with 5-digit IDs this is borderline (~185 chars). Consider TextField() / TEXT to avoid future truncation surprises.
  • claimed=False uses NOT IN (subquery). Fine for current scale, but a LEFT JOIN or NOT EXISTS approach would be more efficient on large tables.

Verdict: REQUEST_CHANGES

The domain model and API design are solid. However, three patterns explicitly fixed in recent merged PRs (#7, #8, #47, #48) are reintroduced here: plaintext token logging (security), raise HTTPException(status_code=200) (correctness), and except Exception (exception specificity). Fix these before merging.

Automated review by Claude PR Reviewer

## AI Code Review — REQUEST_CHANGES ### Files Reviewed - `app/db_engine.py` (modified — new models) - `app/main.py` (modified — router registration) - `app/routers_v2/scout_claims.py` (added) - `app/routers_v2/scout_opportunities.py` (added) - `migrations/2026-03-04_add_scout_opportunities_claims.sql` (added) --- ### Findings #### 🔴 Security — BLOCKING **[1] Bearer token logged in plaintext** — both new routers repeat the exact pattern fixed by PRs #7 and #8: ```python logging.warning(f"Bad Token: {token}") ``` Appears in: - `scout_claims.py` — `post_scout_claim` and `delete_scout_claim` - `scout_opportunities.py` — `post_scout_opportunity` and `delete_scout_opportunity` Remove `{token}` from the log message entirely. Just use a generic warning with no token value. --- #### 🔴 Correctness — BLOCKING **[2] `raise HTTPException(status_code=200, ...)` in delete handlers** — PR #47 was merged specifically to fix this pattern. Both new delete handlers reintroduce it: - `scout_claims.py`: `raise HTTPException(status_code=200, detail=f"Scout claim {claim_id} has been deleted")` - `scout_opportunities.py`: same pattern Replace with `return {"message": f"Scout claim {claim_id} has been deleted"}` (and same for opportunities). **[3] `except Exception` instead of `except peewee.DoesNotExist`** — PR #48 was merged to fix broad exception catching across all routers. The new files reintroduce it in four places: - `scout_claims.py` — `get_one_scout_claim` (line ~41) and `delete_scout_claim` (line ~65) - `scout_opportunities.py` — `get_one_scout_opportunity` (line ~56) and `delete_scout_opportunity` (line ~96) Replace with `except peewee.DoesNotExist:`. --- #### 🟡 Style & Conventions **[4] Unused import in `scout_claims.py`**: `ScoutOpportunity` is imported from `db_engine` but never used in that module. Remove it. **[5] `ScoutOpportunity.opener_team` FK missing `on_delete` in Peewee model** — the SQL migration correctly has `ON DELETE CASCADE`, but the model definition omits it: ```python opener_team = ForeignKeyField(Team) # should be on_delete='CASCADE' ``` The Peewee model and migration are inconsistent; SQLite dev tables won't enforce the cascade. --- #### Suggestions (non-blocking) - `card_ids = CharField()` defaults to max_length=255. For 26 cards with 5-digit IDs this is borderline (~185 chars). Consider `TextField()` / `TEXT` to avoid future truncation surprises. - `claimed=False` uses `NOT IN (subquery)`. Fine for current scale, but a LEFT JOIN or `NOT EXISTS` approach would be more efficient on large tables. --- ### Verdict: REQUEST_CHANGES The domain model and API design are solid. However, three patterns explicitly fixed in recent merged PRs (#7, #8, #47, #48) are reintroduced here: plaintext token logging (security), `raise HTTPException(status_code=200)` (correctness), and `except Exception` (exception specificity). Fix these before merging. --- *Automated review by Claude PR Reviewer*
cal added the
ai-changes-requested
 label 2026-03-05 03:47:43 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working

   
bug

   
enhancement

   
evolution

Card Evolution system

  
performance

   
phase-0

Render Pipeline Optimization

  
phase-1a

Schema & Data Foundation

  
phase-1b

API & Formula Engine

  
phase-1c

Bot Integration

  
phase-1d

Deployment

  
security

   
tech-debt

   
todo
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-database#59
No description provided.
Delete Branch "feat/scout-opportunities-claims"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?