fix(commercial) : validation tous-blocs des onglets collection client + fix 500 NonUniqueResult (ERP-110) (#61)
Auto Tag Develop / tag (push) Successful in 8s
Auto Tag Develop / tag (push) Successful in 8s
## Contexte (ERP-110, dérivé de ERP-107) Sur les onglets à blocs dynamiques d'un client (Contacts / Adresses / RIB), le POST d'une sous-ressource sur un client ayant déjà **≥2 enfants** renvoyait une **500 `NonUniqueResultException`**, court-circuitant la validation (aucune 422 par champ). ## Cause racine Au stade « read » du POST, le `Link` `toProperty` faisait résoudre la collection enfant via `getOneOrNullResult()` (`ItemProvider`) : `SELECT o FROM ClientContact o INNER JOIN o.client c WHERE c.id = :clientId`. Dès 2 enfants → `NonUniqueResult` → 500 **avant** la déserialisation/validation. Les 3 sous-ressources partageaient la même config (même bug latent). Cause secondaire front : la boucle de soumission s'arrêtait au 1er bloc en erreur (`return` dans le `catch`). ## Correctif **Back** — `read: false` sur les 3 opérations `Post` (`ClientContact` / `ClientAddress` / `ClientRib`) : le parent est déjà rattaché manuellement par le `*Processor::linkParent`. Les 3 `linkParent` sont durcis (`NotFoundHttpException` si parent absent → **404 préservé**, sinon régression 500 au persist sur `client_id NOT NULL`). **Front** — nouveau helper `useClientFormErrors().submitRows()` qui tente **tous** les blocs et collecte les erreurs 422 par index (`hasError`), branché sur les 6 sites (`new.vue` + `edit.vue` × contacts/adresses/RIB). Feedback **inline seul** : pas de toast récap, pas de toast succès tant qu'un bloc reste en erreur. ## Tests - Back : non-régression POST contact/adresse/RIB sur client déjà peuplé (≥2 enfants) → 201, + 422 `propertyPath=email` (validation atteinte). Rouge avant fix (500), vert après. - Front : `submitRows` (Vitest) — tente tous les blocs, mappe les erreurs par index, n'arrête pas au 1er échec, délègue le fallback non-422, saute les blocs filtrés. ## Vérifications - `make test` : 474/474 OK - `make php-cs-fixer-allow-risky` : 0 fichier à corriger - `make nuxt-test` : 219/219 OK > Golden path manuel navigateur non exécuté (couvert par les tests automatisés). --------- Co-authored-by: tristan <tristan@yuno.malio.fr> Co-authored-by: Matthieu <contact@malio.fr> Reviewed-on: #61 Co-authored-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr> Co-committed-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr>
This commit was merged in pull request #61.
This commit is contained in:
@@ -5,6 +5,7 @@ declare(strict_types=1);
|
||||
namespace App\Tests\Module\Commercial\Api;
|
||||
|
||||
use App\Module\Commercial\Domain\Entity\Client as ClientEntity;
|
||||
use App\Module\Commercial\Domain\Entity\ClientAddress;
|
||||
use App\Module\Commercial\Domain\Entity\ClientContact;
|
||||
use App\Module\Commercial\Domain\Entity\ClientRib;
|
||||
use App\Module\Commercial\Domain\Entity\PaymentType;
|
||||
@@ -94,6 +95,70 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
|
||||
self::assertSame('L\'adresse email n\'est pas valide.', $byPath['email']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression ERP-110 (bug subresource Link toProperty) : POST d'un contact sur
|
||||
* un client qui en a DEJA >= 2 ne doit pas exploser en 500
|
||||
* (NonUniqueResultException sur la resolution du parent), mais creer (201).
|
||||
*/
|
||||
public function testPostContactOnClientWithTwoExistingContactsReturns201(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
$seed = $this->seedClient('Contact Multi');
|
||||
$this->seedContact($seed, 'Alpha');
|
||||
$this->seedContact($seed, 'Beta');
|
||||
|
||||
$client->request('POST', '/api/clients/'.$seed->getId().'/contacts', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => ['firstName' => 'Gamma'],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(201);
|
||||
}
|
||||
|
||||
/**
|
||||
* Meme contexte (>= 2 contacts existants) : un email invalide doit produire
|
||||
* une 422 par champ (la validation est bien atteinte), pas une 500.
|
||||
*/
|
||||
public function testPostInvalidContactOnPopulatedClientReturns422OnField(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
$seed = $this->seedClient('Contact Multi Bad');
|
||||
$this->seedContact($seed, 'Alpha');
|
||||
$this->seedContact($seed, 'Beta');
|
||||
|
||||
$response = $client->request('POST', '/api/clients/'.$seed->getId().'/contacts', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => ['firstName' => 'Gamma', 'email' => 'pas-un-email'],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(422);
|
||||
$byPath = [];
|
||||
foreach ($response->toArray(false)['violations'] ?? [] as $v) {
|
||||
$byPath[$v['propertyPath']] = $v['message'];
|
||||
}
|
||||
self::assertArrayHasKey('email', $byPath);
|
||||
self::assertSame('L\'adresse email n\'est pas valide.', $byPath['email']);
|
||||
}
|
||||
|
||||
/**
|
||||
* ERP-110 : avec read:false sur le POST, un parent introuvable n'est plus
|
||||
* intercepte au stade lecture. Le 404 est desormais porte par
|
||||
* ClientContactProcessor::linkParent (sinon 500 au persist sur client_id
|
||||
* NOT NULL). Le payload est valide pour atteindre le processor (apres la
|
||||
* validation).
|
||||
*/
|
||||
public function testPostContactOnMissingClientReturns404(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
|
||||
$client->request('POST', '/api/clients/999999/contacts', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => ['firstName' => 'Orphan'],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(404);
|
||||
}
|
||||
|
||||
public function testPatchContactNormalizes(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
@@ -201,6 +266,61 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
|
||||
self::assertResponseStatusCodeSame(422);
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression ERP-110 : POST d'une adresse sur un client qui en a DEJA >= 2 ne
|
||||
* doit pas exploser en 500 (NonUniqueResult sur la resolution du parent). Le
|
||||
* POST porte un site + une categorie (RG-1.10 / RG-1.29) pour etre valide.
|
||||
*/
|
||||
public function testPostAddressOnClientWithTwoExistingAddressesReturns201(): void
|
||||
{
|
||||
$this->skipIfSitesModuleDisabled();
|
||||
$client = $this->createAdminClient();
|
||||
$seed = $this->seedClient('Addr Multi');
|
||||
$siteIri = $this->firstSiteIri();
|
||||
$category = $this->createCategory('SECTEUR');
|
||||
$this->seedAddress($seed, 'Bordeaux');
|
||||
$this->seedAddress($seed, 'Lyon');
|
||||
|
||||
$client->request('POST', '/api/clients/'.$seed->getId().'/addresses', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => [
|
||||
'postalCode' => '75001',
|
||||
'city' => 'Paris',
|
||||
'street' => '2 rue Neuve',
|
||||
'sites' => [$siteIri],
|
||||
'categories' => ['/api/categories/'.$category->getId()],
|
||||
],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(201);
|
||||
}
|
||||
|
||||
/**
|
||||
* ERP-110 : POST adresse sur un client inexistant -> 404 porte par
|
||||
* ClientAddressProcessor::linkParent (read:false). Payload valide (site +
|
||||
* categorie, RG-1.10 / RG-1.29) pour atteindre le processor.
|
||||
*/
|
||||
public function testPostAddressOnMissingClientReturns404(): void
|
||||
{
|
||||
$this->skipIfSitesModuleDisabled();
|
||||
$client = $this->createAdminClient();
|
||||
$siteIri = $this->firstSiteIri();
|
||||
$category = $this->createCategory('SECTEUR');
|
||||
|
||||
$client->request('POST', '/api/clients/999999/addresses', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => [
|
||||
'postalCode' => '75001',
|
||||
'city' => 'Paris',
|
||||
'street' => '2 rue Neuve',
|
||||
'sites' => [$siteIri],
|
||||
'categories' => ['/api/categories/'.$category->getId()],
|
||||
],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(404);
|
||||
}
|
||||
|
||||
// === RIBs ===
|
||||
|
||||
public function testPostRibByAdminReturns201(): void
|
||||
@@ -239,6 +359,43 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
|
||||
self::assertResponseStatusCodeSame(422);
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression ERP-110 : POST d'un RIB sur un client qui en a DEJA >= 2 ne doit
|
||||
* pas exploser en 500 (NonUniqueResult sur la resolution du parent). L'admin
|
||||
* porte commercial.clients.accounting.manage requis par le POST.
|
||||
*/
|
||||
public function testPostRibOnClientWithTwoExistingRibsReturns201(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
$seed = $this->seedClient('Rib Multi');
|
||||
$this->seedRib($seed, 'Compte 1');
|
||||
$this->seedRib($seed, 'Compte 2');
|
||||
|
||||
$client->request('POST', '/api/clients/'.$seed->getId().'/ribs', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => ['label' => 'Compte 3', 'bic' => self::VALID_BIC, 'iban' => self::VALID_IBAN],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(201);
|
||||
}
|
||||
|
||||
/**
|
||||
* ERP-110 : POST RIB sur un client inexistant -> 404 porte par
|
||||
* ClientRibProcessor::linkParent (read:false). L'admin porte
|
||||
* commercial.clients.accounting.manage ; payload valide (BIC / IBAN).
|
||||
*/
|
||||
public function testPostRibOnMissingClientReturns404(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
|
||||
$client->request('POST', '/api/clients/999999/ribs', [
|
||||
'headers' => ['Content-Type' => self::LD, 'Accept' => self::LD],
|
||||
'json' => ['label' => 'Orphan', 'bic' => self::VALID_BIC, 'iban' => self::VALID_IBAN],
|
||||
]);
|
||||
|
||||
self::assertResponseStatusCodeSame(404);
|
||||
}
|
||||
|
||||
public function testDeleteRibNonLcrReturns204(): void
|
||||
{
|
||||
$client = $this->createAdminClient();
|
||||
@@ -306,13 +463,34 @@ final class ClientSubResourceApiTest extends AbstractCommercialApiTestCase
|
||||
}
|
||||
|
||||
/**
|
||||
* Seede un ClientRib valide rattache a un client (sans passer par l'API).
|
||||
* Seede une adresse minimale valide en base (sans passer par l'API) : seules
|
||||
* les colonnes NOT NULL sont posees (CP / ville / rue). Les M2M sites /
|
||||
* categories restent vides — non contraints en base, suffisant pour peupler
|
||||
* un client de plusieurs adresses.
|
||||
*/
|
||||
private function seedRib(ClientEntity $client): ClientRib
|
||||
private function seedAddress(ClientEntity $client, string $city): ClientAddress
|
||||
{
|
||||
$em = $this->getEm();
|
||||
$address = new ClientAddress();
|
||||
$address->setClient($client);
|
||||
$address->setPostalCode('33000');
|
||||
$address->setCity($city);
|
||||
$address->setStreet('1 rue du Test');
|
||||
$em->persist($address);
|
||||
$em->flush();
|
||||
|
||||
return $address;
|
||||
}
|
||||
|
||||
/**
|
||||
* Seede un ClientRib valide rattache a un client (sans passer par l'API). Le
|
||||
* libelle est parametrable pour seeder plusieurs RIB distincts.
|
||||
*/
|
||||
private function seedRib(ClientEntity $client, string $label = 'Seed RIB'): ClientRib
|
||||
{
|
||||
$em = $this->getEm();
|
||||
$rib = new ClientRib();
|
||||
$rib->setLabel('Seed RIB');
|
||||
$rib->setLabel($label);
|
||||
$rib->setBic(self::VALID_BIC);
|
||||
$rib->setIban(self::VALID_IBAN);
|
||||
$rib->setClient($client);
|
||||
|
||||
@@ -8,11 +8,14 @@ use ApiPlatform\Metadata\Operation;
|
||||
use ApiPlatform\State\ProcessorInterface;
|
||||
use ApiPlatform\Validator\Exception\ValidationException;
|
||||
use App\Module\Commercial\Application\Service\ClientFieldNormalizer;
|
||||
use App\Module\Commercial\Application\Validator\ClientAccountingCompletenessValidator;
|
||||
use App\Module\Commercial\Application\Validator\ClientInformationCompletenessValidator;
|
||||
use App\Module\Commercial\Domain\Entity\Bank;
|
||||
use App\Module\Commercial\Domain\Entity\Client;
|
||||
use App\Module\Commercial\Domain\Entity\ClientRib;
|
||||
use App\Module\Commercial\Domain\Entity\PaymentDelay;
|
||||
use App\Module\Commercial\Domain\Entity\PaymentType;
|
||||
use App\Module\Commercial\Domain\Entity\TvaMode;
|
||||
use App\Module\Commercial\Infrastructure\ApiPlatform\State\Processor\ClientProcessor;
|
||||
use App\Shared\Domain\Contract\BusinessRoleAwareInterface;
|
||||
use App\Shared\Domain\Security\BusinessRoles;
|
||||
@@ -280,6 +283,65 @@ final class ClientProcessorTest extends TestCase
|
||||
self::assertInstanceOf(Client::class, $processor->process($client, $this->operation()));
|
||||
}
|
||||
|
||||
public function testFullAccountingSubmitWithEmptyFieldsIsUnprocessable(): void
|
||||
{
|
||||
// spec-front § Onglet Comptabilite : une validation complete de l'onglet
|
||||
// (les 6 champs presents dans le payload) avec des valeurs vides -> 422.
|
||||
// C'est le bug corrige : avant, le back acceptait un onglet tout vide.
|
||||
$client = $this->minimalClient(); // aucun champ comptable renseigne
|
||||
|
||||
$processor = $this->makeProcessor(
|
||||
granted: ['commercial.clients.accounting.manage'],
|
||||
payload: $this->emptyAccountingPayload(),
|
||||
);
|
||||
|
||||
$this->expectException(ValidationException::class);
|
||||
$processor->process($client, $this->operation());
|
||||
}
|
||||
|
||||
public function testFullAccountingSubmitWithAllFieldsPasses(): void
|
||||
{
|
||||
// Les 6 champs obligatoires renseignes + type de reglement neutre
|
||||
// (ni VIREMENT ni LCR -> ni banque ni RIB requis) -> 200.
|
||||
$client = $this->minimalClient();
|
||||
$client->setSiren('123456789');
|
||||
$client->setAccountNumber('00012345678');
|
||||
$client->setTvaMode(new TvaMode());
|
||||
$client->setNTva('FR12345678901');
|
||||
$client->setPaymentDelay(new PaymentDelay());
|
||||
$client->setPaymentType($this->paymentType('CHEQUE'));
|
||||
|
||||
$processor = $this->makeProcessor(
|
||||
granted: ['commercial.clients.accounting.manage'],
|
||||
payload: $this->emptyAccountingPayload(),
|
||||
);
|
||||
|
||||
self::assertInstanceOf(Client::class, $processor->process($client, $this->operation()));
|
||||
}
|
||||
|
||||
public function testPartialAccountingPatchSkipsCompleteness(): void
|
||||
{
|
||||
// Un PATCH ciblant un seul champ comptable n'est pas une validation
|
||||
// d'onglet : la completude n'est pas exigee (les autres champs restent
|
||||
// vides) -> 200. Preserve l'edition ponctuelle (ex. Compta corrige le SIREN).
|
||||
$client = $this->minimalClient();
|
||||
$client->setSiren('999999999');
|
||||
|
||||
$processor = $this->makeProcessor(
|
||||
granted: ['commercial.clients.accounting.manage'],
|
||||
payload: ['siren' => '999999999'],
|
||||
managed: true,
|
||||
originalData: [
|
||||
'siren' => '111111111',
|
||||
'companyName' => 'TEST CO',
|
||||
'triageService' => false,
|
||||
'isArchived' => false,
|
||||
],
|
||||
);
|
||||
|
||||
self::assertInstanceOf(Client::class, $processor->process($client, $this->operation()));
|
||||
}
|
||||
|
||||
public function testCommercialeIncompleteInformationIsUnprocessable(): void
|
||||
{
|
||||
// RG-1.04 : role Commerciale + onglet Information incomplet -> 422.
|
||||
@@ -379,6 +441,7 @@ final class ClientProcessorTest extends TestCase
|
||||
$persist,
|
||||
new ClientFieldNormalizer(),
|
||||
new ClientInformationCompletenessValidator(),
|
||||
new ClientAccountingCompletenessValidator(),
|
||||
$security,
|
||||
$requestStack,
|
||||
$em,
|
||||
@@ -398,6 +461,25 @@ final class ClientProcessorTest extends TestCase
|
||||
return $client;
|
||||
}
|
||||
|
||||
/**
|
||||
* Payload simulant une validation complete de l'onglet Comptabilite : les 6
|
||||
* champs obligatoires presents (le front les envoie toujours ensemble). Les
|
||||
* valeurs importent peu — la completude est evaluee sur l'etat de l'entite.
|
||||
*
|
||||
* @return array<string, mixed>
|
||||
*/
|
||||
private function emptyAccountingPayload(): array
|
||||
{
|
||||
return [
|
||||
'siren' => null,
|
||||
'accountNumber' => null,
|
||||
'tvaMode' => null,
|
||||
'nTva' => null,
|
||||
'paymentDelay' => null,
|
||||
'paymentType' => null,
|
||||
];
|
||||
}
|
||||
|
||||
private function paymentType(string $code): PaymentType
|
||||
{
|
||||
$type = new PaymentType();
|
||||
|
||||
Reference in New Issue
Block a user