Closes#96
Replaces the per-field `json.dumps(value)` probe — which fully serialized
and discarded the result just to check serializability — with a type-check
fast path using `isinstance()`. The `_SERIALIZABLE_TYPES` tuple is defined
at module level so it's not recreated on every log call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Keep both the type: ignore annotation and the logger.info call
in admin_maintenance.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous code attempted to register a maintenance mode gate via
@self.tree.interaction_check inside setup_hook. That pattern is invalid
in discord.py — interaction_check is an overridable method on CommandTree,
not a decorator. The assignment was silently dropped, making maintenance
mode a no-op and producing a RuntimeWarning about an unawaited coroutine.
Changes:
- Add MaintenanceAwareTree(discord.app_commands.CommandTree) that overrides
interaction_check: blocks non-admins when bot.maintenance_mode is True,
always passes admins through, no-op when maintenance mode is off
- Pass tree_cls=MaintenanceAwareTree to super().__init__() in SBABot.__init__
- Add self.maintenance_mode: bool = False to SBABot.__init__
- Update /admin-maintenance command to actually toggle bot.maintenance_mode
- Add tests/test_bot_maintenance_tree.py with 8 unit tests covering all
maintenance mode states, admin pass-through, DM context, and missing attr
Closes#82
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
- Pin redis==7.3.0 and move to requirements.txt (production)
- Create requirements-dev.txt with all dev/test deps pinned to exact versions
(pytest-mock==3.15.1, black==26.1.0, ruff==0.15.0)
- Remove dev/test tools from requirements.txt (not needed in Docker image)
- Document pinning policy and requirements-dev.txt usage in CLAUDE.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
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>
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>
Add centralized `is_admin(interaction)` helper that includes the
`isinstance(interaction.user, discord.Member)` guard, preventing
AttributeError in DM contexts.
Use it in `can_edit_player_image()` which previously accessed
`guild_permissions.administrator` directly without the isinstance
guard. Update the corresponding test to mock the user with
`spec=discord.Member` so the isinstance check passes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The TransactionBuilder cached pre-existing transactions on first load
and never refreshed them. This meant transactions submitted by other
sessions (or newly visible after API fixes) were invisible for the
lifetime of the builder session, causing incorrect roster projections.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Trade validation now automatically fetches the current week from
league_service and validates against next week's projected roster
(including pending transactions), matching /dropadd behavior.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Roster-level errors, warnings, and suggestions now display as
"[BSG] Too many ML players" so users know which team each issue
applies to.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
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>
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>
Adds next-release branch trigger and replaces separate dev/production
build steps with the shared docker-tags action for tag resolution.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removes the @pytest.mark.skip and implements the test using a
patched RosterValidation that raises on the first call, triggering
the except clause in validate_transaction. Verifies the method
returns is_legal=False with a descriptive error message.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both get_disappointment_gif and get_gif previously created a new
ClientSession per call. Replace with a lazily-initialised shared
session stored on the instance, eliminating per-call TCP handshake
and DNS overhead.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both _should_sync_commands and _save_command_hash contained an identical
~35-line block building the command data list and computing the SHA-256 hash.
Extracted into a new synchronous helper method _compute_command_hash() that
both callers now delegate to.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add section headers (Active Roster, Minor League, Injured List) to the
main summary embed in _create_roster_embeds so each roster section is
clearly labeled for users
- Fix incorrect docstring in team_service.get_team_roster that had the
shortil/longil mapping backwards
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add `maintenance_mode: bool = False` flag to `SBABot.__init__`
- Register a global `@tree.interaction_check` that blocks non-admin users
from all commands when maintenance mode is active
- Update `admin_maintenance` command to set `self.bot.maintenance_mode`
and log the state change, replacing the no-op placeholder comment
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Simplify review found that force_refresh=True on every validate_transaction()
call caused redundant API fetches on every embed render and button press.
Instead, invalidate the roster cache after successful submit_transaction() so
the next operation fetches fresh data. This preserves the cache for normal
interaction flows while still preventing stale data after submissions.
Also adds type annotation for roster_svc DI parameter.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TransactionBuilder cached roster data indefinitely via _roster_loaded flag,
causing validation to use stale counts when builders persisted across multiple
/ilmove invocations. Now validate_transaction() always fetches fresh roster
data. Also adds dependency injection for roster_service to improve testability.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codebase audit identified ~50 lazy imports. Moved 42 unnecessary ones to
top-level imports — only keeping those justified by circular imports,
init-order dependencies, or optional dependency guards. Updated test mock
patch targets where needed. See #57 for remaining DI candidates.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
/trade add-player hardcoded from_roster=MAJOR_LEAGUE for all players,
causing incorrect transaction records when trading MiL/IL players. This
cascaded into false roster validation errors for other teams' /dropadd
commands because pre-existing transaction processing misidentified which
roster a player was leaving from.
Now looks up the player's actual team and calls roster_type() to determine
the correct source roster. Same fix applied to /trade supplementary.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevents users from managing injuries for players not on their team.
Admins bypass the check; org affiliates (MiL/IL) are recognized.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
