Self-service tournament administration and yearly rollover - #79
Self-service tournament administration and yearly rollover#79leonshimizu wants to merge 14 commits into
Conversation
✅ Deploy Preview for marianas-open ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
# Conflicts: # README.md
1 participant
Summary
Verification
bundle exec rails test— 7 tests, 31 assertions, 0 failuresbundle exec rails zeitwerk:checkbundle exec brakeman -q --no-pager— 0 high-confidence warnings; 7 pre-existing medium-confidence findings remainnpm run lintnpx tsc -bnpx vite buildOperational notes
Greptile Summary
This PR introduces season lifecycle management (draft → active → archived), a safe annual rollover workflow that pre-stages blobs outside the DB transaction, schedulable sponsor placements with a public featured-bar component, read-only viewer access across all admin APIs, and an audit log trail on core operations.
rollover_committedflag ensures the rescue block only purges blobs if the DB commit never happened. The audit log is written after the commit with a non-raising rescue, avoiding the transaction-abort risk noted in a prior review.publishable?is checked in both theupdateand newpublishactions before a status promotion toupcoming/liveis allowed; the migration handles slug deduplication and main-event uniqueness validation before creating the corresponding indexes.cta_urlis validated tohttp/httpsonly at the model layer (cta_url_is_safe) and again at the frontend (normalizeExternalUrl), closing the stored-XSS vector flagged in the prior review. The gallerybulk_updateaction replacesupdate_allwith per-recordupdate!inside a transaction, resolving the prior Brakeman mass-assignment warning.Confidence Score: 5/5
cta_urlXSS concern from the prior review is closed at both model and frontend layers. The remaining comments are performance/observability suggestions that do not affect correctness.api/app/models/season.rbandapi/app/controllers/api/v1/sponsor_placements_controller.rbhave minor query inefficiencies worth addressing in a follow-up.Important Files Changed
current_seasonhelper, andas_jsonthat runs an explicit preloader per serialization call — the:seasonback-association is re-fetched from DB even though every event in the result already belongs to the season being serialized.committed?to avoid double-purge. Logic is well-structured and correctly handles ownership of prepared attachments.includeslist now covers:seasonandhero_image_attachment.updateguards status promotion behindpublishable?.@season.destroy(without bang) sorestrict_with_errorsets errors on the model and returns false — errors rendered via else branch. Rescue forInvalidForeignKeyis a secondary guard.cta_url_is_safevalidates scheme allowlist,media_is_safevalidates MIME type and size. Previously flagged XSS vector is addressed at both model and frontend layers.Season.current_seasonon every request, which issues a DB query each time. With no caching this becomes a per-request query on a public, unauthenticated endpoint.limit(capped to 100) but has no offset or cursor, making records older than the last 100 inaccessible through the API.deduplicate_and_backfill_event_slugs!before adding the unique index, andensure_one_main_event_per_season!before adding the partial unique index — both pre-existing concerns from prior reviews are addressed.Sequence Diagram
sequenceDiagram participant Admin participant SeasonsController participant SeasonRolloverService participant EventRolloverService participant BlobCopyService participant DB Admin->>SeasonsController: POST /admin/seasons/:id/rollover SeasonsController->>SeasonRolloverService: call(source_season, target_year) loop For each source event SeasonRolloverService->>EventRolloverService: prepare_attachments(event) EventRolloverService->>BlobCopyService: copy hero + accommodation blobs BlobCopyService-->>EventRolloverService: staged blobs (pre-transaction) end loop For each sponsor placement SeasonRolloverService->>BlobCopyService: copy media blob end SeasonRolloverService->>DB: BEGIN TRANSACTION SeasonRolloverService->>DB: INSERT season (draft) loop For each event plan SeasonRolloverService->>EventRolloverService: call(event, target_season, prepared_attachments) EventRolloverService->>DB: INSERT event + schedule + prizes + accommodations EventRolloverService->>DB: ATTACH hero blob end SeasonRolloverService->>DB: INSERT sponsor_placements + ATTACH media SeasonRolloverService->>DB: COMMIT Note over SeasonRolloverService: rollover_committed = true SeasonRolloverService->>DB: INSERT audit_log (rescued on failure) SeasonRolloverService-->>SeasonsController: "{season, events}" SeasonsController-->>Admin: 201 CreatedReviews (13): Last reviewed commit: "Harden admin rollover and sponsor placem..." | Re-trigger Greptile