From c4d57134a815ae8ecf1bae6fa91d7bcefe0e6fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Mike=C5=A1?= Date: Sun, 22 Feb 2026 13:08:19 +0100 Subject: [PATCH 1/2] Player results - implement separate page for better UX+UI --- .../PlayerPuzzleResultsController.php | 56 +++++ src/Query/GetPlayerSolvedPuzzles.php | 233 ++++++++++++++++++ templates/_player_solvings.html.twig | 78 ++---- templates/components/PuzzleTimes.html.twig | 64 +---- .../player-puzzle-results/_modal.html.twig | 60 +++++ .../_times_table.html.twig | 114 +++++++++ .../player-puzzle-results/detail.html.twig | 72 ++++++ .../PlayerPuzzleResultsControllerTest.php | 50 ++++ translations/messages.en.yml | 1 + 9 files changed, 624 insertions(+), 104 deletions(-) create mode 100644 src/Controller/PlayerPuzzleResultsController.php create mode 100644 templates/player-puzzle-results/_modal.html.twig create mode 100644 templates/player-puzzle-results/_times_table.html.twig create mode 100644 templates/player-puzzle-results/detail.html.twig create mode 100644 tests/Controller/PlayerPuzzleResultsControllerTest.php diff --git a/src/Controller/PlayerPuzzleResultsController.php b/src/Controller/PlayerPuzzleResultsController.php new file mode 100644 index 000000000..b8354c82f --- /dev/null +++ b/src/Controller/PlayerPuzzleResultsController.php @@ -0,0 +1,56 @@ + 'solo|duo|team'], + )] + public function __invoke(string $playerId, string $puzzleId, string $category, Request $request): Response + { + $player = $this->getPlayerProfile->byId($playerId); + $results = $this->getPlayerSolvedPuzzles->byPlayerIdPuzzleIdAndCategory($playerId, $puzzleId, $category); + + if ($results === []) { + throw $this->createNotFoundException(); + } + + $ranking = null; + if ($category === 'solo') { + $ranking = $this->getRanking->ofPuzzleForPlayer($puzzleId, $playerId); + } + + $templateData = [ + 'player' => $player, + 'results' => $results, + 'ranking' => $ranking, + 'category' => $category, + ]; + + if ($request->headers->get('Turbo-Frame') === 'modal-frame') { + return $this->render('player-puzzle-results/_modal.html.twig', $templateData); + } + + return $this->render('player-puzzle-results/detail.html.twig', $templateData); + } +} diff --git a/src/Query/GetPlayerSolvedPuzzles.php b/src/Query/GetPlayerSolvedPuzzles.php index a52679f0c..e6fed693f 100644 --- a/src/Query/GetPlayerSolvedPuzzles.php +++ b/src/Query/GetPlayerSolvedPuzzles.php @@ -666,6 +666,239 @@ public function teamByPlayerId( }, $data); } + /** + * @return array + * @throws PlayerNotFound + */ + public function byPlayerIdPuzzleIdAndCategory(string $playerId, string $puzzleId, string $category): array + { + if (Uuid::isValid($puzzleId) === false) { + throw new PlayerNotFound(); + } + + return match ($category) { + // soloByPlayerIdAndPuzzleId returns oldest first, this page renders newest first + 'solo' => array_reverse($this->soloByPlayerIdAndPuzzleId($playerId, $puzzleId)), + 'duo' => $this->duoByPlayerIdAndPuzzleId($playerId, $puzzleId), + 'team' => $this->teamByPlayerIdAndPuzzleId($playerId, $puzzleId), + default => [], + }; + } + + /** + * @return array + * @throws PlayerNotFound + */ + private function duoByPlayerIdAndPuzzleId(string $playerId, string $puzzleId): array + { + if (Uuid::isValid($playerId) === false || Uuid::isValid($puzzleId) === false) { + throw new PlayerNotFound(); + } + + $query = << 'puzzlers') @> jsonb_build_array(jsonb_build_object('player_id', CAST(:playerId AS UUID))) + AND puzzling_type = 'duo' + AND puzzle_id = :puzzleId +) +SELECT + pst.id as time_id, + puzzle.id AS puzzle_id, + puzzle.name AS puzzle_name, + puzzle.alternative_name AS puzzle_alternative_name, + CASE WHEN puzzle.hide_image_until IS NOT NULL AND puzzle.hide_image_until > :now::timestamp THEN NULL ELSE puzzle.image END AS puzzle_image, + CASE WHEN puzzle.hide_image_until IS NOT NULL AND puzzle.hide_image_until > :now::timestamp THEN NULL ELSE puzzle.image_ratio END AS puzzle_image_ratio, + pst.seconds_to_solve AS time, + pst.player_id AS player_id, + pieces_count, + finished_puzzle_photo, + tracked_at, + finished_at, + puzzle.identification_number AS puzzle_identification_number, + pst.comment, + manufacturer.name AS manufacturer_name, + pst.team ->> 'team_id' AS team_id, + first_attempt, + pst.unboxed, + competition.id AS competition_id, + competition.shortcut AS competition_shortcut, + competition.name AS competition_name, + competition.slug AS competition_slug, + pst.suspicious +FROM filtered_pst_ids fids +INNER JOIN puzzle_solving_time pst ON pst.id = fids.id +INNER JOIN puzzle ON puzzle.id = pst.puzzle_id +INNER JOIN manufacturer ON manufacturer.id = puzzle.manufacturer_id +LEFT JOIN competition ON competition.id = pst.competition_id +ORDER BY COALESCE(pst.finished_at, pst.tracked_at) DESC +SQL; + + $data = $this->database + ->executeQuery($query, [ + 'playerId' => $playerId, + 'puzzleId' => $puzzleId, + 'now' => $this->clock->now()->format('Y-m-d H:i:s'), + ]) + ->fetchAllAssociative(); + + /** @var array $timeIds */ + $timeIds = array_column($data, 'time_id'); + + $players = $this->getTeamPlayers->byIds($timeIds); + + return array_map(static function (array $row) use ($players): SolvedPuzzle { + /** + * @var array{ + * time_id: string, + * team_id: null|string, + * player_id: string, + * player_name: null, + * player_code: string, + * player_country: null, + * puzzle_id: string, + * puzzle_name: string, + * puzzle_alternative_name: null|string, + * manufacturer_name: string, + * puzzle_image: null|string, + * puzzle_image_ratio: null|string, + * time: int, + * pieces_count: int, + * comment: null|string, + * finished_puzzle_photo: null|string, + * puzzle_identification_number: null|string, + * tracked_at: string, + * finished_at: null|string, + * first_attempt: bool, + * unboxed: bool, + * competition_id: null|string, + * competition_name: null|string, + * competition_shortcut: null|string, + * competition_slug: null|string, + * suspicious: bool, + * } $row + */ + + $row['players'] = $players[$row['time_id']] ?? null; + + // Dummy placeholder values + $row['player_name'] = null; + $row['player_code'] = ''; + $row['player_country'] = null; + + return SolvedPuzzle::fromDatabaseRow($row); + }, $data); + } + + /** + * @return array + * @throws PlayerNotFound + */ + private function teamByPlayerIdAndPuzzleId(string $playerId, string $puzzleId): array + { + if (Uuid::isValid($playerId) === false || Uuid::isValid($puzzleId) === false) { + throw new PlayerNotFound(); + } + + $query = << 'puzzlers') @> jsonb_build_array(jsonb_build_object('player_id', CAST(:playerId AS UUID))) + AND puzzling_type = 'team' + AND puzzle_id = :puzzleId +) +SELECT + pst.id as time_id, + puzzle.id AS puzzle_id, + puzzle.name AS puzzle_name, + puzzle.alternative_name AS puzzle_alternative_name, + CASE WHEN puzzle.hide_image_until IS NOT NULL AND puzzle.hide_image_until > :now::timestamp THEN NULL ELSE puzzle.image END AS puzzle_image, + CASE WHEN puzzle.hide_image_until IS NOT NULL AND puzzle.hide_image_until > :now::timestamp THEN NULL ELSE puzzle.image_ratio END AS puzzle_image_ratio, + pst.seconds_to_solve AS time, + pst.player_id AS player_id, + pieces_count, + finished_puzzle_photo, + tracked_at, + finished_at, + puzzle.identification_number AS puzzle_identification_number, + pst.comment, + manufacturer.name AS manufacturer_name, + pst.team ->> 'team_id' AS team_id, + first_attempt, + pst.unboxed, + competition.id AS competition_id, + competition.shortcut AS competition_shortcut, + competition.name AS competition_name, + competition.slug AS competition_slug, + pst.suspicious +FROM filtered_pst_ids fids +INNER JOIN puzzle_solving_time pst ON pst.id = fids.id +INNER JOIN puzzle ON puzzle.id = pst.puzzle_id +INNER JOIN manufacturer ON manufacturer.id = puzzle.manufacturer_id +LEFT JOIN competition ON competition.id = pst.competition_id +ORDER BY COALESCE(pst.finished_at, pst.tracked_at) DESC +SQL; + + $data = $this->database + ->executeQuery($query, [ + 'playerId' => $playerId, + 'puzzleId' => $puzzleId, + 'now' => $this->clock->now()->format('Y-m-d H:i:s'), + ]) + ->fetchAllAssociative(); + + /** @var array $timeIds */ + $timeIds = array_column($data, 'time_id'); + + $players = $this->getTeamPlayers->byIds($timeIds); + + return array_map(static function (array $row) use ($players): SolvedPuzzle { + /** + * @var array{ + * time_id: string, + * team_id: null|string, + * player_id: string, + * player_name: null, + * player_code: string, + * player_country: null, + * puzzle_id: string, + * puzzle_name: string, + * puzzle_alternative_name: null|string, + * manufacturer_name: string, + * puzzle_image: null|string, + * puzzle_image_ratio: null|string, + * time: int, + * pieces_count: int, + * comment: null|string, + * finished_puzzle_photo: null|string, + * puzzle_identification_number: null|string, + * tracked_at: string, + * finished_at: null|string, + * first_attempt: bool, + * unboxed: bool, + * competition_id: null|string, + * competition_name: null|string, + * competition_shortcut: null|string, + * competition_slug: null|string, + * suspicious: bool, + * } $row + */ + + $row['players'] = $players[$row['time_id']] ?? null; + + // Dummy placeholder values + $row['player_name'] = null; + $row['player_code'] = ''; + $row['player_country'] = null; + + return SolvedPuzzle::fromDatabaseRow($row); + }, $data); + } + /** * Count distinct puzzles solved by player (solo or in team) * diff --git a/templates/_player_solvings.html.twig b/templates/_player_solvings.html.twig index b05847fa8..cf6ee5226 100644 --- a/templates/_player_solvings.html.twig +++ b/templates/_player_solvings.html.twig @@ -1,4 +1,4 @@ -
+
{% for solved_puzzle in solved_puzzles %} @@ -17,26 +17,6 @@ ) }} - {# Display photo of finished puzzle only to logged user #} - {% if logged_user.profile is not null and logged_user.profile.playerId is same as solved_puzzle[0].playerId %} - {% if solved_puzzle[0].finishedPuzzlePhoto is not null%} -
- - {{ 'puzzle_img_alt'|trans({'%puzzle%': solved_puzzle[0].manufacturerName ~ ' ' ~ solved_puzzle[0].puzzleName}) }} - -
- {% endif %} - - - {% endif %} diff --git a/templates/components/PuzzleTimes.html.twig b/templates/components/PuzzleTimes.html.twig index c56363c5a..a85de8bb0 100644 --- a/templates/components/PuzzleTimes.html.twig +++ b/templates/components/PuzzleTimes.html.twig @@ -307,7 +307,7 @@ {% endif %} -
+
@@ -51,30 +31,23 @@ - {% if solved_puzzle|length > 1 %} - - {% endif %} + {% set puzzle = solved_puzzle[0] %} - {% for puzzle in solved_puzzle %} - + - {% endfor %}
- {% if loop.first %} - - {{ solved_puzzle[0].manufacturerName }} -
- {{ 'pieces_count'|trans({ '%count%': solved_puzzle[0].piecesCount })|raw }} - + + {{ puzzle.manufacturerName }} +
+ {{ 'pieces_count'|trans({ '%count%': puzzle.piecesCount })|raw }} + - {# Display rank for solo (only if ranking exists - won't exist for relax-only puzzles) #} - {% if solved_puzzle[0].players is null and this.ranking[solved_puzzle[0].puzzleId] is defined %} -
- Rank {{ this.ranking[solved_puzzle[0].puzzleId].rank }} ({{ 'ranking_out_of'|trans }} {{ this.ranking[solved_puzzle[0].puzzleId].totalPlayers }})
- {% endif %} + {# Display rank for solo (only if ranking exists - won't exist for relax-only puzzles) #} + {% if puzzle.players is null and this.ranking[puzzle.puzzleId] is defined %} +
+ Rank {{ this.ranking[puzzle.puzzleId].rank }} ({{ 'ranking_out_of'|trans }} {{ this.ranking[puzzle.puzzleId].totalPlayers }})
{% endif %} {% if puzzle.players|length > 0 %} @@ -155,25 +128,24 @@ {% endif %} - {# Display edit icon only to current user #} - {% if logged_user.profile is not null and logged_user.profile.playerId is same as puzzle.playerId %} - {{ include('_edit_time_button.html.twig', { - timeId: puzzle.timeId, - context: 'profile', - }) }} - {% endif %} - - {% if loop.first and solved_puzzle|length > 1 %} + {% if solved_puzzle|length > 1 %}
- - - {{ 'show_more_solving_times'|trans({ '%count%': solved_puzzle|length-1 }) }} - + + {{ 'show_more_solving_times'|trans({ '%count%': solved_puzzle|length-1 }) }} {% endif %}
@@ -416,57 +416,19 @@ {% if aggregated_solver|length > 1 %}
- - - {{ 'show_more_solving_times'|trans({ '%count%': aggregated_solver|length-1 }) }} - + + {{ 'show_more_solving_times'|trans({ '%count%': aggregated_solver|length-1 }) }} - - {% endif %} diff --git a/templates/player-puzzle-results/_modal.html.twig b/templates/player-puzzle-results/_modal.html.twig new file mode 100644 index 000000000..af4923421 --- /dev/null +++ b/templates/player-puzzle-results/_modal.html.twig @@ -0,0 +1,60 @@ + +
+
+
+ {% if results[0].puzzleImage is not null %} + {{ results[0].puzzleName }} + {% endif %} +
+
+ {{ results[0].puzzleName }} +
+ {{ results[0].manufacturerName }} · {{ 'pieces_count'|trans({ '%count%': results[0].piecesCount })|raw }} +
+
+ +
+ +
+
+ {{ include('messaging/_avatar.html.twig', {avatar: player.avatar, size: 32}) }} +
+ + {{ player.playerName ?? ('#' ~ player.code|upper) }} + + {% if player.country is not null %} + + {% endif %} + {% if ranking is not null %} +
+ Rank {{ ranking.rank }} ({{ 'ranking_out_of'|trans }} {{ ranking.totalPlayers }}) + {% endif %} +
+
+ + {% set timed_results = results|filter(r => r.time is not null) %} + {% if timed_results|length >= 2 %} + {% if logged_user.profile is not null and logged_user.profile.activeMembership %} + + {% else %} +
+
+ +
+
+ {% endif %} + {% endif %} + +
+ {% include 'player-puzzle-results/_times_table.html.twig' with {results: results, category: category} %} +
+
+
+ diff --git a/templates/player-puzzle-results/_times_table.html.twig b/templates/player-puzzle-results/_times_table.html.twig new file mode 100644 index 000000000..9d1ab0c82 --- /dev/null +++ b/templates/player-puzzle-results/_times_table.html.twig @@ -0,0 +1,114 @@ +{# Build a chronological list of timed results (oldest first) for delta computation #} +{# Results come ordered DESC (newest first), so we reverse for chronological order #} +{% set timed_chronological = results|reverse|filter(r => r.time is not null) %} +{% set delta_map = {} %} +{% set prev_time = null %} +{% for result in timed_chronological %} + {% if prev_time is not null %} + {% set delta_map = delta_map|merge({ (result.timeId): result.time - prev_time }) %} + {% endif %} + {% set prev_time = result.time %} +{% endfor %} + +
+ + + + + + + + + + {% for result in results %} + + + + + + + + + + {% endfor %} + +
{{ 'time'|trans }}{{ 'date'|trans }}PPM
+ {% if result.time is not null %} + {{ result.time|puzzlingTime }} + {% if delta_map[result.timeId] is defined %} + {% set delta = delta_map[result.timeId] %} +
+ {% if delta < 0 %} + {{ (delta * -1)|puzzlingTime }} + {% elseif delta > 0 %} + +{{ delta|puzzlingTime }} + {% else %} + = + {% endif %} + {% endif %} + {% else %} + + {{ 'badge.relax'|trans }} + + {% endif %} + 		+ {% if result.finishedAt is not null %} + + {% endif %} + + {% if result.time is not null %} + {{ ppm(result.time, result.piecesCount) }} + {% if result.players is not null %} +
({{ result.players|length }}x {{ ppm(result.time, result.piecesCount, result.players|length) }}) + {% endif %} + {% endif %} + 		+ {% if result.firstAttempt %} + {{ 'first_attempt'|trans }} + {% endif %} + + {% if result.unboxed %} + {{ 'unboxed'|trans }} + {% endif %} + + {% if result.suspicious %} + + {{ 'badge.suspicious'|trans }} + + {% endif %} + + {% if result.competitionName %} + {% if result.competitionSlug %} + + {% endif %} + {{ result.competitionShortcut ?? result.competitionName }} + {% if result.competitionSlug %} + + {% endif %} + {% endif %} + + {% if category != 'solo' and result.players|length > 0 %} +
+ + {% for puzzle_solver in result.players %} + {% if not loop.first %}, {% endif %} + {% if puzzle_solver.isPrivate %} + {{ 'secret_puzzler_name'|trans }} + {% elseif puzzle_solver.playerId is not null %} + {{ puzzle_solver.playerName ?? ('#' ~ puzzle_solver.playerCode) }} + {% else %} + {{ puzzle_solver.playerName ?? ('#' ~ puzzle_solver.playerCode) }} + {% endif %} + {% endfor %} + + {% endif %} + + {% if logged_user.profile is not null and logged_user.profile.playerId is same as result.playerId %} + {% if result.finishedPuzzlePhoto is not null %} + + + + {% endif %} + + {% endif %} +
diff --git a/templates/player-puzzle-results/detail.html.twig b/templates/player-puzzle-results/detail.html.twig new file mode 100644 index 000000000..77665436d --- /dev/null +++ b/templates/player-puzzle-results/detail.html.twig @@ -0,0 +1,72 @@ +{% extends 'base.html.twig' %} + +{% block title %}{{ results[0].puzzleName }} - {{ player.playerName ?? ('#' ~ player.code) }}{% endblock %} + +{% block content %} + {% include '_return_back_button.html.twig' with { + fallbackUrl: path('player_profile', {playerId: player.playerId}), + fallbackTitle: player.playerName ?? ('#' ~ player.code) + } %} + +
+
+
+ {% if results[0].puzzleImage is not null %} + + {{ results[0].puzzleName }} + + {% endif %} +
+

+ + {{ results[0].puzzleName }} + +

+ {{ results[0].manufacturerName }} · {{ 'pieces_count'|trans({ '%count%': results[0].piecesCount })|raw }} +
+
+ +
+ {{ include('messaging/_avatar.html.twig', {avatar: player.avatar, size: 32}) }} +
+ + {{ player.playerName ?? ('#' ~ player.code|upper) }} + + {% if player.country is not null %} + + {% endif %} + {% if ranking is not null %} +
+ Rank {{ ranking.rank }} ({{ 'ranking_out_of'|trans }} {{ ranking.totalPlayers }}) + {% endif %} +
+
+
+ +
+ {% set timed_results = results|filter(r => r.time is not null) %} + {% if timed_results|length >= 2 %} + {% if logged_user.profile is not null and logged_user.profile.activeMembership %} + + {% else %} +
+
+ +
+
+ {% endif %} + {% endif %} + +
+ {% include 'player-puzzle-results/_times_table.html.twig' with {results: results, category: category} %} +
+
+
+{% endblock %} diff --git a/tests/Controller/PlayerPuzzleResultsControllerTest.php b/tests/Controller/PlayerPuzzleResultsControllerTest.php new file mode 100644 index 000000000..723f8351c --- /dev/null +++ b/tests/Controller/PlayerPuzzleResultsControllerTest.php @@ -0,0 +1,50 @@ +request('GET', '/en/player/' . PlayerFixture::PLAYER_REGULAR . '/puzzle/' . PuzzleFixture::PUZZLE_500_02 . '/results/solo'); + + $this->assertResponseIsSuccessful(); + } + + public function testModalModeLoads(): void + { + $browser = self::createClient(); + + $browser->request('GET', '/en/player/' . PlayerFixture::PLAYER_REGULAR . '/puzzle/' . PuzzleFixture::PUZZLE_500_02 . '/results/solo', [], [], [ + 'HTTP_TURBO_FRAME' => 'modal-frame', + ]); + + $this->assertResponseIsSuccessful(); + } + + public function testReturns404WhenNoResults(): void + { + $browser = self::createClient(); + + $browser->request('GET', '/en/player/' . PlayerFixture::PLAYER_REGULAR . '/puzzle/' . PuzzleFixture::PUZZLE_1500_02 . '/results/solo'); + + $this->assertResponseStatusCodeSame(404); + } + + public function testDuoResultsPageLoads(): void + { + $browser = self::createClient(); + + $browser->request('GET', '/en/player/' . PlayerFixture::PLAYER_REGULAR . '/puzzle/' . PuzzleFixture::PUZZLE_1000_01 . '/results/duo'); + + $this->assertResponseIsSuccessful(); + } +} diff --git a/translations/messages.en.yml b/translations/messages.en.yml index feeb919f9..5896a3803 100644 --- a/translations/messages.en.yml +++ b/translations/messages.en.yml @@ -58,6 +58,7 @@ puzzler_name: "Name" puzzler: "Puzzler" puzzle: "Puzzle" time: "Time" +date: "Date" pieces_count: "%count% pieces" show_more: "Show more" show_less: "Show less" From f1a0d2cf06bace1e2021d6000c7c9566b383ab88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Mike=C5=A1?= Date: Mon, 27 Jul 2026 15:51:36 +0200 Subject: [PATCH 2/2] Player results: fix show-more link for duo/team rows Two defects in the player-results commit, both pre-existing rather than rebase fallout, found while verifying the rebase against current main. PuzzleTimes: duo/team rows are PuzzleSolversGroup, which has no playerId, so building the link threw a Twig RuntimeError (500 on puzzle detail whenever a team solved the same puzzle more than once). Fall back to the first registered team member, and omit the link when none is registered. _player_solvings: for duo/team rows puzzle.playerId is the solving time's owner, not necessarily the profile owner, so the link could point at a different player's results. Use the component's playerId. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NFMh9JCxZZVmwcLV6tq34z --- templates/_player_solvings.html.twig | 4 +- templates/components/PuzzleTimes.html.twig | 43 +++++++++++++++------- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/templates/_player_solvings.html.twig b/templates/_player_solvings.html.twig index cf6ee5226..53efe4a73 100644 --- a/templates/_player_solvings.html.twig +++ b/templates/_player_solvings.html.twig @@ -130,8 +130,10 @@ {% if solved_puzzle|length > 1 %}
+ {# For duo/team rows puzzle.playerId is the time's owner, who is not + necessarily the profile owner - always link to the profile owner. #} - {{ 'show_more_solving_times'|trans({ '%count%': aggregated_solver|length-1 }) }} - + {# Solo rows are PuzzleSolver (has playerId), duo/team rows are PuzzleSolversGroup + (has no playerId) - fall back to the first registered team member. #} + {% set results_player_id = aggregated_solver[0].playerId|default(null) %} + {% set results_player_name = aggregated_solver[0].playerName|default(null) %} + {% if results_player_id is null %} + {% for puzzle_solver in aggregated_solver[0].players|default([]) %} + {% if results_player_id is null and puzzle_solver.playerId is not null %} + {% set results_player_id = puzzle_solver.playerId %} + {% set results_player_name = puzzle_solver.playerName %} + {% endif %} + {% endfor %} + {% endif %} + + {% if results_player_id is not null %} +
+ + {{ 'show_more_solving_times'|trans({ '%count%': aggregated_solver|length-1 }) }} + + {% endif %} {% endif %}