From de7f41877ccfa640b6d69be7830f2a73af06d143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Mike=C5=A1?= Date: Wed, 19 Aug 2026 20:12:44 +0200 Subject: [PATCH] API: PUT /me/solving-times/{id} keeps the time's competition link The PUT payload has no event field, but the processor dispatched EditPuzzleSolvingTime with competitionId: null and modify() assigns it unconditionally, so every API edit silently detached the time from its competition. Carry the current link through; the round link was never touched by modify() and stays intact. Refs #204 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QUegPi2i9SjwkcrNWyENnL --- docs/features/api/README.md | 1 + src/Api/V1/UpdateSolvingTimeProcessor.php | 8 ++++- .../Api/V1/UpdateSolvingTimeEndpointTest.php | 35 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/docs/features/api/README.md b/docs/features/api/README.md index 1d729769..648adfaf 100644 --- a/docs/features/api/README.md +++ b/docs/features/api/README.md @@ -303,6 +303,7 @@ The three blocks the profile page shows, under the page's own gates (`templates/ - `time` format: `HH:MM:SS` or `MM:SS` - `group_players`: player codes prefixed with `#`, or plain names for unregistered players - `round_id`: optional, nullable. When set, the time is linked to that competition round and automatically to its competition. An invalid or unknown `round_id` returns 404. +- `PUT` never changes the event link: the payload has no `round_id`/competition field, and the processor carries the time's current competition through to the handler (`PuzzleSolvingTime::modify()` assigns whatever it is given, and the round link is never touched by it). Before 2026-08-19 a `PUT` silently detached the time from its competition. - Photo uploads not supported via API (use the website) Response (`SolvingTimeResponse`, shared with `PUT …/solving-times/{timeId}`): diff --git a/src/Api/V1/UpdateSolvingTimeProcessor.php b/src/Api/V1/UpdateSolvingTimeProcessor.php index 9fdc12e7..8dc21526 100644 --- a/src/Api/V1/UpdateSolvingTimeProcessor.php +++ b/src/Api/V1/UpdateSolvingTimeProcessor.php @@ -57,11 +57,17 @@ public function process(mixed $data, Operation $operation, array $uriVariables = $finishedAt = $data->finishedAt !== null ? new DateTimeImmutable($data->finishedAt) : null; + // The API payload has no notion of the event link, but the handler passes the message's + // competition straight to PuzzleSolvingTime::modify(), which assigns it unconditionally — + // so `null` here would silently detach the time from its competition on every PUT. + // Carry the current link through; the round link is never touched by modify(). + $competitionId = $solvingTime->competition?->id->toString(); + $this->messageBus->dispatch( new EditPuzzleSolvingTime( currentUserId: $userId, puzzleSolvingTimeId: $timeId, - competitionId: null, + competitionId: $competitionId, time: $data->time, comment: $data->comment, groupPlayers: $data->groupPlayers, diff --git a/tests/Controller/Api/V1/UpdateSolvingTimeEndpointTest.php b/tests/Controller/Api/V1/UpdateSolvingTimeEndpointTest.php index 7541545b..46ff97ca 100644 --- a/tests/Controller/Api/V1/UpdateSolvingTimeEndpointTest.php +++ b/tests/Controller/Api/V1/UpdateSolvingTimeEndpointTest.php @@ -5,6 +5,8 @@ namespace SpeedPuzzling\Web\Tests\Controller\Api\V1; use Doctrine\DBAL\Connection; +use SpeedPuzzling\Web\Tests\DataFixtures\CompetitionFixture; +use SpeedPuzzling\Web\Tests\DataFixtures\CompetitionRoundFixture; use SpeedPuzzling\Web\Tests\DataFixtures\OAuth2ClientFixture; use SpeedPuzzling\Web\Tests\DataFixtures\PlayerFixture; use SpeedPuzzling\Web\Tests\DataFixtures\PuzzleSolvingTimeFixture; @@ -48,6 +50,39 @@ public function testUpdateOwnTimeKeepsAttributionToPlayer(): void self::assertSame('Updated via API', $row['comment']); } + public function testUpdateKeepsCompetitionAndRoundLink(): void + { + // The PUT payload carries no event information; the processor must carry the time's current + // competition through to the handler, otherwise modify() detaches it (regression: it used to pass null). + $browser = self::createClient(); + + $token = PatTestHelper::createToken($browser, PlayerFixture::PLAYER_REGULAR); + PatTestHelper::addBearerToken($browser, $token); + + $browser->request( + 'PUT', + '/api/v1/me/solving-times/' . PuzzleSolvingTimeFixture::TIME_09, + server: ['CONTENT_TYPE' => 'application/json'], + content: (string) json_encode(['comment' => 'Still a WJPC result']), + ); + + $this->assertResponseIsSuccessful(); + + /** @var Connection $database */ + $database = self::getContainer()->get(Connection::class); + + /** @var array{competition_id: null|string, competition_round_id: null|string, comment: null|string}|false $row */ + $row = $database->fetchAssociative( + 'SELECT competition_id, competition_round_id, comment FROM puzzle_solving_time WHERE id = :id', + ['id' => PuzzleSolvingTimeFixture::TIME_09], + ); + + self::assertNotFalse($row); + self::assertSame('Still a WJPC result', $row['comment']); + self::assertSame(CompetitionFixture::COMPETITION_WJPC_2024, $row['competition_id']); + self::assertSame(CompetitionRoundFixture::ROUND_WJPC_QUALIFICATION, $row['competition_round_id']); + } + public function testUpdateForeignTimeReturnsForbidden(): void { $browser = self::createClient();