From c1fcd9a7c832e2e955dbf38d0aacc6c13aa0204e Mon Sep 17 00:00:00 2001 From: Matthieu Date: Tue, 16 Jun 2026 14:57:45 +0200 Subject: [PATCH] test(transport) : rigueur RG sous-ressources (propertyPath, 404 parent, 401, certif) Repond aux retours de review (rigueur d'assertion transversale) : - mutualise assertViolationOnPath dans AbstractCarrierApiTestCase (au lieu d'un duplicata local a CarrierWriteApiTest) ; - asserte le propertyPath des 422 des sous-ressources (adresses city/street/postalCode, contacts firstName/phones/email, prix clientDeliveryAddress/supplierSupplyAddress/price) -> evite les faux-verts du mapping inline (ERP-101) ; - 404 parent (POST sur /carriers/999999/{addresses,contacts,prices}) ; - 401 anonyme + filtre ?certificationType= sur la collection (trous releves sur le contrat de lecture). --- .../Api/AbstractCarrierApiTestCase.php | 30 +++++++++- .../Transport/Api/CarrierAddressApiTest.php | 24 +++++++- .../Transport/Api/CarrierContactApiTest.php | 36 ++++++++++- .../Module/Transport/Api/CarrierListTest.php | 33 ++++++++++ .../Transport/Api/CarrierPriceApiTest.php | 60 +++++++++++++++++-- .../Transport/Api/CarrierWriteApiTest.php | 16 ----- 6 files changed, 170 insertions(+), 29 deletions(-) diff --git a/tests/Module/Transport/Api/AbstractCarrierApiTestCase.php b/tests/Module/Transport/Api/AbstractCarrierApiTestCase.php index e613d25..5850480 100644 --- a/tests/Module/Transport/Api/AbstractCarrierApiTestCase.php +++ b/tests/Module/Transport/Api/AbstractCarrierApiTestCase.php @@ -17,6 +17,7 @@ use App\Module\Transport\Domain\Entity\CarrierPrice; use App\Module\Transport\Domain\Entity\QualimatCarrier; use App\Tests\Module\Core\Api\AbstractApiTestCase; use DateTimeImmutable; +use Symfony\Contracts\HttpClient\ResponseInterface; /** * Base des tests fonctionnels du repertoire transporteurs (M4). Apporte les @@ -47,9 +48,11 @@ abstract class AbstractCarrierApiTestCase extends AbstractApiTestCase // client/supplier), liberant les Client/Supplier de test pour leur purge. $em->createQuery('DELETE FROM '.Carrier::class)->execute(); $em->createQuery('DELETE FROM '.ClientEntity::class.' c WHERE c.companyName LIKE :p') - ->setParameter('p', self::TEST_REF_PREFIX.'%')->execute(); + ->setParameter('p', self::TEST_REF_PREFIX.'%')->execute() + ; $em->createQuery('DELETE FROM '.Supplier::class.' s WHERE s.companyName LIKE :p') - ->setParameter('p', self::TEST_REF_PREFIX.'%')->execute(); + ->setParameter('p', self::TEST_REF_PREFIX.'%')->execute() + ; // qualimat_carrier : insere en DBAL brut (entite lecture seule) -> purge DBAL. $em->getConnection()->executeStatement( 'DELETE FROM qualimat_carrier WHERE siret LIKE :p', @@ -64,6 +67,27 @@ abstract class AbstractCarrierApiTestCase extends AbstractApiTestCase return $this->authenticatedClient('admin', 'admin'); } + /** + * Garde-fou ERP-101 : verifie qu'une reponse 422 porte une violation sur le + * `propertyPath` attendu (et pas seulement le bon code HTTP). Sans cette + * assertion, une 422 venue d'une AUTRE cause (autre champ manquant, IRI 404) + * ferait passer le test au vert sans prouver le mapping inline par champ. + * + * Mutualise dans la base (au lieu d'un duplicata par fichier) pour que toute + * la stack d'ecriture (formulaire principal + sous-ressources) l'utilise. + */ + protected static function assertViolationOnPath(object $response, string $path): void + { + /** @var ResponseInterface $response */ + $paths = array_column($response->toArray(false)['violations'] ?? [], 'propertyPath'); + + self::assertContains( + $path, + $paths, + sprintf('Aucune violation sur "%s" (paths: %s).', $path, implode(', ', $paths)), + ); + } + /** * Payload minimal valide du formulaire principal (transporteur non-QUALIMAT, * non affrete) : nom + certification GMP_PLUS. Sert de base aux tests @@ -247,7 +271,7 @@ abstract class AbstractCarrierApiTestCase extends AbstractApiTestCase 'status' => 'Valide', 'validity_date' => '2027-12-31', 'is_active' => 'true', - 'last_synced_at' => (new DateTimeImmutable())->format('Y-m-d H:i:s'), + 'last_synced_at' => new DateTimeImmutable()->format('Y-m-d H:i:s'), ]); $qualimat = $em->getRepository(QualimatCarrier::class)->findOneBy(['siret' => $siret]); diff --git a/tests/Module/Transport/Api/CarrierAddressApiTest.php b/tests/Module/Transport/Api/CarrierAddressApiTest.php index 6d163b8..935d2f9 100644 --- a/tests/Module/Transport/Api/CarrierAddressApiTest.php +++ b/tests/Module/Transport/Api/CarrierAddressApiTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace App\Tests\Module\Transport\Api; -use ApiPlatform\Symfony\Bundle\Test\Client; use App\Module\Core\Infrastructure\DataFixtures\RbacDemoFixtures; use App\Module\Transport\Domain\Entity\Carrier; use App\Module\Transport\Domain\Entity\CarrierAddress; @@ -56,11 +55,13 @@ final class CarrierAddressApiTest extends AbstractCarrierApiTestCase $carrier = $this->seedCarrierWithChartered('Cp Invalide', false); $client = $this->createAdminClient(); - $client->request('POST', '/api/carriers/'.$carrier->getId().'/addresses', [ + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/addresses', [ 'headers' => ['Content-Type' => self::LD], 'json' => ['postalCode' => '123'], // 3 chiffres -> Regex KO ]); self::assertResponseStatusCodeSame(422); + // La 422 doit cibler le champ fautif (mapping inline ERP-101), pas juste le code HTTP. + self::assertViolationOnPath($response, 'postalCode'); } public function testInconsistentPostalCodeAndCityIsAccepted(): void @@ -90,11 +91,15 @@ final class CarrierAddressApiTest extends AbstractCarrierApiTestCase $carrier = $this->seedCarrierWithChartered('Affrete Incomplet', true); $client = $this->createAdminClient(); - $client->request('POST', '/api/carriers/'.$carrier->getId().'/addresses', [ + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/addresses', [ 'headers' => ['Content-Type' => self::LD], 'json' => ['postalCode' => '86000'], ]); self::assertResponseStatusCodeSame(422); + // RG-4.05 mappe une violation PAR champ manquant (ville + rue ici) -> chaque + // erreur s'affiche inline sous son champ (ERP-101). + self::assertViolationOnPath($response, 'city'); + self::assertViolationOnPath($response, 'street'); } public function testCharteredCarrierCompleteAddressIsCreated(): void @@ -114,6 +119,19 @@ final class CarrierAddressApiTest extends AbstractCarrierApiTestCase self::assertResponseStatusCodeSame(201); } + public function testPostAddressOnUnknownCarrierReturns404(): void + { + // Sous-ressource en read:false : le parent introuvable n'est plus intercepte + // en amont -> le processor doit lever un 404 explicite (sinon 500 au persist). + $client = $this->createAdminClient(); + + $client->request('POST', '/api/carriers/999999/addresses', [ + 'headers' => ['Content-Type' => self::LD], + 'json' => ['postalCode' => '86000', 'city' => 'Poitiers', 'street' => '1 rue X'], + ]); + self::assertResponseStatusCodeSame(404); + } + public function testPatchAndDeleteSucceedWithManage(): void { $address = $this->seedAddress('Patch Delete', false); diff --git a/tests/Module/Transport/Api/CarrierContactApiTest.php b/tests/Module/Transport/Api/CarrierContactApiTest.php index 189ef45..68f1189 100644 --- a/tests/Module/Transport/Api/CarrierContactApiTest.php +++ b/tests/Module/Transport/Api/CarrierContactApiTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace App\Tests\Module\Transport\Api; use App\Module\Core\Infrastructure\DataFixtures\RbacDemoFixtures; -use App\Module\Transport\Domain\Entity\Carrier; use App\Module\Transport\Domain\Entity\CarrierContact; use Symfony\Bundle\FrameworkBundle\Console\Application; use Symfony\Component\Console\Input\ArrayInput; @@ -56,11 +55,13 @@ final class CarrierContactApiTest extends AbstractCarrierApiTestCase $carrier = $this->seedCarrier('Contact Vide'); $client = $this->createAdminClient(); - $client->request('POST', '/api/carriers/'.$carrier->getId().'/contacts', [ + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/contacts', [ 'headers' => ['Content-Type' => self::LD], 'json' => [], ]); self::assertResponseStatusCodeSame(422); + // RG-4.08 : la violation est rattachee a `firstName` (mapping inline ERP-101). + self::assertViolationOnPath($response, 'firstName'); } public function testSingleFieldContactIsCreated(): void @@ -87,7 +88,7 @@ final class CarrierContactApiTest extends AbstractCarrierApiTestCase $carrier = $this->seedCarrier('Contact Trois Tel'); $client = $this->createAdminClient(); - $client->request('POST', '/api/carriers/'.$carrier->getId().'/contacts', [ + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/contacts', [ 'headers' => ['Content-Type' => self::LD], 'json' => [ 'firstName' => 'Jean', @@ -95,6 +96,35 @@ final class CarrierContactApiTest extends AbstractCarrierApiTestCase ], ]); self::assertResponseStatusCodeSame(422); + // Le max-2 cible le champ virtuel `phones` (mapping inline ERP-101). + self::assertViolationOnPath($response, 'phones'); + } + + public function testInvalidEmailReturns422(): void + { + // L'email du contact porte un Assert\Email (nouvelle contrainte M4) : une + // adresse mal formee -> 422 ciblee sur `email`. + $carrier = $this->seedCarrier('Contact Email Invalide'); + $client = $this->createAdminClient(); + + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/contacts', [ + 'headers' => ['Content-Type' => self::LD], + 'json' => ['lastName' => 'Durand', 'email' => 'pas-un-email'], + ]); + self::assertResponseStatusCodeSame(422); + self::assertViolationOnPath($response, 'email'); + } + + public function testPostContactOnUnknownCarrierReturns404(): void + { + // Parent introuvable (read:false) -> 404 explicite du processor. + $client = $this->createAdminClient(); + + $client->request('POST', '/api/carriers/999999/contacts', [ + 'headers' => ['Content-Type' => self::LD], + 'json' => ['lastName' => 'Martin'], + ]); + self::assertResponseStatusCodeSame(404); } public function testPhonesAreMappedAndNormalized(): void diff --git a/tests/Module/Transport/Api/CarrierListTest.php b/tests/Module/Transport/Api/CarrierListTest.php index f9b1856..790d4c2 100644 --- a/tests/Module/Transport/Api/CarrierListTest.php +++ b/tests/Module/Transport/Api/CarrierListTest.php @@ -51,6 +51,39 @@ final class CarrierListTest extends AbstractCarrierApiTestCase self::assertCount(3, $data['member']); } + public function testAnonymousRequestReturns401(): void + { + // La collection est gatee par is_granted('transport.carriers.view') : un appel + // NON authentifie doit recevoir 401 (spec § 4.1 liste 401 ET 403 ; jusqu'ici + // seuls les exports couvraient le 401). + $http = self::createClient(); + + $http->request('GET', '/api/carriers', ['headers' => ['Accept' => self::LD]]); + self::assertResponseStatusCodeSame(401); + } + + public function testCertificationTypeFilterRestrictsResults(): void + { + // Filtre ?certificationType= (repetable, livre cote repo/provider mais + // jusqu'ici non exerce en collection) : seul le transporteur OVOCOM remonte. + $http = $this->createAdminClient(); + $token = $this->token(); + + $this->seedCarrier($token.' Gmp'); // GMP_PLUS (defaut seedCarrier) + $ovocom = $this->seedCarrier($token.' Ovo'); + $ovocom->setCertificationType('OVOCOM'); + $this->getEm()->flush(); + + $data = $http->request( + 'GET', + '/api/carriers?search='.$token.'&certificationType=OVOCOM', + ['headers' => ['Accept' => self::LD]], + )->toArray(); + + self::assertCount(1, $data['member'], 'Seul le transporteur OVOCOM doit remonter.'); + self::assertStringContainsString('OVO', (string) $data['member'][0]['name']); + } + /** * Anti N+1 (§ 2.11) : le nombre de requetes SQL de la liste ne doit PAS croitre * avec le nombre de transporteurs. On mesure pour N=2 puis N=4 (chacun avec son diff --git a/tests/Module/Transport/Api/CarrierPriceApiTest.php b/tests/Module/Transport/Api/CarrierPriceApiTest.php index 399430d..e669e37 100644 --- a/tests/Module/Transport/Api/CarrierPriceApiTest.php +++ b/tests/Module/Transport/Api/CarrierPriceApiTest.php @@ -99,8 +99,8 @@ final class CarrierPriceApiTest extends AbstractCarrierApiTestCase $this->getEm()->flush(); $siteId = $this->aSiteId(); - $client = $this->createAdminClient(); - $client->request('POST', '/api/carriers/'.$carrier->getId().'/prices', [ + $client = $this->createAdminClient(); + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/prices', [ 'headers' => ['Content-Type' => self::LD], 'json' => [ 'direction' => 'CLIENT', @@ -114,6 +114,9 @@ final class CarrierPriceApiTest extends AbstractCarrierApiTestCase ], ]); self::assertResponseStatusCodeSame(422); + // Faux-vert evite : la 422 doit prouver l'integrite referentielle adresse<->tiers + // (violation sur clientDeliveryAddress), pas une autre cause (RG-4.10, ERP-101). + self::assertViolationOnPath($response, 'clientDeliveryAddress'); } public function testForeignSupplierAddressReturns422(): void @@ -125,8 +128,8 @@ final class CarrierPriceApiTest extends AbstractCarrierApiTestCase $this->getEm()->flush(); $siteId = $this->aSiteId(); - $client = $this->createAdminClient(); - $client->request('POST', '/api/carriers/'.$carrier->getId().'/prices', [ + $client = $this->createAdminClient(); + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/prices', [ 'headers' => ['Content-Type' => self::LD], 'json' => [ 'direction' => 'FOURNISSEUR', @@ -140,6 +143,7 @@ final class CarrierPriceApiTest extends AbstractCarrierApiTestCase ], ]); self::assertResponseStatusCodeSame(422); + self::assertViolationOnPath($response, 'supplierSupplyAddress'); } public function testValidClientPriceIsCreated(): void @@ -192,6 +196,54 @@ final class CarrierPriceApiTest extends AbstractCarrierApiTestCase self::assertJsonContains(['direction' => 'FOURNISSEUR', 'priceState' => 'EN_COURS']); } + public function testNegativePriceReturns422(): void + { + // Le prix porte un Assert\PositiveOrZero : une valeur negative -> 422 sur `price` + // (la branche CLIENT est par ailleurs complete pour isoler la cause). + $carrier = $this->seedCarrier('Prix Negatif'); + $addr = $this->seedClientWithAddress('Client Prix Negatif'); + $this->getEm()->flush(); + $siteId = $this->aSiteId(); + + $client = $this->createAdminClient(); + $response = $client->request('POST', '/api/carriers/'.$carrier->getId().'/prices', [ + 'headers' => ['Content-Type' => self::LD], + 'json' => [ + 'direction' => 'CLIENT', + 'client' => '/api/clients/'.$addr->getClient()?->getId(), + 'clientDeliveryAddress' => '/api/client_addresses/'.$addr->getId(), + 'departureSite' => '/api/sites/'.$siteId, + 'containerType' => 'BENNE', + 'pricingUnit' => 'TONNE', + 'price' => '-5.00', + 'priceState' => 'VALIDE', + ], + ]); + self::assertResponseStatusCodeSame(422); + self::assertViolationOnPath($response, 'price'); + } + + public function testPostPriceOnUnknownCarrierReturns404(): void + { + // Parent introuvable (read:false) -> 404 explicite du processor (linkParent + // s'execute avant validateBranch). Le payload porte les scalaires NotBlank + // (containerType/pricingUnit/price/priceState) pour passer la validation + // d'entite et atteindre le processor, ou le 404 prime. + $client = $this->createAdminClient(); + + $client->request('POST', '/api/carriers/999999/prices', [ + 'headers' => ['Content-Type' => self::LD], + 'json' => [ + 'direction' => 'CLIENT', + 'containerType' => 'BENNE', + 'pricingUnit' => 'TONNE', + 'price' => '42.50', + 'priceState' => 'VALIDE', + ], + ]); + self::assertResponseStatusCodeSame(404); + } + public function testPatchAndDeleteSucceedWithManage(): void { $price = $this->seedClientPrice('Patch Delete'); diff --git a/tests/Module/Transport/Api/CarrierWriteApiTest.php b/tests/Module/Transport/Api/CarrierWriteApiTest.php index 130575b..e13df68 100644 --- a/tests/Module/Transport/Api/CarrierWriteApiTest.php +++ b/tests/Module/Transport/Api/CarrierWriteApiTest.php @@ -221,20 +221,4 @@ final class CarrierWriteApiTest extends AbstractCarrierApiTestCase ]); self::assertResponseStatusCodeSame(422); } - - /** - * Verifie qu'une violation 422 cible bien la propriete attendue (propertyPath), - * gage du mapping inline front (useFormErrors, ERP-101). - */ - private function assertViolationOnPath(object $response, string $path): void - { - /** @var \Symfony\Contracts\HttpClient\ResponseInterface $response */ - $paths = array_column($response->toArray(false)['violations'] ?? [], 'propertyPath'); - - self::assertContains( - $path, - $paths, - sprintf('Aucune violation sur "%s" (paths: %s).', $path, implode(', ', $paths)), - ); - } }