[ERP-53] Migrer les tables Client + sous-collections + référentiels comptables #27

Merged
malio merged 3 commits from feature/ERP-53-migrer-tables-client-m1 into develop 2026-06-01 15:19:44 +00:00
Owner

Contexte

Ticket Lesstime : #53 (ERP-53) — premier ticket back du M1 (Répertoire clients).
Spec back : docs/specs/M1-clients/spec-back.md § 3.2 + § 3.3.

Implémentation

  • Migration Doctrine migrations/Version20260601000000.php (12 tables) + fixture CategoryTypeFixtures.
  • 4 référentiels comptables seedés : tva_mode (3), payment_delay (3), payment_type (4), bank (3).
  • Table client : 31 colonnes (formulaire + Information + Comptabilité + archive + soft-delete + Timestampable/Blamable).
  • 4 sous-collections : client_category (M2M), client_contact, client_address, client_rib + 3 jointures d'adresse (client_address_site, client_address_contact, client_address_category).
  • 4 CHECK : mutex distributor/broker, contact name, address prospect exclusif, billing email conditionnel.
  • 1 index partiel unique : uq_client_company_name_active sur LOWER(company_name) WHERE is_archived=false AND deleted_at IS NULL (décision Q4 — pas d'unicité siren/email).
  • Seed category_type : DISTRIBUTEUR / COURTIER / SECTEUR / AUTRE (ON CONFLICT (code) DO NOTHING en migration pour la prod, + fixture idempotente pour dev/test purgés).
  • COMMENT ON COLUMN sur chaque colonne (convention ERP-67, garde-fou vert).

RG couvertes (niveau BDD)

RG-1.03 (mutex distrib/broker), RG-1.05 (contact name), RG-1.06/07/08 (adresse prospect exclusif), RG-1.11 (billing email), RG-1.16 (unicité company_name — RG-1.15/1.17 supprimées par Q4), RG-1.22 (is_archived + archived_at).

Écarts assumés vs spec (cf. docblock migration + cahier de test du ticket)

  1. Namespace migrations/ racine au lieu de App\Module\Commercial\… : vérifié empiriquement que Doctrine 3.9.6 (AlphabeticalComparator → strcmp FQCN) trierait le namespace Commercial avant DoctrineMigrations → migration client exécutée avant user/category/site → échec FK sur base vide. Le namespace racine garantit l'ordre par timestamp.
  2. DDL aligné Doctrine : INT GENERATED BY DEFAULT AS IDENTITY + TIMESTAMP(0) WITHOUT TIME ZONE (et non SERIAL/TIMESTAMPTZ) → forward-compatible avec les entités du ticket 1.1 (schema:update no-op).
  3. Seed category_type (code, label) sans position : la table M0 n'a pas de colonne position (coquille du pseudo-SQL § 3.3).

Note ERP-54 : à l'arrivée des entités Client*, schema:update droppera leurs COMMENT + l'index partiel. Prévoir l'ajout au ColumnCommentsCatalog + recréation de l'index dans test-db-setup (pattern uq_category_name_type_active).

Tests

  • make php-cs-fixer-allow-risky
  • make db-reset ✓ + vérifications psql manuelles (8 cas : CHECK, unicité partielle, archivage, siren/email dupliqués, seeds)
  • make test ✓ — 312 tests OK, 0 régression
## Contexte Ticket Lesstime : **#53** (ERP-53) — premier ticket back du M1 (Répertoire clients). Spec back : `docs/specs/M1-clients/spec-back.md` § 3.2 + § 3.3. ## Implémentation - Migration Doctrine `migrations/Version20260601000000.php` (12 tables) + fixture `CategoryTypeFixtures`. - **4 référentiels comptables** seedés : `tva_mode` (3), `payment_delay` (3), `payment_type` (4), `bank` (3). - **Table `client`** : 31 colonnes (formulaire + Information + Comptabilité + archive + soft-delete + Timestampable/Blamable). - **4 sous-collections** : `client_category` (M2M), `client_contact`, `client_address`, `client_rib` + **3 jointures** d'adresse (`client_address_site`, `client_address_contact`, `client_address_category`). - **4 CHECK** : mutex distributor/broker, contact name, address prospect exclusif, billing email conditionnel. - **1 index partiel unique** : `uq_client_company_name_active` sur `LOWER(company_name) WHERE is_archived=false AND deleted_at IS NULL` (décision Q4 — **pas** d'unicité siren/email). - **Seed `category_type`** : DISTRIBUTEUR / COURTIER / SECTEUR / AUTRE (`ON CONFLICT (code) DO NOTHING` en migration pour la prod, + fixture idempotente pour dev/test purgés). - `COMMENT ON COLUMN` sur **chaque** colonne (convention ERP-67, garde-fou vert). ## RG couvertes (niveau BDD) RG-1.03 (mutex distrib/broker), RG-1.05 (contact name), RG-1.06/07/08 (adresse prospect exclusif), RG-1.11 (billing email), RG-1.16 (unicité company_name — RG-1.15/1.17 supprimées par Q4), RG-1.22 (is_archived + archived_at). ## Écarts assumés vs spec (cf. docblock migration + cahier de test du ticket) 1. **Namespace `migrations/` racine** au lieu de `App\Module\Commercial\…` : vérifié empiriquement que Doctrine 3.9.6 (AlphabeticalComparator → strcmp FQCN) trierait le namespace Commercial **avant** `DoctrineMigrations` → migration client exécutée avant user/category/site → échec FK sur base vide. Le namespace racine garantit l'ordre par timestamp. 2. **DDL aligné Doctrine** : `INT GENERATED BY DEFAULT AS IDENTITY` + `TIMESTAMP(0) WITHOUT TIME ZONE` (et non SERIAL/TIMESTAMPTZ) → forward-compatible avec les entités du ticket 1.1 (schema:update no-op). 3. **Seed `category_type (code, label)` sans `position`** : la table M0 n'a pas de colonne `position` (coquille du pseudo-SQL § 3.3). > **Note ERP-54** : à l'arrivée des entités Client*, `schema:update` droppera leurs COMMENT + l'index partiel. Prévoir l'ajout au `ColumnCommentsCatalog` + recréation de l'index dans `test-db-setup` (pattern `uq_category_name_type_active`). ## Tests - `make php-cs-fixer-allow-risky` ✓ - `make db-reset` ✓ + vérifications psql manuelles (8 cas : CHECK, unicité partielle, archivage, siren/email dupliqués, seeds) - `make test` ✓ — **312 tests OK, 0 régression**
tristan reviewed 2026-06-01 08:32:39 +00:00
tristan left a comment
Owner

Revue de code ERP-53 (migration tables Client + sous-collections + référentiels comptables).

Globalement très propre : la DDL est fidèle à la spec § 3.2, chaque colonne est commentée (règle n°12 OK), le choix du namespace racine DoctrineMigrations est justifié et correct (tri FQCN), et la double stratégie de seed migration (prod) / fixture idempotente (dev+test, car le purger vide category_type) est valide — vérifié sur le test-db-setup du makefile : les tables non mappées ne sont pas purgées et leurs commentaires survivent au schema:update.

3 remarques ci-dessous (1 correctness sur down(), 2 mineures). Aucun bug bloquant.

Revue de code ERP-53 (migration tables Client + sous-collections + référentiels comptables). Globalement très propre : la DDL est fidèle à la spec § 3.2, chaque colonne est commentée (règle n°12 OK), le choix du namespace racine `DoctrineMigrations` est justifié et correct (tri FQCN), et la double stratégie de seed migration (prod) / fixture idempotente (dev+test, car le purger vide `category_type`) est valide — vérifié sur le `test-db-setup` du makefile : les tables non mappées ne sont pas purgées et leurs commentaires survivent au `schema:update`. 3 remarques ci-dessous (1 correctness sur `down()`, 2 mineures). Aucun bug bloquant.
@@ -0,0 +87,4 @@
// Retire uniquement les 4 types seedes par cette migration. Les autres
// types eventuels (CRUD futur) sont preserves.
$this->addSql(<<<'SQL'
DELETE FROM category_type WHERE code IN ('DISTRIBUTEUR', 'COURTIER', 'SECTEUR', 'AUTRE')
Owner

down() échouera dès qu'une category référence un de ces 4 types.

Contexte : down() supprime les 4 codes seedés, mais category.category_type_id est une FK ON DELETE RESTRICT (cf. M0 Version20260527164000). client_address_category impose justement des catégories de type SECTEUR/AUTRE (RG-1.29) — donc en prod ces types auront des category rattachées.

Cause : DELETE FROM category_type WHERE code IN (...) est bloqué par la FK RESTRICT dès qu'une seule category pointe sur un de ces types → down() lève une exception et le rollback de migration est impossible. (En dev/test ça passe pour l'instant uniquement parce qu'aucune fixture ne crée encore de category.)

Recommandation : ne supprimer que les types orphelins, p.ex. DELETE FROM category_type WHERE code IN (...) AND NOT EXISTS (SELECT 1 FROM category c WHERE c.category_type_id = category_type.id). Cohérent en plus avec le ON CONFLICT DO NOTHING du up() : la migration ne devrait défaire que ce qu'elle a réellement inséré.

**`down()` échouera dès qu'une `category` référence un de ces 4 types.** **Contexte** : `down()` supprime les 4 codes seedés, mais `category.category_type_id` est une FK `ON DELETE RESTRICT` (cf. M0 `Version20260527164000`). `client_address_category` impose justement des catégories de type `SECTEUR`/`AUTRE` (RG-1.29) — donc en prod ces types auront des `category` rattachées. **Cause** : `DELETE FROM category_type WHERE code IN (...)` est bloqué par la FK RESTRICT dès qu'une seule `category` pointe sur un de ces types → `down()` lève une exception et le rollback de migration est impossible. (En dev/test ça passe pour l'instant uniquement parce qu'aucune fixture ne crée encore de `category`.) **Recommandation** : ne supprimer que les types orphelins, p.ex. `DELETE FROM category_type WHERE code IN (...) AND NOT EXISTS (SELECT 1 FROM category c WHERE c.category_type_id = category_type.id)`. Cohérent en plus avec le `ON CONFLICT DO NOTHING` du `up()` : la migration ne devrait défaire que ce qu'elle a réellement inséré.
@@ -0,0 +218,4 @@
$this->addSql('CREATE INDEX idx_client_distributor_id ON client (distributor_id)');
$this->addSql('CREATE INDEX idx_client_broker_id ON client (broker_id)');
$this->addSql('CREATE INDEX idx_client_created_by ON client (created_by)');
$this->addSql('CREATE INDEX idx_client_updated_by ON client (updated_by)');
Owner

Index manquants sur les 4 FK référentiels de client.

Contexte : distributor_id, broker_id, created_by, updated_by ont chacun un index, mais tva_mode_id / payment_delay_id / payment_type_id / bank_id n'en ont pas.

Cause : Postgres n'indexe pas automatiquement les colonnes FK. Comme ces FK sont ON DELETE RESTRICT, toute suppression d'une ligne de référentiel déclenche un seq scan sur client pour vérifier la contrainte (idem pour un futur filtre "clients par type de règlement").

Recommandation : impact faible (référentiels statiques, suppressions rares) et c'est cohérent avec la spec qui les omet aussi — donc soit ajouter les 4 index par cohérence avec les autres FK, soit documenter en commentaire que l'omission est volontaire. Non bloquant.

**Index manquants sur les 4 FK référentiels de `client`.** **Contexte** : `distributor_id`, `broker_id`, `created_by`, `updated_by` ont chacun un index, mais `tva_mode_id` / `payment_delay_id` / `payment_type_id` / `bank_id` n'en ont pas. **Cause** : Postgres n'indexe pas automatiquement les colonnes FK. Comme ces FK sont `ON DELETE RESTRICT`, toute suppression d'une ligne de référentiel déclenche un seq scan sur `client` pour vérifier la contrainte (idem pour un futur filtre "clients par type de règlement"). **Recommandation** : impact faible (référentiels statiques, suppressions rares) et c'est cohérent avec la spec qui les omet aussi — donc soit ajouter les 4 index par cohérence avec les autres FK, soit documenter en commentaire que l'omission est volontaire. Non bloquant.
@@ -0,0 +32,4 @@
* Source unique des 4 types metier : code technique => libelle FR.
* Doit rester aligne sur le seed de la migration Version20260601000000.
*/
private const TYPES = [
Owner

Double source de vérité pour les 4 types.

Contexte : les 4 couples code/libellé (DISTRIBUTEURDistributeur, etc.) sont déclarés ici dans const TYPES ET en dur dans le INSERT de la migration Version20260601000000::seedCategoryTypes().

Cause : deux littéraux indépendants à maintenir en phase — un changement de libellé dans un seul endroit créerait une divergence silencieuse entre prod (migration) et dev/test (fixture). Le commentaire de classe le reconnaît déjà ("Doit rester aligné sur le seed de la migration").

Recommandation : acceptable vu le découpage prod/dev assumé. Si on veut éliminer le risque de drift, centraliser les 4 codes/libellés dans une constante partagée (style ColumnCommentsCatalog) consommée par les deux. Mineur.

**Double source de vérité pour les 4 types.** **Contexte** : les 4 couples code/libellé (`DISTRIBUTEUR` → `Distributeur`, etc.) sont déclarés ici dans `const TYPES` ET en dur dans le `INSERT` de la migration `Version20260601000000::seedCategoryTypes()`. **Cause** : deux littéraux indépendants à maintenir en phase — un changement de libellé dans un seul endroit créerait une divergence silencieuse entre prod (migration) et dev/test (fixture). Le commentaire de classe le reconnaît déjà ("Doit rester aligné sur le seed de la migration"). **Recommandation** : acceptable vu le découpage prod/dev assumé. Si on veut éliminer le risque de drift, centraliser les 4 codes/libellés dans une constante partagée (style `ColumnCommentsCatalog`) consommée par les deux. Mineur.
tristan added the back label 2026-06-01 10:02:57 +00:00
matthieu added the dbtype/feat labels 2026-06-01 11:08:10 +00:00
Author
Owner

Suite review (merci) — corrections poussées en 3f21248 (fix-forward, branche rebasée) :

  • down() vs FK RESTRICT : corrigé en 3f21248. Le DELETE sur category_type est désormais orphan-only (AND NOT EXISTS (SELECT 1 FROM category c WHERE c.category_type_id = category_type.id)), symétrique du ON CONFLICT DO NOTHING du up(). Vérifié par un cycle down→up sur une base où une category de type SECTEUR existe : plus d'erreur FK, SECTEUR préservé, les 3 types orphelins supprimés.
  • Index manquants sur les 4 FK référentiels : corrigé en 3f21248. Ajout de idx_client_tva_mode_id, idx_client_payment_delay_id, idx_client_payment_type_id, idx_client_bank_id.
  • Double source CategoryTypeFixtures / migration : 🟢 volontaire et assumé. Découpage prod (migration ON CONFLICT) vs dev/test (fixtures qui purgent + reseedent) ; le drift potentiel est documenté dans le docblock de la migration. Pas de refacto au M1.
Suite review (merci) — corrections poussées en `3f21248` (fix-forward, branche rebasée) : - **`down()` vs FK RESTRICT** : ✅ corrigé en `3f21248`. Le `DELETE` sur `category_type` est désormais *orphan-only* (`AND NOT EXISTS (SELECT 1 FROM category c WHERE c.category_type_id = category_type.id)`), symétrique du `ON CONFLICT DO NOTHING` du `up()`. Vérifié par un cycle down→up sur une base où une `category` de type `SECTEUR` existe : plus d'erreur FK, `SECTEUR` préservé, les 3 types orphelins supprimés. - **Index manquants sur les 4 FK référentiels** : ✅ corrigé en `3f21248`. Ajout de `idx_client_tva_mode_id`, `idx_client_payment_delay_id`, `idx_client_payment_type_id`, `idx_client_bank_id`. - **Double source `CategoryTypeFixtures` / migration** : 🟢 volontaire et assumé. Découpage prod (migration `ON CONFLICT`) vs dev/test (fixtures qui purgent + reseedent) ; le drift potentiel est documenté dans le docblock de la migration. Pas de refacto au M1.
matthieu added 3 commits 2026-06-01 12:52:04 +00:00
matthieu force-pushed feature/ERP-53-migrer-tables-client-m1 from 3f21248e73 to 034301ceaf 2026-06-01 12:52:04 +00:00 Compare
malio merged commit 56cf492dcc into develop 2026-06-01 15:19:44 +00:00
malio deleted branch feature/ERP-53-migrer-tables-client-m1 2026-06-01 15:19:44 +00:00
Sign in to join this conversation.