From c9460f97874480b61da30f5e15a9157e6214784a Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 27 May 2026 15:30:15 +0200 Subject: [PATCH 1/2] feat(shared) : add Timestampable + Blamable Shared pattern (Trait + Interfaces + Subscriber + test) --- config/packages/doctrine.yaml | 4 + .../Domain/Contract/BlamableInterface.php | 30 ++++ .../Contract/TimestampableInterface.php | 25 +++ .../Trait/TimestampableBlamableTrait.php | 83 +++++++++ .../TimestampableBlamableSubscriber.php | 65 +++++++ .../EntitiesAreTimestampableBlamableTest.php | 124 ++++++++++++++ .../TimestampableBlamableSubscriberTest.php | 159 ++++++++++++++++++ 7 files changed, 490 insertions(+) create mode 100644 src/Shared/Domain/Contract/BlamableInterface.php create mode 100644 src/Shared/Domain/Contract/TimestampableInterface.php create mode 100644 src/Shared/Domain/Trait/TimestampableBlamableTrait.php create mode 100644 src/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriber.php create mode 100644 tests/Architecture/EntitiesAreTimestampableBlamableTest.php create mode 100644 tests/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriberTest.php diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index 48eb309..c7ade33 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -33,6 +33,10 @@ doctrine: # `App\Module\Sites\Domain\Entity\Site` dans User.php. resolve_target_entities: App\Shared\Domain\Contract\SiteInterface: App\Module\Sites\Domain\Entity\Site + # Cible des ManyToOne created_by / updated_by du TimestampableBlamableTrait. + # Permet a Shared de referencer UserInterface dans ses ORM mappings sans + # importer la classe concrete du module Core (cf. spec-back M0 § 2.8). + Symfony\Component\Security\Core\User\UserInterface: App\Module\Core\Domain\Entity\User mappings: Core: type: attribute diff --git a/src/Shared/Domain/Contract/BlamableInterface.php b/src/Shared/Domain/Contract/BlamableInterface.php new file mode 100644 index 0000000..327d47c --- /dev/null +++ b/src/Shared/Domain/Contract/BlamableInterface.php @@ -0,0 +1,30 @@ +createdAt; + } + + public function setCreatedAt(DateTimeImmutable $createdAt): void + { + $this->createdAt = $createdAt; + } + + public function getUpdatedAt(): ?DateTimeImmutable + { + return $this->updatedAt; + } + + public function setUpdatedAt(DateTimeImmutable $updatedAt): void + { + $this->updatedAt = $updatedAt; + } + + public function getCreatedBy(): ?UserInterface + { + return $this->createdBy; + } + + public function setCreatedBy(?UserInterface $user): void + { + $this->createdBy = $user; + } + + public function getUpdatedBy(): ?UserInterface + { + return $this->updatedBy; + } + + public function setUpdatedBy(?UserInterface $user): void + { + $this->updatedBy = $user; + } +} diff --git a/src/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriber.php b/src/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriber.php new file mode 100644 index 0000000..99233f6 --- /dev/null +++ b/src/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriber.php @@ -0,0 +1,65 @@ +getObject(); + $now = new DateTimeImmutable(); + $user = $this->security->getUser(); + + if ($entity instanceof TimestampableInterface) { + $entity->setCreatedAt($now); + $entity->setUpdatedAt($now); + } + + if ($entity instanceof BlamableInterface && $user instanceof UserInterface) { + $entity->setCreatedBy($user); + $entity->setUpdatedBy($user); + } + } + + public function preUpdate(PreUpdateEventArgs $args): void + { + $entity = $args->getObject(); + $user = $this->security->getUser(); + + if ($entity instanceof TimestampableInterface) { + $entity->setUpdatedAt(new DateTimeImmutable()); + } + + if ($entity instanceof BlamableInterface && $user instanceof UserInterface) { + $entity->setUpdatedBy($user); + } + } +} diff --git a/tests/Architecture/EntitiesAreTimestampableBlamableTest.php b/tests/Architecture/EntitiesAreTimestampableBlamableTest.php new file mode 100644 index 0000000..e6fd407 --- /dev/null +++ b/tests/Architecture/EntitiesAreTimestampableBlamableTest.php @@ -0,0 +1,124 @@ +/Domain/Entity/` + * et verifie qu'elles implementent TimestampableInterface ET BlamableInterface + * (via TimestampableBlamableTrait). Empeche tout oubli du pattern sur une + * nouvelle entite metier : la CI passe au rouge. + * + * @internal + */ +final class EntitiesAreTimestampableBlamableTest extends TestCase +{ + /** + * Entites explicitement exemptees du pattern. + * + * Au M0, on whiteliste les 4 entites preexistantes du noyau (creees avant + * l'introduction du pattern) : leur retrofit est une decision archi a part + * entiere, hors scope ERP-52. + * + * - User : referentiel d'authentification, createdAt gere manuellement dans + * le constructeur. Retrofit hors scope M0 (cf. HP-9) : impose de trancher + * la recursion Blamable (un User cree par un User) + casse des tests + * existants. + * - Role : referentiel RBAC synchronise via `app:sync-permissions`, pas de + * tracabilite user-driven necessaire. + * - Permission : idem Role (synchronise, pas pilote utilisateur). + * - Site : referentiel admin-managed, a integrer dans un futur module Sites + * v2 (cf. HP-10). + * + * Les futurs referentiels statiques (ex: CategoryType au ticket 0.2) + * s'ajoutent ici avec une justification. + */ + private const EXCLUDED = [ + User::class, + Role::class, + Permission::class, + Site::class, + ]; + + public function testAllBusinessEntitiesImplementBothInterfaces(): void + { + // Garde : chaque entree de la whitelist doit pointer sur une classe + // reelle. Empeche un FQCN errone de masquer silencieusement un oubli. + foreach (self::EXCLUDED as $excluded) { + self::assertTrue(class_exists($excluded), sprintf('Classe whitelistee inexistante : %s', $excluded)); + } + + $finder = new Finder() + ->files() + ->in(__DIR__.'/../../src/Module') + ->path('Domain/Entity') + ->name('*.php') + ; + + // Garde : si le scan ne trouve rien, le chemin est casse — le test + // deviendrait un faux positif vert. On verifie qu'il a du grain a moudre. + self::assertNotEmpty(iterator_to_array($finder), 'Aucune entite scannee : chemin src/Module invalide ?'); + + foreach ($finder as $file) { + $fqcn = $this->extractFqcn($file->getRealPath()); + if (null === $fqcn || in_array($fqcn, self::EXCLUDED, true)) { + continue; + } + + $reflection = new ReflectionClass($fqcn); + // On ignore les classes abstraites et tout ce qui n'est pas une + // entite Doctrine (value objects, embeddables non mappes, etc.). + if ($reflection->isAbstract() || [] === $reflection->getAttributes(Entity::class)) { + continue; + } + + self::assertTrue( + $reflection->implementsInterface(TimestampableInterface::class) + && $reflection->implementsInterface(BlamableInterface::class), + sprintf( + 'L\'entite %s doit implementer TimestampableInterface ET BlamableInterface ' + .'(utiliser TimestampableBlamableTrait). Si c\'est un referentiel statique ' + .'justifie, l\'ajouter dans EntitiesAreTimestampableBlamableTest::EXCLUDED.', + $fqcn, + ), + ); + } + } + + /** + * Extrait le FQCN (namespace + classe) d'un fichier PHP par lecture du + * source, sans charger le fichier. + */ + private function extractFqcn(string $path): ?string + { + $source = file_get_contents($path); + if (false === $source) { + return null; + } + + if ( + 1 !== preg_match('/^namespace\s+([^;]+);/m', $source, $nsMatch) + || 1 !== preg_match('/^(?:final\s+|abstract\s+|readonly\s+)*class\s+(\w+)/m', $source, $classMatch) + ) { + return null; + } + + return trim($nsMatch[1]).'\\'.$classMatch[1]; + } +} diff --git a/tests/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriberTest.php b/tests/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriberTest.php new file mode 100644 index 0000000..b101606 --- /dev/null +++ b/tests/Shared/Infrastructure/Doctrine/TimestampableBlamableSubscriberTest.php @@ -0,0 +1,159 @@ +createStub(UserInterface::class); + $subscriber = new TimestampableBlamableSubscriber($this->securityReturning($user)); + $entity = new FullAuditableFixture(); + + $subscriber->prePersist($this->prePersistArgs($entity)); + + // Les 4 colonnes sont remplies : dates posees, blame = user courant. + self::assertInstanceOf(DateTimeImmutable::class, $entity->getCreatedAt()); + self::assertInstanceOf(DateTimeImmutable::class, $entity->getUpdatedAt()); + self::assertSame($entity->getCreatedAt(), $entity->getUpdatedAt()); + self::assertSame($user, $entity->getCreatedBy()); + self::assertSame($user, $entity->getUpdatedBy()); + } + + public function testPrePersistWithoutUser(): void + { + $subscriber = new TimestampableBlamableSubscriber($this->securityReturning(null)); + $entity = new FullAuditableFixture(); + + $subscriber->prePersist($this->prePersistArgs($entity)); + + // Hors contexte HTTP (CLI / cron) : dates remplies, blame laisse a null. + self::assertInstanceOf(DateTimeImmutable::class, $entity->getCreatedAt()); + self::assertInstanceOf(DateTimeImmutable::class, $entity->getUpdatedAt()); + self::assertNull($entity->getCreatedBy()); + self::assertNull($entity->getUpdatedBy()); + } + + public function testPreUpdate(): void + { + $user = $this->createStub(UserInterface::class); + $subscriber = new TimestampableBlamableSubscriber($this->securityReturning($user)); + + // On simule une entite deja persistee : createdAt fige dans le passe, + // createdBy positionne par une creation anterieure. + $createdAt = new DateTimeImmutable('2020-01-01 10:00:00'); + $entity = new FullAuditableFixture(); + $entity->setCreatedAt($createdAt); + $entity->setUpdatedAt($createdAt); + + $subscriber->preUpdate($this->preUpdateArgs($entity)); + + // updatedAt avance, createdAt reste fige, updatedBy = user courant. + self::assertSame($createdAt, $entity->getCreatedAt()); + self::assertGreaterThan($createdAt, $entity->getUpdatedAt()); + self::assertSame($user, $entity->getUpdatedBy()); + } + + public function testPartialEntityTimestampableOnly(): void + { + $user = $this->createStub(UserInterface::class); + $subscriber = new TimestampableBlamableSubscriber($this->securityReturning($user)); + $entity = new TimestampableOnlyFixture(); + + // Entite Timestampable mais NON Blamable : seules les dates sont posees, + // aucun appel de blame (et aucune erreur). + $subscriber->prePersist($this->prePersistArgs($entity)); + + self::assertInstanceOf(DateTimeImmutable::class, $entity->getCreatedAt()); + self::assertInstanceOf(DateTimeImmutable::class, $entity->getUpdatedAt()); + } + + /** + * Security stubbee renvoyant l'utilisateur fourni (ou null). + */ + private function securityReturning(?UserInterface $user): Security + { + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn($user); + + return $security; + } + + private function prePersistArgs(object $entity): PrePersistEventArgs + { + return new PrePersistEventArgs($entity, $this->createStub(EntityManagerInterface::class)); + } + + private function preUpdateArgs(object $entity): PreUpdateEventArgs + { + $changeSet = []; + + return new PreUpdateEventArgs($entity, $this->createStub(EntityManagerInterface::class), $changeSet); + } +} + +/** + * Fixture interne : entite metier complete (Timestampable + Blamable) via le + * Trait reel teste. + * + * @internal + */ +final class FullAuditableFixture implements TimestampableInterface, BlamableInterface +{ + use TimestampableBlamableTrait; +} + +/** + * Fixture interne : entite Timestampable seule (sans Blamable), pour verifier + * la dissociation des deux contrats par le Subscriber. + * + * @internal + */ +final class TimestampableOnlyFixture implements TimestampableInterface +{ + private ?DateTimeImmutable $createdAt = null; + private ?DateTimeImmutable $updatedAt = null; + + public function getCreatedAt(): ?DateTimeImmutable + { + return $this->createdAt; + } + + public function setCreatedAt(DateTimeImmutable $createdAt): void + { + $this->createdAt = $createdAt; + } + + public function getUpdatedAt(): ?DateTimeImmutable + { + return $this->updatedAt; + } + + public function setUpdatedAt(DateTimeImmutable $updatedAt): void + { + $this->updatedAt = $updatedAt; + } +} -- 2.39.5 From 5ed775cf86a6bd8c948858ddf3d07b41502ccb4a Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 27 May 2026 15:31:45 +0200 Subject: [PATCH 2/2] docs(catalog) : document EXCLUDED rationale and add HP-9/HP-10 --- .claude/rules/backend.md | 21 +++++++++++++++++++++ docs/specs/M0-categories/spec-back.md | 17 +++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/.claude/rules/backend.md b/.claude/rules/backend.md index 40c6a05..7534f0c 100644 --- a/.claude/rules/backend.md +++ b/.claude/rules/backend.md @@ -40,6 +40,27 @@ Format obligatoire : `module.resource[.subresource].action` en snake_case. - Audit ManyToMany : trace automatiquement `{fieldName: {added: [ids], removed: [ids]}}` — aucune action supplementaire - Spec complete : @doc/audit-log.md +## Timestampable + Blamable (obligatoire pour entites metier) + +Toute **nouvelle** entite metier sous `src/Module/*/Domain/Entity/` doit porter les 4 colonnes `created_at` / `updated_at` / `created_by` / `updated_by`, remplies automatiquement. Trois lignes a ajouter a l'entite : + +```php +use App\Shared\Domain\Contract\BlamableInterface; +use App\Shared\Domain\Contract\TimestampableInterface; +use App\Shared\Domain\Trait\TimestampableBlamableTrait; + +class MyEntity implements TimestampableInterface, BlamableInterface +{ + use TimestampableBlamableTrait; // porte les 4 props + getters/setters + // ... reste metier +} +``` + +- Le `TimestampableBlamableSubscriber` (`Shared/Infrastructure/Doctrine/`) remplit les colonnes au `prePersist` / `preUpdate`. Hors contexte HTTP (CLI, cron, migration), le blame reste `null` (libelle « Systeme » cote front). +- La migration de l'entite doit creer les 4 colonnes (`created_at` / `updated_at` NOT NULL, `created_by` / `updated_by` nullable `ON DELETE SET NULL`). +- **Garde-fou CI** : `tests/Architecture/EntitiesAreTimestampableBlamableTest` echoue si une entite oublie le pattern. Un referentiel statique justifie (ex: `CategoryType`) doit etre explicitement whiteliste dans la constante `EXCLUDED` avec un commentaire. +- Spec complete : @docs/specs/M0-categories/spec-back.md § 2.8 + § 2.8.bis + ## Serialization Pour embarquer une relation dans le JSON (au lieu d'un IRI Hydra), ajouter le groupe du parent sur les proprietes de l'entite cible. diff --git a/docs/specs/M0-categories/spec-back.md b/docs/specs/M0-categories/spec-back.md index 330ccb1..f9aeea2 100644 --- a/docs/specs/M0-categories/spec-back.md +++ b/docs/specs/M0-categories/spec-back.md @@ -441,6 +441,21 @@ final class EntitiesAreTimestampableBlamableTest extends TestCase Coût d'écriture : 1h. Coût en CI : ~50ms. Bénéfice : 0 oubli possible. À écrire dans le ticket 0.0. +#### Décision M0 sur la whitelist `EXCLUDED` (ERP-52) + +Au moment d'introduire le pattern (ticket 0.0), 4 entités préexistantes vivent déjà sous `src/Module/*/Domain/Entity/` : `User`, `Role`, `Permission` (Core) et `Site` (Sites). Aucune n'implémente le pattern. Le test L3 les détecterait et passerait au rouge. + +**Décision** : on **whiteliste explicitement ces 4 entités** dans `EntitiesAreTimestampableBlamableTest::EXCLUDED` avec une justification par entrée, plutôt que de les rétrofiter dans ERP-52 : + +| Entité | Justification du `EXCLUDED` | +|---|---| +| `User` | Référentiel d'authentification ; `createdAt` géré manuellement dans le constructeur. Rétrofit non trivial : impose de trancher la récursion Blamable (un `User` créé par un `User`) et casse des tests existants → **HP-9**. | +| `Role` | Référentiel RBAC synchronisé via `app:sync-permissions`, pas de traçabilité user-driven nécessaire. | +| `Permission` | Idem `Role` (synchronisé, pas piloté utilisateur). | +| `Site` | Référentiel admin-managed, rétrofit à intégrer dans un futur module Sites v2 → **HP-10**. | + +**Règle dure pour la suite** : toute **nouvelle** entité métier (`Category` au M0, puis `Client`, `Fournisseur`, `Prestataire`, etc.) **doit** implémenter `TimestampableInterface` + `BlamableInterface` via le Trait. La whitelist `EXCLUDED` est réservée aux référentiels statiques justifiés (ex : `CategoryType` au ticket 0.2) — toute nouvelle entrée doit être documentée. + #### Tests Subscriber Tests unitaires du Subscriber : créer une entité de test minimale (fixture interne aux tests) qui `use` le Trait + implements les interfaces, vérifier que `prePersist` + `preUpdate` remplissent les 4 colonnes. À écrire dans le ticket 0.0. @@ -979,6 +994,8 @@ Les deux mécanismes sont indépendants : on peut désactiver `#[Auditable]` (pa - **HP-6** : **Filtres avancés / recherche serveur** dans la liste. Pas pertinent à 300 entrées (pagination front). - **HP-7** : **Catégories hiérarchiques** (parent / enfant). Pas demandé. Si besoin futur → migration ajout colonne `parent_id` + spec dédiée. - **HP-8** : **Création des rôles métier Bureau / Compta / Commerciale / Usine.** Ces rôles font partie du modèle MALIO mais leur seed initial dans `role` + leur attribution aux users est hors du périmètre M0 (probablement un M-RBAC dédié, ou seedés dans `AppFixtures` / `SeedE2ECommand` au fil des modules). +- **HP-9** : **Rétrofit de `User` vers Timestampable + Blamable.** L'entité `User` est whitelistée dans `EntitiesAreTimestampableBlamableTest::EXCLUDED` au M0 (cf. § 2.8.bis). Son rétrofit nécessite une **décision archi dédiée** : récursion Blamable (un `User` créé/modifié par un `User`, FK auto-référente `created_by` / `updated_by` sur la table `user`), impact sur le `createdAt` déjà géré dans le constructeur, et migration des données existantes. À traiter dans un ticket scopé hors M0. +- **HP-10** : **Rétrofit de `Site` vers Timestampable + Blamable.** Même logique que HP-9 pour le référentiel `Site` (whitelisté `EXCLUDED`). À intégrer dans un futur module Sites v2, avec la migration ajoutant les 4 colonnes + FK `user`. ## 10. Liens & dépendances -- 2.39.5