Skip to content

US4: Replay Upload & Storage#96

Merged
Ghee-clarified-butter merged 4 commits into
mainfrom
user-story-4-replay-upload
May 3, 2026
Merged

US4: Replay Upload & Storage#96
Ghee-clarified-butter merged 4 commits into
mainfrom
user-story-4-replay-upload

Conversation

@mollup
Copy link
Copy Markdown
Collaborator

@mollup mollup commented May 3, 2026

Implement User Story 4: Replay Upload & Storage

This PR adds replay upload and storage functionality for tournament organizers.

Key Features:

  • Upload replays with metadata (title, player names, game)
  • 2GB file size limit validation
  • Replay retrieval by ID and tournament
  • Authorization: only organizers can upload
  • Only tournament organizers can upload replays for their events

Technical Implementation:

  • Replay interface with video URL, file size, metadata
  • Store functions for creating and retrieving replays
  • RESTful endpoints: POST /api/replays, GET /api/replays/:id, GET /api/tournaments/:id/replays
  • Comprehensive test coverage (14 tests)
  • File size validation with proper error messages

Machine Acceptance Criteria Verified:
✓ POST /api/replays creates a replay and returns HTTP 201
✓ GET /api/replays/:id returns replay metadata
✓ 2GB file size limit enforced with 413 status
✓ Organizer-only access with proper authentication

Tests: 14/14 passing

mollup added 4 commits May 3, 2026 14:15
- Add score fields (player1Score, player2Score) to BracketMatch type
- Add tournamentWinner field to BracketResponse
- Implement PUT /api/tournaments/:id/matches/:matchId/score endpoint
- Add submitMatchScore function to store with validation
- Update getTournamentBracket to include tournament winner
- Add comprehensive test suite with 14 test cases covering:
  - Score submission and validation
  - Bracket advancement and winner determination
  - Real-time updates
  - Score history preservation
  - Edge cases (byes, completed matches)
- All 73 tests passing
- Add Replay type to types.ts with all required fields
- Create replay storage in store.ts with Maps for replays and replaysByTournament
- Implement createReplay, getReplayById, getReplaysByTournament, validateReplayFileSize functions
- Create src/routes/replays.ts with POST /api/replays and GET /api/replays/:id
- Add GET /api/tournaments/:id/replays endpoint in tournaments.ts
- Implement 2GB file size limit validation
- Add authentication checks (organizer-only upload)
- Ensure organizers can only upload to their own tournaments
- Replays accessible publicly without authentication
- Add comprehensive test suite with 14 tests covering all edge cases
- All 96 tests passing
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Claude automated code review

Summary

This PR implements US4 (Replay Upload & Storage) with endpoints for uploading replays with metadata, retrieving replays by ID, and listing tournament replays. It adds comprehensive test coverage (14 tests) plus a large new automation documentation file (561 lines). The implementation includes 2GB file-size validation and organizer-only authorization. However, the diff is truncated and the automation document appears unrelated to US4's core functionality.

Risk assessment

Medium — The replay upload logic is straightforward and well-tested, but the PR introduces an unrelated 561-line automation architecture document that should be split into a separate PR, and score-tracking tests appear to test US3 features (not US4).

Findings

  1. [major] AUTOMATION-ARCHITECTURE-SUMMARY.md — This 561-line document describes CI/LLM automation for US11–US13, not US4. It should be removed from this PR or split into a separate documentation PR. The PR scope is for replay upload (US4), not automation infrastructure.

  2. [major] src/score-tracking.test.ts — This test file implements US3 (Live Score Tracking), not US4. It should be in a separate US3 PR. Including it here dilutes the PR's focus and makes reviewers question whether the wrong tests were committed.

  3. [minor] src/bracket/singleElimination.ts lines 63, 64, 82, 83 — Adding player1Score and player2Score fields to bracket initialization is US3 scope (score tracking), not US4. These should be in the US3 PR. The replay feature has no dependency on these fields.

  4. [minor] src/routes/replays.ts line 1 — Missing /api/tournaments/:id/replays endpoint in the replays route file. The endpoint is correctly added in tournaments.ts but logically belongs in replays.ts for consistency. Consider adding it here as well or clearly document why it's split.

  5. [nit] src/replay-upload.test.ts lines 289–293 — Test "should validate at least one player name required" passes an empty array but the schema in replays.ts line 22 already enforces min(1). Confirm this validation works end-to-end; if it does, test is good; if not, the schema needs tightening.

  6. [nit] src/routes/tournaments.ts lines 324–359 — The submitMatchScore endpoint and its tests belong to US3, not US4. These should be removed from this PR.

Test coverage

14 replay-specific tests are well-written and cover: upload with metadata (201), organizer-only access (403), file-size limits (413), missing fields (400), tournament existence (404), organizer ownership (403), retrieval by ID, public access, listing by tournament, metadata preservation, and timestamp validation. However, score-tracking.test.ts (487 lines) tests US3 features and should not be in this PR. Missing: tests for invalid URLs (beyond schema validation) and edge cases like very-long player names or special characters.

Security

  • No injection vulnerabilities identified; Zod schema validates all inputs.
  • Authorization check correctly verifies organizer ownership of tournament before allowing replay upload.
  • File-size validation prevents abuse (2GB limit enforced).
  • Public read access to replays is intentional (spectators).
  • No secrets or credentials exposed.

No security concerns identified.

Suggested follow-ups

  • Create a separate PR for US3 score-tracking implementation (move score-tracking.test.ts, submitMatchScore endpoint, and bracket score fields).
  • Create a separate PR for automation documentation (move AUTOMATION-ARCHITECTURE-SUMMARY.md).
  • Consider adding tests for malformed URLs and very-long strings to stress the Zod schemas.
  • Document why GET /api/tournaments/:id/replays is in tournaments.ts rather than replays.ts, or refactor for consistency.

Advisory only — a human reviewer still approves the merge. Re-run with /claude-review in a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants