[ERP-54] Créer les entités Client + sous-entités + référentiels #29

Merged
malio merged 5 commits from feature/ERP-54-creer-entites-client-m1 into develop 2026-06-01 15:20:24 +00:00
Owner

Contexte

Ticket Lesstime #54 (1.1 / Backend / M) — spec docs/specs/M1-clients/spec-back.md § 3.4 / § 3.5.

🔗 MR stackée sur ERP-53 — cible feature/ERP-53-migrer-tables-client-m1, pas develop. À repointer vers develop quand ERP-53 sera mergé (cf. STACK-BRANCHES-PROCEDURE.md). Le diff ne montre que les fichiers d'ERP-54.

Contenu

9 entités (src/Module/Commercial/Domain/Entity/) :

  • Métier : Client, ClientContact, ClientAddress, ClientRib#[Auditable] + Timestampable/Blamable.
  • Référentiels statiques lecture seule : TvaMode, PaymentDelay, PaymentType, Bank — whitelistés dans EntitiesAreTimestampableBlamableTest::EXCLUDED.

8 repositories interfaces (Domain/Repository/) + impl Doctrine (Infrastructure/Doctrine/).

La spec § 3.5 ne définit que 8 entités (4 métier + 4 référentiels) ; pas de 9ᵉ entité malgré la formulation « 9 paires » du ticket.

Décisions

  • Aucun #[ApiResource] dans ce ticket : le bloc ApiResource du Client (§ 3.4) référence ClientProvider/ClientProcessor = périmètre ERP-55. L'inclure casserait cache:clear/make test/schema:validate. Les entités sont des entités Doctrine pures (ORM + Assert + Groups). Endpoints lecture seule des référentiels → ticket dédié.
  • Q4 : Client sans #[ORM\UniqueConstraint] — unicité du nom de société portée par l'index partiel Postgres uq_client_company_name_active (inexprimable en attribut ORM).
  • Audit RIB (29/05) : aucun #[AuditIgnore] sur ClientRib.iban/bic (tous champs audités, audit admin-only).
  • Cross-module (règle n°1) : M2M Category via le contrat Shared\Domain\Contract\CategoryInterface + resolve_target_entities (pas d'import direct Catalog→Commercial) ; ClientAddress.sites via SiteInterface existant.

Infra nécessaire (découvert pendant le dev)

  • doctrine.yaml : mapping ORM du module Commercial (mappings explicites par module) + résolution CategoryInterface → Category.
  • CommercialReferentialFixtures créée (n'existait pas — ERP-53 avait seedé les CategoryType côté Catalog) : re-seed idempotent des 4 référentiels, sinon vidés au db-reset (désormais tables mappées).
  • ColumnCommentsCatalog étendu (colonnes M1) pour le chemin schema:update/test — sinon ColumnsHaveSqlCommentTest (garde-fou n°12) échoue.
  • Migration retrofit Version20260528120000 (ERP-67) rendue résiliente ($schema->hasTable()) : elle rejouait tout le catalogue mais s'exécute avant la création des tables M1 → relation tva_mode does not exist. Conforme à son docblock (« les futures migrations posent leurs propres COMMENT »).
  • makefile test-db-setup : recréation de l'index partiel uq_client_company_name_active (analogue de la ligne existante pour category).

Vérifications

  • make php-cs-fixer-allow-risky
  • make db-reset ✓ (bout en bout ; 4 référentiels + 4 CategoryType présents, 2 index partiels créés)
  • make test312/312 (Architecture vert, 0 régression M0)
  • doctrine:schema:validate : Mapping OK ; « not in sync » = bruit cosmétique pré-existant du projet (clear COMMENT hors-ORM, drop index partiels, renommages d'index). Seul diff introduit : renommage cosmétique de l'index M2M idx_client_category_category (même colonne) — aucun écart de type/colonne/FK vs migration ERP-53.
## Contexte Ticket Lesstime **#54** (1.1 / Backend / M) — spec `docs/specs/M1-clients/spec-back.md` § 3.4 / § 3.5. > 🔗 **MR stackée sur ERP-53** — cible `feature/ERP-53-migrer-tables-client-m1`, **pas** `develop`. À repointer vers `develop` quand ERP-53 sera mergé (cf. `STACK-BRANCHES-PROCEDURE.md`). Le diff ne montre que les fichiers d'ERP-54. ## Contenu **9 entités** (`src/Module/Commercial/Domain/Entity/`) : - Métier : `Client`, `ClientContact`, `ClientAddress`, `ClientRib` — `#[Auditable]` + Timestampable/Blamable. - Référentiels statiques lecture seule : `TvaMode`, `PaymentDelay`, `PaymentType`, `Bank` — whitelistés dans `EntitiesAreTimestampableBlamableTest::EXCLUDED`. **8 repositories** interfaces (`Domain/Repository/`) + impl Doctrine (`Infrastructure/Doctrine/`). > La spec § 3.5 ne définit que 8 entités (4 métier + 4 référentiels) ; pas de 9ᵉ entité malgré la formulation « 9 paires » du ticket. ## Décisions - **Aucun `#[ApiResource]` dans ce ticket** : le bloc ApiResource du `Client` (§ 3.4) référence `ClientProvider`/`ClientProcessor` = périmètre **ERP-55**. L'inclure casserait `cache:clear`/`make test`/`schema:validate`. Les entités sont des entités Doctrine pures (ORM + Assert + Groups). Endpoints lecture seule des référentiels → ticket dédié. - **Q4** : `Client` sans `#[ORM\UniqueConstraint]` — unicité du nom de société portée par l'index partiel Postgres `uq_client_company_name_active` (inexprimable en attribut ORM). - **Audit RIB (29/05)** : aucun `#[AuditIgnore]` sur `ClientRib.iban`/`bic` (tous champs audités, audit admin-only). - **Cross-module (règle n°1)** : M2M `Category` via le contrat `Shared\Domain\Contract\CategoryInterface` + `resolve_target_entities` (pas d'import direct Catalog→Commercial) ; `ClientAddress.sites` via `SiteInterface` existant. ## Infra nécessaire (découvert pendant le dev) - `doctrine.yaml` : mapping ORM du module `Commercial` (mappings explicites par module) + résolution `CategoryInterface → Category`. - `CommercialReferentialFixtures` **créée** (n'existait pas — ERP-53 avait seedé les CategoryType côté Catalog) : re-seed idempotent des 4 référentiels, sinon vidés au `db-reset` (désormais tables mappées). - `ColumnCommentsCatalog` étendu (colonnes M1) pour le chemin `schema:update`/test — sinon `ColumnsHaveSqlCommentTest` (garde-fou n°12) échoue. - Migration retrofit `Version20260528120000` (ERP-67) rendue résiliente (`$schema->hasTable()`) : elle rejouait tout le catalogue mais s'exécute avant la création des tables M1 → `relation tva_mode does not exist`. Conforme à son docblock (« les futures migrations posent leurs propres COMMENT »). - `makefile test-db-setup` : recréation de l'index partiel `uq_client_company_name_active` (analogue de la ligne existante pour `category`). ## Vérifications - `make php-cs-fixer-allow-risky` ✓ - `make db-reset` ✓ (bout en bout ; 4 référentiels + 4 CategoryType présents, 2 index partiels créés) - `make test` ✓ **312/312** (Architecture vert, 0 régression M0) - `doctrine:schema:validate` : Mapping **OK** ; « not in sync » = bruit cosmétique pré-existant du projet (clear COMMENT hors-ORM, drop index partiels, renommages d'index). Seul diff introduit : renommage cosmétique de l'index M2M `idx_client_category_category` (même colonne) — aucun écart de type/colonne/FK vs migration ERP-53.
tristan reviewed 2026-06-01 08:43:10 +00:00
tristan left a comment
Owner

Revue de code ERP-54 (entités Client + sous-entités + référentiels comptables), stackée sur ERP-53.

Très bon travail, mapping ORM cohérent et bien documenté. Points vérifiés OK :

  • resolve_target_entities : CategoryInterface ajouté, SiteInterface déjà présent → les M2M Client.categories / ClientAddress.{sites,categories} résolvent bien (règle n°1 respectée, pas d'import inter-modules). Category::getName(): ?string matche le contrat.
  • Bidirectionnel OneToMany/ManyToOne cohérent (mappedBy/inversedBy alignés), cascade+orphanRemoval corrects, FK onDelete alignés sur la migration ERP-53.
  • EntitiesAreTimestampableBlamableTest : découverte par Finder → les 4 entités client (Timestampable/Blamable) passent, les 4 référentiels statiques sont whitelistés à juste titre. Aucun test n'enforce #[Auditable], donc référentiels non-audités OK.
  • ColumnCommentsCatalog complété pour TOUTES les colonnes des nouvelles tables → après schema:update (qui drop les COMMENT des tables désormais mappées), app:apply-column-comments les restaure → ColumnsHaveSqlCommentTest reste vert. Le fix du filtre hasTable() dans la migration retrofit 0528 est nécessaire et correct.
  • IBAN/BIC audités sans #[AuditIgnore] : décision explicite (docblock + spec § 6.1), conforme (rule n°3 ne vise que password/token/secret).

3 remarques mineures / vérification ci-dessous. Aucun bug bloquant.

Revue de code ERP-54 (entités Client + sous-entités + référentiels comptables), stackée sur ERP-53. Très bon travail, mapping ORM cohérent et bien documenté. Points vérifiés OK : - `resolve_target_entities` : `CategoryInterface` ajouté, `SiteInterface` déjà présent → les M2M `Client.categories` / `ClientAddress.{sites,categories}` résolvent bien (règle n°1 respectée, pas d'import inter-modules). `Category::getName(): ?string` matche le contrat. - Bidirectionnel OneToMany/ManyToOne cohérent (mappedBy/inversedBy alignés), `cascade`+`orphanRemoval` corrects, FK `onDelete` alignés sur la migration ERP-53. - `EntitiesAreTimestampableBlamableTest` : découverte par Finder → les 4 entités client (Timestampable/Blamable) passent, les 4 référentiels statiques sont whitelistés à juste titre. Aucun test n'enforce `#[Auditable]`, donc référentiels non-audités OK. - `ColumnCommentsCatalog` complété pour TOUTES les colonnes des nouvelles tables → après `schema:update` (qui drop les COMMENT des tables désormais mappées), `app:apply-column-comments` les restaure → `ColumnsHaveSqlCommentTest` reste vert. Le fix du filtre `hasTable()` dans la migration retrofit 0528 est nécessaire et correct. - IBAN/BIC audités sans `#[AuditIgnore]` : décision explicite (docblock + spec § 6.1), conforme (rule n°3 ne vise que password/token/secret). 3 remarques mineures / vérification ci-dessous. Aucun bug bloquant.
@@ -69,0 +74,4 @@
# les tables (client, sous-collections, referentiels comptables) creees
# par la migration M1 (Version20260601000000) doivent etre connues de
# l'ORM. L'activation fonctionnelle passe par config/modules.php.
Commercial:
Owner

Vérifier que schema:update est bien un no-op sur les nouvelles tables.

Contexte : les entités sont mappées à la main pour coller à une migration déjà écrite (ERP-53). Toute la correction du schéma dev/test repose sur le fait que doctrine:schema:update --force (étape 2 de test-db-setup) ne génère aucun diff destructif sur client + sous-tables + tables de jointure M2M.

Cause : si un attribut ORM diverge de la DDL (type, longueur, onDelete, nom de FK de jointure), schema:update peut émettre des ALTER silencieux sur la base de test — et comme il n'y a pas encore de test fonctionnel en ERP-54, une divergence ne serait visible qu'en ERP-55. Les CHECK (chk_client_*) et l'index partiel uq_client_company_name_active ne sont pas exprimables en ORM : le 1er survit (non géré par Doctrine), le 2nd est bien recréé par le makefile — mais c'est précisément le signe que le diff n'est pas vide.

Recommandation : lancer php bin/console doctrine:schema:update --env=test --dump-sql après migrations:migrate et confirmer qu'il ne reste QUE les drops attendus (index partiel + COMMENT), rien sur les colonnes/FK. À documenter dans la description de MR pour ERP-55.

**Vérifier que `schema:update` est bien un no-op sur les nouvelles tables.** **Contexte** : les entités sont mappées à la main pour coller à une migration déjà écrite (ERP-53). Toute la correction du schéma dev/test repose sur le fait que `doctrine:schema:update --force` (étape 2 de `test-db-setup`) ne génère aucun diff destructif sur `client` + sous-tables + tables de jointure M2M. **Cause** : si un attribut ORM diverge de la DDL (type, longueur, `onDelete`, nom de FK de jointure), `schema:update` peut émettre des ALTER silencieux sur la base de test — et comme il n'y a pas encore de test fonctionnel en ERP-54, une divergence ne serait visible qu'en ERP-55. Les CHECK (chk_client_*) et l'index partiel `uq_client_company_name_active` ne sont pas exprimables en ORM : le 1er survit (non géré par Doctrine), le 2nd est bien recréé par le makefile — mais c'est précisément le signe que le diff n'est pas vide. **Recommandation** : lancer `php bin/console doctrine:schema:update --env=test --dump-sql` après `migrations:migrate` et confirmer qu'il ne reste QUE les drops attendus (index partiel + COMMENT), rien sur les colonnes/FK. À documenter dans la description de MR pour ERP-55.
@@ -43,0 +46,4 @@
// leur migration dediee (regle ABSOLUE n°12). Garde-fou indispensable,
// sinon l'ajout d'un module au catalogue casse ce retrofit avec un
// "relation X does not exist".
$existingTables = array_values(array_filter(
Owner

Édition d'une migration déjà livrée — OK ici, mais à acter.

Contexte : on modifie le corps d'une migration historique (retrofit ERP-67) pour filtrer sur $schema->hasTable(), car le catalogue partagé liste maintenant des tables (client…) qui n'existent pas encore à l'instant T de cette migration.

Cause : en principe une migration appliquée est immuable. Ici c'est sans risque car le filtre ne change le comportement que pour des tables absentes à ce stade (elles n'auraient de toute façon pas pu être commentées) — une prod déjà passée par 0528 ne rejoue pas, et un db-reset repart proprement. Sans ce garde-fou, l'ajout du module au catalogue casserait le retrofit avec « relation client does not exist ».

Recommandation : rien à changer, le fix est correct et nécessaire. Juste s'assurer qu'aucun environnement n'a une version partielle du catalogue figée ; le commentaire ajouté documente déjà bien le pourquoi.

**Édition d'une migration déjà livrée — OK ici, mais à acter.** **Contexte** : on modifie le corps d'une migration historique (retrofit ERP-67) pour filtrer sur `$schema->hasTable()`, car le catalogue partagé liste maintenant des tables (client…) qui n'existent pas encore à l'instant T de cette migration. **Cause** : en principe une migration appliquée est immuable. Ici c'est sans risque car le filtre ne change le comportement que pour des tables absentes à ce stade (elles n'auraient de toute façon pas pu être commentées) — une prod déjà passée par 0528 ne rejoue pas, et un `db-reset` repart proprement. Sans ce garde-fou, l'ajout du module au catalogue casserait le retrofit avec « relation client does not exist ». **Recommandation** : rien à changer, le fix est correct et nécessaire. Juste s'assurer qu'aucun environnement n'a une version partielle du catalogue figée ; le commentaire ajouté documente déjà bien le pourquoi.
@@ -0,0 +35,4 @@
*
* @var array<class-string, array<string, array{string, int}>>
*/
private const REFERENTIALS = [
Owner

Triple source de vérité pour les valeurs des 4 référentiels.

Contexte : les couples code/label/position sont désormais déclarés à 3 endroits — l'INSERT de la migration Version20260601000000 (ERP-53, sert la prod), ce const REFERENTIALS (sert dev/test), et les libellés de ColumnCommentsCatalog côté commentaires.

Cause : ces littéraux doivent rester strictement alignés. Un changement de label (ex. « À réception ») ou un nouveau code dans un seul endroit crée une divergence silencieuse prod ↔ dev/test, non détectée par les tests actuels. Le docblock le reconnaît déjà (« Doit rester aligné sur le seed de la migration »).

Recommandation : acceptable vu le découpage prod/dev assumé (même pattern que CategoryTypeFixtures). Pour éliminer le risque, on pourrait centraliser code/label/position dans une constante partagée consommée par la migration ET la fixture. Mineur, à arbitrer.

**Triple source de vérité pour les valeurs des 4 référentiels.** **Contexte** : les couples code/label/position sont désormais déclarés à 3 endroits — l'`INSERT` de la migration `Version20260601000000` (ERP-53, sert la prod), ce `const REFERENTIALS` (sert dev/test), et les libellés de `ColumnCommentsCatalog` côté commentaires. **Cause** : ces littéraux doivent rester strictement alignés. Un changement de label (ex. « À réception ») ou un nouveau code dans un seul endroit crée une divergence silencieuse prod ↔ dev/test, non détectée par les tests actuels. Le docblock le reconnaît déjà (« Doit rester aligné sur le seed de la migration »). **Recommandation** : acceptable vu le découpage prod/dev assumé (même pattern que `CategoryTypeFixtures`). Pour éliminer le risque, on pourrait centraliser code/label/position dans une constante partagée consommée par la migration ET la fixture. Mineur, à arbitrer.
tristan added the back label 2026-06-01 10:02:52 +00:00
matthieu added the type/feat label 2026-06-01 11:08:11 +00:00
Author
Owner

Suite review (merci) — pas de changement de code requis, ces 3 points sont des confirmations :

  • schema:update no-op : vérifié. make db-reset (migrations puis schema:update --force en test) ne génère aucun diff destructif sur client + sous-tables + jointures M2M ; la suite complète (370 tests) passe sur base fraîche.
  • Triple source des référentiels : 🟢 assumé. Les littéraux migration (prod) / fixtures (dev-test) / ColumnCommentsCatalog (commentaires) doivent rester alignés — documenté, pas de factorisation au M1.
  • Édition d'une migration déjà livrée (Version20260528120000, retrofit ERP-67) : 🟢 acté. Le filtre $schema->hasTable() est sans risque ici (table absente à l'instant T) ; on note l'entorse au principe d'immuabilité.

(Branche rebasée sur ERP-53 corrigé → nouveau SHA, pas de changement de contenu propre à ERP-54.)

Suite review (merci) — pas de changement de code requis, ces 3 points sont des confirmations : - **`schema:update` no-op** : ✅ vérifié. `make db-reset` (migrations puis `schema:update --force` en test) ne génère aucun diff destructif sur `client` + sous-tables + jointures M2M ; la suite complète (370 tests) passe sur base fraîche. - **Triple source des référentiels** : 🟢 assumé. Les littéraux migration (prod) / fixtures (dev-test) / `ColumnCommentsCatalog` (commentaires) doivent rester alignés — documenté, pas de factorisation au M1. - **Édition d'une migration déjà livrée (`Version20260528120000`, retrofit ERP-67)** : 🟢 acté. Le filtre `$schema->hasTable()` est sans risque ici (table absente à l'instant T) ; on note l'entorse au principe d'immuabilité. _(Branche rebasée sur ERP-53 corrigé → nouveau SHA, pas de changement de contenu propre à ERP-54.)_
malio changed target branch from feature/ERP-53-migrer-tables-client-m1 to develop 2026-06-01 15:19:48 +00:00
malio added 4 commits 2026-06-01 15:19:48 +00:00
fix(commercial) : down() orphan-only + index FK referentiels (review ERP-53)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 1m29s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m15s
034301ceaf
Entites metier (Client, ClientContact, ClientAddress, ClientRib) avec
#[Auditable] + Timestampable/Blamable, et 4 referentiels comptables statiques
(TvaMode, PaymentDelay, PaymentType, Bank). 8 repositories interfaces + impl
Doctrine. Aucun ApiResource (Provider/Processor = ERP-55).

- Client : 2 FK auto-referentes distributor/broker (mutuellement exclusives,
  CHECK en base), M2M categories, FK referentiels comptables, groupes de
  serialisation par onglet. Pas de #[ORM\UniqueConstraint] : unicite du nom de
  societe portee par l'index partiel Postgres (decision Q4).
- ClientRib : tous les champs audites, aucun #[AuditIgnore] sur iban/bic
  (decision 29/05, audit admin-only).
- M2M Category via le contrat Shared CategoryInterface + resolve_target_entities
  (regle n°1, pas d'import inter-modules) ; sites via SiteInterface.
- CommercialReferentialFixtures : re-seed idempotent des 4 referentiels (sinon
  vides apres db-reset car desormais tables mappees, purgees par les fixtures).
- Referentiels whitelistes dans EntitiesAreTimestampableBlamableTest::EXCLUDED.
- doctrine.yaml : mapping ORM du module Commercial + resolve CategoryInterface.
- ColumnCommentsCatalog : ajout des colonnes M1 (chemin schema:update/test) ;
  migration retrofit Version20260528120000 filtree sur les tables existantes
  pour ne pas casser sur les tables des modules crees plus tard.
- makefile test-db-setup : recreation de l'index partiel uq_client_company_name_active.

Refs ERP-54.
malio added 1 commit 2026-06-01 15:20:10 +00:00
Merge branch 'develop' into feature/ERP-54-creer-entites-client-m1
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Failing after 21s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Failing after 2m16s
a7c597160c
malio merged commit b495e4030a into develop 2026-06-01 15:20:24 +00:00
malio deleted branch feature/ERP-54-creer-entites-client-m1 2026-06-01 15:20:25 +00:00
Sign in to join this conversation.