From 9cda225bdf078d97f15027f9a2c9a04fe6a981bc Mon Sep 17 00:00:00 2001 From: THOLOT DECHENE Matthieu Date: Mon, 8 Jun 2026 08:47:43 +0000 Subject: [PATCH] Correctifs post-review M2 fournisseurs (P1 + P2/P3 + alignement M1) (#74) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctifs issus de la review lead du stack M2 fournisseurs (ERP-84→113), répartis en priorités. Base : `develop`. Suite verte : `make test` 577 tests / 2475 assertions, `php-cs-fixer` 0 correction. ## P1 — défauts bloquants - **ERP-89** — Le message de complétude Information ne fuit plus le nom de champ technique (`(champ "%s")` retiré). Correction miroir appliquée aux deux validators (Supplier + Client), accent uniformisé. Le `propertyPath` est conservé pour le mapping inline front. - **ERP-112** — La fixture fournisseurs résout désormais la catégorie en filtrant sur le type `FOURNISSEUR` (via `CategoryInterface::getCategoryTypeCode()`), évitant de rattacher une catégorie homonyme d'un autre type (RG-2.10). - **ERP-113** — Tests d'export complétés : dédup F3 (fournisseur multi-catégories rendu sur une seule ligne) ; gating SIREN prouvé via un utilisateur minimal non-admin portant `suppliers.view` + `suppliers.accounting.view` (nouveau helper `createUserWithPermissions`). ## P2 / P3 - **ERP-86** — `maxMessage` explicite sur `competitors` (Supplier). - **ERP-92** — Garde `skipIfSitesModuleDisabled()` sur le test POST adresse sans site (évite un faux positif si le module Sites est désactivé). - **ERP-89 bis** — Nouveau test : Admin authentifié non-Commerciale + Information incomplète → 200 (distinct du cas `user=null`). - **ERP-85** — `down()` de la migration fournisseurs en `DROP TABLE IF EXISTS`. - **ERP-87** — Reset de la mémoïsation payload en début de `process()` du SupplierProcessor + documentation du filtre `?archivedOnly` de l'export (parité avec le provider liste). - **spec-back.md (M2)** — Alignée sur le code (le code fait foi) : security PATCH `manage or accounting.manage`, gating accounting par ajout de groupe (`SupplierReadGroupContextBuilder`), anti-N+1 via `hydrateListCollections` (pas de fetch-join), types de colonnes réels (`IDENTITY` / `TIMESTAMP(0)`). ## Alignement M1 ↔ M2 - **ERP-86/87 (Client)** — Mêmes corrections appliquées aux jumeaux M1 : message `competitors` explicite + reset mémoïsation `ClientProcessor`. ## Décision actée - **RG-2.10 (catégorie)** : court-circuit conservé (une seule violation sur `categories`). Les violations partageant path + message sont fusionnées côté front ; ERP-101 (toutes les erreurs en un aller-retour) est déjà respecté car le Callback n'interrompt pas la validation des autres champs. --------- Co-authored-by: admin malio Co-authored-by: Matthieu Reviewed-on: https://gitea.malio.fr/MALIO-DEV/Starseed/pulls/74 Co-authored-by: THOLOT DECHENE Matthieu Co-committed-by: THOLOT DECHENE Matthieu --- docs/specs/M2-suppliers/spec-back.md | 34 +++++++---- migrations/Version20260605130000.php | 16 ++--- ...ClientInformationCompletenessValidator.php | 5 +- ...pplierInformationCompletenessValidator.php | 5 +- .../Commercial/Domain/Entity/Client.php | 2 +- .../Commercial/Domain/Entity/Supplier.php | 2 +- .../State/Processor/ClientProcessor.php | 6 ++ .../State/Processor/SupplierProcessor.php | 6 ++ .../Controller/SupplierExportController.php | 5 ++ .../DataFixtures/SupplierFixtures.php | 28 ++++++--- .../Api/SupplierExportControllerTest.php | 59 ++++++++++++++++++- .../Api/SupplierSubResourceApiTest.php | 4 ++ .../Commercial/Unit/SupplierProcessorTest.php | 45 ++++++++++++++ tests/Module/Core/Api/AbstractApiTestCase.php | 48 +++++++++++---- 14 files changed, 220 insertions(+), 45 deletions(-) diff --git a/docs/specs/M2-suppliers/spec-back.md b/docs/specs/M2-suppliers/spec-back.md index 6906ef3..5d3e542 100644 --- a/docs/specs/M2-suppliers/spec-back.md +++ b/docs/specs/M2-suppliers/spec-back.md @@ -126,7 +126,7 @@ Toutes les entités métier nouvelles implémentent `TimestampableInterface` + ` Notes (miroir M1) : - **Compta édite uniquement l'onglet Comptabilité** (`accounting.manage`) d'un fournisseur existant. Compta ne peut pas **créer** un fournisseur (pas de `manage` global). -- **Commerciale** a `view` + `manage` mais **pas** `accounting.view` → l'onglet Comptabilité est masqué (front) et filtré (back, 2 niveaux : `security` API Platform + `SupplierProvider`). +- **Commerciale** a `view` + `manage` mais **pas** `accounting.view` → l'onglet Comptabilité est masqué (front) et filtré (back). Mécanisme réel (le code fait foi) : le groupe de lecture `supplier:read:accounting` n'est **pas** dans le contexte de sérialisation par défaut ; le `SupplierReadGroupContextBuilder` ne l'**ajoute** dynamiquement que si l'utilisateur porte `commercial.suppliers.accounting.view` (gating **par ajout** de groupe, jamais par retrait). Sans la permission, les champs comptables (et les RIB) ne sont donc jamais sérialisés. La colonne SIREN de l'export XLSX suit la même règle (`accounting.view`). - **Bureau** : `view` + `manage` (tout sauf Comptabilité). - **Usine** : aucune permission → item sidebar invisible, accès direct 403. @@ -159,9 +159,11 @@ final class SupplierFieldNormalizer Le formatage `XX XX XX XX XX` est fait à l'affichage côté front. Le back stocke `0612345678` (chiffres seuls). -### 2.12 Liste : embed catégories + sites + fetch-joins (cohérence M1/ERP-62) +### 2.12 Liste : embed catégories + sites + hydratation anti-N+1 (cohérence M1/ERP-62) -Décision d'alignement (02/06/2026) : la **liste** `GET /api/suppliers` **embarque** les `categories[]` (avec `code`/`name`) et les `sites[]` (avec `name`/`postalCode` — pas de `code`), comme la liste Clients après ERP-62 — et **non** des champs dérivés aplatis. Conséquence performance : le `DoctrineSupplierRepository` **DOIT** poser des **fetch-joins** (`leftJoin`+`addSelect`) sur `categories` et `addresses.sites` dans la requête de liste pour éviter le N+1. Les `sites` de la liste sont agrégés/dédoublonnés via `Supplier::getSites()` (cf. § 3.3). Le contrat de sérialisation (groupes `category:read` / `site:read` dans le contexte) est posé **une seule fois** sur l'entité — source de vérité unique, le front ne le redéfinit pas. +Décision d'alignement (02/06/2026) : la **liste** `GET /api/suppliers` **embarque** les `categories[]` (avec `code`/`name`) et les `sites[]` (avec `name`/`postalCode` — pas de `code`), comme la liste Clients après ERP-62 — et **non** des champs dérivés aplatis. + +Conséquence performance — **implémentation réelle (le code fait foi)** : le `DoctrineSupplierRepository` **ne fetch-joine PAS** les to-many dans la requête de sélection (`createListQueryBuilder` ne fait que filtres + tri). L'anti-N+1 passe par `hydrateListCollections()` (puis `hydrateContacts()`) : une fois le jeu de fournisseurs borné (page ou export), des requêtes **`IN` bornées séparées** remplissent `categories`, puis `addresses.sites`, puis `contacts` sur les **mêmes** instances `Supplier` (identity map). Ce découpage évite le **produit cartésien** qu'un fetch-join combiné `categories × addresses.sites` imposerait aux chemins non paginés (export, `?pagination=false`). Les `sites` de la liste sont agrégés/dédoublonnés via `Supplier::getSites()` (cf. § 3.3). Le contrat de sérialisation (groupes `category:read` / `site:read` dans le contexte) est posé **une seule fois** sur l'entité — source de vérité unique, le front ne le redéfinit pas. > Dépendance confirmée sur le JSON réel (#82 mergé) : `Category` expose `code`/`name` sous `category:read` ; `Site` expose `name`/`postalCode`/`city`/`color` sous `site:read` (**pas de `code`**). L'embed est pleinement matérialisé. @@ -213,6 +215,8 @@ Namespace : **`DoctrineMigrations` (racine `migrations/`)** — fichier `migrati > **Rappel règle ABSOLUE n°12** : chaque colonne créée ci-dessous DOIT recevoir son `COMMENT ON COLUMN`. Les 4 colonnes Timestampable/Blamable passent par le helper `addStandardTimestampableBlamableComments($schema, '')`. Le SQL ci-dessous montre la structure ; les `COMMENT ON COLUMN` (un par colonne métier) sont à écrire dans la migration (exemples §3.2.bis). +> **Types réels de la migration (le code fait foi)** : le SQL ci-dessous est *illustratif*. La migration mergée (`Version20260605130000`) utilise le **style aligné M1** : clés primaires en `INT GENERATED BY DEFAULT AS IDENTITY` (et **non** `SERIAL`) et horodatages en `TIMESTAMP(0) WITHOUT TIME ZONE` (et **non** `TIMESTAMPTZ`, car le `TimestampableBlamableTrait` mappe `datetime_immutable`). Garantit que `schema:update` reste un no-op une fois les entités mappées. + ```sql -- ===================================================================== -- Seed taxonomie : nouveau type FOURNISSEUR (référentiels comptables = M1, non recréés) @@ -422,8 +426,10 @@ use Symfony\Component\Validator\Constraints as Assert; // Cohérence M1/ERP-62 : la LISTE embarque catégories + sites (pas de // champ dérivé aplati). Maillon (c) : category:read + site:read dans // le contexte pour exposer Category(code/name) + Site(name/postalCode). - // ⚠ Le SupplierRepository DOIT fetch-join categories + addresses.sites - // pour éviter le N+1 sur la liste (cf. § 2.12). + // ⚠ Anti-N+1 : pas de fetch-join dans la requête de liste — le + // SupplierRepository hydrate categories/sites/contacts via des requêtes + // IN bornées séparées (hydrateListCollections), pour éviter le produit + // cartésien sur les chemins non paginés (export) — cf. § 2.12. normalizationContext: ['groups' => [ 'supplier:read', 'category:read', @@ -442,13 +448,14 @@ use Symfony\Component\Validator\Constraints as Assert; normalizationContext: ['groups' => [ 'supplier:read', 'supplier:item:read', // embed contacts / addresses - 'supplier:read:accounting', // scalaires compta + embed ribs (filtré par le Provider selon accounting.view) + // ⚠ supplier:read:accounting est volontairement ABSENT ici : il est + // AJOUTÉ dynamiquement par le SupplierReadGroupContextBuilder quand + // l'user porte accounting.view (gating par ajout, pas par retrait — + // parade bug #4 M1). Il porte les scalaires compta + l'embed ribs. 'category:read', // embed des Category (id/code/name) — relation imbriquée 'site:read', // embed des Site (id/name/postalCode/city/color, pas de code) — relation imbriquée 'default:read', ]], - // Le Provider RETIRE supplier:read:accounting du contexte si l'user - // n'a pas is_granted('commercial.suppliers.accounting.view'). provider: SupplierProvider::class, ), new Post( @@ -458,10 +465,13 @@ use Symfony\Component\Validator\Constraints as Assert; processor: SupplierProcessor::class, ), new Patch( - security: "is_granted('commercial.suppliers.manage')", - // Le SupplierProcessor inspecte les groupes envoyés pour autoriser - // onglet par onglet (cf. § 2.10 + § 5). Patch des champs comptables - // exige is_granted('commercial.suppliers.accounting.manage') ; + // Security élargie : `manage` OU `accounting.manage` — le rôle Compta + // n'a pas `manage` mais doit pouvoir éditer l'onglet Comptabilité d'un + // fournisseur existant (§ 2.9). Le SupplierProcessor re-gate ensuite + // onglet par onglet (mode strict RG-2.16) : + security: "is_granted('commercial.suppliers.manage') or is_granted('commercial.suppliers.accounting.manage')", + // Patch des champs comptables exige accounting.manage (guardAccounting) ; + // champs main/information exigent manage (guardManage) ; // patch isArchived exige is_granted('commercial.suppliers.archive'). normalizationContext: ['groups' => ['supplier:read', 'default:read']], denormalizationContext: ['groups' => [ diff --git a/migrations/Version20260605130000.php b/migrations/Version20260605130000.php index de1a1c1..70bdb48 100644 --- a/migrations/Version20260605130000.php +++ b/migrations/Version20260605130000.php @@ -82,14 +82,14 @@ final class Version20260605130000 extends AbstractMigration // Ordre inverse des dependances FK : jointures et sous-collections // d'abord, puis supplier. Les referentiels comptables et le // CategoryType FOURNISSEUR ne sont pas touches (crees ailleurs). - $this->addSql('DROP TABLE supplier_address_category'); - $this->addSql('DROP TABLE supplier_address_contact'); - $this->addSql('DROP TABLE supplier_address_site'); - $this->addSql('DROP TABLE supplier_rib'); - $this->addSql('DROP TABLE supplier_address'); - $this->addSql('DROP TABLE supplier_contact'); - $this->addSql('DROP TABLE supplier_category'); - $this->addSql('DROP TABLE supplier'); + $this->addSql('DROP TABLE IF EXISTS supplier_address_category'); + $this->addSql('DROP TABLE IF EXISTS supplier_address_contact'); + $this->addSql('DROP TABLE IF EXISTS supplier_address_site'); + $this->addSql('DROP TABLE IF EXISTS supplier_rib'); + $this->addSql('DROP TABLE IF EXISTS supplier_address'); + $this->addSql('DROP TABLE IF EXISTS supplier_contact'); + $this->addSql('DROP TABLE IF EXISTS supplier_category'); + $this->addSql('DROP TABLE IF EXISTS supplier'); } // ================================================================= diff --git a/src/Module/Commercial/Application/Validator/ClientInformationCompletenessValidator.php b/src/Module/Commercial/Application/Validator/ClientInformationCompletenessValidator.php index fe374fe..ec5c775 100644 --- a/src/Module/Commercial/Application/Validator/ClientInformationCompletenessValidator.php +++ b/src/Module/Commercial/Application/Validator/ClientInformationCompletenessValidator.php @@ -43,7 +43,10 @@ final class ClientInformationCompletenessValidator foreach ($fields as $property => $value) { if ($this->isMissing($value)) { $violations->add(new ConstraintViolation( - sprintf('Ce champ est obligatoire pour le role Commerciale (champ "%s").', $property), + // Pas de nom de champ technique dans le message : la violation est + // deja rattachee au bon champ via son propertyPath (mappe inline + // cote front par useFormErrors). + 'Ce champ est obligatoire pour le rôle Commerciale.', null, [], $client, diff --git a/src/Module/Commercial/Application/Validator/SupplierInformationCompletenessValidator.php b/src/Module/Commercial/Application/Validator/SupplierInformationCompletenessValidator.php index f5db864..21d4d84 100644 --- a/src/Module/Commercial/Application/Validator/SupplierInformationCompletenessValidator.php +++ b/src/Module/Commercial/Application/Validator/SupplierInformationCompletenessValidator.php @@ -47,7 +47,10 @@ final class SupplierInformationCompletenessValidator foreach ($fields as $property => $value) { if ($this->isMissing($value)) { $violations->add(new ConstraintViolation( - sprintf('Ce champ est obligatoire pour le rôle Commerciale (champ "%s").', $property), + // Pas de nom de champ technique dans le message : la violation est + // deja rattachee au bon champ via son propertyPath (mappe inline + // cote front par useFormErrors). + 'Ce champ est obligatoire pour le rôle Commerciale.', null, [], $supplier, diff --git a/src/Module/Commercial/Domain/Entity/Client.php b/src/Module/Commercial/Domain/Entity/Client.php index 69e6b39..387f94f 100644 --- a/src/Module/Commercial/Domain/Entity/Client.php +++ b/src/Module/Commercial/Domain/Entity/Client.php @@ -188,7 +188,7 @@ class Client implements TimestampableInterface, BlamableInterface private ?string $description = null; #[ORM\Column(length: 255, nullable: true)] - #[Assert\Length(max: 255, maxMessage: 'Ce champ ne peut dépasser {{ limit }} caractères.', normalizer: 'trim')] + #[Assert\Length(max: 255, maxMessage: 'La liste des concurrents ne peut dépasser {{ limit }} caractères.', normalizer: 'trim')] #[Groups(['client:read', 'client:write:information'])] private ?string $competitors = null; diff --git a/src/Module/Commercial/Domain/Entity/Supplier.php b/src/Module/Commercial/Domain/Entity/Supplier.php index 85c9b36..38d3951 100644 --- a/src/Module/Commercial/Domain/Entity/Supplier.php +++ b/src/Module/Commercial/Domain/Entity/Supplier.php @@ -181,7 +181,7 @@ class Supplier implements TimestampableInterface, BlamableInterface private ?string $description = null; #[ORM\Column(length: 255, nullable: true)] - #[Assert\Length(max: 255, maxMessage: 'Ce champ ne peut dépasser {{ limit }} caractères.', normalizer: 'trim')] + #[Assert\Length(max: 255, maxMessage: 'La liste des concurrents ne peut dépasser {{ limit }} caractères.', normalizer: 'trim')] #[Groups(['supplier:read', 'supplier:write:information'])] private ?string $competitors = null; diff --git a/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/ClientProcessor.php b/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/ClientProcessor.php index e451ee7..976ec6f 100644 --- a/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/ClientProcessor.php +++ b/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/ClientProcessor.php @@ -121,6 +121,12 @@ final class ClientProcessor implements ProcessorInterface return $this->persistProcessor->process($data, $operation, $uriVariables, $context); } + // Reinitialisation de la memoisation du payload en debut de traitement : + // le service est partage (stateful), on repart du corps de LA requete + // courante et on n'herite jamais des cles decodees d'une requete passee. + $this->decodedContent = null; + $this->decodedPayloadKeys = []; + $writableKeys = $this->writablePayloadKeys(); $isArchiveRequest = $this->guardArchive($data, $writableKeys); diff --git a/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/SupplierProcessor.php b/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/SupplierProcessor.php index b2685d1..e66eb3c 100644 --- a/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/SupplierProcessor.php +++ b/src/Module/Commercial/Infrastructure/ApiPlatform/State/Processor/SupplierProcessor.php @@ -114,6 +114,12 @@ final class SupplierProcessor implements ProcessorInterface return $this->persistProcessor->process($data, $operation, $uriVariables, $context); } + // Reinitialisation de la memoisation du payload en debut de traitement : + // le service est partage (stateful), on repart du corps de LA requete + // courante et on n'herite jamais des cles decodees d'une requete passee. + $this->decodedContent = null; + $this->decodedPayloadKeys = []; + $writableKeys = $this->writablePayloadKeys(); $isArchiveRequest = $this->guardArchive($data, $writableKeys); diff --git a/src/Module/Commercial/Infrastructure/Controller/SupplierExportController.php b/src/Module/Commercial/Infrastructure/Controller/SupplierExportController.php index 037e4dc..78f8edb 100644 --- a/src/Module/Commercial/Infrastructure/Controller/SupplierExportController.php +++ b/src/Module/Commercial/Infrastructure/Controller/SupplierExportController.php @@ -57,6 +57,11 @@ final class SupplierExportController #[IsGranted('commercial.suppliers.view')] public function __invoke(Request $request): Response { + // Memes filtres d'archivage que la vue liste (SupplierProvider) pour que + // l'export reflete exactement ce que l'utilisateur voit a l'ecran : + // - includeArchived : inclut les archives en plus des actifs ; + // - archivedOnly : restreint aux seules archives (prioritaire, cf. + // createListQueryBuilder). $includeArchived = $this->readBool($request->query->get('includeArchived')); $archivedOnly = $this->readBool($request->query->get('archivedOnly')); $search = $request->query->getString('search') ?: null; diff --git a/src/Module/Commercial/Infrastructure/DataFixtures/SupplierFixtures.php b/src/Module/Commercial/Infrastructure/DataFixtures/SupplierFixtures.php index ac89f22..6ef37fe 100644 --- a/src/Module/Commercial/Infrastructure/DataFixtures/SupplierFixtures.php +++ b/src/Module/Commercial/Infrastructure/DataFixtures/SupplierFixtures.php @@ -82,6 +82,12 @@ use Symfony\Component\DependencyInjection\Attribute\Autowire; */ class SupplierFixtures extends Fixture implements DependentFixtureInterface { + /** + * Type de categorie exige pour un fournisseur et ses adresses (RG-2.10). + * Miroir de Supplier::REQUIRED_CATEGORY_TYPE_CODE (non importable — regle n°1). + */ + private const string SUPPLIER_CATEGORY_TYPE_CODE = 'FOURNISSEUR'; + /** Cache des categories resolues par nom (evite des requetes repetees). */ private array $categoryCache = []; @@ -415,19 +421,27 @@ class SupplierFixtures extends Fixture implements DependentFixtureInterface return $this->categoryCache[$name]; } - $category = $manager->getRepository(CategoryInterface::class)->findOneBy([ + // RG-2.10 : on filtre explicitement sur le type FOURNISSEUR. Un lookup par + // le seul `name` rattacherait une categorie homonyme d'un autre type (ex. + // futur PRESTA) — donc du MAUVAIS type — ce qui violerait « au moins une + // categorie de type FOURNISSEUR ». Le filtre type est porte cote PHP + // (findBy ne sait pas filtrer une propriete imbriquee categoryType.code). + $candidates = $manager->getRepository(CategoryInterface::class)->findBy([ 'name' => $name, 'deletedAt' => null, ]); - if (!$category instanceof CategoryInterface) { - throw new RuntimeException(sprintf( - 'Categorie "%s" introuvable : CategoryFixtures doit tourner avant SupplierFixtures.', - $name, - )); + foreach ($candidates as $candidate) { + if ($candidate instanceof CategoryInterface + && self::SUPPLIER_CATEGORY_TYPE_CODE === $candidate->getCategoryTypeCode()) { + return $this->categoryCache[$name] = $candidate; + } } - return $this->categoryCache[$name] = $category; + throw new RuntimeException(sprintf( + 'Categorie FOURNISSEUR "%s" introuvable : CategoryFixtures doit tourner avant SupplierFixtures.', + $name, + )); } /** diff --git a/tests/Module/Commercial/Api/SupplierExportControllerTest.php b/tests/Module/Commercial/Api/SupplierExportControllerTest.php index ca00e63..f39f741 100644 --- a/tests/Module/Commercial/Api/SupplierExportControllerTest.php +++ b/tests/Module/Commercial/Api/SupplierExportControllerTest.php @@ -15,8 +15,9 @@ use PhpOffice\PhpSpreadsheet\IOFactory; * Couvre : reponse 200 (Content-Type + Content-Disposition), exclusion des * archives par defaut, respect du filtre ?search, peuplement des colonnes * contact principal / categories / sites, gating de la colonne SIREN selon - * commercial.suppliers.accounting.view, 403 sans commercial.suppliers.view, - * 401 anonyme. + * commercial.suppliers.accounting.view (admin ET user minimal a permission + * explicite), dedup F3 (fournisseur multi-categories rendu sur une seule ligne), + * 403 sans commercial.suppliers.view, 401 anonyme. * * @internal */ @@ -178,6 +179,60 @@ final class SupplierExportControllerTest extends AbstractSupplierApiTestCase self::assertStringNotContainsString('987654321', $this->flatten($grid)); } + /** + * Gating SIREN prouve via une permission EXPLICITE (et non le bypass admin) : + * un user minimal portant uniquement commercial.suppliers.view + + * commercial.suppliers.accounting.view voit bien la colonne SIREN et sa + * valeur. Complement de testSirenColumnPresentWithAccountingView (admin), qui + * ne prouve pas que accounting.view SEULE suffit (l'admin bypasse le RBAC). + * Le pendant negatif (sans accounting.view -> colonne absente) est couvert par + * testSirenColumnAbsentWithoutAccountingView. + */ + public function testSirenColumnPresentForMinimalUserWithAccountingView(): void + { + // Seed via admin, puis relecture par un user non-admin a 2 permissions. + $this->createAdminClient(); + $supplier = $this->seedSupplier('Gated Siren Co'); + $em = $this->getEm(); + $supplier->setSiren('456789123'); + $em->flush(); + + $creds = $this->createUserWithPermissions([ + 'commercial.suppliers.view', + 'commercial.suppliers.accounting.view', + ]); + $viewer = $this->authenticatedClient($creds['username'], $creds['password']); + + $grid = $this->gridFromResponse($viewer->request('GET', self::EXPORT_URL)->getContent()); + + self::assertContains('SIREN', $grid[0]); + self::assertStringContainsString('456789123', $this->flatten($grid)); + } + + /** + * Dedup F3 : un fournisseur portant >= 2 categories FOURNISSEUR est multiplie + * par la jointure (selection/hydratation des collections) ; l'export doit le + * rendre sur UNE SEULE ligne. On seede un fournisseur a 2 categories et on + * assert qu'il n'apparait qu'une fois dans la colonne « Nom fournisseur ». + */ + public function testExportDeduplicatesSupplierWithMultipleCategories(): void + { + $client = $this->createAdminClient(); + $supplier = $this->seedSupplier('Multi Cat Co', false, 'NEGOCIANT'); + // 2e categorie FOURNISSEUR sur le meme fournisseur (RG-2.10). + $supplier->addCategory($this->supplierCategory('GROSSISTE')); + $this->getEm()->flush(); + + $names = $this->companyNames($client->request('GET', self::EXPORT_URL)->getContent()); + + $occurrences = count(array_filter($names, static fn (string $name): bool => 'MULTI CAT CO' === $name)); + self::assertSame( + 1, + $occurrences, + 'Un fournisseur multi-categories doit apparaitre sur une seule ligne (dedup F3).', + ); + } + public function testForbiddenWithoutSuppliersViewPermission(): void { $creds = $this->createUserWithPermission('core.users.view'); diff --git a/tests/Module/Commercial/Api/SupplierSubResourceApiTest.php b/tests/Module/Commercial/Api/SupplierSubResourceApiTest.php index 9a4aa25..77df0a9 100644 --- a/tests/Module/Commercial/Api/SupplierSubResourceApiTest.php +++ b/tests/Module/Commercial/Api/SupplierSubResourceApiTest.php @@ -126,6 +126,10 @@ final class SupplierSubResourceApiTest extends AbstractSupplierApiTestCase public function testPostAddressWithoutSiteReturns422(): void { + // Sans cette garde, un module Sites desactive renverrait 404 (route + // /addresses indisponible) et le test passerait pour la MAUVAISE raison + // au lieu de prouver RG-2.06 (Assert\Count min 1 sur sites). + $this->skipIfSitesModuleDisabled(); $client = $this->createAdminClient(); $seed = $this->seedSupplier('Address No Site'); diff --git a/tests/Module/Commercial/Unit/SupplierProcessorTest.php b/tests/Module/Commercial/Unit/SupplierProcessorTest.php index 19b26f4..250ae0a 100644 --- a/tests/Module/Commercial/Unit/SupplierProcessorTest.php +++ b/tests/Module/Commercial/Unit/SupplierProcessorTest.php @@ -101,6 +101,24 @@ final class SupplierProcessorTest extends TestCase self::assertInstanceOf(Supplier::class, $processor->process($supplier, $this->operation())); } + public function testAdminIncompleteInformationPasses(): void + { + // Distinct du cas user=null : un utilisateur AUTHENTIFIE mais non-Commerciale + // (ici un admin, BusinessRoleAwareInterface renvoyant false pour tout role + // metier) n'est pas soumis a la completude Information -> 200 malgre un + // onglet Information incomplet. Prouve que le gate porte bien sur le ROLE + // metier Commerciale, et pas sur « il y a un utilisateur connecte ». + $supplier = $this->minimalSupplier(); + $supplier->setDescription('Une description'); + + $processor = $this->makeProcessor( + payload: ['description' => 'Une description'], + user: $this->adminUser(), + ); + + self::assertInstanceOf(Supplier::class, $processor->process($supplier, $this->operation())); + } + /** * @param list $granted Permissions accordees a l'utilisateur courant * @param array $payload Corps JSON simule de la requete @@ -175,6 +193,33 @@ final class SupplierProcessorTest extends TestCase return $this->createStub(Operation::class); } + /** + * Utilisateur authentifie non-Commerciale (profil admin) : porte + * BusinessRoleAwareInterface mais ne reconnait aucun role metier. Sert a + * distinguer « pas de role Commerciale » de « pas d'utilisateur » (null). + */ + private function adminUser(): UserInterface + { + return new class implements UserInterface, BusinessRoleAwareInterface { + public function hasBusinessRole(string $roleCode): bool + { + return false; + } + + public function getRoles(): array + { + return ['ROLE_ADMIN']; + } + + public function eraseCredentials(): void {} + + public function getUserIdentifier(): string + { + return 'admin-test'; + } + }; + } + private function commercialeUser(): UserInterface { return new class implements UserInterface, BusinessRoleAwareInterface { diff --git a/tests/Module/Core/Api/AbstractApiTestCase.php b/tests/Module/Core/Api/AbstractApiTestCase.php index 9f7bcc2..a258f32 100644 --- a/tests/Module/Core/Api/AbstractApiTestCase.php +++ b/tests/Module/Core/Api/AbstractApiTestCase.php @@ -90,6 +90,26 @@ abstract class AbstractApiTestCase extends ApiTestCase * @return array{username: string, password: string} Les identifiants pour authenticatedClient() */ protected function createUserWithPermission(string $permissionCode): array + { + return $this->createUserWithPermissions([$permissionCode]); + } + + /** + * Variante multi-permissions de {@see createUserWithPermission()} : cree un + * utilisateur non-admin portant PLUSIEURS permissions via un unique role + * jetable. Utile pour prouver qu'une combinaison precise de permissions + * (sans le bypass admin) suffit a debloquer un comportement — ex. la colonne + * SIREN de l'export, gatee par accounting.view EN PLUS de suppliers.view. + * + * Memes garanties que le singulier : suffixe aleatoire, password "testpass", + * rattachement a tous les sites, echec explicite si une permission est + * introuvable en base. + * + * @param list $permissionCodes codes des permissions a accorder + * + * @return array{username: string, password: string} identifiants pour authenticatedClient() + */ + protected function createUserWithPermissions(array $permissionCodes): array { if (!self::$kernel) { self::bootKernel(); @@ -97,17 +117,6 @@ abstract class AbstractApiTestCase extends ApiTestCase $em = $this->getEm(); - /** @var null|Permission $permission */ - $permission = $em->getRepository(Permission::class)->findOneBy(['code' => $permissionCode]); - - self::assertNotNull( - $permission, - sprintf( - 'Permission "%s" introuvable en base. Assurez-vous que `app:sync-permissions` a ete execute.', - $permissionCode, - ), - ); - $suffix = substr(bin2hex(random_bytes(4)), 0, 8); $username = 'testuser_'.$suffix; $password = 'testpass'; @@ -116,7 +125,22 @@ abstract class AbstractApiTestCase extends ApiTestCase $hasher = self::getContainer()->get(UserPasswordHasherInterface::class); $role = new Role('test_'.$suffix, 'Test Role '.$suffix, false); - $role->addPermission($permission); + + foreach ($permissionCodes as $permissionCode) { + /** @var null|Permission $permission */ + $permission = $em->getRepository(Permission::class)->findOneBy(['code' => $permissionCode]); + + self::assertNotNull( + $permission, + sprintf( + 'Permission "%s" introuvable en base. Assurez-vous que `app:sync-permissions` a ete execute.', + $permissionCode, + ), + ); + + $role->addPermission($permission); + } + $em->persist($role); $user = new User();