🐛 fix: don't bump freshness timestamps on position-only reorder

docs/features.md

@@ -56,7 +56,7 @@ The frontend is built with **React.js** (via Symfony UX / Webpack Encore) for al
 | F2.30  | Personal deck flag                   | Medium   | Done   | Owner can mark any deck (Expanded **or** Standard) as **personal**: excluded from borrow workflow and event registration. **Orthogonal to `public`** — a personal deck can still be public and URL-viewable (showcased without being lendable). Printable labels (F5.1) and lost-and-found resolution (F5.3) still work. A "Personal" badge appears on the owner's deck list alongside other badges. Cannot toggle on while the deck has active borrows or event registrations. Complements F2.23 (`DeckFormat::Standard`) — Standard already excludes from lending; this flag extends the same opt-out to Expanded-format decks. Depends on F2.1, F2.23. |
 | F2.28  | Preserve imported list order         | Low      | Done   | When a deck list is pasted (`DeckListParser`), capture the source-line index on each parsed card and persist it in a new `DeckCard.sortOrder` column (indexed alongside `deck_version_id`). New imports populate `sortOrder` automatically at every controller call site. Historical decks have null `sortOrder`; an admin dashboard tile (`AdminTechnicalController`) dispatches a `BackfillDeckCardSortOrderMessage` per version with a stored `rawList`, and the handler re-parses the raw list and matches cards by `(setCode, cardNumber, cardName)` to populate the column idempotently. The deck-show UI toggle ("Grouped" / "Import order") is deferred to a follow-up issue — this row covers the data layer only. Depends on F2.1, F6.1. |
 | F2.29  | Restrict inline archetype creation to editors | Medium | Done | The deck creation/edit forms' archetype combobox previously let any authenticated user inline-create a brand-new archetype. Tightened to **`ROLE_ARCHETYPE_EDITOR`**: only editors see the "Create new archetype" affordance and can POST to `/api/archetype` (server-side gate on `ArchetypeController` mirroring `AdminArchetypeController`). Non-editors see a graceful empty state ("No matching archetype. Ask an archetype editor to add it.") and receive a 403 if they bypass the UI. Protects the curated archetype catalogue from accidental pollution (typos, duplicates, casing variants, jokes). Depends on F1.4, F2.6. |
-| F2.27  | Archetype publication dates                  | Low      | Done   | Strict publish semantic on `Archetype`: nullable `firstPublishedAt` is stamped the first time `isPublished` transitions to true; `lastPublishedAt` refreshes on every persist while the entity is published. Drafts never bump either field. Shared with CMS pages (F11.4) through `PublishableTimestampsTrait`. **Variant-driven freshness:** `ArchetypeFreshnessListener` (`postPersist` + `postUpdate` on `Deck` / `DeckVersion`, deferred to `postFlush`) bumps `Archetype.lastPublishedAt` whenever one of the archetype's variant decks (`owner IS NULL`) or its versions is created or modified, so the catalog reflects real variant activity rather than only direct edits to the archetype metadata. The bump is emitted as a single bulk SQL `UPDATE` after the originating flush to avoid re-entering Archetype's own lifecycle. The initial migration's backfill mirrors the same rule via `GREATEST(updated_at, max variant Deck.updated_at, max variant DeckVersion.created_at)` so production rows arrive with already-correct freshness at deploy time. **Archetype catalog (F2.16):** each card displays "Updated on <long-date>" when republished or after variant churn, "Published on <long-date>" otherwise. A new "Most recently updated" sort option (`?sort=updatedAt`) orders the list by `COALESCE(lastPublishedAt, firstPublishedAt, createdAt) DESC` — the `COALESCE` keeps the sort stable for rows that pre-date the backfill or were never republished. **Archetype detail (F2.10) variant selector (F18.16):** each variant carries an `effectiveUpdatedAt = max(Deck.updatedAt, latest DeckVersion.createdAt)` computed in a single non-N+1 lookup (`DeckRepository::findEffectiveUpdatedAtByDeckIds()`); the React selector renders "Updated on <date>" under the selected variant. **JSON-LD (F18.27):** the `Article` payload now uses the publication timestamps for `datePublished` / `dateModified` instead of falling back to `createdAt`/`updatedAt`. Backfilled migration sets the two columns from `created_at` / `updated_at` for previously published rows. Depends on F2.6, F2.10, F2.16, F18.16. |
+| F2.27  | Archetype publication dates                  | Low      | Done   | Strict publish semantic on `Archetype`: nullable `firstPublishedAt` is stamped the first time `isPublished` transitions to true; `lastPublishedAt` refreshes on every persist while the entity is published. Drafts never bump either field. Shared with CMS pages (F11.4) through `PublishableTimestampsTrait`. **Variant-driven freshness:** `ArchetypeFreshnessListener` (`postPersist` + `preUpdate` on `Deck` / `DeckVersion`, deferred to `postFlush`) bumps `Archetype.lastPublishedAt` whenever one of the archetype's variant decks (`owner IS NULL`) or its versions is created or modified, so the catalog reflects real variant activity rather than only direct edits to the archetype metadata. The update collection runs on `preUpdate` (not `postUpdate`) so Doctrine's change-set is reliably available: a **position-only change is skipped** — drag-and-drop variant reordering (F18.19) is structural, not content activity, and must not bump freshness. The reorder guard is shared by the entities themselves via `StructuralChangeTrait` (`Archetype` reorder F18.11/F18.12 and `Deck` variant reorder F18.19 leave `updatedAt` / `lastPublishedAt` untouched when `position` is the only changed field). The bump is emitted as a single bulk SQL `UPDATE` after the originating flush to avoid re-entering Archetype's own lifecycle. The initial migration's backfill mirrors the same rule via `GREATEST(updated_at, max variant Deck.updated_at, max variant DeckVersion.created_at)` so production rows arrive with already-correct freshness at deploy time. **Archetype catalog (F2.16):** each card displays "Updated on <long-date>" when republished or after variant churn, "Published on <long-date>" otherwise. A new "Most recently updated" sort option (`?sort=updatedAt`) orders the list by `COALESCE(lastPublishedAt, firstPublishedAt, createdAt) DESC` — the `COALESCE` keeps the sort stable for rows that pre-date the backfill or were never republished. **Archetype detail (F2.10) variant selector (F18.16):** each variant carries an `effectiveUpdatedAt = max(Deck.updatedAt, latest DeckVersion.createdAt)` computed in a single non-N+1 lookup (`DeckRepository::findEffectiveUpdatedAtByDeckIds()`); the React selector renders "Updated on <date>" under the selected variant. **JSON-LD (F18.27):** the `Article` payload now uses the publication timestamps for `datePublished` / `dateModified` instead of falling back to `createdAt`/`updatedAt`. Backfilled migration sets the two columns from `created_at` / `updated_at` for previously published rows. Depends on F2.6, F2.10, F2.16, F18.16. |
 
 ## F3 — Event Management
 

docs/models/deck.md

@@ -31,7 +31,7 @@ Represents a **physical** Pokemon TCG deck — the deck box with a label. A deck
 `DeckCard` gains a nullable `sortOrder` (int) column, indexed on `(deck_version_id, sort_order)`. New imports record the zero-based source-line index of each card in the rawList; historical rows have `null` until the admin dashboard backfill runs. The rendering path stays grouped by default — the "Import order" toggle is a follow-up.
 | `currentVersion`   | `DeckVersion`      | Yes      | The latest/active version of this deck. Null only before the first list import. |
 | `createdAt`        | `DateTimeImmutable` | No      | Deck registration timestamp. |
-| `updatedAt`        | `DateTimeImmutable` | Yes     | Last modification timestamp. |
+| `updatedAt`        | `DateTimeImmutable` | Yes     | Last modification timestamp. A position-only reorder does not bump it (see reorder rule below). |
 
 ### Status Enum: `App\Enum\DeckStatus`
 
@@ -155,7 +155,7 @@ A managed archetype entry representing a deck strategy (e.g. "Lugia VSTAR", "Iro
 | `isPublished`      | `bool`             | No       | Controls visibility of the archetype detail page (F2.10). Default: `false`. |
 | `deletedAt`        | `DateTimeImmutable` | Yes     | Soft-delete timestamp. Null = active. When set, the archetype is hidden from all lists (admin and public) and its detail page returns 404. See Soft-Delete Rules below. |
 | `createdAt`        | `DateTimeImmutable` | No      | Creation timestamp. |
-| `updatedAt`        | `DateTimeImmutable` | Yes     | Last modification timestamp. |
+| `updatedAt`        | `DateTimeImmutable` | Yes     | Last modification timestamp. A position-only reorder does not bump it (see reorder rule below). |
 | `firstPublishedAt` | `DateTimeImmutable` | Yes     | First time `isPublished` transitioned to true (F2.27). Drafts stay null. Powers the catalog freshness caption and the `Article` JSON-LD `datePublished`. |
 | `lastPublishedAt`  | `DateTimeImmutable` | Yes     | Most recent persist while published (F2.27). Drafts and unpublish saves never bump it. Powers the catalog "Updated on" caption and `Article` JSON-LD `dateModified`. |
 
@@ -165,6 +165,7 @@ A managed archetype entry representing a deck strategy (e.g. "Lugia VSTAR", "Iro
 - `slug`: required, unique, auto-generated from name via `AsciiSlugger`
 - `metaDescription`: max 255 characters
 - `firstPublishedAt` / `lastPublishedAt`: managed via [`PublishableTimestampsTrait`](../../src/Entity/PublishableTimestampsTrait.php); shared with [`Page`](cms.md#page) (F11.4). Variant freshness is computed lazily by `DeckRepository::findEffectiveUpdatedAtByDeckIds()` — no schema change on `Deck`
+- **Reorder rule:** a drag-and-drop reorder that changes only `position` is structural, not a content update, and must not bump `updatedAt` / `lastPublishedAt`. Both `Archetype` and `Deck` use [`StructuralChangeTrait`](../../src/Entity/StructuralChangeTrait.php) in their `#[ORM\PreUpdate]` hook to skip stamping when `position` is the sole changed field; `ArchetypeFreshnessListener` applies the same guard so a variant reorder does not bump the parent archetype's freshness either. Reordering archetypes is F18.11/F18.12; reordering variants within an archetype is F18.19
 - **Soft-delete guard:** an archetype can only be soft-deleted when it has **zero associated decks** (including non-public and retired decks). If any deck references the archetype, deletion is refused with an error message
 
 ### Relations

src/Entity/Archetype.php

@@ -32,6 +32,7 @@
 class Archetype
 {
     use PublishableTimestampsTrait;
+    use StructuralChangeTrait;
 
     #[ORM\Id]
     #[ORM\GeneratedValue]
@@ -311,6 +312,12 @@ public function onPrePersist(): void
     #[ORM\PreUpdate]
     public function onPreUpdate(PreUpdateEventArgs $args): void
     {
+        // A drag-and-drop reorder only moves `position`; that is not a content
+        // update, so leave the freshness timestamps untouched (F18.11).
+        if ($this->isStructuralOnlyChange($args)) {
+            return;
+        }
+
         $this->updatedAt = new \DateTimeImmutable();
         $this->generateSlug();
         $this->stampPublicationOnUpdate($args);

src/Entity/Deck.php

@@ -19,6 +19,7 @@
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Collection;
 use Doctrine\DBAL\Types\Types;
+use Doctrine\ORM\Event\PreUpdateEventArgs;
 use Doctrine\ORM\Mapping as ORM;
 use Symfony\Component\Validator\Constraints as Assert;
 
@@ -29,6 +30,8 @@
 #[ORM\HasLifecycleCallbacks]
 class Deck
 {
+    use StructuralChangeTrait;
+
     #[ORM\Id]
     #[ORM\GeneratedValue]
     #[ORM\Column]
@@ -546,8 +549,15 @@ public function onPrePersist(): void
     }
 
     #[ORM\PreUpdate]
-    public function onPreUpdate(): void
+    public function onPreUpdate(PreUpdateEventArgs $args): void
     {
+        // Variant reordering only moves `position`; that is not a content
+        // update, so leave `updatedAt` (sitemap lastmod, JSON-LD dateModified)
+        // untouched (F18.19).
+        if ($this->isStructuralOnlyChange($args)) {
+            return;
+        }
+
         $this->updatedAt = new \DateTimeImmutable();
     }
 

src/Entity/StructuralChangeTrait.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Expanded Decks project.
+ *
+ * (c) Expanded Decks contributors
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Entity;
+
+use Doctrine\ORM\Event\PreUpdateEventArgs;
+
+/**
+ * Lets a `#[ORM\PreUpdate]` hook distinguish a "real" content update from a
+ * purely structural one (display ordering). Reordering an entity should not be
+ * advertised as a content change to users or search engines, so timestamp
+ * stamping is skipped when `position` is the only field that moved.
+ *
+ * @see docs/features.md F18.11 — Archetype relevance ordering
+ * @see docs/features.md F18.19 — Archetype variant ordering
+ */
+trait StructuralChangeTrait
+{
+    /**
+     * Display-ordering fields — changing one of these alone is not a content update.
+     *
+     * @var list<string>
+     */
+    private const array STRUCTURAL_ONLY_FIELDS = ['position'];
+
+    private function isStructuralOnlyChange(PreUpdateEventArgs $args): bool
+    {
+        $changedFields = array_keys($args->getEntityChangeSet());
+
+        return [] !== $changedFields
+            && [] === array_diff($changedFields, self::STRUCTURAL_ONLY_FIELDS);
+    }
+}

src/EventListener/ArchetypeFreshnessListener.php

@@ -19,7 +19,7 @@
 use Doctrine\DBAL\ArrayParameterType;
 use Doctrine\ORM\Event\PostFlushEventArgs;
 use Doctrine\ORM\Event\PostPersistEventArgs;
-use Doctrine\ORM\Event\PostUpdateEventArgs;
+use Doctrine\ORM\Event\PreUpdateEventArgs;
 use Doctrine\ORM\Events;
 
 /**
@@ -35,10 +35,18 @@
  * @see docs/features.md F2.27 — Archetype publication dates
  */
 #[AsDoctrineListener(event: Events::postPersist)]
-#[AsDoctrineListener(event: Events::postUpdate)]
+#[AsDoctrineListener(event: Events::preUpdate)]
 #[AsDoctrineListener(event: Events::postFlush)]
 final class ArchetypeFreshnessListener
 {
+    /**
+     * Display-ordering fields whose change alone is not variant activity.
+     * Mirrors {@see \App\Entity\StructuralChangeTrait}.
+     *
+     * @var list<string>
+     */
+    private const array STRUCTURAL_ONLY_FIELDS = ['position'];
+
     /** @var array<int, true> */
     private array $pendingArchetypeIds = [];
 
@@ -47,8 +55,17 @@ public function postPersist(PostPersistEventArgs $args): void
         $this->collect($args->getObject());
     }
 
-    public function postUpdate(PostUpdateEventArgs $args): void
+    public function preUpdate(PreUpdateEventArgs $args): void
     {
+        // A pure reorder only moves `position`; it isn't variant activity that
+        // should refresh the archetype's freshness signal (F18.19). The
+        // change-set is read here (preUpdate) because that is where Doctrine
+        // reliably exposes it.
+        $changedFields = array_keys($args->getEntityChangeSet());
+        if ([] !== $changedFields && [] === array_diff($changedFields, self::STRUCTURAL_ONLY_FIELDS)) {
+            return;
+        }
+
         $this->collect($args->getObject());
     }
 

tests/Entity/DeckTest.php

@@ -19,6 +19,7 @@
 use App\Entity\User;
 use App\Enum\DeckFormat;
 use App\Enum\DeckStatus;
+use Doctrine\ORM\Event\PreUpdateEventArgs;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -94,7 +95,11 @@ public function testOnPreUpdateSetsUpdatedAt(): void
         $deck = new Deck();
         self::assertNull($deck->getUpdatedAt());
 
-        $deck->onPreUpdate();
+        // A content change (here: name) must stamp updatedAt.
+        $args = $this->createStub(PreUpdateEventArgs::class);
+        $args->method('getEntityChangeSet')->willReturn(['name' => ['Old', 'New']]);
+
+        $deck->onPreUpdate($args);
 
         self::assertInstanceOf(\DateTimeImmutable::class, $deck->getUpdatedAt());
     }

tests/Entity/StructuralChangeTest.php

@@ -0,0 +1,114 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Expanded Decks project.
+ *
+ * (c) Expanded Decks contributors
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Entity;
+
+use App\Entity\Archetype;
+use App\Entity\Deck;
+use Doctrine\ORM\Event\PreUpdateEventArgs;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * A position-only change (drag-and-drop reorder) is structural, not a content
+ * update, so it must not bump any freshness timestamp.
+ *
+ * @see docs/features.md F18.11 — Archetype relevance ordering
+ * @see docs/features.md F18.19 — Archetype variant ordering
+ */
+final class StructuralChangeTest extends TestCase
+{
+    /**
+     * @param array<string, array{mixed, mixed}> $changeSet
+     */
+    private function changeSetArgs(array $changeSet): PreUpdateEventArgs
+    {
+        $args = $this->createStub(PreUpdateEventArgs::class);
+        $args->method('getEntityChangeSet')->willReturn($changeSet);
+        $args->method('hasChangedField')->willReturn(false);
+
+        return $args;
+    }
+
+    private function setTimestamp(object $entity, string $property, ?\DateTimeImmutable $value): void
+    {
+        $reflection = new \ReflectionProperty($entity, $property);
+        $reflection->setValue($entity, $value);
+    }
+
+    public function testArchetypePositionOnlyChangeKeepsTimestamps(): void
+    {
+        $original = new \DateTimeImmutable('2024-01-01T10:00:00+00:00');
+        $archetype = (new Archetype())->setName('Iron Thorns');
+        $archetype->setIsPublished(true);
+        $this->setTimestamp($archetype, 'updatedAt', $original);
+        $this->setTimestamp($archetype, 'lastPublishedAt', $original);
+
+        $archetype->onPreUpdate($this->changeSetArgs(['position' => [0, 5]]));
+
+        self::assertSame($original, $archetype->getUpdatedAt());
+        self::assertSame($original, $archetype->getLastPublishedAt());
+    }
+
+    public function testArchetypeContentChangeBumpsTimestamps(): void
+    {
+        $original = new \DateTimeImmutable('2024-01-01T10:00:00+00:00');
+        $archetype = (new Archetype())->setName('Iron Thorns');
+        $archetype->setIsPublished(true);
+        $this->setTimestamp($archetype, 'updatedAt', $original);
+        $this->setTimestamp($archetype, 'lastPublishedAt', $original);
+
+        $archetype->onPreUpdate($this->changeSetArgs(['name' => ['Iron Thorns', 'Iron Thorns ex']]));
+
+        self::assertGreaterThan($original, $archetype->getUpdatedAt());
+        self::assertGreaterThan($original, $archetype->getLastPublishedAt());
+    }
+
+    public function testArchetypePositionAlongsideContentBumpsTimestamps(): void
+    {
+        $original = new \DateTimeImmutable('2024-01-01T10:00:00+00:00');
+        $archetype = (new Archetype())->setName('Iron Thorns');
+        $archetype->setIsPublished(true);
+        $this->setTimestamp($archetype, 'updatedAt', $original);
+        $this->setTimestamp($archetype, 'lastPublishedAt', $original);
+
+        $archetype->onPreUpdate($this->changeSetArgs([
+            'position' => [0, 5],
+            'name' => ['Iron Thorns', 'Iron Thorns ex'],
+        ]));
+
+        self::assertGreaterThan($original, $archetype->getUpdatedAt());
+        self::assertGreaterThan($original, $archetype->getLastPublishedAt());
+    }
+
+    public function testDeckPositionOnlyChangeKeepsUpdatedAt(): void
+    {
+        $original = new \DateTimeImmutable('2024-01-01T10:00:00+00:00');
+        $deck = (new Deck())->setName('Variant A');
+        $this->setTimestamp($deck, 'updatedAt', $original);
+
+        $deck->onPreUpdate($this->changeSetArgs(['position' => [0, 1]]));
+
+        self::assertSame($original, $deck->getUpdatedAt());
+    }
+
+    public function testDeckContentChangeBumpsUpdatedAt(): void
+    {
+        $original = new \DateTimeImmutable('2024-01-01T10:00:00+00:00');
+        $deck = (new Deck())->setName('Variant A');
+        $this->setTimestamp($deck, 'updatedAt', $original);
+
+        $deck->onPreUpdate($this->changeSetArgs(['name' => ['Variant A', 'Variant B']]));
+
+        self::assertGreaterThan($original, $deck->getUpdatedAt());
+    }
+}