cal/major-domo-v2
1
0
Fork 0
Code Issues 6 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: prevent partial DB writes on scorecard submission failure #79

Closed
cal wants to merge 12 commits from fix/scorecard-submission-resilience into next-release
pull from: fix/scorecard-submission-resilience
merge into: cal:next-release
Conversation 0 Commits 12 Files Changed 13 +1006 -922
cal commented 2026-03-11 01:54:11 +00:00
Owner
Copy Link

Summary

  • Read all spreadsheet data (plays, box score, pitching decisions) before any database writes, preventing partial commits when the spreadsheet has formula errors (e.g. #N/A)
  • Preserve detailed SheetsException messages through the error chain instead of wrapping them in generic "Unable to read pitching decisions"
  • Add dedicated SheetsException handler that shows users the specific cell and error type instead of "An unexpected error occurred"

Context

On 2026-03-10, a CLS vs MKE scorecard submission failed because the Pitcherstats tab had a #N/A formula error in team_id (Row 4). The old code had already committed 74 plays and updated the game score before attempting to read pitching decisions — leaving the DB in a partial state. The user saw only: "An unexpected error occurred. Please try again or contact an admin."

What changed

  • commands/league/submit_scorecard.py: Moved all three spreadsheet reads into a single Phase 6 before any DB writes (Phase 7-9). Added except SheetsException handler with user-friendly detailed message.
  • services/sheets_service.py: Added except SheetsException: raise in read_pitching_decisions and read_box_score so detailed error messages aren't swallowed by the generic wrapper.

Test plan

  • 930 tests pass, 3 skipped
  • Submit a scorecard with a #N/A formula error — verify no data written to DB and user sees detailed error
  • Submit a valid scorecard — verify normal flow still works end-to-end

Closes #78

🤖 Generated with Claude Code

## Summary - Read all spreadsheet data (plays, box score, pitching decisions) **before** any database writes, preventing partial commits when the spreadsheet has formula errors (e.g. `#N/A`) - Preserve detailed `SheetsException` messages through the error chain instead of wrapping them in generic "Unable to read pitching decisions" - Add dedicated `SheetsException` handler that shows users the specific cell and error type instead of "An unexpected error occurred" ## Context On 2026-03-10, a CLS vs MKE scorecard submission failed because the Pitcherstats tab had a `#N/A` formula error in `team_id` (Row 4). The old code had already committed 74 plays and updated the game score before attempting to read pitching decisions — leaving the DB in a partial state. The user saw only: "An unexpected error occurred. Please try again or contact an admin." ## What changed - **`commands/league/submit_scorecard.py`**: Moved all three spreadsheet reads into a single Phase 6 before any DB writes (Phase 7-9). Added `except SheetsException` handler with user-friendly detailed message. - **`services/sheets_service.py`**: Added `except SheetsException: raise` in `read_pitching_decisions` and `read_box_score` so detailed error messages aren't swallowed by the generic wrapper. ## Test plan - [x] 930 tests pass, 3 skipped - [ ] Submit a scorecard with a `#N/A` formula error — verify no data written to DB and user sees detailed error - [ ] Submit a valid scorecard — verify normal flow still works end-to-end Closes #78 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 12 commits 2026-03-11 01:54:12 +00:00
fix: replace create_item_in_table placeholder with direct endpoint call (#30)
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m10s
Details
18ae65a8e2 
Remove the generic placeholder method from BaseService and replace the
single call site in CustomCommandsService.get_or_create_creator with a
direct client.post("custom_commands/creators", ...) call, consistent
with how _update_creator_stats and _update_creator_info already work.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewed-on: #69
Quick Status previously only showed "X errors found" with no details.
Now lists each error and suggestion inline. Also stripped all emoji
from embed titles, field names, values, buttons, and messages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix: trade validation now checks against next week's projected roster
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m5s
Details
58fe9f22de 
validate_trade() was passing next_week=None to each team's
validate_transaction(), which skipped load_existing_transactions()
entirely. Trades were validated against the current roster only,
ignoring pending /dropadd transactions for next week.

Now auto-fetches current week from league_service and passes
next_week=current_week+1, matching /dropadd validation behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewed-on: #70
fix: add trailing slashes to API URLs to prevent 307 redirects dropping POST bodies
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m13s
Details
9379ba587a 
The FastAPI server returns 307 redirects for URLs without trailing slashes.
aiohttp follows these redirects but converts POST to GET, silently dropping
the request body. This caused play-by-play and decision data from
/submit-scorecard to never be persisted to the database despite the API
returning success.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewed-on: #73
fix: use targeted trailing slashes instead of universal (hotfix)
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m7s
Details
f6a25aa16d 
Reverts universal trailing slash in _build_url which broke custom_commands
endpoints (401 on /execute/). Instead, add trailing slashes only to the
two batch POST endpoints (plays/, decisions/) that need them to avoid
307 redirects dropping request bodies.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewed-on: #74
fix: add trailing slashes to all collection POST calls
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m16s
Details
ba55ed3109 
Ensures all client.post() calls to collection endpoints include
trailing slashes, matching the standardized database API routes.
Covers BaseService.create(), TransactionService, InjuryService,
and DraftListService POST calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewed-on: #75
Read all spreadsheet data (plays, box score, pitching decisions) before any
database writes so formula errors like #N/A don't leave the DB in a partial
state. Also preserve SheetsException detail through the error chain and show
users the specific cell/error instead of a generic failure message.

Closes #78

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cal closed this pull request 2026-03-11 01:55:22 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-working

   
in-next-release

Fix included in the next-release branch, pending merge to main

  
status/in-progress

Work is actively in progress

  
status/pr-open

A pull request has been opened for this issue

No Label
ai-changes-requested
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
in-next-release
status/in-progress
status/pr-open
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/major-domo-v2#79
No description provided.
Delete Branch "fix/scorecard-submission-resilience"

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?