test(transport) : couverture RG-4.01→4.14 + contrat + fixtures (ERP-163) #120
Reference in New Issue
Block a user
Delete Branch "feat/erp-163-carrier-tests"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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é
qualimatCarrier, comptage requêtes constant), triname ASC, échappatoire?pagination=false(règle n°13).audit_logentity_type='transport.Carrier'; le diff d'archivage traceisArchived(RG-4.14).carriervierge en test).CarrierSerializationContractTest(dumpCARRIER_DOD_DUMP=1) — les 3 pièges hérités M1 vérifiés verts (embed client/supplier/sites en objet,isArchived/isCharteredprésents,qualimatCarrierembarqué).Vérifs
make test: 791 tests OK (exit 0). Suite Transport : 89/89.make php-cs-fixer-allow-risky: vert.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, viadoctrine.debug_data_holder) — la bonne façon de prouver le batch du fetch-joinqualimatCarrier; + tri name ASC +?pagination=false(ce dernier comble le trou que j'avais signalé sur #112).CarrierAuditTest: POST/PATCH/archive →audit_logentity_type='transport.Carrier', avec assertion du diff (isArchivedtracé, 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).test) + idempotentes (findOneByavant création) + spec § 4.0.bis (JSON réel capturé).Un 🟡 d'opportunité en inline (le trou d'assertion
propertyPathdes sous-ressources d'écriture aurait pu être comblé ici). Rien qui bloque.@@ -63,6 +63,26 @@ final class CarrierAddressApiTest extends AbstractCarrierApiTestCaseself::assertResponseStatusCodeSame(422);}public function testInconsistentPostalCodeAndCityIsAccepted(): void🟢 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
propertyPathque 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 helperassertViolationOnPathdansAbstractCarrierApiTestCase+ 1 test 404 par sous-ressource.@@ -0,0 +75,4 @@);}public function testArchiveCarrierIsAudited(): void🟢
testArchiveCarrierIsAuditedassert le contenu du diff (changescontientisArchived, RG-4.14) et pas seulement la présence d'une ligne d'audit — c'est le bon niveau de rigueur. Bonne séparation viadoctrine.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🟢 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
qualimatCarrierest batché (et non par ligne) — bien plus robuste qu'un simple assert de comptage absolu. L'usage dedoctrine.debug_data_holder(actif viaprofiling: trueen test, indépendant d'APP_DEBUG) est correct. AvectestPaginationDisabledReturnsFullCollection, cette PR comble aussi le trou?pagination=falseque j'avais relevé sur #112. 👍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 permissionsPOST /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.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).29e4dbb67btoc1fcd9a7c8