fix(admin): conflits HTTP 409 au lieu de 422 (delete FK-bloque + course unicite) (#33)
This commit is contained in:
parent
c2a4854083
commit
0666a22562
10 changed files with 31 additions and 29 deletions
|
|
@ -257,7 +257,7 @@ stable).
|
||||||
| `NOT_FOUND` | 404 | ressource introuvable |
|
| `NOT_FOUND` | 404 | ressource introuvable |
|
||||||
| `METHOD_NOT_ALLOWED` | 405 | methode non autorisee sur ce chemin |
|
| `METHOD_NOT_ALLOWED` | 405 | methode non autorisee sur ce chemin |
|
||||||
| `VALIDATION_ERROR` | 422 | entree invalide (champ, longueur, enum) |
|
| `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) |
|
| `AUTH_REQUIRED` | 401 | authentification requise (prevu, API admin) |
|
||||||
| `FORBIDDEN` | 403 | permission insuffisante, ou jeton CSRF invalide cote formulaire |
|
| `FORBIDDEN` | 403 | permission insuffisante, ou jeton CSRF invalide cote formulaire |
|
||||||
| `RATE_LIMITED` | 429 | throttling (prevu) |
|
| `RATE_LIMITED` | 429 | throttling (prevu) |
|
||||||
|
|
|
||||||
|
|
@ -17,7 +17,7 @@ use App\Core\DatabaseInterface;
|
||||||
* - menu_slot_option.menu_slot_id : CASCADE ; .product_id : RESTRICT.
|
* - menu_slot_option.menu_slot_id : CASCADE ; .product_id : RESTRICT.
|
||||||
* - order_item.menu_id : RESTRICT -> la suppression dure est bloquee si le menu
|
* - 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
|
* 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
|
* 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).
|
* (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 ;
|
* Suppression dure. CASCADE retire menu_slot + menu_slot_option ;
|
||||||
* order_item.menu_id (RESTRICT) bloque si une commande historique reference le
|
* 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
|
public function delete(int $id): int
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -14,7 +14,7 @@ use App\Core\DatabaseInterface;
|
||||||
* suppression dure :
|
* suppression dure :
|
||||||
* - RESTRICT (bloquent la suppression) : order_item, menu.burger_product_id,
|
* - RESTRICT (bloquent la suppression) : order_item, menu.burger_product_id,
|
||||||
* menu_slot_option, order_item_selection. Le controleur attrape la violation
|
* 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
|
* - CASCADE : product_ingredient (la recette appartient au produit ; la
|
||||||
* supprimer avec le produit est voulu). La suppression n'est donc PAS bloquee
|
* supprimer avec le produit est voulu). La suppression n'est donc PAS bloquee
|
||||||
* par une recette existante. TODO (phase stock/recettes, table aujourd'hui
|
* par une recette existante. TODO (phase stock/recettes, table aujourd'hui
|
||||||
|
|
|
||||||
|
|
@ -245,9 +245,11 @@ class CategoryController extends AdminController
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Traduit une violation de contrainte d'unicite (SQLSTATE 23000) en
|
* Traduit une violation de contrainte d'unicite (SQLSTATE 23000) en
|
||||||
* re-affichage 422 du formulaire plutot qu'en 500. Couvre la fenetre de
|
* re-affichage 409 du formulaire plutot qu'en 500. Conflit remonte par la
|
||||||
* concurrence entre le controle nameExists/slugExists et l'ecriture. Tout
|
* base (slug/name deja pris) = 409 Conflict, aligne sur le contrat d'API
|
||||||
* autre code d'erreur est repropage (vrai incident interne).
|
* (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<string, mixed> $form
|
* @param array<string, mixed> $form
|
||||||
*/
|
*/
|
||||||
|
|
@ -256,7 +258,7 @@ class CategoryController extends AdminController
|
||||||
// getCode() rend la chaine SQLSTATE pour une vraie PDOException ; le cast
|
// getCode() rend la chaine SQLSTATE pour une vraie PDOException ; le cast
|
||||||
// couvre aussi un code entier (23000 = violation de contrainte d'integrite).
|
// couvre aussi un code entier (23000 = violation de contrainte d'integrite).
|
||||||
if ((string) $exception->getCode() === '23000') {
|
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;
|
throw $exception;
|
||||||
|
|
|
||||||
|
|
@ -24,7 +24,7 @@ use App\Core\Response;
|
||||||
* vat_rate ; la sensibilite fiscale est au niveau composant -> hors RG-T13) ;
|
* vat_rate ; la sensibilite fiscale est au niveau composant -> hors RG-T13) ;
|
||||||
* - delete (menu.delete) : action sensible -> PIN equipier + audit (RG-T13/T14,
|
* - 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
|
* 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
|
* La configuration de slots est soumise en un champ cache `slots_json` (le
|
||||||
* builder vanilla JS la serialise) : Request::formBody() ne retient que les
|
* builder vanilla JS la serialise) : Request::formBody() ne retient que les
|
||||||
|
|
@ -236,7 +236,7 @@ class MenuController extends AdminController
|
||||||
|
|
||||||
$name = (string) ($menu['name'] ?? '');
|
$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).
|
// menu_slot / menu_slot_option sont CASCADE (supprimes avec le menu).
|
||||||
try {
|
try {
|
||||||
$this->db()->transaction(function (DatabaseInterface $db) use ($id, $actor, $name): void {
|
$this->db()->transaction(function (DatabaseInterface $db) use ($id, $actor, $name): void {
|
||||||
|
|
@ -247,7 +247,7 @@ class MenuController extends AdminController
|
||||||
});
|
});
|
||||||
} catch (PDOException $exception) {
|
} catch (PDOException $exception) {
|
||||||
if ((string) $exception->getCode() === '23000') {
|
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;
|
throw $exception;
|
||||||
|
|
@ -478,7 +478,7 @@ class MenuController extends AdminController
|
||||||
/**
|
/**
|
||||||
* @param array<string, mixed> $menu
|
* @param array<string, mixed> $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', [
|
return $this->adminView('admin/menus/delete', [
|
||||||
'title' => 'Supprimer un menu - Wakdo Admin',
|
'title' => 'Supprimer un menu - Wakdo Admin',
|
||||||
|
|
@ -486,7 +486,7 @@ class MenuController extends AdminController
|
||||||
'menuId' => $id,
|
'menuId' => $id,
|
||||||
'name' => (string) ($menu['name'] ?? ''),
|
'name' => (string) ($menu['name'] ?? ''),
|
||||||
'error' => $error,
|
'error' => $error,
|
||||||
], $guard, $error !== null ? 422 : 200);
|
], $guard, $status ?? ($error !== null ? 422 : 200));
|
||||||
}
|
}
|
||||||
|
|
||||||
private function notFound(GuardResult $guard): Response
|
private function notFound(GuardResult $guard): Response
|
||||||
|
|
|
||||||
|
|
@ -22,7 +22,7 @@ use App\Core\Response;
|
||||||
* - update (product.update) : PIN equipier + audit UNIQUEMENT si prix ou TVA
|
* - update (product.update) : PIN equipier + audit UNIQUEMENT si prix ou TVA
|
||||||
* change (mlt 8.2 RG-4) ; sinon mise a jour simple ;
|
* change (mlt 8.2 RG-4) ; sinon mise a jour simple ;
|
||||||
* - delete (product.delete) : PIN equipier + audit, suppression dure seulement si
|
* - 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
|
* 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).
|
* 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'] ?? '');
|
$name = (string) ($product['name'] ?? '');
|
||||||
|
|
||||||
// FK RESTRICT (order_item / menu / menu_slot_option / order_item_selection)
|
// FK RESTRICT (order_item / menu / menu_slot_option / order_item_selection)
|
||||||
// -> PDOException 23000 -> 422 (catch ci-dessous). product_ingredient est
|
// -> PDOException 23000 -> 409 Conflit (catch ci-dessous). product_ingredient
|
||||||
// CASCADE (recette possedee par le produit) : supprimee avec lui, jamais
|
// est CASCADE (recette possedee par le produit) : supprimee avec lui, jamais
|
||||||
// bloquante (cf. docblock ProductRepository).
|
// bloquante (cf. docblock ProductRepository).
|
||||||
try {
|
try {
|
||||||
$this->db()->transaction(function (DatabaseInterface $db) use ($id, $actor, $name): void {
|
$this->db()->transaction(function (DatabaseInterface $db) use ($id, $actor, $name): void {
|
||||||
|
|
@ -264,7 +264,7 @@ class ProductController extends AdminController
|
||||||
});
|
});
|
||||||
} catch (PDOException $exception) {
|
} catch (PDOException $exception) {
|
||||||
if ((string) $exception->getCode() === '23000') {
|
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;
|
throw $exception;
|
||||||
|
|
@ -272,7 +272,7 @@ class ProductController extends AdminController
|
||||||
|
|
||||||
// PIN valide et suppression effective : reinitialise le compteur de l'acteur
|
// 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
|
// 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->pinThrottle()->reset($actorId);
|
||||||
|
|
||||||
$this->setFlash('Produit supprime.');
|
$this->setFlash('Produit supprime.');
|
||||||
|
|
@ -447,7 +447,7 @@ class ProductController extends AdminController
|
||||||
/**
|
/**
|
||||||
* @param array<string, mixed> $product
|
* @param array<string, mixed> $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', [
|
return $this->adminView('admin/products/delete', [
|
||||||
'title' => 'Supprimer un produit - Wakdo Admin',
|
'title' => 'Supprimer un produit - Wakdo Admin',
|
||||||
|
|
@ -455,7 +455,7 @@ class ProductController extends AdminController
|
||||||
'productId' => $id,
|
'productId' => $id,
|
||||||
'name' => (string) ($product['name'] ?? ''),
|
'name' => (string) ($product['name'] ?? ''),
|
||||||
'error' => $error,
|
'error' => $error,
|
||||||
], $guard, $error !== null ? 422 : 200);
|
], $guard, $status ?? ($error !== null ? 422 : 200));
|
||||||
}
|
}
|
||||||
|
|
||||||
private function notFound(GuardResult $guard): Response
|
private function notFound(GuardResult $guard): Response
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,7 @@ declare(strict_types=1);
|
||||||
/**
|
/**
|
||||||
* Confirmation de suppression d'un menu (action sensible RG-T13/mlt 8.6) : exige
|
* 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 /
|
* 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.
|
* Injecte dans admin/layout.php.
|
||||||
*
|
*
|
||||||
* @var int $menuId
|
* @var int $menuId
|
||||||
|
|
|
||||||
|
|
@ -237,10 +237,10 @@ final class CategoryControllerTest extends TestCase
|
||||||
self::assertFalse($this->wroteContaining($db, 'INSERT INTO category'));
|
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 ;
|
// Fenetre de concurrence : la base leve une violation 23000 a l'insertion.
|
||||||
// le controleur doit re-afficher le formulaire (422), pas remonter un 500.
|
// Conflit remonte par la base -> 409 (re-affiche le formulaire), pas un 500.
|
||||||
$db = $this->permittedDb();
|
$db = $this->permittedDb();
|
||||||
$db->failOnExecute = new \PDOException('duplicate', 23000);
|
$db->failOnExecute = new \PDOException('duplicate', 23000);
|
||||||
$request = $this->post(
|
$request = $this->post(
|
||||||
|
|
@ -250,7 +250,7 @@ final class CategoryControllerTest extends TestCase
|
||||||
|
|
||||||
$response = $this->controller($request, $db)->store();
|
$response = $this->controller($request, $db)->store();
|
||||||
|
|
||||||
self::assertSame(422, $response->status());
|
self::assertSame(409, $response->status());
|
||||||
self::assertStringContainsString('existe deja', $response->body());
|
self::assertStringContainsString('existe deja', $response->body());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -269,7 +269,7 @@ final class MenuControllerTest extends TestCase
|
||||||
self::assertSame(1, $reset['params']['uid'] ?? null);
|
self::assertSame(1, $reset['params']['uid'] ?? null);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testDestroyReferencedByOrderReturns422(): void
|
public function testDestroyReferencedByOrderReturns409(): void
|
||||||
{
|
{
|
||||||
$db = $this->permittedDb();
|
$db = $this->permittedDb();
|
||||||
$db->menuRow = ['id' => 5, 'name' => 'Best Of'];
|
$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']);
|
$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());
|
self::assertStringContainsString('suppression impossible', $response->body());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -304,7 +304,7 @@ final class ProductControllerTest extends TestCase
|
||||||
$this->assertAuditWithinTransaction($db);
|
$this->assertAuditWithinTransaction($db);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testDestroyReferencedReturns422(): void
|
public function testDestroyReferencedReturns409(): void
|
||||||
{
|
{
|
||||||
$db = $this->permittedDb();
|
$db = $this->permittedDb();
|
||||||
$db->productRow = ['id' => 5, 'name' => 'Big Mac'];
|
$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']);
|
$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());
|
self::assertStringContainsString('reference', $response->body());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue