test(transport) : couverture RG-4.01→4.14 + contrat + fixtures (ERP-163) #120

Merged
matthieu merged 13 commits from feat/erp-163-carrier-tests into develop 2026-06-16 13:42:47 +00:00
Owner

Matrice RG-4.01→4.14 + RBAC + audit + anti-N+1 + CarrierSerializationContractTest (contrat JSON réel capturé en § 4.0.bis) + CarrierFixtures complète idempotente. Lien ticket ERP-163.

Livré

  • CarrierListTest : anti-N+1 liste (fetch-join qualimatCarrier, comptage requêtes constant), tri name ASC, échappatoire ?pagination=false (règle n°13).
  • CarrierAuditTest : POST/PATCH/archive → audit_log entity_type='transport.Carrier' ; le diff d'archivage trace isArchived (RG-4.14).
  • CarrierAddressApiTest : CP/ville incohérents → 201 (RG-4.06, format borné mais pas de contrôle de cohérence serveur).
  • CarrierFixtures : fixtures dev complètes et idempotentes (lookup par nom normalisé) couvrant le § 8.4 — 1 QUALIMAT (validité passée RG-4.04), 1 AUTRE+décharge (RG-4.02), 1 affrété (RG-4.03), 1 LIOT (RG-4.01), 1 complet (contacts/adresses/prix CLIENT+FOURNISSEUR), 1 archivé. Env-gated (dev uniquement, table carrier vierge en test).
  • spec-back § 4.0.bis : JSON RÉEL (liste + détail) capturé via CarrierSerializationContractTest (dump CARRIER_DOD_DUMP=1) — les 3 pièges hérités M1 vérifiés verts (embed client/supplier/sites en objet, isArchived/isChartered présents, qualimatCarrier embarqué).

Vérifs

  • make test : 791 tests OK (exit 0). Suite Transport : 89/89.
  • make php-cs-fixer-allow-risky : vert.

Pile : base = feat/erp-162-carrier-export (sommet de pile).

Matrice RG-4.01→4.14 + RBAC + audit + anti-N+1 + CarrierSerializationContractTest (contrat JSON réel capturé en § 4.0.bis) + CarrierFixtures complète idempotente. Lien ticket ERP-163. ## Livré - **CarrierListTest** : anti-N+1 liste (fetch-join `qualimatCarrier`, comptage requêtes constant), tri `name ASC`, échappatoire `?pagination=false` (règle n°13). - **CarrierAuditTest** : POST/PATCH/archive → `audit_log` `entity_type='transport.Carrier'` ; le diff d'archivage trace `isArchived` (RG-4.14). - **CarrierAddressApiTest** : CP/ville incohérents → 201 (RG-4.06, format borné mais pas de contrôle de cohérence serveur). - **CarrierFixtures** : fixtures dev complètes et **idempotentes** (lookup par nom normalisé) couvrant le § 8.4 — 1 QUALIMAT (validité passée RG-4.04), 1 AUTRE+décharge (RG-4.02), 1 affrété (RG-4.03), 1 LIOT (RG-4.01), 1 complet (contacts/adresses/prix CLIENT+FOURNISSEUR), 1 archivé. Env-gated (dev uniquement, table `carrier` vierge en test). - **spec-back § 4.0.bis** : JSON RÉEL (liste + détail) capturé via `CarrierSerializationContractTest` (dump `CARRIER_DOD_DUMP=1`) — les 3 pièges hérités M1 vérifiés verts (embed client/supplier/sites en objet, `isArchived`/`isChartered` présents, `qualimatCarrier` embarqué). ## Vérifs - `make test` : **791 tests OK** (exit 0). Suite Transport : 89/89. - `make php-cs-fixer-allow-risky` : vert. > Pile : base = `feat/erp-162-carrier-export` (sommet de pile).
matthieu added the backM4-Transporteurtype/test labels 2026-06-16 09:41:26 +00:00
tristan reviewed 2026-06-16 10:08:20 +00:00
tristan left a comment
Owner

Code review ERP-163 (couverture RG-4.01→4.14 + contrat JSON + fixtures) — PR de tests, dernière de la stack M4.

Verdict : excellente PR de tests, aucun constat bloquant. Elle ajoute de la couverture à forte valeur :

  • CarrierListTest : test anti-N+1 par comptage de requêtes (N=2 vs N=4 → identique, via doctrine.debug_data_holder) — la bonne façon de prouver le batch du fetch-join qualimatCarrier ; + tri name ASC + ?pagination=false (ce dernier comble le trou que j'avais signalé sur #112).
  • CarrierAuditTest : POST/PATCH/archive → audit_log entity_type='transport.Carrier', avec assertion du diff (isArchived tracé, RG-4.14) et non d'un simple count.
  • CarrierAddressApiTest : edge RG-4.06 (CP valide + ville incohérente → 201, pas de contrôle de cohérence serveur).
  • Fixtures dev env-gated (skip en test) + idempotentes (findOneBy avant création) + spec § 4.0.bis (JSON réel capturé).

Un 🟡 d'opportunité en inline (le trou d'assertion propertyPath des sous-ressources d'écriture aurait pu être comblé ici). Rien qui bloque.

Code review ERP-163 (couverture RG-4.01→4.14 + contrat JSON + fixtures) — PR de tests, dernière de la stack M4. **Verdict : excellente PR de tests, aucun constat bloquant.** Elle ajoute de la couverture à forte valeur : - **`CarrierListTest`** : test **anti-N+1 par comptage de requêtes** (N=2 vs N=4 → identique, via `doctrine.debug_data_holder`) — la bonne façon de prouver le batch du fetch-join `qualimatCarrier` ; + tri name ASC + `?pagination=false` (ce dernier **comble le trou que j'avais signalé sur #112**). - **`CarrierAuditTest`** : POST/PATCH/archive → `audit_log` `entity_type='transport.Carrier'`, avec assertion du **diff** (`isArchived` tracé, RG-4.14) et non d'un simple count. - **`CarrierAddressApiTest`** : edge RG-4.06 (CP valide + ville incohérente → 201, pas de contrôle de cohérence serveur). - Fixtures dev **env-gated** (skip en `test`) + **idempotentes** (`findOneBy` avant création) + spec § 4.0.bis (JSON réel capturé). Un 🟡 d'opportunité en inline (le trou d'assertion `propertyPath` des sous-ressources d'écriture aurait pu être comblé ici). Rien qui bloque.
@@ -63,6 +63,26 @@ final class CarrierAddressApiTest extends AbstractCarrierApiTestCase
self::assertResponseStatusCodeSame(422);
}
public function testInconsistentPostalCodeAndCityIsAccepted(): void
Owner

🟢 Bon edge RG-4.06 : prouve que le serveur borne le format du CP mais ne contrôle pas la cohérence CP↔ville (déléguée à l'autocomplete BAN front).

🟡 Opportunité manquée : cette PR de tests était l'endroit naturel pour combler le trou d'assertion propertyPath que j'ai signalé sur #113/#115/#116/#117 (les 422 des sous-ressources d'écriture n'assertent que le code HTTP, pas le champ fautif — enjeu ERP-101). Idem le 404 parent non testé sur addresses/contacts/prix. Ces trous restent ouverts après cette PR. Suggestion : un helper assertViolationOnPath dans AbstractCarrierApiTestCase + 1 test 404 par sous-ressource.

🟢 Bon edge RG-4.06 : prouve que le serveur borne le **format** du CP mais ne contrôle pas la cohérence CP↔ville (déléguée à l'autocomplete BAN front). 🟡 Opportunité manquée : cette PR de tests était l'endroit naturel pour combler le trou d'assertion **`propertyPath`** que j'ai signalé sur #113/#115/#116/#117 (les 422 des sous-ressources d'écriture n'assertent que le code HTTP, pas le champ fautif — enjeu ERP-101). Idem le **404 parent** non testé sur addresses/contacts/prix. Ces trous restent ouverts après cette PR. Suggestion : un helper `assertViolationOnPath` dans `AbstractCarrierApiTestCase` + 1 test 404 par sous-ressource.
@@ -0,0 +75,4 @@
);
}
public function testArchiveCarrierIsAudited(): void
Owner

🟢 testArchiveCarrierIsAudited assert le contenu du diff (changes contient isArchived, RG-4.14) et pas seulement la présence d'une ligne d'audit — c'est le bon niveau de rigueur. Bonne séparation via doctrine.dbal.audit_connection. Les 3 actions create/update/archive sont couvertes.

🟢 `testArchiveCarrierIsAudited` assert le **contenu du diff** (`changes` contient `isArchived`, RG-4.14) et pas seulement la présence d'une ligne d'audit — c'est le bon niveau de rigueur. Bonne séparation via `doctrine.dbal.audit_connection`. Les 3 actions create/update/archive sont couvertes.
@@ -0,0 +57,4 @@
* lien QUALIMAT embarque) et on exige un compte IDENTIQUE preuve que le
* fetch-join `qualimatCarrier` est batche et non par ligne.
*/
public function testListQueryCountDoesNotGrowWithRowCount(): void
Owner

🟢 Très bon test anti-N+1 : mesurer le nombre de requêtes SQL pour N=2 puis N=4 et exiger l'égalité est la bonne façon de prouver que le fetch-join qualimatCarrier est batché (et non par ligne) — bien plus robuste qu'un simple assert de comptage absolu. L'usage de doctrine.debug_data_holder (actif via profiling: true en test, indépendant d'APP_DEBUG) est correct. Avec testPaginationDisabledReturnsFullCollection, cette PR comble aussi le trou ?pagination=false que j'avais relevé sur #112. 👍

🟢 Très bon test anti-N+1 : mesurer le nombre de requêtes SQL pour N=2 puis N=4 et exiger l'égalité est la **bonne** façon de prouver que le fetch-join `qualimatCarrier` est batché (et non par ligne) — bien plus robuste qu'un simple assert de comptage absolu. L'usage de `doctrine.debug_data_holder` (actif via `profiling: true` en test, indépendant d'`APP_DEBUG`) est correct. Avec `testPaginationDisabledReturnsFullCollection`, cette PR comble aussi le trou `?pagination=false` que j'avais relevé sur #112. 👍
matthieu changed target branch from feat/erp-162-carrier-export to develop 2026-06-16 13:14:48 +00:00
matthieu added 12 commits 2026-06-16 13:15:02 +00:00
feat(transport) : permissions carriers + sidebar (ERP-153)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 2m41s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m24s
2be9cd05d4
Socle RBAC du module Transport (M4 § 5) :
- TransportModule::permissions() declare transport.carriers.{view,manage,archive}
- RbacSeeder::MATRIX (§ 5.2) : Bureau (view+manage), Commerciale (view) ;
  Compta/Usine aucun acces ; archive admin seul
- config/sidebar.php : section Transport + item /carriers (gate transport.carriers.view)
- i18n sidebar.transport.{section,carriers}
- 3 miroirs RBAC alignes : sidebar.php, personas.ts (user-full), SeedE2ECommand.php
- TransportModuleTest : garde-fou sur le jeu de permissions
Schéma BDD du répertoire transporteurs (M4) + entités + contrat de lecture
(liste + détail), socle du front.

- Migration Version20260615150000 : tables carrier / carrier_address /
  carrier_contact / carrier_price (FK cross-module, CHECK enum, index partiel
  uq_carrier_name_active, COMMENT ON COLUMN). uploaded_document et
  qualimat_carrier réutilisées (non recréées).
- Entités Carrier* (#[Auditable], Timestampable/Blamable) + ApiResource
  LECTURE seule (GetCollection + Get via CarrierProvider, anti-N+1, exclusion
  archivés + ?includeArchived). Écriture (POST/PATCH + Processor) reportée WT4+.
- QualimatCarrier : mapping ORM lecture seule sur la table référentielle
  existante (sortie du schema_filter, mapping aligné DDL ERP-39, schema:update
  no-op) + endpoint de recherche read-only (§ 4.7).
- Relations cross-module des prix (Client/Supplier/adresses) via contrats
  Shared (ClientInterface, SupplierInterface, ClientAddressInterface,
  SupplierAddressInterface) + resolve_target_entities — sans import inter-module
  (règle n°1). Ajout du groupe supplier_address:read aux champs de
  SupplierAddress pour l'embed.
- Garde-fous : ColumnCommentsCatalog (carrier* + qualimat_carrier), makefile
  test-db-setup (index partiel carrier), i18n audit (transport_carrier*),
  EntitiesAreTimestampableBlamableTest (QualimatCarrier whitelisté).
- CarrierSerializationContractTest : contrat JSON liste + détail vérifié
  (embeds objet, booléens, enveloppe Hydra) ; JSON réel capturé dans
  spec-back § 4.0.bis.

make db-reset OK, make test vert (731), make nuxt-test vert (480),
php-cs-fixer OK.
Aligne CarrierProvider/DoctrineCarrierRepository sur Client/Supplier/Provider :
?archivedOnly=true n'expose que les archives (prioritaire sur includeArchived),
pour que le toggle « Voir les archives » du front (ERP-173/ERP-164) soit operant.
Parametre optionnel en fin de signature : retro-compatible avec les appels existants.
Ecriture du formulaire principal transporteur (M4, WT4) : POST/PATCH via
CarrierProcessor + CarrierFieldNormalizer, contraintes conditionnelles sur
l'entite Carrier.

- RG-4.01 : POST qualimatCarrier -> certificationType=QUALIMAT + FK persistee ;
  cas LIOT (name=LIOT) -> certification non requise, liotPlates accepte.
- RG-4.02 : certificationType=AUTRE sans dischargeDocument -> 422 (Assert\Callback).
- RG-4.03 : isChartered=true sans indexationRate/containerType/volumeM3 -> 422.
- RG-4.12 : doublon de nom (parmi actifs) -> 409 (index partiel uq_carrier_name_active).
- RG-4.13 : normalisation serveur (name UPPER, liotPlates ;-split/trim/UPPER) +
  methodes personne/telephone/email pour les sous-ressources Contact (WT7).
- RG-4.14 : PATCH isArchived exige transport.carriers.archive (Admin seul),
  mode strict -> 403 + 422 si autre champ ; restauration en conflit -> 409.

Operations Post/Patch ajoutees a l'ApiResource (lecture posee au WT3 conservee).
RG conditionnelles portees par validateMainFormConsistency (Assert\Callback +
->atPath()) pour un propertyPath mappable inline (useFormErrors, ERP-101).

certificationType / containerType whitelistes dans EXCLUDED_LENGTH_MIRROR (Choice
borne deja les valeurs, miroir SupplierAddress::addressType).

Tests : CarrierWriteApiTest (RG-4.01->4.03/4.12->4.14), CarrierRBACMatrixTest
(matrice bureau/compta/commerciale/usine), CarrierArchiveTest (409 restauration),
CarrierFieldNormalizerTest (RG-4.13). make test vert (750).
GET /api/qualimat_carriers?search= pour la saisie assistee du nom (RG-4.01,
spec-back § 4.7) : recherche fuzzy sur name (+ siret), restreinte aux lignes
actives (is_active = true), triee name ASC, paginee (regle n°13).

- QualimatCarrierRepositoryInterface + DoctrineQualimatCarrierRepository :
  QueryBuilder de recherche (forcage is_active cote serveur, fuzzy multi-champs).
- QualimatCarrierSearchProvider : provider de la GetCollection (pagination Hydra
  + echappatoire ?pagination=false), branche uniquement sur la collection.
- ApiResource : provider custom sur GetCollection, retrait des ApiFilter natifs
  (incapables d'unifier name/siret sous ?search= ni d'imposer l'actif). Mapping
  ORM inchange (schema:update reste no-op). Aucune ecriture exposee.
- Tests : actifs seuls, tri name, match siret, pagination Hydra, 403 sans perm.
POST /api/carriers/{id}/addresses + PATCH/DELETE /api/carrier_addresses/{id}
(security transport.carriers.manage), spec-back § 4.5. Jumelle de SupplierAddress
(M2) / ProviderAddress (M3), sans address_type ni M2M.

- CarrierAddress : ajout #[ApiResource] (Get/Post/Patch/Delete) + groupe
  d'ecriture carrier:write:addresses + contraintes FR. RG-4.06 : code postal
  ^[0-9]{4,5}$ (Assert\Regex). Mapping ORM/colonnes inchange.
- CarrierAddressProcessor : rattachement parent (404 si absent) + RG-4.05
  (transporteur affrete -> Pays/CP/Ville/Adresse obligatoires, 422 par champ).
  RG-4.05 portee par le processor car le parent est indisponible a la validation
  Symfony sur un POST sous-ressource read:false. RG-4.07 = front (PATCH accepte).
- EXCLUDED_LENGTH_MIRROR : CarrierAddress::postalCode (Regex borne la longueur).
- Tests : CP invalide 422, affrete incomplet 422, affrete complet 201,
  PATCH/DELETE OK (manage), 403 sans manage.
POST /api/carriers/{id}/prices + PATCH/DELETE /api/carrier_prices/{id}
(security transport.carriers.manage) via CarrierPriceProcessor.

RG-4.09->4.11 : coherence de branche CLIENT/FOURNISSEUR (champs requis +
appartenance de l'adresse de livraison au client / de l'adresse d'appro au
fournisseur, sinon 422), nettoyage de la branche opposee (CHECK BDD). Champs
communs obligatoires via Assert\NotBlank + Assert\Choice.

Les contrats Shared ClientAddressInterface / SupplierAddressInterface exposent
desormais getClient() / getSupplier() (canal cross-module, regle n°1) pour la
verification d'appartenance. Colonnes enum du prix whitelistees dans le miroir
Assert\Length (deja bornees par Choice).
GET /api/carriers/export.xlsx (mêmes filtres que la liste : includeArchived,
search, certificationType) et GET /api/carriers/{id}/prices/export.xlsx (tableau
Prix regroupé Benne / Fond Mouvant). Controllers Symfony custom avec
#[Route(priority: 1)] pour éviter le conflit API Platform {id}, génération
déléguée au service Shared SpreadsheetExporterInterface.
L'export XLSX du repertoire reflete la vue liste : il propage desormais
?archivedOnly comme CarrierProvider (sinon l'export divergerait de l'ecran
quand le toggle « Voir les archives » est actif).
- CarrierListTest : anti-N+1 liste (fetch-join qualimat), tri name ASC,
  echappatoire ?pagination=false (regle n°13)
- CarrierAuditTest : POST/PATCH/archive -> audit_log entity_type='transport.Carrier'
- CarrierAddressApiTest : CP/ville incoherents acceptes (RG-4.06, pas de
  controle de coherence serveur)
- CarrierFixtures : fixtures dev completes et idempotentes (QUALIMAT validite
  passee, AUTRE+decharge, affrete, LIOT, complet prix CLIENT+FOURNISSEUR,
  archive) ; env-gated dev uniquement
- spec-back § 4.0.bis : JSON reel capture (liste + detail) via CarrierSerializationContractTest
test(transport) : rigueur RG sous-ressources (propertyPath, 404 parent, 401, certif)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Failing after 54s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m33s
c1fcd9a7c8
Repond aux retours de review (rigueur d'assertion transversale) :
- mutualise assertViolationOnPath dans AbstractCarrierApiTestCase (au lieu d'un
  duplicata local a CarrierWriteApiTest) ;
- asserte le propertyPath des 422 des sous-ressources (adresses city/street/postalCode,
  contacts firstName/phones/email, prix clientDeliveryAddress/supplierSupplyAddress/price)
  -> evite les faux-verts du mapping inline (ERP-101) ;
- 404 parent (POST sur /carriers/999999/{addresses,contacts,prices}) ;
- 401 anonyme + filtre ?certificationType= sur la collection (trous releves sur le
  contrat de lecture).
matthieu force-pushed feat/erp-163-carrier-tests from 29e4dbb67b to c1fcd9a7c8 2026-06-16 13:15:02 +00:00 Compare
matthieu closed this pull request 2026-06-16 13:16:18 +00:00
matthieu reopened this pull request 2026-06-16 13:16:21 +00:00
matthieu added 1 commit 2026-06-16 13:36:45 +00:00
style(transport) : conformite php-cs-fixer (lint CI projet entier)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 3m10s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m38s
6dab7cfd17
2 nits cs preexistants masques par le cache local (.php-cs-fixer.cache) et
revele par la CI (check projet entier, sans cache) : QualimatCarrierSearchProvider
et CarrierFixtures. Sans incidence fonctionnelle.
matthieu merged commit c60daebf3e into develop 2026-06-16 13:42:47 +00:00
matthieu deleted branch feat/erp-163-carrier-tests 2026-06-16 13:42:47 +00:00
Sign in to join this conversation.