test(commercial) : tests PHPUnit M2 fournisseurs (matrice RG + contrat sérialisation + DoD JSON réel) (ERP-92) #71

Merged
malio merged 2 commits from feature/ERP-92-tests-phpunit-m2 into develop 2026-06-08 07:49:17 +00:00
Owner

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

  • Contrat de sérialisation (SupplierSerializationContractTest) : 4 régressions M1 re-testées — RIB gaté absent pour la Commerciale, booléens triageProvider/isArchived présents, embed categories[].code/name, embed sites[].name/postalCode (objet, pas IRI) — + enveloppe AP4 (member/totalItems/view, archivés exclus) + suppression du contact inline.
  • Matrice RBAC réelle (app:seed-rbac, pas de mock) : bureau/compta/commerciale/usine 200/403, gating accounting par omission de clé, mode strict PATCH (RG-2.16).
  • Matrice RG-2.03 → RG-2.17 (création, normalisation RG-2.12, catégorie FOURNISSEUR RG-2.10, unicité RG-2.11, archivage RG-2.14/2.15, RG-2.07/2.08 compta, sous-ressources RG-2.04/2.05/2.06/2.09).
  • Anti N+1 liste : nombre de requêtes constant entre 2 et 4 fournisseurs. Audit Supplier + RIB (iban/bic dans le diff).

Fix de contrat (découvert par la DoD)

Les référentiels comptables (TvaMode/PaymentType/PaymentDelay/Bank) ne portaient que client:read:accounting (M1) → sur un fournisseur ils sortaient en IRI nu. Ajout de supplier: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-setup recrée l'index partiel uq_supplier_company_name_active (droppé par schema:update comme 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 correction
## 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 - **Contrat de sérialisation** (`SupplierSerializationContractTest`) : 4 régressions M1 re-testées — RIB gaté **absent** pour la Commerciale, booléens `triageProvider`/`isArchived` présents, embed `categories[].code/name`, embed `sites[].name/postalCode` (objet, pas IRI) — + enveloppe AP4 (`member`/`totalItems`/`view`, archivés exclus) + suppression du contact inline. - **Matrice RBAC réelle** (`app:seed-rbac`, pas de mock) : bureau/compta/commerciale/usine 200/403, gating `accounting` par **omission de clé**, mode strict PATCH (RG-2.16). - **Matrice RG-2.03 → RG-2.17** (création, normalisation RG-2.12, catégorie FOURNISSEUR RG-2.10, unicité RG-2.11, archivage RG-2.14/2.15, RG-2.07/2.08 compta, sous-ressources RG-2.04/2.05/2.06/2.09). - **Anti N+1 liste** : nombre de requêtes constant entre 2 et 4 fournisseurs. **Audit** Supplier + RIB (`iban`/`bic` dans le diff). ### Fix de contrat (découvert par la DoD) Les référentiels comptables (`TvaMode`/`PaymentType`/`PaymentDelay`/`Bank`) ne portaient que `client:read:accounting` (M1) → sur un fournisseur ils sortaient en **IRI nu**. Ajout de `supplier: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-setup` recrée l'index partiel `uq_supplier_company_name_active` (droppé par `schema:update` comme 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 correction
matthieu added the backM2-Fournisseurtype/test labels 2026-06-07 08:45:51 +00:00
matthieu reviewed 2026-06-07 09:00:25 +00:00
matthieu left a comment
Author
Owner

Review 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.

Review 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(): void
Author
Owner

Les happy-paths testVirementWithBankReturns200 / testLcrWithRibReturns200 (l.66) n'assertent que le 200 ; la persistance de paymentType/bank n'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.

Les happy-paths `testVirementWithBankReturns200` / `testLcrWithRibReturns200` (l.66) n'assertent que le 200 ; la persistance de `paymentType`/`bank` n'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) {
Author
Owner

Dans la boucle assertArrayNotHasKey, phoneSecondary n'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.

Dans la boucle `assertArrayNotHasKey`, `phoneSecondary` n'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);
Author
Owner

testRestoreConflictReturns409 n'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() puis assertTrue($reloaded->isArchived()).

`testRestoreConflictReturns409` n'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()` puis `assertTrue($reloaded->isArchived())`.
@@ -0,0 +72,4 @@
]);
self::assertResponseStatusCodeSame(200);
self::assertGreaterThanOrEqual(
Author
Owner

testPatchSupplierIsAudited ne contrôle que count(update) >= 1, jamais que le diff porte bien companyName (old/new). Contrairement à testArchiveSupplierIsAudited qui décode changes, 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é.

`testPatchSupplierIsAudited` ne contrôle que `count(update) >= 1`, jamais que le diff porte bien `companyName` (old/new). Contrairement à `testArchiveSupplierIsAudited` qui décode `changes`, 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.');
Author
Owner

testArchiveSupplierIsAudited vérifie assertArrayHasKey('isArchived', $changes) mais pas la valeur. Un audit émis avec new=false (entité non réellement archivée) ou old/new inversés passerait. Suggéré : assertSame(true, $changes['isArchived']['new']).

`testArchiveSupplierIsAudited` vérifie `assertArrayHasKey('isArchived', $changes)` mais pas la valeur. Un audit émis avec `new=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).');
Author
Owner

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éel is_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 NULL seul) resterait verte. Suggéré : assertStringContainsString('is_archived', $def) + assertStringContainsString('deleted_at', $def).

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éel `is_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 NULL` seul) 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);
Author
Owner

testComptaCanEditAccountingOnly n'asserte que le 200 sur le PATCH siren ; la valeur n'est jamais relue. testComptaDetailHasAccountingFields lit un seed direct (seedCompleteSupplier), pas l'écriture API → le write accounting n'est prouvé bout-en-bout nulle part. Un bug acceptant-puis-ignorant siren (mauvais groupe supplier:write:accounting, processor sortant avant persist) passerait. Suggéré : em->clear() puis relire getSiren().

`testComptaCanEditAccountingOnly` n'asserte que le 200 sur le PATCH `siren` ; la valeur n'est jamais relue. `testComptaDetailHasAccountingFields` lit un seed direct (`seedCompleteSupplier`), pas l'écriture API → le write accounting n'est prouvé bout-en-bout nulle part. Un bug acceptant-puis-ignorant `siren` (mauvais groupe `supplier:write:accounting`, processor sortant avant persist) passerait. Suggéré : `em->clear()` puis relire `getSiren()`.
@@ -0,0 +209,4 @@
'headers' => ['Content-Type' => self::LD],
'json' => $this->validMainPayload('Commerciale Post'),
]);
self::assertResponseStatusCodeSame(422);
Author
Owner

testCommercialeHasViewAndManageButNoAccountingNoArchive attend 422 sans vérifier le propertyPath. 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).

`testCommercialeHasViewAndManageButNoAccountingNoArchive` attend 422 sans vérifier le `propertyPath`. 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(): void
Author
Owner

testPostAddressWithoutSiteReturns422 est le seul test d'adresse sans skipIfSitesModuleDisabled() (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 relation sites non mappée plutôt que de RG-2.06 (Assert\Count min 1). Ajouter le guard pour aligner le comportement module-off.

`testPostAddressWithoutSiteReturns422` est le seul test d'adresse sans `skipIfSitesModuleDisabled()` (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 relation `sites` non mappée plutôt que de RG-2.06 (`Assert\Count` min 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());
Author
Owner

Sans em->clear() avant les find(), l'identity map renvoie l'entité déjà gérée : les deux assertSame('123456789', …) sont tautologiques (la valeur a été set en mémoire lignes 28-29, jamais relue depuis la DB). Seule l'absence d'exception au flush teste réellement l'absence d'index unique. Un em->clear() rendrait les assertions porteuses de sens.

Sans `em->clear()` avant les `find()`, l'identity map renvoie l'entité déjà gérée : les deux `assertSame('123456789', …)` sont tautologiques (la valeur a été `set` en mémoire lignes 28-29, jamais relue depuis la DB). Seule l'absence d'exception au `flush` teste réellement l'absence d'index unique. Un `em->clear()` rendrait les assertions porteuses de sens.
matthieu added 2 commits 2026-06-08 07:49:09 +00:00
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.
test(commercial) : fix CI anti-N+1 (profiling test) + durcissement 422/gating M2 fournisseurs (ERP-92)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Failing after 31s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m6s
de4821b355
- config/packages/test/doctrine.yaml : force dbal profiling en test pour que
  doctrine.debug_data_holder existe sous APP_DEBUG=0 (CI). Le test anti-N+1
  SupplierListTest passait en local (debug=1) mais cassait en CI.
- RBACMatrix/SupplierApi : les 422 RG-2.03 et RG-2.14 assertent desormais le
  propertyPath / message (plus seulement le code) — un 422 orthogonal ne peut
  plus faire passer le test.
- RBACMatrix : gating bureau/commerciale verifie l'ensemble des champs
  comptables (accountNumber/nTva/tvaMode/paymentType), plus seulement siren/ribs.
- violationsByPath() mutualise dans AbstractSupplierApiTestCase (dedup).
matthieu force-pushed feature/ERP-92-tests-phpunit-m2 from 85963ec3ff to de4821b355 2026-06-08 07:49:09 +00:00 Compare
malio merged commit 6f9bb68170 into develop 2026-06-08 07:49:17 +00:00
malio deleted branch feature/ERP-92-tests-phpunit-m2 2026-06-08 07:49:18 +00:00
Sign in to join this conversation.