feat(order): modifier une commande avant paiement, sans creer de doublon #129

Merged
Corentin merged 2 commits from feat/modifier-commande-avant-paiement into dev 2026-07-31 15:25:00 +02:00
Owner
No description provided.
feat(order): modifier une commande avant paiement, sans creer de doublon
All checks were successful
CI / secret-scan (push) Successful in 13s
CI / php-lint (push) Successful in 26s
CI / static-tests (push) Successful in 1m2s
CI / js-tests (push) Successful in 33s
1867d9dca9
Le flux borne fait deux appels HTTP (creation puis encaissement). Entre les deux,
le client peut repartir modifier son panier. La cle d'idempotence renvoyait alors
la commande existante EN IGNORANT les lignes envoyees : la borne contournait en
changeant de cle a chaque entree sur l'ecran de paiement, ce qui creait une
SECONDE commande et laissait la premiere en attente jusqu'au balayage de 2h.

Le client obtenait ce qu'il voulait -- le defaut etait economique, pas
fonctionnel : chaque hesitation brulait un numero et salissait le compteur
"en attente". replaceItems supprime l'orpheline a la source. La cle identifie
desormais une SESSION de paiement : une commande, dont le contenu suit le panier.
Cle sur commande annulee ou expiree -> ORDER_CANCELLED (409), la borne repart
d'une cle neuve UNE fois (la colonne etant UNIQUE, elle est consommee) ; sans ca
un client dont la commande a ete annulee pendant qu'il hesitait restait bloque.

Le verrou de ligne explicite est le point a defendre, et il est etabli par
l'experience, pas par l'intuition. Deux mesures independantes sur la vraie base :
l'instantane de lecture InnoDB est pose PARESSEUSEMENT, a la premiere lecture non
verrouillante -- donc le motif garde-dans-le-WHERE du projet semblait suffire.
La piece decisive dit le contraire : gardes actives, UNE seule lecture simple
ajoutee dans la transaction d'encaissement, et le stock est debite pour un
produit que le client n'a pas commande, les deux UPDATE rendant 1 ligne affectee
donc AUCUN signal. La protection etait accidentelle, suspendue a un detail non
ecrit. Mesure complementaire : sur des totaux inchanges le meme UPDATE rend 1
puis 0 ligne selon la seconde en cours, donc "0 ligne = course perdue" ne tient
pas pour un remplacement. lockOrder() est donc pris par les deux operations, en
premiere instruction, et un test de non-regression verrouille cet ordre.

pay() relit aussi son total SOUS le verrou : une commande en attente etant
desormais modifiable, facturer le total lu avant la transaction serait un ecart
entre le montant annonce et le contenu reel. Corrige au passage : la course
perdue renvoyait 'preparing' au lieu du statut reel (une commande deja 'ready'
etait annoncee en preparation), et un commentaire affirmait que le projet
n'utilise aucun SELECT FOR UPDATE.

Verifie : 588 tests unitaires, 79 d'integration sur base reelle (dont l'empreinte
stock rendue intacte, verifiee chiffre par chiffre), 190 JS, PHPStan niveau 6
propre. Contrat HTTP eprouve en direct sur la production : meme cle + panier
different -> meme numero K235, total passe de 680 a 2040, UNE seule commande
creee (19 -> 20), base nettoyee derriere.

Regles : mlt.md 3.3bis. Cycle de vie : state-commande.md (boucle M1). ADR-0016.
fix(order): cinq trous fermes sur la modification avant paiement
All checks were successful
CI / secret-scan (push) Successful in 15s
CI / secret-scan (pull_request) Successful in 14s
CI / php-lint (push) Successful in 25s
CI / static-tests (push) Successful in 58s
CI / js-tests (push) Successful in 36s
CI / php-lint (pull_request) Successful in 25s
CI / static-tests (pull_request) Successful in 1m6s
CI / js-tests (pull_request) Successful in 32s
9d91f6d0dd
Une relecture adversariale du lot precedent a trouve cinq defauts reels. Le plus
grave ne demandait meme pas de concurrence.

1. L'EN-TETE ne suivait pas le panier. replaceItems ne rafraichissait que les
   lignes et les totaux : service_mode et service_tag restaient ceux de la
   creation. Sequence realisable par des gestes normaux -- client "sur place"
   chevalet 12, paiement echoue, il repasse "a emporter", il paie -> commande
   ENCAISSEE en dine_in chevalet 12. C'est le marqueur que le projet traite comme
   la distinction fiscale (TVA salle contre vente a emporter) qui etait faux sur
   une commande payee, donc sur le chiffre d'affaires, et un plateau partait vers
   une table vide. Le sens miroir jetait le chevalet saisi. Aucune trace, aucun
   code d'erreur. L'en-tete est rafraichi dans la meme transaction, avec la
   validation de la creation extraite en resolveHeader, et RG-T09 enoncee dans le
   code plutot que laissee a la contrainte de base.

2. Le verrou seul donnait une FAUSSE impression de protection structurelle -- ce
   qui rendait le piege plus facile a poser qu'avant. Mesure : avec une lecture
   simple posee avant le verrou, le FOR UPDATE rendait le nouveau total et la
   lecture suivante les anciennes lignes. Les lectures du CONTENU sont donc
   verrouillantes a leur tour (order_item et ses enfants) : la fraicheur devient
   structurelle au lieu de dependre de l'ordre des instructions. Le catalogue
   n'est pas verrouille (donnee de reference partagee).

3. replaceItems levait INVALID_TRANSITION sur une commande annulee, alors que la
   borne ne reprend que sur ORDER_CANCELLED : le client voyait un echec generique
   et ne se debloquait qu'au SECOND appui, ce qui se lit comme une borne en panne.

4. La cle vivait le temps de l'ONGLET, pas du panier. Sur une borne partagee, le
   client B qui abandonnait le panier de A voyait sa commande s'installer DANS
   celle de A -- heritant son mode de service, son horloge d'expiration et son
   numero deja affiche a A. clearCheckoutKey est appele partout ou clearCart
   l'est hors succes : panier vide vaut session de paiement soldee.

5. Le balayage selectionnait sur created_at. Une commande creee a 10h00 et
   modifiee a 12h01 mourait a 12h02, juste apres que le serveur lui a confirme sa
   modification, et l'audit affirmait une expiration "restee en attente". Le
   predicat porte desormais sur GREATEST(created_at, updated_at). Amende ADR-0014.

Inscrit comme reliquat plutot que corrige en silence : la garde de rupture est
evaluee avant le verrou et pay() ne la rejoue pas, donc deux clients peuvent
encaisser la derniere portion d'un ingredient et le stock passe en negatif sans
erreur. Fenetre pre-existante, non aggravee ici, et sa correction est une autre
fonctionnalite (ADR-0016, consequences).

Verifie : 594 tests unitaires, 84 d'integration, 191 JS, PHPStan niveau 6 propre.
Correctif 1 eprouve en direct sur la production : dine_in/chevalet 12 devient
takeaway/aucun sur la meme commande, base nettoyee derriere.
Corentin scheduled this pull request to auto merge when all checks succeed 2026-07-31 15:20:14 +02:00
Sign in to join this conversation.
No reviewers
No labels
auto-merge
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
AcadeNice/corentin_wakdo!129
No description provided.