[ERP-50] Implémenter les composables useCategoriesAdmin et useCategoryForm #25

Merged
tristan merged 2 commits from feature/ERP-50-0-8-frontend-m-implementer-les-composables-usecate into develop 2026-05-29 09:18:30 +00:00
Owner

Lien Lesstime : #50

Résumé

Refacto : extraction de la logique fetch/CRUD inline de la page categories (ERP-49) vers deux composables dédiés, conformément au pattern Starseed (useSidebar / useModules).

  • useCategoriesAdmin : singleton state (categories + types + loading + error). Pré-chargement des types au mount de la page (au lieu d'un fetch par ouverture du drawer). Reset au logout via onAuthSessionCleared + appel explicite dans logout.vue.
  • useCategoryForm : state local par form (pas singleton, contrairement à useCategoriesAdmin). Valide côté client en miroir des RG back (RG-1.02 / RG-1.04 / RG-1.05), mappe les erreurs 409 (RG-1.07 doublon) et 422 (violations API Platform) sur les bons champs. submitCreate / submitUpdate / submitDelete renvoient la ressource ou null pour découpler la décision de fermeture du drawer.

La page et le drawer deviennent purement présentationnels — aucune régression UX attendue (mêmes validations, mêmes toasts, même bascule view → edit via isDirty exposé par le composable).

Décisions

  • useCategoriesAdmin porte aussi les types (fetchTypes), pas seulement categories — sinon le drawer continuerait à fetcher tout seul et la refacto n'aurait rien centralisé.
  • buildCreatePayload retourne Record<string, unknown> (pas CategoryCreateInput) car la signature useApi.post(body: AnyObject) n'accepte pas les types stricts (variance TS).
  • Reset au logout : double mécanisme conservé (auto via onAuthSessionCleared pour 401, explicite dans logout.vue pour logout volontaire — pattern existant Starseed).

Tests

  • npx nuxi typecheck ✓ 0 erreur nouvelle (1 erreur pré-existante sur modules/catalog/nuxt.config.ts héritée d'ERP-49)
  • make nuxt-test ✓ 43/43, 0 régression
  • PHPUnit ✓ 311/311 (pre-commit)
  • Manuel navigateur : à valider (cahier de test consigné dans Lesstime #50)

⚠ Note d'intégration

La branche contient encore les 3 commits ERP-49 (4046910, 216f388, 934a12b) car elle a été créée depuis la branche ERP-49 avant son merge sur develop. Selon l'ordre de merge : soit ERP-49 est mergée d'abord (cette MR ne contiendra plus que le commit ERP-50 après rebase auto), soit cette MR embarque tout l'historique catalog.

Lien Lesstime : #50 ## Résumé Refacto : extraction de la logique fetch/CRUD inline de la page categories (ERP-49) vers deux composables dédiés, conformément au pattern Starseed (useSidebar / useModules). - **useCategoriesAdmin** : singleton state (`categories` + `types` + `loading` + `error`). Pré-chargement des types au mount de la page (au lieu d'un fetch par ouverture du drawer). Reset au logout via `onAuthSessionCleared` + appel explicite dans `logout.vue`. - **useCategoryForm** : state local par form (pas singleton, contrairement à `useCategoriesAdmin`). Valide côté client en miroir des RG back (RG-1.02 / RG-1.04 / RG-1.05), mappe les erreurs 409 (RG-1.07 doublon) et 422 (violations API Platform) sur les bons champs. `submitCreate` / `submitUpdate` / `submitDelete` renvoient la ressource ou `null` pour découpler la décision de fermeture du drawer. La page et le drawer deviennent purement présentationnels — aucune régression UX attendue (mêmes validations, mêmes toasts, même bascule view → edit via `isDirty` exposé par le composable). ## Décisions - `useCategoriesAdmin` porte aussi les types (`fetchTypes`), pas seulement `categories` — sinon le drawer continuerait à fetcher tout seul et la refacto n'aurait rien centralisé. - `buildCreatePayload` retourne `Record<string, unknown>` (pas `CategoryCreateInput`) car la signature `useApi.post(body: AnyObject)` n'accepte pas les types stricts (variance TS). - Reset au logout : double mécanisme conservé (auto via `onAuthSessionCleared` pour 401, explicite dans `logout.vue` pour logout volontaire — pattern existant Starseed). ## Tests - `npx nuxi typecheck` ✓ 0 erreur nouvelle (1 erreur pré-existante sur `modules/catalog/nuxt.config.ts` héritée d'ERP-49) - `make nuxt-test` ✓ 43/43, 0 régression - PHPUnit ✓ 311/311 (pre-commit) - Manuel navigateur : à valider (cahier de test consigné dans Lesstime #50) ## ⚠ Note d'intégration La branche contient encore les 3 commits ERP-49 (`4046910`, `216f388`, `934a12b`) car elle a été créée depuis la branche ERP-49 avant son merge sur develop. Selon l'ordre de merge : soit ERP-49 est mergée d'abord (cette MR ne contiendra plus que le commit ERP-50 après rebase auto), soit cette MR embarque tout l'historique catalog.
matthieu reviewed 2026-05-29 07:50:09 +00:00
matthieu left a comment
Owner

Review ERP-50 (delta vs ERP-49) — composables useCategoriesAdmin / useCategoryForm. 7 points, surtout une regression sur le feedback d'erreur de chargement des types et de la duplication a remonter en shared.

Review ERP-50 (delta vs ERP-49) — composables useCategoriesAdmin / useCategoryForm. 7 points, surtout une regression sur le feedback d'erreur de chargement des types et de la duplication a remonter en shared.
@@ -0,0 +79,4 @@
const { t } = useI18n()
const { can } = usePermissions()
const { types, loadingTypes, fetchTypes } = useCategoriesAdmin()
Owner

Regression vs ERP-49 : le drawer ne destructure que { types, loadingTypes, fetchTypes }, pas error. Avant, l'echec de chargement des types posait errors._global = typesLoadFailed et affichait un message inline. Desormais fetchTypes() range l'echec dans useCategoriesAdmin.error, qui n'est lu ni ici ni dans le template (seul form.errors.value._global est rendu). Resultat : si /category_types tombe, le select reste vide et desactive sans aucun message, et la cle i18n admin.categories.toast.typesLoadFailed devient orpheline. A brancher (afficher useCategoriesAdmin.error dans le drawer) ou a repositionner sur form._global.

Regression vs ERP-49 : le drawer ne destructure que { types, loadingTypes, fetchTypes }, pas `error`. Avant, l'echec de chargement des types posait `errors._global = typesLoadFailed` et affichait un message inline. Desormais fetchTypes() range l'echec dans useCategoriesAdmin.error, qui n'est lu ni ici ni dans le template (seul form.errors.value._global est rendu). Resultat : si /category_types tombe, le select reste vide et desactive sans aucun message, et la cle i18n admin.categories.toast.typesLoadFailed devient orpheline. A brancher (afficher useCategoriesAdmin.error dans le drawer) ou a repositionner sur form._global.
@@ -0,0 +25,4 @@
const types = ref<CategoryType[]>([])
const loading = ref(false)
const loadingTypes = ref(false)
const error = ref<string | null>(null)
Owner

error est peuple par fetchAll/fetchTypes et exporte, mais consomme nulle part (ni la page ni le drawer ne le destructurent). State mort — et c'est precisement le canal qui aurait du porter l'echec de chargement des types (cf. mon commentaire sur le drawer). Soit on le branche cote UI, soit on le retire.

`error` est peuple par fetchAll/fetchTypes et exporte, mais consomme nulle part (ni la page ni le drawer ne le destructurent). State mort — et c'est precisement le canal qui aurait du porter l'echec de chargement des types (cf. mon commentaire sur le drawer). Soit on le branche cote UI, soit on le retire.
@@ -0,0 +87,4 @@
* Toast desactive : on stocke l'erreur dans `error` plutot que de
* spammer un toast le drawer affichera l'erreur inline s'il y a lieu.
*/
async function fetchTypes(): Promise<void> {
Owner

Race possible : fetchTypes() est appele par categories.vue (onMounted) et par le drawer (watch modelValue), sur le meme state singleton, sans garde in-flight. Deux appels concurrents partent en double sur /category_types, et le premier qui repond remet loadingTypes=false alors que le second tourne encore (select reactive trop tot). Pas de corruption (meme payload), impact UX mineur. Une garde if (loadingTypes.value) return ou un petit cache suffit.

Race possible : fetchTypes() est appele par categories.vue (onMounted) et par le drawer (watch modelValue), sur le meme state singleton, sans garde in-flight. Deux appels concurrents partent en double sur /category_types, et le premier qui repond remet loadingTypes=false alors que le second tourne encore (select reactive trop tot). Pas de corruption (meme payload), impact UX mineur. Une garde `if (loadingTypes.value) return` ou un petit cache suffit.
@@ -0,0 +167,4 @@
* Extrait un message d'erreur lisible depuis un payload Hydra (champs
* `hydra:description`, `detail`, `description`).
*/
function extractErrorMessage(data: unknown): string {
Owner

extractErrorMessage est reimplemente alors que useApi en a deja une version plus complete (hydra:description, detail, message, error, title, hydra:title + fallback FetchError.message). Ici on ne couvre que hydra:description / detail / description : on rate notamment message et title, donc certaines 500 afficheront le fallback generique 'Une erreur est survenue.' la ou useApi aurait sorti un vrai message. Le helper de useApi n'est pas exporte aujourd'hui : autant l'exposer et le partager plutot que de maintenir une 2e version divergente.

extractErrorMessage est reimplemente alors que useApi en a deja une version plus complete (hydra:description, detail, message, error, title, hydra:title + fallback FetchError.message). Ici on ne couvre que hydra:description / detail / description : on rate notamment `message` et `title`, donc certaines 500 afficheront le fallback generique 'Une erreur est survenue.' la ou useApi aurait sorti un vrai message. Le helper de useApi n'est pas exporte aujourd'hui : autant l'exposer et le partager plutot que de maintenir une 2e version divergente.
@@ -0,0 +185,4 @@
* Retourne true si l'erreur a ete reconnue et traitee, false sinon
* (utile pour les tests).
*/
function handleApiError(e: unknown, attemptedName: string): boolean {
Owner

Altitude : handleApiError + mapServerViolations (violations vs hydra:violations, strategie 409/422/global) sont transverses et vont se repeter dans chaque composable de form (clients, produits...). Les dupliquer par module garantit le drift — c'est exactement ce qui arrive deja avec extractErrorMessage juste au-dessus. Leur place est dans frontend/shared (un useFormErrors() ou un helper dans utils/api.ts), parametre par la liste des champs connus. OK pour ce M0 mono-form, mais a faire avant que le pattern se duplique.

Altitude : handleApiError + mapServerViolations (violations vs hydra:violations, strategie 409/422/global) sont transverses et vont se repeter dans chaque composable de form (clients, produits...). Les dupliquer par module garantit le drift — c'est exactement ce qui arrive deja avec extractErrorMessage juste au-dessus. Leur place est dans frontend/shared (un useFormErrors() ou un helper dans utils/api.ts), parametre par la liste des champs connus. OK pour ce M0 mono-form, mais a faire avant que le pattern se duplique.
@@ -0,0 +315,4 @@
* Reset complet du formulaire utilise par le drawer apres save ou
* fermeture pour ne pas garder de donnees stale entre deux ouvertures.
*/
function reset(): void {
Owner

reset() est exporte mais jamais appele (aucun .reset() dans modules/catalog). loadFrom(null) couvre deja le reset complet. A supprimer, ou justifier s'il est prevu pour les tests.

reset() est exporte mais jamais appele (aucun .reset() dans modules/catalog). loadFrom(null) couvre deja le reset complet. A supprimer, ou justifier s'il est prevu pour les tests.
@@ -0,0 +49,4 @@
const { t } = useI18n()
const { can } = usePermissions()
const { categories, fetchAll, fetchTypes } = useCategoriesAdmin()
const { submitDelete } = useCategoryForm()
Owner

categories.vue cree une instance complete de useCategoryForm uniquement pour submitDelete. Cette instance n'appelle jamais loadFrom, donc name.value reste vide et est passe a handleApiError — inoffensif aujourd'hui (un DELETE ne renvoie pas 409, branche generique n'utilise pas le nom), mais couplage fragile : toute la machinerie de form pour une methode quasi stateless. submitDelete gagnerait a etre une fonction a part (ou vivre dans useCategoriesAdmin).

categories.vue cree une instance complete de useCategoryForm uniquement pour submitDelete. Cette instance n'appelle jamais loadFrom, donc name.value reste vide et est passe a handleApiError — inoffensif aujourd'hui (un DELETE ne renvoie pas 409, branche generique n'utilise pas le nom), mais couplage fragile : toute la machinerie de form pour une methode quasi stateless. submitDelete gagnerait a etre une fonction a part (ou vivre dans useCategoriesAdmin).
tristan added 2 commits 2026-05-29 09:09:05 +00:00
Extrait la logique fetch/CRUD inline de la page categories (ERP-49) vers
deux composables dedies, conformement au pattern Starseed :

- useCategoriesAdmin : singleton state (categories + types + loading +
  error). Pre-chargement des types au mount de la page (au lieu du
  fetch par ouverture du drawer). Reset au logout via
  onAuthSessionCleared + appel explicite dans logout.vue.

- useCategoryForm : state local par form (pas singleton). Valide
  cote client en miroir des RG back (RG-1.02 / RG-1.04 / RG-1.05),
  mappe les erreurs 409 (doublon RG-1.07) et 422 (violations API
  Platform) sur les bons champs. submitCreate / submitUpdate /
  submitDelete renvoient la ressource ou null pour decoupler la
  decision de fermeture du drawer.

La page et le drawer deviennent purement presentationnels. Aucune
regression UX : meme validations, memes toasts, meme pattern
view -> edit du drawer (via isDirty expose par useCategoryForm).
refactor(catalog) : address Matthieu review on composables (constant + shared helpers)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 1m18s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m0s
62e1b019a1
- useCategoriesAdmin : extract `HYDRA_NO_PAGINATION = 999` to a named
  constant (was duplicated between fetchAll and fetchTypes) + comment
  the post-M0 server-pagination debt.

- useCategoryForm + useApi + shared/utils/api : drop the local copy of
  `extractErrorMessage` in useCategoryForm (it was duplicating the one
  in useApi), and centralize Hydra error / violation extraction in
  `shared/utils/api.ts` via two new helpers :
    - extractApiErrorMessage(data) : tries hydra:description, detail,
      description, message, error, title, hydra:title — used by both
      useApi.onResponseError and useCategoryForm.handleApiError.
    - extractApiViolations(data) : returns the ApiPlatform 422
      violations as a typed array (handles `violations` and
      `hydra:violations`), letting each caller map them onto its own
      fields. useCategoryForm now uses this helper instead of an
      inline loop, ready for the next form drawer to reuse.

handleApiError keeps a manual fallback toast on non-409/422 errors :
the native useApi toast is disabled by design (`toast: false`) to allow
fine-grained 409/422 mapping, so the catch-all branch must re-emit one
or a 500 would be silent.

No behavior change — 43/43 Vitest tests still pass.
tristan force-pushed feature/ERP-50-0-8-frontend-m-implementer-les-composables-usecate from 9ed42e9d91 to 62e1b019a1 2026-05-29 09:09:05 +00:00 Compare
tristan merged commit 58589e93d0 into develop 2026-05-29 09:18:30 +00:00
tristan deleted branch feature/ERP-50-0-8-frontend-m-implementer-les-composables-usecate 2026-05-29 09:18:30 +00:00
Sign in to join this conversation.