[ERP-73] Paginer toutes les listes côté front + composable de liste paginée réutilisable #30

Merged
tristan merged 2 commits from feature/ERP-73-frontend-l-paginer-toutes-les-listes-cote-front-co into develop 2026-06-01 09:54:55 +00:00
Owner

Contexte

Ticket Lesstime : #73 (id 492) — volet front de la pagination (groupe Transversal).
Dépend du back ERP-72 (déjà mergé sur develop). Pas de spec docs/specs ; référence = description #73 + .claude/rules/frontend.md.

Implémentation

  • Composable réutilisable usePaginatedList (frontend/shared/composables/) générique, branché directement sur MalioDataTable (props page/perPage/totalItems + events update:page/update:per-page).
  • Force Accept: application/ld+json (sans Accept, API Platform renvoie un tableau plat sans pagination).
  • Migration des pages admin existantes (M0 catégories, Sites, Utilisateurs, Rôles) vers le composable.
  • Refactor de useCategoriesAdmin : ne porte plus la liste paginée (déplacée vers usePaginatedList<Category> dans la page) et concentre son rôle sur le référentiel CategoryType (chargé en une fois via ?pagination=false, échappatoire prévue par pagination_client_enabled: true côté back).
  • Cas limites couverts : liste vide (pas de contrôle pagination affiché), page hors borne après filtre (retombe sur la dernière page valide), items/page 10/25/50, reset filtres/tri, swallow erreur réseau.
  • Pattern « liste paginée » documenté dans .claude/rules/frontend.md (section dédiée + exemple).

Décision URL

Le ticket suggérait « idéalement page/tri/filtre dans l'URL » — arbitré explicitement par Tristan en faveur de la règle ABSOLUE n°6 du CLAUDE.md (state local uniquement, jamais persisté dans l'URL). Aucun reflet URL implémenté ; comportement homogène entre toutes les listes migrées.

Tests

  • make nuxt-test : 101/101 OK (22 nouveaux tests sur usePaginatedList, 6 anciens tests useCategoriesAdmin.fetchAll retirés en cohérence avec la refacto).
  • Vérification manuelle dans le navigateur (make dev-nuxt) : Sites, Utilisateurs, Rôles, Catégories affichent le sélecteur Lignes : 10 et les boutons Prev/Next ; audit-log (non migré, composable spécifique) intact avec ses 3 pages.
  • Aucun test E2E ajouté (règle d'or projet).
  • Pre-commit hook : ESLint + PHPUnit 322/322 OK.

Hors périmètre

  • audit-log.vue non migré : composable useAuditLog spécifique (cache partagé page/timeline, filtres complexes, persistance URL préexistante). Refactor risqué et net-zéro pour ERP-73.
  • M1 répertoire clients : pas encore livré sur develop (seules les specs sont mergées via #23). Le futur écran consommera usePaginatedList dès sa création.
## Contexte Ticket Lesstime : #73 (id 492) — volet front de la pagination (groupe Transversal). Dépend du back ERP-72 (déjà mergé sur develop). Pas de spec docs/specs ; référence = description #73 + .claude/rules/frontend.md. ## Implémentation - Composable réutilisable `usePaginatedList` (`frontend/shared/composables/`) générique, branché directement sur `MalioDataTable` (props page/perPage/totalItems + events update:page/update:per-page). - Force `Accept: application/ld+json` (sans Accept, API Platform renvoie un tableau plat sans pagination). - Migration des pages admin existantes (M0 catégories, Sites, Utilisateurs, Rôles) vers le composable. - Refactor de `useCategoriesAdmin` : ne porte plus la liste paginée (déplacée vers `usePaginatedList<Category>` dans la page) et concentre son rôle sur le référentiel `CategoryType` (chargé en une fois via `?pagination=false`, échappatoire prévue par `pagination_client_enabled: true` côté back). - Cas limites couverts : liste vide (pas de contrôle pagination affiché), page hors borne après filtre (retombe sur la dernière page valide), items/page 10/25/50, reset filtres/tri, swallow erreur réseau. - Pattern « liste paginée » documenté dans `.claude/rules/frontend.md` (section dédiée + exemple). ## Décision URL Le ticket suggérait « idéalement page/tri/filtre dans l'URL » — arbitré explicitement par Tristan en faveur de la règle ABSOLUE n°6 du CLAUDE.md (state local uniquement, jamais persisté dans l'URL). Aucun reflet URL implémenté ; comportement homogène entre toutes les listes migrées. ## Tests - `make nuxt-test` : 101/101 OK (22 nouveaux tests sur `usePaginatedList`, 6 anciens tests `useCategoriesAdmin.fetchAll` retirés en cohérence avec la refacto). - Vérification manuelle dans le navigateur (`make dev-nuxt`) : Sites, Utilisateurs, Rôles, Catégories affichent le sélecteur `Lignes : 10` et les boutons Prev/Next ; audit-log (non migré, composable spécifique) intact avec ses 3 pages. - Aucun test E2E ajouté (règle d'or projet). - Pre-commit hook : ESLint + PHPUnit 322/322 OK. ## Hors périmètre - `audit-log.vue` non migré : composable `useAuditLog` spécifique (cache partagé page/timeline, filtres complexes, persistance URL préexistante). Refactor risqué et net-zéro pour ERP-73. - M1 répertoire clients : pas encore livré sur develop (seules les specs sont mergées via #23). Le futur écran consommera `usePaginatedList` dès sa création.
tristan added 1 commit 2026-05-29 14:41:14 +00:00
feat(front) : add usePaginatedList composable + paginate all admin lists via MalioDataTable
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 2m1s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m7s
43c3220873
- frontend/shared/composables/usePaginatedList.ts : composable generique de liste paginee serveur (Hydra), branche directement sur MalioDataTable
- 22 tests Vitest (navigation, bornes, parse Hydra, hors-borne, reset, filtres, tri, swallow erreur)
- Migration des pages admin existantes : sites, users, roles, categories
- Refactor de useCategoriesAdmin pour ne porter que le referentiel CategoryType (charge en une fois via ?pagination=false)
- Etat page/tri/filtre 100% local dans le composable (respect regle ABSOLUE n°6 — pas de persistance URL)
- Section dediee dans .claude/rules/frontend.md documentant le pattern obligatoire pour toute nouvelle liste

ERP-73 — volet front de la pagination, depend du back ERP-72 deja merge.
Author
Owner

Review ERP-73 — usePaginatedList + migration des listes admin

Relecture du diff complet (composable, tests, 4 pages migrées, refacto useCategoriesAdmin, doc). Beau travail dans l'ensemble : l'abstraction est propre, bien documentée en FR, et la couverture de tests est sérieuse. Quelques recommandations ci-dessous, classées par importance.

Points positifs

  • sites.vue : vrai correctif de fond. L'ancien loadSites faisait itemsPerPage: 999 → il court-circuitait la pagination et violait la règle ABSOLUE n°13. La bascule sur usePaginatedList corrige ça réellement, pas juste cosmétiquement.
  • useCategoriesAdmin recentré sur le référentiel CategoryType via ?pagination=false (échappatoire prévue) — séparation des responsabilités claire, le commentaire explique bien le « pourquoi singleton ».
  • Mode contrôlé respecté : MalioDataTable ne slice pas en interne (il rend items tel quel et émet update:page/update:per-page), donc passer la page serveur + totalItems est exactement le contrat attendu. RAS de ce côté.
  • Règle n°6 respectée (aucun state tableau dans l'URL) + décision arbitrée et tracée dans la description. 22 tests couvrant les invariants critiques.

Recommandations

1. [Correctness — moyen] Pas de garde anti-réponse périmée (race condition).
fetch() ne séquence pas les requêtes concurrentes. Si l'utilisateur enchaîne goToPage / setItemsPerPage / setFilters rapidement (réseau lent), c'est la dernière réponse arrivée qui gagne, pas la dernière demandée → le tableau peut afficher une page périmée avec un currentPage incohérent.
👉 Ajouter un jeton de séquence incrémenté à chaque fetch() et ignorer la réponse si le jeton n'est plus le dernier :

let fetchToken = 0
async function fetch() {
    const token = ++fetchToken
    // ...
    if (token !== fetchToken) return // une requête plus récente a pris la main
    items.value = data.member ?? []
    // ...
}

2. [UX/observabilité — moyen] L'erreur réseau/403 est indiscernable d'une liste vide.
Dans le catch, on fait items=[], totalItems=0, hasFetched=trueisEmpty=true. Le composable n'expose aucun error : un 403 ou une coupure réseau s'affiche désormais exactement comme une liste légitimement vide (l'empty-message). L'ancien code par page avait le même défaut, mais le composable est l'occasion de mieux faire.
👉 Exposer un error: Ref<unknown | null> (set dans le catch, reset en début de fetch) pour que les pages puissent afficher un bandeau d'erreur / un bouton « réessayer ». A minima, documenter explicitement que isEmpty confond « vide » et « échec ».

3. [Qualité de test — faible] Le test « page hors borne après filtre » ne teste pas ce que son nom annonce.
Dans usePaginatedList.test.ts, le test 'page hors borne apres filtre retombe sur la derniere page valide' arme deux mockResolvedValueOnce puis appelle setFilters — or setFilters remet currentPage = 1, donc le chemin de retry hors-borne n'est jamais exercé (seul le 1ᵉʳ mock est consommé, le 2ᵉ est mort). Le commentaire interne l'admet d'ailleurs (« c'est le cas standard, pas le hors borne »). Le vrai retry est couvert par le test suivant ('declenche le retry sur derniere page...').
👉 Soit supprimer le mock mort + renommer ce test pour ce qu'il vérifie réellement (« setFilters retombe en page 1 »), soit le faire réellement déclencher le hors-borne. En l'état il donne une fausse impression de couverture.

4. [Doc — faible] Commentaire extraQuery trompeur sur la réactivité.
La JSDoc dit « Reactifs si une ref / computed est fournie via refresh() ». Or extraQuery est un Record<string, unknown> figé à l'init, et buildQuery fait Object.assign(query, options.extraQuery) : passer une ref copierait l'objet ref non déballé dans le query param. Aucun appelant n'utilise ce cas aujourd'hui, donc c'est latent.
👉 Soit accepter MaybeRefOrGetter et toValue() les entrées, soit corriger le commentaire pour dire que extraQuery est un snapshot statique.

5. [Robustesse — faible/défensif] Les filtres peuvent écraser les clés de pagination.
Dans buildQuery, Object.assign(query, filters.value) est appliqué en dernier : un filtre nommé page, itemsPerPage ou order[...] écraserait silencieusement la pagination/le tri. Peu probable mais le composable est générique.
👉 Assigner les filtres avant la pagination, ou logger/ignorer ces clés réservées.

6. [Nit] loading exposé mais non câblé.
Le composable expose loading, mais aucune des pages migrées ne le branche sur MalioDataTable (qui n'a pas de prop loading ici, mais on pourrait au moins masquer/griser pendant un changement de page). Optionnel — à garder en tête pour l'UX de changement de page sur réseau lent.

7. [Nit/futur] Pas de debounce pour les filtres texte.
setFilters déclenche un fetch immédiat. C'est OK pour l'usage actuel (refetch piloté par le drawer), mais dès qu'un filtre texte « live » sera câblé, chaque frappe fera un appel. À documenter côté appelant ou à offrir en option le moment venu.


Rien de bloquant. Les points 1 et 2 méritent à mon sens un traitement (ou au moins une décision explicite) avant merge ; le reste peut suivre.

## Review ERP-73 — `usePaginatedList` + migration des listes admin Relecture du diff complet (composable, tests, 4 pages migrées, refacto `useCategoriesAdmin`, doc). Beau travail dans l'ensemble : l'abstraction est propre, bien documentée en FR, et la couverture de tests est sérieuse. Quelques recommandations ci-dessous, classées par importance. ### Points positifs - **`sites.vue` : vrai correctif de fond.** L'ancien `loadSites` faisait `itemsPerPage: 999` → il court-circuitait la pagination et violait la règle ABSOLUE n°13. La bascule sur `usePaginatedList` corrige ça réellement, pas juste cosmétiquement. - **`useCategoriesAdmin` recentré** sur le référentiel `CategoryType` via `?pagination=false` (échappatoire prévue) — séparation des responsabilités claire, le commentaire explique bien le « pourquoi singleton ». - **Mode contrôlé respecté** : `MalioDataTable` ne slice pas en interne (il rend `items` tel quel et émet `update:page`/`update:per-page`), donc passer la page serveur + `totalItems` est exactement le contrat attendu. RAS de ce côté. - **Règle n°6 respectée** (aucun state tableau dans l'URL) + décision arbitrée et tracée dans la description. 22 tests couvrant les invariants critiques. --- ### Recommandations **1. [Correctness — moyen] Pas de garde anti-réponse périmée (race condition).** `fetch()` ne séquence pas les requêtes concurrentes. Si l'utilisateur enchaîne `goToPage` / `setItemsPerPage` / `setFilters` rapidement (réseau lent), c'est la **dernière réponse arrivée** qui gagne, pas la dernière demandée → le tableau peut afficher une page périmée avec un `currentPage` incohérent. 👉 Ajouter un jeton de séquence incrémenté à chaque `fetch()` et ignorer la réponse si le jeton n'est plus le dernier : ```ts let fetchToken = 0 async function fetch() { const token = ++fetchToken // ... if (token !== fetchToken) return // une requête plus récente a pris la main items.value = data.member ?? [] // ... } ``` **2. [UX/observabilité — moyen] L'erreur réseau/403 est indiscernable d'une liste vide.** Dans le `catch`, on fait `items=[]`, `totalItems=0`, `hasFetched=true` → `isEmpty=true`. Le composable **n'expose aucun `error`** : un 403 ou une coupure réseau s'affiche désormais exactement comme une liste légitimement vide (l'`empty-message`). L'ancien code par page avait le même défaut, mais le composable est l'occasion de mieux faire. 👉 Exposer un `error: Ref<unknown | null>` (set dans le `catch`, reset en début de `fetch`) pour que les pages puissent afficher un bandeau d'erreur / un bouton « réessayer ». A minima, documenter explicitement que `isEmpty` confond « vide » et « échec ». **3. [Qualité de test — faible] Le test « page hors borne après filtre » ne teste pas ce que son nom annonce.** Dans `usePaginatedList.test.ts`, le test `'page hors borne apres filtre retombe sur la derniere page valide'` arme deux `mockResolvedValueOnce` puis appelle `setFilters` — or `setFilters` remet `currentPage = 1`, donc le chemin de retry hors-borne n'est jamais exercé (seul le 1ᵉʳ mock est consommé, le 2ᵉ est mort). Le commentaire interne l'admet d'ailleurs (« c'est le cas standard, pas le hors borne »). Le vrai retry est couvert par le test suivant (`'declenche le retry sur derniere page...'`). 👉 Soit supprimer le mock mort + renommer ce test pour ce qu'il vérifie réellement (« setFilters retombe en page 1 »), soit le faire réellement déclencher le hors-borne. En l'état il donne une fausse impression de couverture. **4. [Doc — faible] Commentaire `extraQuery` trompeur sur la réactivité.** La JSDoc dit « *Reactifs si une ref / computed est fournie via `refresh()`* ». Or `extraQuery` est un `Record<string, unknown>` figé à l'init, et `buildQuery` fait `Object.assign(query, options.extraQuery)` : passer une `ref` copierait l'objet ref **non déballé** dans le query param. Aucun appelant n'utilise ce cas aujourd'hui, donc c'est latent. 👉 Soit accepter `MaybeRefOrGetter` et `toValue()` les entrées, soit corriger le commentaire pour dire que `extraQuery` est un snapshot statique. **5. [Robustesse — faible/défensif] Les filtres peuvent écraser les clés de pagination.** Dans `buildQuery`, `Object.assign(query, filters.value)` est appliqué **en dernier** : un filtre nommé `page`, `itemsPerPage` ou `order[...]` écraserait silencieusement la pagination/le tri. Peu probable mais le composable est générique. 👉 Assigner les filtres **avant** la pagination, ou logger/ignorer ces clés réservées. **6. [Nit] `loading` exposé mais non câblé.** Le composable expose `loading`, mais aucune des pages migrées ne le branche sur `MalioDataTable` (qui n'a pas de prop loading ici, mais on pourrait au moins masquer/griser pendant un changement de page). Optionnel — à garder en tête pour l'UX de changement de page sur réseau lent. **7. [Nit/futur] Pas de debounce pour les filtres texte.** `setFilters` déclenche un `fetch` immédiat. C'est OK pour l'usage actuel (refetch piloté par le drawer), mais dès qu'un filtre texte « live » sera câblé, chaque frappe fera un appel. À documenter côté appelant ou à offrir en option le moment venu. --- Rien de bloquant. Les points **1** et **2** méritent à mon sens un traitement (ou au moins une décision explicite) avant merge ; le reste peut suivre.
tristan added 1 commit 2026-05-31 11:14:11 +00:00
fix(front) : usePaginatedList — garde anti-reponse periemee, expose error, durcit buildQuery
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 2m25s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m14s
f770812b7e
- ajoute un jeton de sequence dans fetch() : ignore les reponses
  arrivees apres une requete plus recente (race page/tri/filtres sur
  reseau lent)
- expose une ref error pour distinguer liste vide d'un echec reseau/403
  (isEmpty ne suffit pas a lever l'ambiguite)
- buildQuery : cles reservees (page/itemsPerPage/order) assignees en
  dernier pour qu'un filtre homonyme ne les ecrase pas
- corrige le commentaire trompeur sur extraQuery (snapshot statique)
- nettoie le test hors-borne qui testait en realite le cas standard +
  ajoute tests error/reset-error/race
Owner

Review ERP-73 — pagination front + usePaginatedList

Le composable est propre et bien testé : garde anti-réponse périmée par token, error exposé pour distinguer « vide » d'« échec », MalioDataTable correctement piloté en mode contrôlé serveur (pas de slice client), useApi forwarde bien headers (Accept ld+json OK), chargement initial via onMounted conservé. 👍

En revanche, la pagination ne fonctionne pas de bout en bout : la PR câble la moitié front mais la moitié back n'est pas alignée. Les tests passent car ils mockent useApi, donc ces deux points ne sont pas couverts.

🔴 1 — /categories n'est pas paginé côté serveur (bloquant)

CategoryProvider::provide() retourne ->getResult() (tableau brut). API Platform ne pagine pas un tableau brut renvoyé par un provider custom : il sérialise toute la liste et totalItems = count(liste), en ignorant page et itemsPerPage.

Conséquence dans categories.vue : usePaginatedList reçoit l'intégralité des catégories → MalioDataTable affiche toutes les lignes sur la « page 1 », la barre annonce N/10 pages, et cliquer sur une autre page refetch… toute la liste. La pagination est cosmétique.

Fix : faire renvoyer un paginator par le provider (cf. AuditLogProvider qui retourne un DbalPaginator, ou un ApiPlatform\State\Pagination\TraversablePaginator / ArrayPaginator), ou laisser API Platform paginer un QueryBuilder Doctrine.

🟠 2 — itemsPerPage ignoré sur /sites, /roles, /users

Aucune des ressources Site / Role / User ne déclare paginationClientItemsPerPage, et config/packages/api_platform.yaml > defaults ne fixe aucune clé de pagination. On retombe donc sur les défauts framework : pagination_client_items_per_page: false et pagination_items_per_page: 30.

Conséquence : le itemsPerPage=10 envoyé par le composable est silencieusement ignoré, le serveur pagine par 30.

  • le sélecteur 10/25/50 est inopérant (toujours 30/page) ;
  • totalPages calculé côté front (÷10) ne correspond pas au paging serveur (÷30) ;
  • toute liste de 11 à 30 éléments affiche une page 2 vide (le front croit qu'il y a 2 pages, le serveur a tout renvoyé en page 1).

Fix : ajouter paginationClientItemsPerPage: true + paginationItemsPerPage: 10 sur les GetCollection concernées (ou globalement dans defaults), pour que le serveur respecte la taille de page demandée.

🟡 3 — Référence à une « règle ABSOLUE n°13 » inexistante

.claude/rules/frontend.md et le docblock de usePaginatedList.ts citent la « règle ABSOLUE n°13 (toute collection paginée côté back) ». CLAUDE.md s'arrête à la règle 12 : cette règle n'existe pas. Soit l'ajouter à CLAUDE.md, soit corriger la référence — d'autant qu'avec les points 1 & 2, le back ne la respecte pas encore.


En l'état, je recommande de compléter la pagination côté back (points 1 & 2) avant merge, sinon les barres de pagination introduites sont trompeuses pour l'utilisateur.

## Review ERP-73 — pagination front + `usePaginatedList` Le composable est propre et bien testé : garde anti-réponse périmée par token, `error` exposé pour distinguer « vide » d'« échec », `MalioDataTable` correctement piloté en mode contrôlé serveur (pas de slice client), `useApi` forwarde bien `headers` (Accept ld+json OK), chargement initial via `onMounted` conservé. 👍 En revanche, la pagination **ne fonctionne pas de bout en bout** : la PR câble la moitié front mais la moitié back n'est pas alignée. Les tests passent car ils mockent `useApi`, donc ces deux points ne sont pas couverts. ### 🔴 1 — `/categories` n'est pas paginé côté serveur (bloquant) `CategoryProvider::provide()` retourne `->getResult()` (tableau brut). API Platform **ne pagine pas** un tableau brut renvoyé par un provider custom : il sérialise toute la liste et `totalItems = count(liste)`, en ignorant `page` et `itemsPerPage`. Conséquence dans `categories.vue` : `usePaginatedList` reçoit l'intégralité des catégories → `MalioDataTable` affiche toutes les lignes sur la « page 1 », la barre annonce N/10 pages, et cliquer sur une autre page refetch… toute la liste. La pagination est cosmétique. **Fix** : faire renvoyer un paginator par le provider (cf. `AuditLogProvider` qui retourne un `DbalPaginator`, ou un `ApiPlatform\State\Pagination\TraversablePaginator` / `ArrayPaginator`), ou laisser API Platform paginer un `QueryBuilder` Doctrine. ### 🟠 2 — `itemsPerPage` ignoré sur `/sites`, `/roles`, `/users` Aucune des ressources `Site` / `Role` / `User` ne déclare `paginationClientItemsPerPage`, et `config/packages/api_platform.yaml > defaults` ne fixe aucune clé de pagination. On retombe donc sur les défauts framework : `pagination_client_items_per_page: false` et `pagination_items_per_page: 30`. Conséquence : le `itemsPerPage=10` envoyé par le composable est **silencieusement ignoré**, le serveur pagine par 30. - le sélecteur 10/25/50 est inopérant (toujours 30/page) ; - `totalPages` calculé côté front (÷10) ne correspond pas au paging serveur (÷30) ; - toute liste de 11 à 30 éléments affiche une **page 2 vide** (le front croit qu'il y a 2 pages, le serveur a tout renvoyé en page 1). **Fix** : ajouter `paginationClientItemsPerPage: true` + `paginationItemsPerPage: 10` sur les `GetCollection` concernées (ou globalement dans `defaults`), pour que le serveur respecte la taille de page demandée. ### 🟡 3 — Référence à une « règle ABSOLUE n°13 » inexistante `.claude/rules/frontend.md` et le docblock de `usePaginatedList.ts` citent la « règle ABSOLUE n°13 (toute collection paginée côté back) ». `CLAUDE.md` s'arrête à la règle 12 : cette règle n'existe pas. Soit l'ajouter à `CLAUDE.md`, soit corriger la référence — d'autant qu'avec les points 1 & 2, le back ne la respecte pas encore. --- En l'état, je recommande de compléter la pagination côté back (points 1 & 2) avant merge, sinon les barres de pagination introduites sont trompeuses pour l'utilisateur.
tristan self-assigned this 2026-06-01 08:25:06 +00:00
tristan merged commit ad20d1f4c9 into develop 2026-06-01 09:54:55 +00:00
tristan deleted branch feature/ERP-73-frontend-l-paginer-toutes-les-listes-cote-front-co 2026-06-01 09:54:55 +00:00
Sign in to join this conversation.