store: PR review: major-domo-v2#61 — dynamic roster type detection in /trade add-player
This commit is contained in:
parent
0735caa537
commit
7be08f281d
@ -0,0 +1,37 @@
|
||||
---
|
||||
id: 4cb03629-1d2d-467f-90af-01dc405abcd1
|
||||
type: workflow
|
||||
title: "PR review: major-domo-v2#61 — dynamic roster type detection in /trade add-player"
|
||||
tags: [pr-reviewer, major-domo, discord-bot, python, trade, roster-type, approved]
|
||||
importance: 0.4
|
||||
confidence: 0.8
|
||||
created: "2026-03-03T06:37:31.339038+00:00"
|
||||
updated: "2026-03-03T06:37:31.339038+00:00"
|
||||
---
|
||||
|
||||
## PR Review: major-domo-v2 #61
|
||||
|
||||
**Verdict**: APPROVED (could not post — Gitea blocks self-review on cal's own PRs)
|
||||
|
||||
**Branch**: `ai/major-domo-v2-29` → `main`
|
||||
|
||||
**Files reviewed**: `commands/transactions/trade.py`
|
||||
|
||||
## Key Findings
|
||||
|
||||
### Correctness
|
||||
- Fix replaces hardcoded `RosterType.MAJOR_LEAGUE` defaults with dynamic detection.
|
||||
- `from_roster`: three-way check — `player.team` object → fetch via `team_service.get_team(player.team_id)` → free agent fallback to MAJOR_LEAGUE. Correct per Player model structure.
|
||||
- `to_roster`: delegates to `dest_team.roster_type()` — safe since dest_team is a full Team object.
|
||||
- `team_service` imported at module level (not lazy mid-file), per CLAUDE.md.
|
||||
|
||||
### No Issues
|
||||
- Security: no new input paths.
|
||||
- Style: all other diff changes are Black/ruff formatting only.
|
||||
|
||||
### Minor Notes
|
||||
- No new tests for command-layer `from_roster` detection logic. Service layer tests cover `add_player_move()` with explicit roster types. Acceptable.
|
||||
- Silent fallback to MAJOR_LEAGUE when `team_service.get_team()` returns None — acceptable since service layer validates.
|
||||
|
||||
## Gitea Self-Review Limitation
|
||||
Gitea prevents a user from approving their own PR. The review body was prepared but could not be posted. This is expected when the issue-worker agent creates PRs under the `cal` account.
|
||||
Loading…
Reference in New Issue
Block a user