feat(commercial) : validation back de la relation + suivi de revue MR (ERP-119)

- validation serveur « relation choisie => FK obligatoire » : champ transitoire relationType (non persiste) + Assert\Callback portant la 422 sur distributor / broker, que le back ne pouvait pas deriver des seules FK nullable - mutualisation des payloads d'ecriture clients : new.vue consomme buildMainPayload / buildAddressPayload / buildRibPayload (fin de la duplication create/edit) - COMMENT ON TABLE client_address : ajout des types Courtier / Distributeur (catalogue + migration Version20260609120000) - tests : violationsByPath remonte dans AbstractCommercialApiTestCase (fin des copies inline) + couverture de la nouvelle RG relation
2026-06-09 21:42:47 +02:00
parent 5b3ce0eb13
commit 8bfaa3f640
11 changed files with 185 additions and 80 deletions
@@ -389,7 +389,6 @@ import { computed, onMounted, reactive, ref, watch } from 'vue'
 import { useClientReferentials, type RefOption } from '~/modules/commercial/composables/useClientReferentials'
 import { useClientFormErrors } from '~/modules/commercial/composables/useClientFormErrors'
 import {
-    ADDRESS_REQUIRED_NON_NULLABLE_KEYS,
    buildClientFormTabKeys,
    CLIENT_FORM_PLACEHOLDER_TABS,
    isAddressValid,
@@ -401,11 +400,13 @@ import {
    isRibComplete,
    isRibRequiredForPaymentType,
    lastFillableTabKey,
-    MAIN_REQUIRED_NON_NULLABLE_KEYS,
-    omitEmptyRequired,
-    RIB_REQUIRED_NON_NULLABLE_KEYS,
    showsRelationAndTriageFields,
 } from '~/modules/commercial/utils/clientFormRules'
+import {
+    buildAddressPayload,
+    buildMainPayload,
+    buildRibPayload,
+} from '~/modules/commercial/utils/clientEdit'
 import {
    emptyAddress,
    emptyContact,
@@ -538,15 +539,9 @@ async function submitMain(): Promise<void> {
    mainSubmitting.value = true
    mainErrors.clearErrors()
    try {
-        // companyName omis si vide -> 422 NotBlank au lieu d'un 400 de type (ERP-119).
-        const payload: Record<string, unknown> = omitEmptyRequired({
-            companyName: main.companyName,
-            categories: main.categoryIris,
-            distributor: main.relationType === 'distributeur' ? main.distributorIri : null,
-            broker: main.relationType === 'courtier' ? main.brokerIri : null,
-            triageService: main.triageService,
-        }, MAIN_REQUIRED_NON_NULLABLE_KEYS)
-        const created = await api.post<ClientResponse>('/clients', payload, {
+        // Payload partage avec l'edition (buildMainPayload) : meme logique
+        // d'omission des requis vides et meme envoi de relationType (ERP-119).
+        const created = await api.post<ClientResponse>('/clients', buildMainPayload(main), {
            headers: { Accept: 'application/ld+json' },
            toast: false,
        })
@@ -826,24 +821,8 @@ async function submitAddresses(): Promise<void> {
            addresses.value,
            addressErrors,
            async (address) => {
-                // postalCode / city / street omis si vides -> 422 NotBlank (ERP-119).
-                const body = omitEmptyRequired({
-                    isProspect: address.isProspect,
-                    isDelivery: address.isDelivery,
-                    isBilling: address.isBilling,
-                    isBroker: address.isBroker,
-                    isDistributor: address.isDistributor,
-                    country: address.country,
-                    postalCode: address.postalCode || null,
-                    city: address.city || null,
-                    street: address.street || null,
-                    streetComplement: address.streetComplement || null,
-                    categories: address.categoryIris,
-                    sites: address.siteIris,
-                    contacts: address.contactIris,
-                    billingEmail: isBillingEmailRequired(address) ? (address.billingEmail || null) : null,
-                    billingEmailSecondary: isBillingEmailRequired(address) && address.hasSecondaryBillingEmail ? (address.billingEmailSecondary || null) : null,
-                }, ADDRESS_REQUIRED_NON_NULLABLE_KEYS)
+                // Payload partage avec l'edition (buildAddressPayload, ERP-119).
+                const body = buildAddressPayload(address, isBillingEmailRequired(address))
                if (address.id === null) {
                    const created = await api.post<{ id: number }>(
                        `/clients/${clientId.value}/addresses`,
@@ -947,9 +926,8 @@ async function submitAccounting(): Promise<void> {
            ribs.value,
            ribErrors,
            async (rib) => {
-                // label / bic / iban omis si vides -> 422 NotBlank au lieu d'un 400
-                // de type sur un RIB partiel (ex. IBAN seul). ERP-119.
-                const body = omitEmptyRequired({ label: rib.label, bic: rib.bic, iban: rib.iban }, RIB_REQUIRED_NON_NULLABLE_KEYS)
+                // Payload partage avec l'edition (buildRibPayload, ERP-119).
+                const body = buildRibPayload(rib)
                if (rib.id === null) {
                    const created = await api.post<{ id: number }>(
                        `/clients/${clientId.value}/ribs`,
@@ -61,7 +61,9 @@ function accountingDraft(overrides: Partial<AccountingFormDraft> = {}): Accounti
 // Le contact inline (nom/prenom/telephones/email) ne fait plus partie du groupe
 // main : les coordonnees vivent desormais sur la sous-ressource ClientContact.
 const MAIN_KEYS = [
-    'companyName', 'categories', 'distributor', 'broker', 'triageService',
+    // relationType : champ transitoire envoye au back pour la validation croisee
+    // « relation choisie => FK obligatoire » (RG-1.03 bis, ERP-119).
+    'companyName', 'categories', 'relationType', 'distributor', 'broker', 'triageService',
 ]
 const INFORMATION_KEYS = [
    'description', 'competitors', 'foundedAt', 'employeesCount',
@@ -100,6 +102,12 @@ describe('buildMainPayload — scoping strict groupe client:write:main', () => {
        expect(payload.broker).toBeNull()
    })

+    it('transmet relationType au back pour la validation croisee (RG-1.03 bis)', () => {
+        expect(buildMainPayload(mainDraft({ relationType: 'distributeur' })).relationType).toBe('distributeur')
+        expect(buildMainPayload(mainDraft({ relationType: 'courtier' })).relationType).toBe('courtier')
+        expect(buildMainPayload(mainDraft({ relationType: null })).relationType).toBeNull()
+    })
+
    // ERP-119 : companyName est requis ET adosse a une colonne NON-nullable. Si le
    // champ est vide, on OMET la cle (au lieu d'envoyer null) pour que le back
    // renvoie une 422 NotBlank (propertyPath companyName) et non un 400 de type.
@@ -146,9 +146,16 @@ export function mapAccountingFormDraft(client: ClientDetail): AccountingFormDraf
 */
 export function buildMainPayload(main: MainFormDraft): Record<string, unknown> {
    // companyName omis si vide -> 422 NotBlank au lieu d'un 400 de type (ERP-119).
+    // relationType : champ transitoire (non persiste cote back) qui porte
+    // l'intention UI « ce client depend d'un distributeur / courtier ». Il sert
+    // a la validation croisee serveur (RG-1.03 bis) : si une relation est choisie,
+    // la FK correspondante devient obligatoire -> 422 sur distributor / broker.
+    // Sans equivalent derivable cote back (FK nullable), c'est la seule facon de
+    // rester sur « on soumet, le back tranche » plutot qu'une garde front-only.
    return omitEmptyRequired({
        companyName: main.companyName,
        categories: main.categoryIris,
+        relationType: main.relationType,
        distributor: main.relationType === 'distributeur' ? main.distributorIri : null,
        broker: main.relationType === 'courtier' ? main.brokerIri : null,
        triageService: main.triageService,
@@ -50,10 +50,15 @@ final class Version20260609120000 extends AbstractMigration

        $this->comment('client_address', 'is_broker', 'Adresse Courtier — type autonome exclusif de tout autre usage (chk_client_address_broker_exclusive). Faux par defaut.');
        $this->comment('client_address', 'is_distributor', 'Adresse Distributeur — type autonome exclusif de tout autre usage (chk_client_address_distributor_exclusive). Faux par defaut.');
+
+        // Le commentaire de table mentionnait seulement prospect/livraison/facturation :
+        // on y ajoute les types autonomes Courtier / Distributeur (cf. ColumnCommentsCatalog).
+        $this->addSql('COMMENT ON TABLE client_address IS $_$Adresses d un client (1:n) — types prospect / livraison / facturation (exclusivites RG-1.06/07/08) + Courtier / Distributeur autonomes (exclusifs de tout autre usage), >= 1 site rattache (RG-1.10).$_$');
    }

    public function down(Schema $schema): void
    {
+        $this->addSql('COMMENT ON TABLE client_address IS $_$Adresses d un client (1:n) — prospect exclusif de livraison/facturation (RG-1.06/07/08), >= 1 site rattache (RG-1.10).$_$');
        $this->addSql('ALTER TABLE client_address DROP CONSTRAINT IF EXISTS chk_client_address_broker_exclusive');
        $this->addSql('ALTER TABLE client_address DROP CONSTRAINT IF EXISTS chk_client_address_distributor_exclusive');
        $this->addSql('ALTER TABLE client_address DROP COLUMN is_distributor');
@@ -25,6 +25,7 @@ use Doctrine\ORM\Mapping as ORM;
 use Symfony\Component\Serializer\Attribute\Groups;
 use Symfony\Component\Serializer\Attribute\SerializedName;
 use Symfony\Component\Validator\Constraints as Assert;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;

 /**
 * Client (M1 Commercial) — entite racine du repertoire clients. Porte le
@@ -171,6 +172,17 @@ class Client implements TimestampableInterface, BlamableInterface
    #[Groups(['client:read', 'client:write:main'])]
    private bool $triageService = false;

+    // Champ transitoire (NON persiste : aucune colonne ORM) portant l'intention UI
+    // « ce client depend d'un distributeur / courtier ». Write-only (groupe
+    // d'ecriture main uniquement, pas de groupe de lecture -> jamais serialise en
+    // sortie). Sert exclusivement a la validation croisee validateRelationName :
+    // si une relation est choisie, la FK correspondante (distributor / broker)
+    // devient obligatoire. Non mappe ORM -> non audite, et toujours null une fois
+    // l'entite rechargee depuis la base (ne sert qu'au cycle d'une ecriture).
+    #[Assert\Choice(choices: ['distributeur', 'courtier'], message: 'Le type de relation est invalide.')]
+    #[Groups(['client:write:main'])]
+    private ?string $relationType = null;
+
    // RG : au moins une categorie (Count min 1). M2M vers Category via le contrat
    // CategoryInterface (resolve_target_entities -> Category).
    /** @var Collection<int, CategoryInterface> */
@@ -333,6 +345,45 @@ class Client implements TimestampableInterface, BlamableInterface
        return $this;
    }

+    public function getRelationType(): ?string
+    {
+        return $this->relationType;
+    }
+
+    public function setRelationType(?string $relationType): static
+    {
+        $this->relationType = $relationType;
+
+        return $this;
+    }
+
+    /**
+     * RG-1.03 bis : si l'utilisateur declare une relation (« depend d'un
+     * distributeur / courtier » via le champ transitoire relationType), la FK
+     * correspondante est obligatoire. Le back ne peut pas deviner cette intention
+     * a partir des seules FK nullable (distributor=null ne distingue pas « pas de
+     * relation » de « relation choisie sans nom »), d'ou relationType qui la porte.
+     * Violation portee sur distributor / broker (champ fautif cote formulaire), de
+     * sorte que useFormErrors la mappe inline sous le bon select (ERP-101).
+     */
+    #[Assert\Callback]
+    public function validateRelationName(ExecutionContextInterface $context): void
+    {
+        if ('distributeur' === $this->relationType && null === $this->distributor) {
+            $context->buildViolation('Le nom du distributeur est obligatoire.')
+                ->atPath('distributor')
+                ->addViolation()
+            ;
+        }
+
+        if ('courtier' === $this->relationType && null === $this->broker) {
+            $context->buildViolation('Le nom du courtier est obligatoire.')
+                ->atPath('broker')
+                ->addViolation()
+            ;
+        }
+    }
+
    public function isTriageService(): bool
    {
        return $this->triageService;
@@ -219,7 +219,7 @@ final class ColumnCommentsCatalog
            ] + self::timestampableBlamableComments(),

            'client_address' => [
-                '_table'                  => 'Adresses d un client (1:n) — prospect exclusif de livraison/facturation (RG-1.06/07/08), >= 1 site rattache (RG-1.10).',
+                '_table'                  => 'Adresses d un client (1:n) — types prospect / livraison / facturation (exclusivites RG-1.06/07/08) + Courtier / Distributeur autonomes (exclusifs de tout autre usage), >= 1 site rattache (RG-1.10).',
                'id'                      => 'Identifiant interne auto-incremente.',
                'client_id'               => 'FK -> client.id, ON DELETE CASCADE — client proprietaire de l adresse.',
                'is_prospect'             => 'Adresse de prospection — exclusive de is_delivery/is_billing (RG-1.06/07/08, chk_client_address_prospect_exclusive). Faux par defaut.',
@@ -137,6 +137,27 @@ abstract class AbstractCommercialApiTestCase extends AbstractApiTestCase
        return $client;
    }

+    /**
+     * Indexe les violations d'un corps de reponse 422 par propertyPath. Permet
+     * d'asserter qu'un 422 porte bien sur le champ attendu (et n'est pas un 422
+     * orthogonal) : un test qui se contente du code 422 passerait meme si la RG
+     * visee etait cassee pour une autre raison. Mutualise ici (et non dans la
+     * sous-classe Supplier) pour etre accessible a tous les tests Commercial.
+     *
+     * @param array<string, mixed> $body corps decode de la reponse (toArray(false))
+     *
+     * @return array<string, string> propertyPath => message
+     */
+    protected function violationsByPath(array $body): array
+    {
+        $byPath = [];
+        foreach ($body['violations'] ?? [] as $v) {
+            $byPath[$v['propertyPath']] = $v['message'];
+        }
+
+        return $byPath;
+    }
+
    private function cleanupCommercialTestData(): void
    {
        $em = $this->getEm();
@@ -298,26 +298,6 @@ abstract class AbstractSupplierApiTestCase extends AbstractCommercialApiTestCase
        return $this->referential(Bank::class, $code);
    }

-    /**
-     * Indexe les violations d'un corps de reponse 422 par propertyPath. Permet
-     * d'asserter qu'un 422 porte bien sur le champ attendu (et n'est pas un 422
-     * orthogonal) : un test qui se contente du code 422 passerait meme si la RG
-     * visee etait cassee pour une autre raison.
-     *
-     * @param array<string, mixed> $body corps decode de la reponse (toArray(false))
-     *
-     * @return array<string, string> propertyPath => message
-     */
-    protected function violationsByPath(array $body): array
-    {
-        $byPath = [];
-        foreach ($body['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
-
-        return $byPath;
-    }
-
    /**
     * Recupere un referentiel comptable seede (CommercialReferentialFixtures) par
     * code. Echoue explicitement si absent (fixtures non chargees).
@@ -241,10 +241,7 @@ final class ClientAddressTest extends AbstractCommercialApiTestCase
        ])->toArray(false);

        self::assertResponseStatusCodeSame(422);
-        $byPath = [];
-        foreach ($body['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
+        $byPath = $this->violationsByPath($body);
        self::assertArrayHasKey('billingEmailSecondary', $byPath);
    }

@@ -402,10 +399,7 @@ final class ClientAddressTest extends AbstractCommercialApiTestCase
        ])->toArray(false);

        self::assertResponseStatusCodeSame(422);
-        $byPath = [];
-        foreach ($body['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
+        $byPath = $this->violationsByPath($body);
        self::assertArrayHasKey('isProspect', $byPath);
        self::assertSame('Le type d\'adresse est obligatoire.', $byPath['isProspect']);
    }
@@ -484,10 +478,7 @@ final class ClientAddressTest extends AbstractCommercialApiTestCase
        ])->toArray(false);

        self::assertResponseStatusCodeSame(422);
-        $byPath = [];
-        foreach ($body['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
+        $byPath = $this->violationsByPath($body);
        self::assertArrayHasKey('isProspect', $byPath);
        self::assertSame('Une adresse Courtier ne peut pas avoir d\'autre type.', $byPath['isProspect']);
    }
@@ -85,4 +85,77 @@ final class ClientFormulaireMainTest extends AbstractCommercialApiTestCase
        self::assertNotNull($persisted);
        self::assertSame('LEGACY FIELDS SARL', $persisted->getCompanyName());
    }
+
+    /**
+     * RG-1.03 bis : declarer une relation « depend d'un distributeur »
+     * (relationType, champ transitoire) sans renseigner la FK distributor doit
+     * produire une 422 portee sur `distributor`. Le back ne peut pas deviner
+     * l'intention depuis la seule FK nullable (distributor=null = client
+     * independant), d'ou relationType qui la transporte.
+     */
+    public function testRelationDistributeurSansDistributeurEst422(): void
+    {
+        $client = $this->createAdminClient();
+        $cat    = $this->createCategory('SECTEUR');
+
+        $body = $client->request('POST', '/api/clients', [
+            'headers' => ['Content-Type' => self::LD],
+            'json'    => [
+                'companyName'  => 'Relation Sans Distrib SARL',
+                'categories'   => ['/api/categories/'.$cat->getId()],
+                'relationType' => 'distributeur',
+            ],
+        ])->toArray(false);
+
+        self::assertResponseStatusCodeSame(422);
+        $byPath = $this->violationsByPath($body);
+        self::assertArrayHasKey('distributor', $byPath);
+        self::assertSame('Le nom du distributeur est obligatoire.', $byPath['distributor']);
+    }
+
+    /** Idem courtier : relationType=courtier sans broker -> 422 portee sur `broker`. */
+    public function testRelationCourtierSansCourtierEst422(): void
+    {
+        $client = $this->createAdminClient();
+        $cat    = $this->createCategory('SECTEUR');
+
+        $body = $client->request('POST', '/api/clients', [
+            'headers' => ['Content-Type' => self::LD],
+            'json'    => [
+                'companyName'  => 'Relation Sans Courtier SARL',
+                'categories'   => ['/api/categories/'.$cat->getId()],
+                'relationType' => 'courtier',
+            ],
+        ])->toArray(false);
+
+        self::assertResponseStatusCodeSame(422);
+        $byPath = $this->violationsByPath($body);
+        self::assertArrayHasKey('broker', $byPath);
+        self::assertSame('Le nom du courtier est obligatoire.', $byPath['broker']);
+    }
+
+    /**
+     * Le champ transitoire relationType ne casse pas la creation nominale : avec
+     * la FK correspondante renseignee, le client se cree (201) et relationType
+     * n'est jamais serialise en sortie (write-only, aucun groupe de lecture).
+     */
+    public function testRelationDistributeurAvecDistributeurEst201(): void
+    {
+        $client      = $this->createAdminClient();
+        $cat         = $this->createCategory('SECTEUR');
+        $distributor = $this->seedClient('Distrib Cible', false, 'DISTRIBUTEUR');
+
+        $data = $client->request('POST', '/api/clients', [
+            'headers' => ['Content-Type' => self::LD],
+            'json'    => [
+                'companyName'  => 'Relation Ok SARL',
+                'categories'   => ['/api/categories/'.$cat->getId()],
+                'relationType' => 'distributeur',
+                'distributor'  => '/api/clients/'.$distributor->getId(),
+            ],
+        ])->toArray();
+
+        self::assertResponseStatusCodeSame(201);
+        self::assertArrayNotHasKey('relationType', $data);
+    }
 }
@@ -89,10 +89,7 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
        ]);

        self::assertResponseStatusCodeSame(422);
-        $byPath = [];
-        foreach ($response->toArray(false)['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
+        $byPath = $this->violationsByPath($response->toArray(false));

        self::assertArrayHasKey('email', $byPath, 'La violation email doit porter propertyPath=email (mapping front).');
        self::assertSame('L\'adresse email n\'est pas valide.', $byPath['email']);
@@ -135,10 +132,7 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
        ]);

        self::assertResponseStatusCodeSame(422);
-        $byPath = [];
-        foreach ($response->toArray(false)['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
+        $byPath = $this->violationsByPath($response->toArray(false));
        self::assertArrayHasKey('email', $byPath);
        self::assertSame('L\'adresse email n\'est pas valide.', $byPath['email']);
    }
@@ -386,10 +380,7 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
        ]);

        self::assertResponseStatusCodeSame(422);
-        $byPath = [];
-        foreach ($response->toArray(false)['violations'] ?? [] as $v) {
-            $byPath[$v['propertyPath']] = $v['message'];
-        }
+        $byPath = $this->violationsByPath($response->toArray(false));

        self::assertArrayHasKey('bic', $byPath, 'Le mismatch pays BIC/IBAN doit porter propertyPath=bic (mapping front).');
        self::assertSame('Le BIC ne correspond pas au pays de l\'IBAN.', $byPath['bic']);