MALIO-DEV/Starseed
3
0
Fork 0
Code Issues Pull Requests 14 Actions Packages Projects Releases Wiki Activity

feat(transport) : CarrierProcessor (RG-4.01→4.03/4.12→4.14) (ERP-158) #113

Merged
matthieu merged 1 commits from feat/erp-158-carrier-processor into feat/erp-155-carrier-schema-entities 2026-06-16 13:47:38 +00:00
Conversation 1 Commits 1 Files Changed 9
+1040 -23
matthieu commented 2026-06-16 06:15:12 +00:00
Owner
Copy Link
Copy Source

Normalisation + champs conditionnels + archive. Ticket ERP-158.

Ecriture du formulaire principal transporteur (M4, WT4) : Post/Patch via CarrierProcessor + CarrierFieldNormalizer, contraintes conditionnelles sur l'entite Carrier. Cible la branche WT3 (feat/erp-155-carrier-schema-entities), pas develop — stack.

Perimetre

  • CarrierProcessor (normalisation, gating archive mode strict, 409 doublon)
  • CarrierFieldNormalizer (name UPPER, person Title, phone digits, email lower, liotPlates ;-split/trim/UPPER)
  • Contraintes formulaire principal sur Carrier (NotBlank/Length name, Choice certif/contenant, Callback conditionnel)
  • Operations Post/Patch ajoutees a l'ApiResource (lecture WT3 conservee)

Regles de gestion

  • RG-4.01 : Post qualimatCarrier -> certificationType=QUALIMAT + FK ; cas LIOT -> certification non requise, liotPlates accepte
  • RG-4.02 : certificationType=AUTRE sans dischargeDocument -> 422
  • RG-4.03 : isChartered=true sans indexationRate/containerType/volumeM3 -> 422
  • RG-4.12 : doublon de nom (actifs) -> 409
  • RG-4.13 : normalisation serveur
  • RG-4.14 : Patch isArchived exige transport.carriers.archive (Admin seul), mode strict 403/422, restauration en conflit -> 409

Tests

CarrierWriteApiTest, CarrierRBACMatrixTest, CarrierArchiveTest, CarrierFieldNormalizerTest. make test vert (750), php-cs-fixer clean.

Normalisation + champs conditionnels + archive. Ticket ERP-158. Ecriture du formulaire principal transporteur (M4, WT4) : Post/Patch via CarrierProcessor + CarrierFieldNormalizer, contraintes conditionnelles sur l'entite Carrier. Cible la branche WT3 (feat/erp-155-carrier-schema-entities), pas develop — stack. ## Perimetre - CarrierProcessor (normalisation, gating archive mode strict, 409 doublon) - CarrierFieldNormalizer (name UPPER, person Title, phone digits, email lower, liotPlates ;-split/trim/UPPER) - Contraintes formulaire principal sur Carrier (NotBlank/Length name, Choice certif/contenant, Callback conditionnel) - Operations Post/Patch ajoutees a l'ApiResource (lecture WT3 conservee) ## Regles de gestion - RG-4.01 : Post qualimatCarrier -> certificationType=QUALIMAT + FK ; cas LIOT -> certification non requise, liotPlates accepte - RG-4.02 : certificationType=AUTRE sans dischargeDocument -> 422 - RG-4.03 : isChartered=true sans indexationRate/containerType/volumeM3 -> 422 - RG-4.12 : doublon de nom (actifs) -> 409 - RG-4.13 : normalisation serveur - RG-4.14 : Patch isArchived exige transport.carriers.archive (Admin seul), mode strict 403/422, restauration en conflit -> 409 ## Tests CarrierWriteApiTest, CarrierRBACMatrixTest, CarrierArchiveTest, CarrierFieldNormalizerTest. make test vert (750), php-cs-fixer clean.
matthieu added the backM4-Transporteurtype/feat labels 2026-06-16 06:15:31 +00:00
tristan reviewed 2026-06-16 09:27:29 +00:00
tristan left a comment
Owner
Copy Link
Copy Source

Code review ERP-158 (CarrierProcessor + champs conditionnels du formulaire principal) — relue contre docs/specs/M4-transporteurs/spec-back.md (RG-4.01→4.03 / 4.12→4.14) et les conventions backend.

Verdict : PR solide, aucun constat bloquant. C'est la PR d'écriture, et tous les points sensibles sont conformes :

  • Sécurité manage vs archive : Post/Patch gated transport.carriers.manage au niveau opération, puis re-gating fin transport.carriers.archive dans le CarrierProcessor (uniquement quand isArchived bascule réellement) → Bureau reçoit bien 403 sur l'archivage. Mode strict (archive + autre champ → 422) présent.
  • Contraintes de validation (mon point de vigilance de la PR précédente, levé) : message FR sur chaque #[Assert\*], miroir Length(max:255)↔Column(length:255) sur name, enums exacts, et les RG conditionnelles en #[Assert\Callback] + ->atPath() sur l'entité (meilleur que la spec qui disait « dans le Processor » : propertyPath mappable useFormErrors).
  • 409 doublon/restauration géré via catch UniqueConstraintViolationException (pas de 500).
  • Isolation inter-modules respectée, EXCLUDED_LENGTH_MIRROR (+2) justifié (Choice borné < length).

Tests : matrice RBAC complète (dont le piège Bureau-manage-sans-archive→403, non-faux-vert), RG couvertes avec assertions sur le propertyPath, 409 doublon+restauration, normalizer unitaire solide.

Quelques 🟡 mineurs en inline (valeurs conditionnelles résiduelles, hypothèse mono-index du catch 409, 2-3 trous de tests). Rien qui bloque le merge.

Code review ERP-158 (CarrierProcessor + champs conditionnels du formulaire principal) — relue contre `docs/specs/M4-transporteurs/spec-back.md` (RG-4.01→4.03 / 4.12→4.14) et les conventions backend. **Verdict : PR solide, aucun constat bloquant.** C'est la PR d'écriture, et tous les points sensibles sont conformes : - **Sécurité manage vs archive** : `Post`/`Patch` gated `transport.carriers.manage` au niveau opération, puis re-gating fin `transport.carriers.archive` dans le `CarrierProcessor` (uniquement quand `isArchived` bascule réellement) → Bureau reçoit bien 403 sur l'archivage. Mode strict (archive + autre champ → 422) présent. - **Contraintes de validation** (mon point de vigilance de la PR précédente, levé) : message FR sur chaque `#[Assert\*]`, miroir `Length(max:255)`↔`Column(length:255)` sur `name`, enums exacts, et les RG conditionnelles en `#[Assert\Callback]` + `->atPath()` sur l'**entité** (meilleur que la spec qui disait « dans le Processor » : propertyPath mappable `useFormErrors`). - **409** doublon/restauration géré via catch `UniqueConstraintViolationException` (pas de 500). - **Isolation inter-modules** respectée, `EXCLUDED_LENGTH_MIRROR` (+2) justifié (Choice borné < length). Tests : matrice RBAC complète (dont le piège Bureau-manage-sans-archive→403, non-faux-vert), RG couvertes avec assertions sur le `propertyPath`, 409 doublon+restauration, normalizer unitaire solide. Quelques 🟡 mineurs en inline (valeurs conditionnelles résiduelles, hypothèse mono-index du catch 409, 2-3 trous de tests). Rien qui bloque le merge.
src/Module/Transport/Domain/Entity/Carrier.php Outdated
@@ -103,3 +135,3 @@
#[ORM\Column(length: 255)]
#[Groups(['carrier:read'])]
#[Assert\NotBlank(message: 'Le nom du transporteur est obligatoire.', normalizer: 'trim')]
tristan commented 2026-06-16 09:27:29 +00:00
Owner
Copy Link
Copy Source

🟢 Contraintes conformes (mon point « à vérifier sur la PR d'écriture » de #112 est levé) : NotBlank + Length(min:2, max:255) avec messages FR, max:255 miroir exact du ORM\Column(length:255), cohérence 3 niveaux (colonne NOT NULL ↔ NotBlank). Les enums (certificationType, containerType) sont en Assert\Choice avec les valeurs exactes de la spec + messages FR. RAS.

🟢 Contraintes conformes (mon point « à vérifier sur la PR d'écriture » de #112 est levé) : `NotBlank` + `Length(min:2, max:255)` avec messages FR, `max:255` miroir exact du `ORM\Column(length:255)`, cohérence 3 niveaux (colonne NOT NULL ↔ NotBlank). Les enums (`certificationType`, `containerType`) sont en `Assert\Choice` avec les valeurs exactes de la spec + messages FR. RAS.
src/Module/Transport/Domain/Entity/Carrier.php Outdated
@@ -179,0 +243,4 @@
* pas de 422 sur la presence residuelle »). Le nom est compare en majuscules
* car la normalisation UPPER n'intervient qu'au processor (apres validation).
*/
#[Assert\Callback]
tristan commented 2026-06-16 09:27:29 +00:00
Owner
Copy Link
Copy Source

🟢 Les RG conditionnelles (RG-4.01 certif requise hors LIOT, RG-4.02 AUTRE→décharge, RG-4.03 affrété→indexation/contenant/volume) sont en #[Assert\Callback] avec ->atPath() sur le champ exact + messages FR, court-circuit LIOT en tête. À noter : la spec disait « Callback dans le CarrierProcessor » — l'avoir mis sur l'entité est un meilleur choix (chaque 422 porte un propertyPath consommable par useFormErrors, objectif ERP-101). Divergence volontaire et supérieure à la spec.

🟢 Les RG conditionnelles (RG-4.01 certif requise hors LIOT, RG-4.02 AUTRE→décharge, RG-4.03 affrété→indexation/contenant/volume) sont en `#[Assert\Callback]` avec `->atPath()` sur le champ exact + messages FR, court-circuit LIOT en tête. À noter : la spec disait « Callback **dans le CarrierProcessor** » — l'avoir mis sur l'**entité** est un meilleur choix (chaque 422 porte un propertyPath consommable par `useFormErrors`, objectif ERP-101). Divergence volontaire et supérieure à la spec.
src/Module/Transport/Infrastructure/ApiPlatform/State/Processor/CarrierProcessor.php Outdated
@@ -0,0 +99,4 @@
try {
return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
} catch (UniqueConstraintViolationException $e) {
tristan commented 2026-06-16 09:27:29 +00:00
Owner
Copy Link
Copy Source

🟡 Mineur (piège latent). Le 409 repose sur le catch large de UniqueConstraintViolationException, en attribuant TOUT conflit d'unicité au message « nom déjà pris ». C'est correct aujourd'hui car uq_carrier_name_active est le seul index unique de la table — mais cet invariant est load-bearing : si un futur index unique est ajouté sur carrier, son conflit serait mal libellé. Reco (optionnel) : commenter explicitement que l'unicité repose sur cet unique index, ou discriminer via $e->getViolatedConstraint() si exposé. L'approche catch (vs pré-check) est par ailleurs le bon choix (pas de race condition).

🟡 Mineur (piège latent). Le 409 repose sur le catch large de `UniqueConstraintViolationException`, en attribuant TOUT conflit d'unicité au message « nom déjà pris ». C'est correct aujourd'hui car `uq_carrier_name_active` est le **seul** index unique de la table — mais cet invariant est *load-bearing* : si un futur index unique est ajouté sur `carrier`, son conflit serait mal libellé. Reco (optionnel) : commenter explicitement que l'unicité repose sur cet unique index, ou discriminer via `$e->getViolatedConstraint()` si exposé. L'approche catch (vs pré-check) est par ailleurs le bon choix (pas de race condition).
src/Module/Transport/Infrastructure/ApiPlatform/State/Processor/CarrierProcessor.php Outdated
@@ -0,0 +143,4 @@
return false;
}
if (!$this->security->isGranted(self::PERM_ARCHIVE)) {
tristan commented 2026-06-16 09:27:29 +00:00
Owner
Copy Link
Copy Source

🟢 Séparation RBAC manage/archive correcte. La security d'opération (manage) ne peut pas distinguer « ce PATCH est-il un archivage ? » : le re-gating transport.carriers.archive est donc fait ici, et seulement quand isArchived bascule réellement (guardArchive court-circuite si l'entité n'est pas managée = POST, et si la valeur ne change pas). Un Bureau (manage sans archive) reçoit bien 403. Mode strict (archive + autre champ → 422) également présent. Conforme RG-4.14 / matrice § 5.2.

🟢 Séparation RBAC manage/archive correcte. La security d'opération (`manage`) ne peut pas distinguer « ce PATCH est-il un archivage ? » : le re-gating `transport.carriers.archive` est donc fait ici, et **seulement quand `isArchived` bascule réellement** (`guardArchive` court-circuite si l'entité n'est pas managée = POST, et si la valeur ne change pas). Un Bureau (manage sans archive) reçoit bien 403. Mode strict (archive + autre champ → 422) également présent. Conforme RG-4.14 / matrice § 5.2.
src/Module/Transport/Infrastructure/ApiPlatform/State/Processor/CarrierProcessor.php Outdated
@@ -0,0 +171,4 @@
* processor (sous-ressource, WT7). Les setters ne sont touches que si une valeur
* est presente, pour ne jamais ecraser l'existant lors d'un PATCH partiel.
*/
private function normalize(Carrier $data): void
tristan commented 2026-06-16 09:27:29 +00:00
Owner
Copy Link
Copy Source

🟡 Durcissement optionnel (non bloquant, conforme spec). normalize() ne touche que name/liotPlates — aucun nettoyage des champs conditionnels. C'est conforme à la spec § (« le back ne maintient pas de state machine : il stocke ce qu'il reçoit et valide la cohérence »), et la validation Callback garantit le sens « affrété ⇒ champs requis ».

Mais le sens inverse n'est pas borné : un PATCH {isChartered:false, indexationRate:'12.5'} (ou décharge sur une certif ≠ AUTRE, ou champs d'affrètement sur un LIOT) persiste une valeur sémantiquement morte. Le front masque ces champs, donc en pratique le risque est faible. À considérer : soit vider activement le champ quand son déclencheur est off, soit une garde de cohérence inverse. À trancher avec l'auteur de la spec — le code actuel est défendable.

🟡 Durcissement optionnel (non bloquant, conforme spec). `normalize()` ne touche que `name`/`liotPlates` — aucun nettoyage des champs conditionnels. C'est **conforme** à la spec § (« le back ne maintient pas de state machine : il stocke ce qu'il reçoit et valide la cohérence »), et la validation `Callback` garantit le sens « affrété ⇒ champs requis ». Mais le sens inverse n'est pas borné : un PATCH `{isChartered:false, indexationRate:'12.5'}` (ou décharge sur une certif ≠ AUTRE, ou champs d'affrètement sur un LIOT) persiste une valeur **sémantiquement morte**. Le front masque ces champs, donc en pratique le risque est faible. À considérer : soit vider activement le champ quand son déclencheur est off, soit une garde de cohérence inverse. À trancher avec l'auteur de la spec — le code actuel est défendable.
tests/Module/Transport/Api/CarrierWriteApiTest.php Outdated
@@ -0,0 +85,4 @@
* RG-4.02 : certificationType=AUTRE sans dischargeDocument -> 422 cible ; une
* certification != AUTRE sans decharge passe (201).
*/
public function testAutreCertificationRequiresDischarge(): void
tristan commented 2026-06-16 09:27:29 +00:00
Owner
Copy Link
Copy Source

🟡 Trous de couverture (non bloquants) :

  1. RG-4.02 chemin positif manquant : ce test couvre AUTRE-sans-décharge→422 et GMP_PLUS-sans-décharge→201, mais jamais AUTRE avec dischargeDocument→201. Le seul chemin qui prouve que la Callback laisse passer quand la décharge est fournie n'est pas exercé (l'infra uploaded_document existe).
  2. PATCH partiel sans écrasement : la logique merge-patch du Processor (setters touchés seulement si valeur présente + mécanique fieldChanged/originalData) est non triviale et non testée e2e — un PATCH {certificationType:'OVOCOM'} devrait conserver name/liotPlates.

Le reste des RG est très bien couvert, avec assertions sur le propertyPath (pas seulement le code HTTP). À compléter quand les filtres seront consolidés (ERP-162/163).

🟡 Trous de couverture (non bloquants) : 1. **RG-4.02 chemin positif manquant** : ce test couvre AUTRE-sans-décharge→422 et GMP_PLUS-sans-décharge→201, mais jamais **AUTRE *avec* `dischargeDocument`→201**. Le seul chemin qui prouve que la Callback laisse passer quand la décharge est fournie n'est pas exercé (l'infra `uploaded_document` existe). 2. **PATCH partiel sans écrasement** : la logique merge-patch du Processor (setters touchés seulement si valeur présente + mécanique `fieldChanged`/`originalData`) est non triviale et non testée e2e — un PATCH `{certificationType:'OVOCOM'}` devrait conserver `name`/`liotPlates`. Le reste des RG est très bien couvert, avec assertions sur le `propertyPath` (pas seulement le code HTTP). À compléter quand les filtres seront consolidés (ERP-162/163).
matthieu force-pushed feat/erp-158-carrier-processor from 97d7cacd2c to cbabe8f9ac 2026-06-16 13:06:28 +00:00 Compare
matthieu added 1 commit 2026-06-16 13:15:02 +00:00
feat(transport) : CarrierProcessor + champs conditionnels formulaire principal (ERP-158) 13d4a08bc9 
Ecriture du formulaire principal transporteur (M4, WT4) : POST/PATCH via
CarrierProcessor + CarrierFieldNormalizer, contraintes conditionnelles sur
l'entite Carrier.

- RG-4.01 : POST qualimatCarrier -> certificationType=QUALIMAT + FK persistee ;
  cas LIOT (name=LIOT) -> certification non requise, liotPlates accepte.
- RG-4.02 : certificationType=AUTRE sans dischargeDocument -> 422 (Assert\Callback).
- RG-4.03 : isChartered=true sans indexationRate/containerType/volumeM3 -> 422.
- RG-4.12 : doublon de nom (parmi actifs) -> 409 (index partiel uq_carrier_name_active).
- RG-4.13 : normalisation serveur (name UPPER, liotPlates ;-split/trim/UPPER) +
  methodes personne/telephone/email pour les sous-ressources Contact (WT7).
- RG-4.14 : PATCH isArchived exige transport.carriers.archive (Admin seul),
  mode strict -> 403 + 422 si autre champ ; restauration en conflit -> 409.

Operations Post/Patch ajoutees a l'ApiResource (lecture posee au WT3 conservee).
RG conditionnelles portees par validateMainFormConsistency (Assert\Callback +
->atPath()) pour un propertyPath mappable inline (useFormErrors, ERP-101).

certificationType / containerType whitelistes dans EXCLUDED_LENGTH_MIRROR (Choice
borne deja les valeurs, miroir SupplierAddress::addressType).

Tests : CarrierWriteApiTest (RG-4.01->4.03/4.12->4.14), CarrierRBACMatrixTest
(matrice bureau/compta/commerciale/usine), CarrierArchiveTest (409 restauration),
CarrierFieldNormalizerTest (RG-4.13). make test vert (750).
matthieu force-pushed feat/erp-158-carrier-processor from cbabe8f9ac to 13d4a08bc9 2026-06-16 13:15:02 +00:00 Compare
matthieu merged commit 13d4a08bc9 into feat/erp-155-carrier-schema-entities 2026-06-16 13:47:38 +00:00
matthieu deleted branch feat/erp-158-carrier-processor 2026-06-16 13:47:38 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
tristan
Labels
Clear labels
back
Backend PHP / Symfony / API Platform
 blocked
Bloqué par une dépendance externe
 breaking
Breaking change — impact API / contrat
 db
Migrations Doctrine / schéma BDD
 front
Frontend Nuxt / Vue / @malio/layer-ui
 infra
Docker / nginx / config infra
 M0-Categorie
M0 — Catégories
 M1-Client
M1 — Clients
 M2-Fournisseur
M2 — Fournisseurs
 M3-Prestataire
M3 — Prestataires
 M4-Transporteur
M4 — Transporteurs
 M5-Ticket-pesee
M5 — Tickets de pesée
 type/chore
Tâche de maintenance (bump, conf, etc.)
 type/ci
CI / pipeline / workflows
 type/docs
Documentation
 type/feat
Nouvelle fonctionnalité
 type/fix
Correction de bug
 type/perf
Amélioration de performance
 type/refactor
Refacto sans changement fonctionnel
 type/test
Ajout ou modification de tests
 wip
Work in progress — ne pas merger
No Label back M4-Transporteur type/feat
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
malio (admin malio) matthieu (THOLOT DECHENE Matthieu) tristan (Autin )
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: MALIO-DEV/Starseed#113
Delete Branch "feat/erp-158-carrier-processor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?