test(commercial) : tests PHPUnit M2 fournisseurs (matrice RG + contrat sérialisation + DoD JSON réel) (ERP-92) #71
Reference in New Issue
Block a user
Delete Branch "feature/ERP-92-tests-phpunit-m2"
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?
ERP-92 — Tests PHPUnit M2 fournisseurs (#521)
Suite fonctionnelle M2 assertant sur le corps JSON (jamais les annotations), jumelle de la suite clients M1.
Couverture
SupplierSerializationContractTest) : 4 régressions M1 re-testées — RIB gaté absent pour la Commerciale, booléenstriageProvider/isArchivedprésents, embedcategories[].code/name, embedsites[].name/postalCode(objet, pas IRI) — + enveloppe AP4 (member/totalItems/view, archivés exclus) + suppression du contact inline.app:seed-rbac, pas de mock) : bureau/compta/commerciale/usine 200/403, gatingaccountingpar omission de clé, mode strict PATCH (RG-2.16).iban/bicdans le diff).Fix de contrat (découvert par la DoD)
Les référentiels comptables (
TvaMode/PaymentType/PaymentDelay/Bank) ne portaient queclient:read:accounting(M1) → sur un fournisseur ils sortaient en IRI nu. Ajout desupplier:read:accounting→ objet{id, code, label}embarqué (additif, zéro impact M1). Sans ce fix, #95/#96 auraient été développés contre un contrat faux.Infra
makefile:test-db-setuprecrée l'index partieluq_supplier_company_name_active(droppé parschema:updatecomme celui du client — oubli M2).DoD ✅
§ 4.0.bis : réponses JSON réelles (liste + détail admin/commerciale) collées. Front #93→#96 peuvent démarrer.
Vérifs
make test: 574 tests OK (suite complète verte)make php-cs-fixer-allow-risky: 0 correctionReview ERP-92 — tests M2 fournisseurs. Verdict favorable : code prod minimal et correct, helper soigné, couverture RG large. Les 10 remarques ci-dessous sont des renforcements d'assertions (tests plus faibles que ce que leur docblock annonce, donc susceptibles de rester verts malgré certaines régressions) — aucun bloquant.
@@ -0,0 +31,4 @@self::assertArrayHasKey('bank', $this->violationsByPath($response->toArray(false)));}public function testVirementWithBankReturns200(): voidLes happy-paths
testVirementWithBankReturns200/testLcrWithRibReturns200(l.66) n'assertent que le 200 ; la persistance depaymentType/bankn'est pas relue. Les négatifs 422 couvrent l'essentiel (le callback ne bloque plus à tort), mais un PATCH renvoyant 200 sans écrire le champ resterait vert. Mineur — un re-read confirmerait le write.@@ -0,0 +58,4 @@])->toArray();self::assertResponseStatusCodeSame(201);foreach (['firstName', 'lastName', 'phonePrimary', 'phoneSecondary', 'email'] as $key) {Dans la boucle
assertArrayNotHasKey,phoneSecondaryn'est jamais envoyé dans le payload (l.49-56) : l'assertion est un no-op qui ne peut pas échouer. Soit l'ajouter au payload (pour prouver qu'il est aussi ignoré), soit le retirer de la liste. Trivial.@@ -0,0 +31,4 @@'json' => ['isArchived' => false],]);self::assertResponseStatusCodeSame(409);testRestoreConflictReturns409n'asserte que le code 409 ; l'invariant « le fournisseur est resté archivé » n'est pas vérifié. Un bug renvoyant 409 après avoir tout de même persistéisArchived=false(rollback manqué → deux actifs de même nom, RG-2.15 cassée) passerait. Suggéré :em->clear()puisassertTrue($reloaded->isArchived()).@@ -0,0 +72,4 @@]);self::assertResponseStatusCodeSame(200);self::assertGreaterThanOrEqual(testPatchSupplierIsAuditedne contrôle quecount(update) >= 1, jamais que le diff porte biencompanyName(old/new). Contrairement àtestArchiveSupplierIsAuditedqui décodechanges, ce test ne lit pas le changeset alors que c'est l'objet même de l'audit. Un diff vide ou portant le mauvais champ ne serait pas détecté.@@ -0,0 +98,4 @@/** @var array<string, mixed> $changes */$changes = json_decode((string) $rows[0]['changes'], true, flags: JSON_THROW_ON_ERROR);self::assertArrayHasKey('isArchived', $changes, 'Le diff d\'archivage doit tracer isArchived.');testArchiveSupplierIsAuditedvérifieassertArrayHasKey('isArchived', $changes)mais pas la valeur. Un audit émis avecnew=false(entité non réellement archivée) ou old/new inversés passerait. Suggéré :assertSame(true, $changes['isArchived']['new']).@@ -0,0 +30,4 @@self::assertStringContainsString('unique', $def);self::assertStringContainsString('lower', $def);self::assertStringContainsString('company_name', $def);self::assertStringContainsString('where', $def, 'L\'index doit etre partiel (clause WHERE sur les actifs).');Le docblock annonce RG-2.11 (« partiel sur actifs non archivés / non supprimés ») mais l'index n'est vérifié que par
contains('where'). Le prédicat réelis_archived = FALSE AND deleted_at IS NULL— cœur de l'unicité et de la logique de conflit de restauration (RG-2.15) — n'est pas asserté. Une régression qui réduirait le prédicat (ex.WHERE deleted_at IS NULLseul) resterait verte. Suggéré :assertStringContainsString('is_archived', $def)+assertStringContainsString('deleted_at', $def).@@ -0,0 +156,4 @@'headers' => ['Content-Type' => self::MERGE],'json' => ['siren' => '123456789'],]);self::assertResponseStatusCodeSame(200);testComptaCanEditAccountingOnlyn'asserte que le 200 sur le PATCHsiren; la valeur n'est jamais relue.testComptaDetailHasAccountingFieldslit un seed direct (seedCompleteSupplier), pas l'écriture API → le write accounting n'est prouvé bout-en-bout nulle part. Un bug acceptant-puis-ignorantsiren(mauvais groupesupplier:write:accounting, processor sortant avant persist) passerait. Suggéré :em->clear()puis reliregetSiren().@@ -0,0 +209,4 @@'headers' => ['Content-Type' => self::LD],'json' => $this->validMainPayload('Commerciale Post'),]);self::assertResponseStatusCodeSame(422);testCommercialeHasViewAndManageButNoAccountingNoArchiveattend 422 sans vérifier lepropertyPath. L'intention (la Commerciale passe la security d'opération mais bute sur RG-2.03) n'est pas prouvée : tout 422 d'une autre cause (champ accounting wrongly-required, régression catégorie…) ferait passer le test. Suggéré : asserter qu'une violation porte sur un champ Information (cf.SupplierAccountingApiTest::violationsByPath).@@ -0,0 +124,4 @@self::assertSame('DEPART', $data['addressType']);}public function testPostAddressWithoutSiteReturns422(): voidtestPostAddressWithoutSiteReturns422est le seul test d'adresse sansskipIfSitesModuleDisabled()(présent en 104 et 147). Incohérence de suite : si le module Sites est désactivé, ce test s'exécute quand même et son 422 pourrait venir d'une relationsitesnon mappée plutôt que de RG-2.06 (Assert\Countmin 1). Ajouter le guard pour aligner le comportement module-off.@@ -0,0 +30,4 @@$em->flush();// Aucune exception : pas d'index unique sur siren (§ 2.6).self::assertSame('123456789', $em->getRepository(Supplier::class)->find($one->getId())?->getSiren());Sans
em->clear()avant lesfind(), l'identity map renvoie l'entité déjà gérée : les deuxassertSame('123456789', …)sont tautologiques (la valeur a étéseten mémoire lignes 28-29, jamais relue depuis la DB). Seule l'absence d'exception auflushteste réellement l'absence d'index unique. Unem->clear()rendrait les assertions porteuses de sens.Suite fonctionnelle M2 assertant sur le CORPS JSON (jamais les annotations), jumelle de la suite clients M1 : - contrat de sérialisation : 4 régressions M1 re-testées (RIB gaté absent pour Commerciale, booléens triageProvider/isArchived présents, embed categories[].code/name, embed sites[].name/postalCode objet) + enveloppe AP4 (member/totalItems/view, archivés exclus) + suppression du contact inline ; - matrice RBAC réelle (app:seed-rbac) bureau/compta/commerciale/usine 200/403, gating accounting par omission de clé, mode strict PATCH (RG-2.16) ; - RG-2.03/2.04/2.05/2.06/2.07/2.08/2.09/2.10/2.11/2.12/2.14/2.15/2.17 ; - sous-ressources contacts/adresses/ribs (CRUD, sécurité, normalisation) ; - anti N+1 liste (compte de requêtes constant), audit Supplier + RIB iban/bic. Fix de contrat découvert et corrigé (sinon DoD figée sur un contrat faux) : les référentiels comptables (TvaMode/PaymentType/PaymentDelay/Bank) ne portaient que le groupe client:read:accounting (M1) → sur un fournisseur ils sortaient en IRI nu. Ajout de supplier:read:accounting → objet {id, code, label} embarqué. makefile : test-db-setup recrée l'index partiel uq_supplier_company_name_active (droppé par schema:update comme pour le client) — oubli M2. DoD § 4.0.bis : réponses JSON RÉELLES (liste + détail admin/commerciale) collées, capturées via SupplierSerializationContractTest.85963ec3fftode4821b355