From a5d48cacbd68cdfa056beee46811199ee4cb5fbb Mon Sep 17 00:00:00 2001 From: Imugiii Date: Wed, 17 Jun 2026 07:45:29 +0000 Subject: [PATCH] fix(admin): conflits HTTP 409 au lieu de 422 (delete FK-bloque + course unicite) Aligne les controleurs admin sur le contrat byan-api.md / conventions.md : un conflit remonte par la base (SQLSTATE 23000) renvoie 409 Conflict, pas 422. Couvre la suppression dure bloquee par une FK RESTRICT (Product, Menu) et la course d'unicite slug/name a l'insertion (Category). La validation simple en amont (champ/format/bornes, PIN invalide, acteur verrouille) et la pre-verification nameExists/slugExists restent en 422. renderDelete gagne un parametre de statut optionnel (defaut inchange). Tests : assertions de conflit flippees en 409 (TDD). 201 tests, 597 assertions, PHPStan L6 propre. --- docs/api/conventions.md | 2 +- src/app/Catalogue/MenuRepository.php | 4 ++-- src/app/Catalogue/ProductRepository.php | 2 +- src/app/Controllers/CategoryController.php | 10 ++++++---- src/app/Controllers/MenuController.php | 10 +++++----- src/app/Controllers/ProductController.php | 14 +++++++------- src/app/Views/admin/menus/delete.php | 2 +- tests/Unit/Admin/CategoryControllerTest.php | 8 ++++---- tests/Unit/Admin/MenuControllerTest.php | 4 ++-- tests/Unit/Admin/ProductControllerTest.php | 4 ++-- 10 files changed, 31 insertions(+), 29 deletions(-) diff --git a/docs/api/conventions.md b/docs/api/conventions.md index 3599803..395012d 100644 --- a/docs/api/conventions.md +++ b/docs/api/conventions.md @@ -257,7 +257,7 @@ stable). | `NOT_FOUND` | 404 | ressource introuvable | | `METHOD_NOT_ALLOWED` | 405 | methode non autorisee sur ce chemin | | `VALIDATION_ERROR` | 422 | entree invalide (champ, longueur, enum) | -| `CONFLICT` | 409 | conflit d'etat (ex. transition de commande concurrente) | +| `CONFLICT` | 409 | conflit d'etat (ex. transition de commande concurrente) ; suppression dure bloquee par une reference (FK RESTRICT) ; unicite slug/name deja prise (remontee par la base). La validation simple en amont (champ/format/bornes) reste `VALIDATION_ERROR` 422 | | `AUTH_REQUIRED` | 401 | authentification requise (prevu, API admin) | | `FORBIDDEN` | 403 | permission insuffisante, ou jeton CSRF invalide cote formulaire | | `RATE_LIMITED` | 429 | throttling (prevu) | diff --git a/src/app/Catalogue/MenuRepository.php b/src/app/Catalogue/MenuRepository.php index 20f67a2..0ca9854 100644 --- a/src/app/Catalogue/MenuRepository.php +++ b/src/app/Catalogue/MenuRepository.php @@ -17,7 +17,7 @@ use App\Core\DatabaseInterface; * - menu_slot_option.menu_slot_id : CASCADE ; .product_id : RESTRICT. * - order_item.menu_id : RESTRICT -> la suppression dure est bloquee si le menu * est reference par une commande historique (mlt 8.6 RG-1 : le controleur - * traduit la violation en 422 et propose la desactivation). + * traduit la violation en 409 et propose la desactivation). * * create() et update() ecrivent menu + slots + options dans UNE transaction * (RG-T08). update() reconstruit les slots en delete-and-reinsert (mlt 8.5 RG-2). @@ -170,7 +170,7 @@ final class MenuRepository /** * Suppression dure. CASCADE retire menu_slot + menu_slot_option ; * order_item.menu_id (RESTRICT) bloque si une commande historique reference le - * menu (le controleur attrape SQLSTATE 23000 -> 422). + * menu (le controleur attrape SQLSTATE 23000 -> 409). */ public function delete(int $id): int { diff --git a/src/app/Catalogue/ProductRepository.php b/src/app/Catalogue/ProductRepository.php index 3a93f3a..38e7ba8 100644 --- a/src/app/Catalogue/ProductRepository.php +++ b/src/app/Catalogue/ProductRepository.php @@ -14,7 +14,7 @@ use App\Core\DatabaseInterface; * suppression dure : * - RESTRICT (bloquent la suppression) : order_item, menu.burger_product_id, * menu_slot_option, order_item_selection. Le controleur attrape la violation - * (SQLSTATE 23000) -> 422, plutot que de pre-tester chaque reference. + * (SQLSTATE 23000) -> 409 Conflit, plutot que de pre-tester chaque reference. * - CASCADE : product_ingredient (la recette appartient au produit ; la * supprimer avec le produit est voulu). La suppression n'est donc PAS bloquee * par une recette existante. TODO (phase stock/recettes, table aujourd'hui diff --git a/src/app/Controllers/CategoryController.php b/src/app/Controllers/CategoryController.php index 85d64da..60c098e 100644 --- a/src/app/Controllers/CategoryController.php +++ b/src/app/Controllers/CategoryController.php @@ -245,9 +245,11 @@ class CategoryController extends AdminController /** * Traduit une violation de contrainte d'unicite (SQLSTATE 23000) en - * re-affichage 422 du formulaire plutot qu'en 500. Couvre la fenetre de - * concurrence entre le controle nameExists/slugExists et l'ecriture. Tout - * autre code d'erreur est repropage (vrai incident interne). + * re-affichage 409 du formulaire plutot qu'en 500. Conflit remonte par la + * base (slug/name deja pris) = 409 Conflict, aligne sur le contrat d'API + * (SLUG_EXISTS). La pre-verification nameExists/slugExists reste, elle, en + * 422 (validation du formulaire) ; ce catch couvre la fenetre de concurrence + * entre ce controle et l'ecriture. Tout autre code d'erreur est repropage. * * @param array $form */ @@ -256,7 +258,7 @@ class CategoryController extends AdminController // getCode() rend la chaine SQLSTATE pour une vraie PDOException ; le cast // couvre aussi un code entier (23000 = violation de contrainte d'integrite). if ((string) $exception->getCode() === '23000') { - return $this->renderForm($guard, $id, $form, ['slug' => 'Ce libelle ou ce slug existe deja.'], 422); + return $this->renderForm($guard, $id, $form, ['slug' => 'Ce libelle ou ce slug existe deja.'], 409); } throw $exception; diff --git a/src/app/Controllers/MenuController.php b/src/app/Controllers/MenuController.php index 955aa55..cfd2199 100644 --- a/src/app/Controllers/MenuController.php +++ b/src/app/Controllers/MenuController.php @@ -24,7 +24,7 @@ use App\Core\Response; * vat_rate ; la sensibilite fiscale est au niveau composant -> hors RG-T13) ; * - delete (menu.delete) : action sensible -> PIN equipier + audit (RG-T13/T14, * mlt 8.6), suppression dure seulement si non reference par order_item.menu_id - * (FK RESTRICT -> 422 sinon, proposer la desactivation). + * (FK RESTRICT -> 409 sinon, proposer la desactivation). * * La configuration de slots est soumise en un champ cache `slots_json` (le * builder vanilla JS la serialise) : Request::formBody() ne retient que les @@ -236,7 +236,7 @@ class MenuController extends AdminController $name = (string) ($menu['name'] ?? ''); - // FK order_item.menu_id RESTRICT -> PDOException 23000 -> 422 (catch). + // FK order_item.menu_id RESTRICT -> PDOException 23000 -> 409 Conflit (catch). // menu_slot / menu_slot_option sont CASCADE (supprimes avec le menu). try { $this->db()->transaction(function (DatabaseInterface $db) use ($id, $actor, $name): void { @@ -247,7 +247,7 @@ class MenuController extends AdminController }); } catch (PDOException $exception) { if ((string) $exception->getCode() === '23000') { - return $this->renderDelete($guard, $id, $menu, 'Menu reference par des commandes : suppression impossible. Desactivez-le plutot.'); + return $this->renderDelete($guard, $id, $menu, 'Menu reference par des commandes : suppression impossible. Desactivez-le plutot.', 409); } throw $exception; @@ -478,7 +478,7 @@ class MenuController extends AdminController /** * @param array $menu */ - private function renderDelete(GuardResult $guard, int $id, array $menu, ?string $error): Response + private function renderDelete(GuardResult $guard, int $id, array $menu, ?string $error, ?int $status = null): Response { return $this->adminView('admin/menus/delete', [ 'title' => 'Supprimer un menu - Wakdo Admin', @@ -486,7 +486,7 @@ class MenuController extends AdminController 'menuId' => $id, 'name' => (string) ($menu['name'] ?? ''), 'error' => $error, - ], $guard, $error !== null ? 422 : 200); + ], $guard, $status ?? ($error !== null ? 422 : 200)); } private function notFound(GuardResult $guard): Response diff --git a/src/app/Controllers/ProductController.php b/src/app/Controllers/ProductController.php index 6b26e53..062f8ce 100644 --- a/src/app/Controllers/ProductController.php +++ b/src/app/Controllers/ProductController.php @@ -22,7 +22,7 @@ use App\Core\Response; * - update (product.update) : PIN equipier + audit UNIQUEMENT si prix ou TVA * change (mlt 8.2 RG-4) ; sinon mise a jour simple ; * - delete (product.delete) : PIN equipier + audit, suppression dure seulement si - * le produit n'est reference nulle part (FK RESTRICT -> 422 sinon). + * le produit n'est reference nulle part (FK RESTRICT -> 409 sinon). * Le PIN suit le modele "identifiant equipier + PIN" : email + PIN resolus en un * acting_user_id ecrit dans audit_log, dans la meme transaction que l'effet (RG-T08). * @@ -252,8 +252,8 @@ class ProductController extends AdminController $name = (string) ($product['name'] ?? ''); // FK RESTRICT (order_item / menu / menu_slot_option / order_item_selection) - // -> PDOException 23000 -> 422 (catch ci-dessous). product_ingredient est - // CASCADE (recette possedee par le produit) : supprimee avec lui, jamais + // -> PDOException 23000 -> 409 Conflit (catch ci-dessous). product_ingredient + // est CASCADE (recette possedee par le produit) : supprimee avec lui, jamais // bloquante (cf. docblock ProductRepository). try { $this->db()->transaction(function (DatabaseInterface $db) use ($id, $actor, $name): void { @@ -264,7 +264,7 @@ class ProductController extends AdminController }); } catch (PDOException $exception) { if ((string) $exception->getCode() === '23000') { - return $this->renderDelete($guard, $id, $product, 'Produit reference par des commandes ou menus : suppression impossible. Masquez-le plutot.'); + return $this->renderDelete($guard, $id, $product, 'Produit reference par des commandes ou menus : suppression impossible. Masquez-le plutot.', 409); } throw $exception; @@ -272,7 +272,7 @@ class ProductController extends AdminController // PIN valide et suppression effective : reinitialise le compteur de l'acteur // de session (RG-T22, cle = $actorId). Apres le try/catch : non atteint si la - // FK a bloque (422), ce qui est benin (l'acteur n'est pas un attaquant). + // FK a bloque (409), ce qui est benin (l'acteur n'est pas un attaquant). $this->pinThrottle()->reset($actorId); $this->setFlash('Produit supprime.'); @@ -447,7 +447,7 @@ class ProductController extends AdminController /** * @param array $product */ - private function renderDelete(GuardResult $guard, int $id, array $product, ?string $error): Response + private function renderDelete(GuardResult $guard, int $id, array $product, ?string $error, ?int $status = null): Response { return $this->adminView('admin/products/delete', [ 'title' => 'Supprimer un produit - Wakdo Admin', @@ -455,7 +455,7 @@ class ProductController extends AdminController 'productId' => $id, 'name' => (string) ($product['name'] ?? ''), 'error' => $error, - ], $guard, $error !== null ? 422 : 200); + ], $guard, $status ?? ($error !== null ? 422 : 200)); } private function notFound(GuardResult $guard): Response diff --git a/src/app/Views/admin/menus/delete.php b/src/app/Views/admin/menus/delete.php index 24d8dc7..8973af4 100644 --- a/src/app/Views/admin/menus/delete.php +++ b/src/app/Views/admin/menus/delete.php @@ -5,7 +5,7 @@ declare(strict_types=1); /** * Confirmation de suppression d'un menu (action sensible RG-T13/mlt 8.6) : exige * l'email + le PIN de l'equipier. La suppression cascade vers menu_slot / - * menu_slot_option ; bloquee (422) si reference par une commande historique. + * menu_slot_option ; bloquee (409) si reference par une commande historique. * Injecte dans admin/layout.php. * * @var int $menuId diff --git a/tests/Unit/Admin/CategoryControllerTest.php b/tests/Unit/Admin/CategoryControllerTest.php index 6362291..3941090 100644 --- a/tests/Unit/Admin/CategoryControllerTest.php +++ b/tests/Unit/Admin/CategoryControllerTest.php @@ -237,10 +237,10 @@ final class CategoryControllerTest extends TestCase self::assertFalse($this->wroteContaining($db, 'INSERT INTO category')); } - public function testStoreTranslatesUniqueViolationTo422(): void + public function testStoreTranslatesUniqueViolationTo409(): void { - // Fenetre de concurrence : la base leve une violation 23000 a l'insertion ; - // le controleur doit re-afficher le formulaire (422), pas remonter un 500. + // Fenetre de concurrence : la base leve une violation 23000 a l'insertion. + // Conflit remonte par la base -> 409 (re-affiche le formulaire), pas un 500. $db = $this->permittedDb(); $db->failOnExecute = new \PDOException('duplicate', 23000); $request = $this->post( @@ -250,7 +250,7 @@ final class CategoryControllerTest extends TestCase $response = $this->controller($request, $db)->store(); - self::assertSame(422, $response->status()); + self::assertSame(409, $response->status()); self::assertStringContainsString('existe deja', $response->body()); } diff --git a/tests/Unit/Admin/MenuControllerTest.php b/tests/Unit/Admin/MenuControllerTest.php index 3701850..bc18a3e 100644 --- a/tests/Unit/Admin/MenuControllerTest.php +++ b/tests/Unit/Admin/MenuControllerTest.php @@ -269,7 +269,7 @@ final class MenuControllerTest extends TestCase self::assertSame(1, $reset['params']['uid'] ?? null); } - public function testDestroyReferencedByOrderReturns422(): void + public function testDestroyReferencedByOrderReturns409(): void { $db = $this->permittedDb(); $db->menuRow = ['id' => 5, 'name' => 'Best Of']; @@ -278,7 +278,7 @@ final class MenuControllerTest extends TestCase $response = $this->controller($this->post(['_csrf' => $this->csrf, 'pin_email' => 'staff@wakdo.local', 'pin' => '4729'], '/admin/menus/5/delete'), $db)->destroy(['id' => '5']); - self::assertSame(422, $response->status()); + self::assertSame(409, $response->status()); self::assertStringContainsString('suppression impossible', $response->body()); } diff --git a/tests/Unit/Admin/ProductControllerTest.php b/tests/Unit/Admin/ProductControllerTest.php index 1a476eb..317d379 100644 --- a/tests/Unit/Admin/ProductControllerTest.php +++ b/tests/Unit/Admin/ProductControllerTest.php @@ -304,7 +304,7 @@ final class ProductControllerTest extends TestCase $this->assertAuditWithinTransaction($db); } - public function testDestroyReferencedReturns422(): void + public function testDestroyReferencedReturns409(): void { $db = $this->permittedDb(); $db->productRow = ['id' => 5, 'name' => 'Big Mac']; @@ -313,7 +313,7 @@ final class ProductControllerTest extends TestCase $response = $this->controller($this->post(['_csrf' => $this->csrf, 'pin_email' => 'staff@wakdo.local', 'pin' => '4729'], '/admin/products/5/delete'), $db)->destroy(['id' => '5']); - self::assertSame(422, $response->status()); + self::assertSame(409, $response->status()); self::assertStringContainsString('reference', $response->body()); }