diff --git a/docs/features.md b/docs/features.md index 2c5594e1..e2d0f07d 100644 --- a/docs/features.md +++ b/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 " when republished or after variant churn, "Published on " 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 " 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 " when republished or after variant churn, "Published on " 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 " 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 diff --git a/docs/models/deck.md b/docs/models/deck.md index c788e77a..a1040c72 100644 --- a/docs/models/deck.md +++ b/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 diff --git a/src/Entity/Archetype.php b/src/Entity/Archetype.php index d9893afa..4b86d3b1 100644 --- a/src/Entity/Archetype.php +++ b/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); diff --git a/src/Entity/Deck.php b/src/Entity/Deck.php index cc95cdea..58122c25 100644 --- a/src/Entity/Deck.php +++ b/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(); } diff --git a/src/Entity/StructuralChangeTrait.php b/src/Entity/StructuralChangeTrait.php new file mode 100644 index 00000000..26b3c005 --- /dev/null +++ b/src/Entity/StructuralChangeTrait.php @@ -0,0 +1,43 @@ + + */ + 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); + } +} diff --git a/src/EventListener/ArchetypeFreshnessListener.php b/src/EventListener/ArchetypeFreshnessListener.php index 54daa466..5ea9e6d3 100644 --- a/src/EventListener/ArchetypeFreshnessListener.php +++ b/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 + */ + private const array STRUCTURAL_ONLY_FIELDS = ['position']; + /** @var array */ 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()); } diff --git a/tests/Entity/DeckTest.php b/tests/Entity/DeckTest.php index cad8ec74..e8711470 100644 --- a/tests/Entity/DeckTest.php +++ b/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()); } diff --git a/tests/Entity/StructuralChangeTest.php b/tests/Entity/StructuralChangeTest.php new file mode 100644 index 00000000..f3cde891 --- /dev/null +++ b/tests/Entity/StructuralChangeTest.php @@ -0,0 +1,114 @@ + $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()); + } +} diff --git a/tests/Functional/ArchetypeFreshnessListenerTest.php b/tests/Functional/ArchetypeFreshnessListenerTest.php index b0fbd308..ece61998 100644 --- a/tests/Functional/ArchetypeFreshnessListenerTest.php +++ b/tests/Functional/ArchetypeFreshnessListenerTest.php @@ -100,4 +100,67 @@ public function testOwnedDeckDoesNotBumpArchetype(): void $em->refresh($archetype); self::assertEquals($before, $archetype->getLastPublishedAt()); } + + /** + * A drag-and-drop variant reorder only moves `position`; it must touch + * neither the archetype's freshness signal nor the deck's own `updatedAt`. + * + * @see docs/features.md F18.19 — Archetype variant ordering + */ + public function testReorderingAVariantDoesNotBumpTimestamps(): void + { + $em = $this->getEntityManager(); + + $archetype = $em->getRepository(Archetype::class)->findOneBy(['slug' => 'regidrago']); + self::assertNotNull($archetype); + $em->refresh($archetype); + $archetypeBefore = $archetype->getLastPublishedAt(); + self::assertNotNull($archetypeBefore); + + $variant = $em->getRepository(Deck::class)->findOneBy(['archetype' => $archetype, 'owner' => null]); + self::assertNotNull($variant); + $deckBefore = $variant->getUpdatedAt(); + + sleep(1); + + $variant->setPosition($variant->getPosition() + 100); + $em->flush(); + + $em->refresh($archetype); + $em->refresh($variant); + + self::assertEquals($archetypeBefore, $archetype->getLastPublishedAt()); + self::assertEquals($deckBefore, $variant->getUpdatedAt()); + } + + /** + * Editing a variant's content (not its position) is real activity and must + * still bump both the archetype's freshness signal and the deck timestamp. + * + * @see docs/features.md F18.19 — Archetype variant ordering + */ + public function testEditingAVariantStillBumpsTimestamps(): void + { + $em = $this->getEntityManager(); + + $archetype = $em->getRepository(Archetype::class)->findOneBy(['slug' => 'regidrago']); + self::assertNotNull($archetype); + $em->refresh($archetype); + $archetypeBefore = $archetype->getLastPublishedAt(); + self::assertNotNull($archetypeBefore); + + $variant = $em->getRepository(Deck::class)->findOneBy(['archetype' => $archetype, 'owner' => null]); + self::assertNotNull($variant); + + sleep(1); + + $variant->setName($variant->getName().' (edited)'); + $em->flush(); + + $em->refresh($archetype); + $em->refresh($variant); + + self::assertGreaterThan($archetypeBefore, $archetype->getLastPublishedAt()); + self::assertNotNull($variant->getUpdatedAt()); + } }