Propose puzzle changes / duplicates (#73)

* Reporting

* Proposals improvements

* Code review

* Translations, description, fixes

* New badge

* Fixes

* Fix duplicates

* Improvements

* Improvements

* Improvements

Author: JanMikes

.claude/fixtures.md

@@ -35,6 +35,9 @@ Most lent puzzles are **owned by `PLAYER_WITH_STRIPE`**:
 | `LENT_03` | PUZZLE_1000_01 (1000 pcs) | PLAYER_WITH_STRIPE | - | **Returned** | "Returned in good condition" |
 | `LENT_04` | PUZZLE_500_03 (500 pcs) | PLAYER_WITH_STRIPE | PLAYER_WITH_FAVORITES | Active (passed) | "For testing purposes" |
 | `LENT_05` | PUZZLE_1500_02 (1500 pcs) | PLAYER_REGULAR | PLAYER_WITH_STRIPE | Active | - |
+| `LENT_06` | PUZZLE_3000 (3000 pcs) | PLAYER_REGULAR | PLAYER_WITH_STRIPE | Active | - |
+| `LENT_07` | PUZZLE_500_05 (500 pcs) | PLAYER_REGULAR | PLAYER_ADMIN | Active | "Merge test puzzle" |
+| `LENT_08` | PUZZLE_500_04 (500 pcs) | PLAYER_REGULAR | PLAYER_WITH_FAVORITES | Active | "Deduplication test puzzle" |
 
 ### Transfer History
 
@@ -65,7 +68,7 @@ Most lent puzzles are **owned by `PLAYER_WITH_STRIPE`**:
 
 ### Collection Items Distribution
 
-**COLLECTION_PUBLIC** (PLAYER_WITH_STRIPE): PUZZLE_500_01, PUZZLE_500_02, PUZZLE_1000_01, PUZZLE_1000_03, PUZZLE_1000_05, PUZZLE_300, PUZZLE_500_04
+**COLLECTION_PUBLIC** (PLAYER_WITH_STRIPE): PUZZLE_500_01, PUZZLE_500_02, PUZZLE_1000_01, PUZZLE_1000_03, PUZZLE_1000_05, PUZZLE_300, PUZZLE_500_04, PUZZLE_500_05
 
 **COLLECTION_PRIVATE** (PLAYER_REGULAR): PUZZLE_1500_01, PUZZLE_2000, PUZZLE_3000, PUZZLE_1500_02
 
@@ -105,13 +108,15 @@ Most lent puzzles are **owned by `PLAYER_WITH_STRIPE`**:
 | `SELLSWAP_05` | PUZZLE_1000_02 | Swap | - | LikeNew |
 | `SELLSWAP_06` | PUZZLE_1500_01 | Both | 60.00 | MissingPieces |
 | `SELLSWAP_07` | PUZZLE_1000_03 | Sell | 35.00 | Normal |
+| `SELLSWAP_08` | PUZZLE_500_05 | Sell | 20.00 | Normal | (PLAYER_ADMIN)
+| `SELLSWAP_09` | PUZZLE_500_04 | Swap | - | LikeNew | (PLAYER_ADMIN)
 
 ## Wishlists
 
 | Player | Puzzles |
 |--------|---------|
-| PLAYER_REGULAR | PUZZLE_4000, PUZZLE_5000, PUZZLE_6000 |
-| PLAYER_WITH_STRIPE | PUZZLE_9000, PUZZLE_3000 |
+| PLAYER_REGULAR | PUZZLE_4000, PUZZLE_5000, PUZZLE_6000, PUZZLE_500_05, PUZZLE_500_04 |
+| PLAYER_WITH_STRIPE | PUZZLE_9000, PUZZLE_3000, PUZZLE_500_01 |
 | PLAYER_PRIVATE | PUZZLE_4000 |
 
 ## Puzzles (20 total)
@@ -214,3 +219,29 @@ Most lent puzzles are **owned by `PLAYER_WITH_STRIPE`**:
 | Multiple collections | PLAYER_WITH_STRIPE (2), PLAYER_REGULAR (2) |
 | Puzzle in 3 collections | PLAYER_WITH_STRIPE: PUZZLE_500_02 (system + PUBLIC + STRIPE_TREFL) |
 | Borrowed + in collection | PLAYER_WITH_STRIPE: PUZZLE_1500_02 (borrowed + in system collection) |
+
+## Puzzle Merge Testing
+
+PUZZLE_500_04 (survivor) and PUZZLE_500_05 (duplicate) are set up for puzzle merge testing:
+
+### Deduplication Scenarios
+These scenarios test that when a player has BOTH puzzles, only the survivor entry is kept:
+
+| Entity Type | Player | Survivor Entry | Duplicate Entry | After Merge |
+|-------------|--------|----------------|-----------------|-------------|
+| CollectionItem | PLAYER_WITH_STRIPE | ITEM_21 (PUBLIC) | ITEM_27 (PUBLIC) | ITEM_27 removed |
+| WishListItem | PLAYER_REGULAR | WISHLIST_09 | WISHLIST_08 | WISHLIST_08 removed |
+| SellSwapListItem | PLAYER_ADMIN | SELLSWAP_09 | SELLSWAP_08 | SELLSWAP_08 removed |
+| LentPuzzle | PLAYER_REGULAR | LENT_08 | LENT_07 | LENT_07 removed |
+
+### Migration Scenarios
+These entries migrate from duplicate to survivor (no deduplication needed):
+
+| Entity Type | Entry | Player |
+|-------------|-------|--------|
+| CollectionItem | ITEM_25 | PLAYER_ADMIN |
+| CollectionItem | ITEM_26 | PLAYER_PRIVATE |
+| PuzzleSolvingTime | TIME_43 | (all solving times migrate) |
+| PuzzleSolvingTime | TIME_44 | (all solving times migrate) |
+| SoldSwappedItem | SOLD_01 | (all historical records migrate) |
+| SoldSwappedItem | SOLD_02 | (all historical records migrate) |

assets/controllers/dynamic_modal_controller.js

@@ -5,7 +5,7 @@ import { Modal } from 'bootstrap';
  * Dynamic Modal Controller
  *
  * Handles a global modal that loads content dynamically via Turbo Frames.
- * Opens automatically when the turbo-frame starts fetching content,
+ * Opens automatically when frame content is loaded (turbo:frame-load event),
  * closes automatically when the frame becomes empty.
  *
  * Usage:
@@ -18,7 +18,7 @@ export default class extends Controller {
 
     modal = null;
     observer = null;
-    openTimeout = null;
+    pendingOpen = false;
 
     connect() {
         // Disable focus trap to allow interaction with tom-select dropdowns
@@ -27,9 +27,12 @@ export default class extends Controller {
             focus: false
         });
 
-        // Open modal when frame starts fetching content
+        // Track when a fetch starts (we want to open modal when content arrives)
         this.frameTarget.addEventListener('turbo:before-fetch-request', this.handleBeforeFetch);
 
+        // Open modal when content arrives
+        this.frameTarget.addEventListener('turbo:frame-load', this.handleFrameLoad);
+
         // Watch for frame becoming empty (close trigger)
         this.observer = new MutationObserver(this.handleMutation);
         this.observer.observe(this.frameTarget, { childList: true, subtree: true });
@@ -43,18 +46,23 @@ export default class extends Controller {
 
     disconnect() {
         this.frameTarget.removeEventListener('turbo:before-fetch-request', this.handleBeforeFetch);
+        this.frameTarget.removeEventListener('turbo:frame-load', this.handleFrameLoad);
         this.observer?.disconnect();
         document.removeEventListener('keydown', this.handleKeydown);
         document.removeEventListener('modal:close', this.handleClose);
-        clearTimeout(this.openTimeout);
     }
 
     handleBeforeFetch = () => {
-        // Delay modal opening to allow content to load first
-        clearTimeout(this.openTimeout);
-        this.openTimeout = setTimeout(() => {
+        // Mark that we want to open the modal when content arrives
+        this.pendingOpen = true;
+    };
+
+    handleFrameLoad = () => {
+        // Content has arrived - open modal if we were waiting for it
+        if (this.pendingOpen) {
+            this.pendingOpen = false;
             this.open();
-        }, 150);
+        }
     };
 
     handleMutation = () => {
@@ -75,12 +83,16 @@ export default class extends Controller {
     };
 
     open() {
+        // Don't open modal if frame is empty (safety check for edge cases)
+        if (this.frameTarget.innerHTML.trim() === '') {
+            return;
+        }
         this.modal.show();
         document.body.classList.add('modal-open');
     }
 
     close() {
-        clearTimeout(this.openTimeout);
+        this.pendingOpen = false;
         this.modal.hide();
         document.body.classList.remove('modal-open');
         this.frameTarget.innerHTML = '';

assets/controllers/report_duplicate_form_controller.js

@@ -0,0 +1,123 @@
+import { Controller } from '@hotwired/stimulus';
+
+export default class extends Controller {
+    static targets = ['manufacturer', 'puzzle'];
+
+    static values = {
+        currentPuzzleId: String,
+    };
+
+    uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+
+    initialize() {
+        this._onManufacturerConnect = this._onManufacturerConnect.bind(this);
+        this._onPuzzleConnect = this._onPuzzleConnect.bind(this);
+    }
+
+    connect() {
+        this.manufacturerTarget.addEventListener('autocomplete:pre-connect', this._onManufacturerConnect);
+        this.puzzleTarget.addEventListener('autocomplete:pre-connect', this._onPuzzleConnect);
+    }
+
+    disconnect() {
+        this.manufacturerTarget.removeEventListener('autocomplete:pre-connect', this._onManufacturerConnect);
+        this.puzzleTarget.removeEventListener('autocomplete:pre-connect', this._onPuzzleConnect);
+    }
+
+    _onManufacturerConnect(event) {
+        event.detail.options.onChange = (value) => {
+            this.onManufacturerValueChanged(value);
+        };
+    }
+
+    _onPuzzleConnect(event) {
+        event.detail.options.onChange = (value) => {
+            this.onPuzzleValueChanged(value);
+        };
+
+        // Initialize puzzle field state after TomSelect is ready
+        event.detail.options.onInitialize = () => {
+            this.handleInitialState();
+        };
+    }
+
+    onManufacturerValueChanged(value) {
+        const puzzleTom = this.puzzleTarget.tomselect;
+        if (!puzzleTom) return;
+
+        puzzleTom.clear();
+        puzzleTom.clearOptions();
+
+        if (value && this.uuidRegex.test(value)) {
+            puzzleTom.enable();
+            puzzleTom.settings.placeholder = this.puzzleTarget.dataset.choosePuzzlePlaceholder;
+            puzzleTom.inputState();
+
+            this.fetchPuzzleOptions(value);
+        } else {
+            this.disablePuzzleField();
+        }
+    }
+
+    onPuzzleValueChanged(value) {
+        const puzzleTom = this.puzzleTarget.tomselect;
+        if (value && puzzleTom) {
+            puzzleTom.blur();
+        }
+    }
+
+    fetchPuzzleOptions(manufacturerId) {
+        const fetchUrl = this.manufacturerTarget.getAttribute('data-fetch-url');
+        const currentPuzzleId = this.currentPuzzleIdValue;
+
+        fetch(`${fetchUrl}?brand=${manufacturerId}`)
+            .then(response => {
+                if (!response.ok) {
+                    console.error('Network response was not ok');
+                    return null;
+                }
+                return response.json();
+            })
+            .then(data => {
+                if (data && data.results) {
+                    // Filter out current puzzle from options
+                    const filteredResults = data.results.filter(
+                        puzzle => puzzle.value !== currentPuzzleId
+                    );
+                    this.updatePuzzleSelectValues(filteredResults);
+                }
+            })
+            .catch(error => {
+                console.error('Error fetching puzzle options:', error);
+            });
+    }
+
+    updatePuzzleSelectValues(data) {
+        const puzzleTomSelect = this.puzzleTarget.tomselect;
+        if (!puzzleTomSelect) return;
+
+        puzzleTomSelect.clearOptions();
+        puzzleTomSelect.addOptions(data);
+        puzzleTomSelect.refreshOptions(true);
+    }
+
+    handleInitialState() {
+        // Check if manufacturer already has a value (shouldn't normally happen on fresh form)
+        const manufacturerValue = this.manufacturerTarget.value;
+        if (manufacturerValue && this.uuidRegex.test(manufacturerValue)) {
+            this.fetchPuzzleOptions(manufacturerValue);
+        } else {
+            this.disablePuzzleField();
+        }
+    }
+
+    disablePuzzleField() {
+        const puzzleTomSelect = this.puzzleTarget.tomselect;
+        if (!puzzleTomSelect) return;
+
+        puzzleTomSelect.clearOptions();
+        puzzleTomSelect.disable();
+        puzzleTomSelect.settings.placeholder = this.puzzleTarget.dataset.chooseManufacturerPlaceholder;
+        puzzleTomSelect.inputState();
+    }
+}

config/packages/messenger.php

@@ -12,7 +12,10 @@
         'messenger' => [
             'buses' => [
                 'command_bus' => [
-                    'middleware' => ['doctrine_transaction'],
+                    'middleware' => [
+                        'SpeedPuzzling\Web\Services\MessengerMiddleware\ClearEntityManagerMiddleware',
+                        'doctrine_transaction',
+                    ],
                 ],
             ],
             'failure_transport' => 'failed',
@@ -34,6 +37,12 @@
                 'SpeedPuzzling\Web\Events\PuzzleBorrowed' => 'sync',
                 'SpeedPuzzling\Web\Events\PuzzleAddedToCollection' => 'sync',
                 'SpeedPuzzling\Web\Events\LendingTransferCompleted' => 'sync',
+                // Events that must run synchronously for statistics recalculation
+                'SpeedPuzzling\Web\Events\PuzzleSolved' => 'sync',
+                'SpeedPuzzling\Web\Events\PuzzleSolvingTimeModified' => 'sync',
+                'SpeedPuzzling\Web\Events\PuzzleSolvingTimeDeleted' => 'sync',
+                // Events that must run synchronously for proper transaction ordering
+                'SpeedPuzzling\Web\Events\PuzzleMergeApproved' => 'sync',
                 // All other events can run asynchronously
                 'SpeedPuzzling\Web\Events\*' => 'async',
             ],

migrations/Version20251218233751.php

@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SpeedPuzzling\Web\Migrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Auto-generated Migration: Please modify to your needs!
+ */
+final class Version20251218233751 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Add puzzle change request and merge request tables for community feedback system';
+    }
+
+    public function up(Schema $schema): void
+    {
+        // this up() migration is auto-generated, please modify it to your needs
+        $this->addSql('CREATE TABLE puzzle_change_request (status VARCHAR(255) NOT NULL, reviewed_at TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL, rejection_reason TEXT DEFAULT NULL, id UUID NOT NULL, submitted_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL, proposed_name VARCHAR(255) DEFAULT NULL, proposed_pieces_count INT DEFAULT NULL, proposed_ean VARCHAR(255) DEFAULT NULL, proposed_identification_number VARCHAR(255) DEFAULT NULL, proposed_image VARCHAR(255) DEFAULT NULL, original_name VARCHAR(255) NOT NULL, original_manufacturer_id UUID DEFAULT NULL, original_pieces_count INT NOT NULL, original_ean VARCHAR(255) DEFAULT NULL, original_identification_number VARCHAR(255) DEFAULT NULL, original_image VARCHAR(255) DEFAULT NULL, reviewed_by_id UUID DEFAULT NULL, puzzle_id UUID NOT NULL, reporter_id UUID NOT NULL, proposed_manufacturer_id UUID DEFAULT NULL, PRIMARY KEY (id))');
+        $this->addSql('CREATE INDEX IDX_3668C37CFC6B21F1 ON puzzle_change_request (reviewed_by_id)');
+        $this->addSql('CREATE INDEX IDX_3668C37CD9816812 ON puzzle_change_request (puzzle_id)');
+        $this->addSql('CREATE INDEX IDX_3668C37CE1CFE6F5 ON puzzle_change_request (reporter_id)');
+        $this->addSql('CREATE INDEX IDX_3668C37C4971441F ON puzzle_change_request (proposed_manufacturer_id)');
+        $this->addSql('CREATE TABLE puzzle_merge_request (status VARCHAR(255) NOT NULL, reviewed_at TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL, rejection_reason TEXT DEFAULT NULL, survivor_puzzle_id UUID DEFAULT NULL, merged_puzzle_ids JSON DEFAULT \'[]\' NOT NULL, id UUID NOT NULL, submitted_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL, reported_duplicate_puzzle_ids JSON NOT NULL, reviewed_by_id UUID DEFAULT NULL, source_puzzle_id UUID NOT NULL, reporter_id UUID NOT NULL, PRIMARY KEY (id))');
+        $this->addSql('CREATE INDEX IDX_C53156FBFC6B21F1 ON puzzle_merge_request (reviewed_by_id)');
+        $this->addSql('CREATE INDEX IDX_C53156FBB11FFBDC ON puzzle_merge_request (source_puzzle_id)');
+        $this->addSql('CREATE INDEX IDX_C53156FBE1CFE6F5 ON puzzle_merge_request (reporter_id)');
+        $this->addSql('ALTER TABLE puzzle_change_request ADD CONSTRAINT FK_3668C37CFC6B21F1 FOREIGN KEY (reviewed_by_id) REFERENCES player (id) ON DELETE SET NULL NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE puzzle_change_request ADD CONSTRAINT FK_3668C37CD9816812 FOREIGN KEY (puzzle_id) REFERENCES puzzle (id) ON DELETE CASCADE NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE puzzle_change_request ADD CONSTRAINT FK_3668C37CE1CFE6F5 FOREIGN KEY (reporter_id) REFERENCES player (id) ON DELETE CASCADE NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE puzzle_change_request ADD CONSTRAINT FK_3668C37C4971441F FOREIGN KEY (proposed_manufacturer_id) REFERENCES manufacturer (id) ON DELETE SET NULL NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE puzzle_merge_request ADD CONSTRAINT FK_C53156FBFC6B21F1 FOREIGN KEY (reviewed_by_id) REFERENCES player (id) ON DELETE SET NULL NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE puzzle_merge_request ADD CONSTRAINT FK_C53156FBB11FFBDC FOREIGN KEY (source_puzzle_id) REFERENCES puzzle (id) ON DELETE CASCADE NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE puzzle_merge_request ADD CONSTRAINT FK_C53156FBE1CFE6F5 FOREIGN KEY (reporter_id) REFERENCES player (id) ON DELETE CASCADE NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE notification ADD target_change_request_id UUID DEFAULT NULL');
+        $this->addSql('ALTER TABLE notification ADD target_merge_request_id UUID DEFAULT NULL');
+        $this->addSql('ALTER TABLE notification ADD CONSTRAINT FK_BF5476CA96566C4C FOREIGN KEY (target_change_request_id) REFERENCES puzzle_change_request (id) ON DELETE CASCADE NOT DEFERRABLE');
+        $this->addSql('ALTER TABLE notification ADD CONSTRAINT FK_BF5476CAFB320824 FOREIGN KEY (target_merge_request_id) REFERENCES puzzle_merge_request (id) ON DELETE CASCADE NOT DEFERRABLE');
+        $this->addSql('CREATE INDEX IDX_BF5476CA96566C4C ON notification (target_change_request_id)');
+        $this->addSql('CREATE INDEX IDX_BF5476CAFB320824 ON notification (target_merge_request_id)');
+    }
+
+    public function down(Schema $schema): void
+    {
+        // this down() migration is auto-generated, please modify it to your needs
+        $this->addSql('ALTER TABLE puzzle_change_request DROP CONSTRAINT FK_3668C37CFC6B21F1');
+        $this->addSql('ALTER TABLE puzzle_change_request DROP CONSTRAINT FK_3668C37CD9816812');
+        $this->addSql('ALTER TABLE puzzle_change_request DROP CONSTRAINT FK_3668C37CE1CFE6F5');
+        $this->addSql('ALTER TABLE puzzle_change_request DROP CONSTRAINT FK_3668C37C4971441F');
+        $this->addSql('ALTER TABLE puzzle_merge_request DROP CONSTRAINT FK_C53156FBFC6B21F1');
+        $this->addSql('ALTER TABLE puzzle_merge_request DROP CONSTRAINT FK_C53156FBB11FFBDC');
+        $this->addSql('ALTER TABLE puzzle_merge_request DROP CONSTRAINT FK_C53156FBE1CFE6F5');
+        $this->addSql('DROP TABLE puzzle_change_request');
+        $this->addSql('DROP TABLE puzzle_merge_request');
+        $this->addSql('ALTER TABLE notification DROP CONSTRAINT FK_BF5476CA96566C4C');
+        $this->addSql('ALTER TABLE notification DROP CONSTRAINT FK_BF5476CAFB320824');
+        $this->addSql('DROP INDEX IDX_BF5476CA96566C4C');
+        $this->addSql('DROP INDEX IDX_BF5476CAFB320824');
+        $this->addSql('ALTER TABLE notification DROP target_change_request_id');
+        $this->addSql('ALTER TABLE notification DROP target_merge_request_id');
+    }
+}