fix: lifecycle guard for play_card API endpoint (#2631)#2708
fix: lifecycle guard for play_card API endpoint (#2631)#2708immortal71 wants to merge 2 commits intoOWASP:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #2631 by adding a game lifecycle guard to the PUT /api/games/:game_id/players/:player_id/card endpoint so cards can’t be played before a game starts or after it finishes.
Changes:
- Add
started_at/finished_atlifecycle checks toplay_card/2, returning422 Unprocessable Entitywhen invalid. - Update and extend controller tests to cover lifecycle validation and keep existing tests passing by setting
started_at. - Adjust control flow in
PlayerLive.Show’s"next_round"handler (block scoping change).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| copi.owasp.org/lib/copi_web/controllers/api_controller.ex | Adds lifecycle cond guard to play_card/2 before allowing card play logic. |
| copi.owasp.org/test/copi_web/controllers/api_controller_test.exs | Updates existing tests to set started_at and adds new 422 tests for unstarted/ended games. |
| copi.owasp.org/lib/copi_web/live/player_live/show.ex | Changes block scoping in "next_round" to keep reload/broadcast/assign within the else branch. |
| {:ok, updated_game} = Game.find(game.id) | ||
|
|
||
| CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game) | ||
| CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game) | ||
|
|
||
| {:noreply, assign(socket, :game, updated_game)} | ||
| {:noreply, assign(socket, :game, updated_game)} | ||
| end |
There was a problem hiding this comment.
This PR description focuses on adding a lifecycle guard to the play_card API endpoint, but this diff also changes control-flow in handle_event("next_round", ...) by moving the Game.find/1 + broadcast + {:noreply, ...} block under the else branch (via the relocated end). If this behavioral change is intentional, it should be called out in the PR description (or split into a separate PR) and ideally covered by a test, since it affects LiveView round progression logic beyond the stated scope.
Reject card plays with 422 Unprocessable Entity when: - game.started_at == nil (game has not started yet) - game.finished_at != nil (game has already ended) Add tests covering both lifecycle guard cases, and update existing tests that exercise the success and card/player validation paths to first start the game, since those paths now require an active game.
immortal71
force-pushed
the
fix/2631-play-card-lifecycle-guard
branch
from
1c7ebf1 to
0f774fb
Compare
|
@sydseter is this good to go .... |
Summary
Fixes #2631
The PUT /api/games/:game_id/players/:player_id/card endpoint in api_controller.ex had no game lifecycle check, allowing cards to be played against games that had not yet started or had already finished.
Root Cause
The play_card/2 handler jumped straight to player and card lookups after finding the game, with no guard on game.started_at or game.finished_at.
Fix
Added a cond block at the top of the game-found branch that rejects the request with 422 Unprocessable Entity when:
All other existing checks (card already played, player already played in round) proceed unchanged once the lifecycle is confirmed.
Tests