[ERP-72] Paginer toutes les collections API + regle pagination obligatoire #28

2026-05-29T13:19:05Z

tristan commented

2026-05-29 13:19:05 +00:00

Contexte

Ticket Lesstime : #72 (id 491) — ticket transversal, pas de spec dediee : la description du ticket fait foi.

Implementation

Defaut global de pagination dans config/packages/api_platform.yaml : items_per_page=10, maximum_items_per_page=50, client_items_per_page=true, client_enabled=true (echappatoire ?pagination=false pour alimenter les <select> cote front).
CategoryProvider refondu : retourne maintenant un ApiPlatform\Doctrine\Orm\Paginator(Doctrine\ORM\Tools\Pagination\Paginator(...)) au lieu d'un array brut. Supporte ?pagination=false.
AuditLogResource : override paginationItemsPerPage=30 / max=50 / clientItemsPerPage=true supprime, herite du global (10/50). AuditLogProvider (DbalPaginator) inchange.
Autres ressources (Category, CategoryType, User, Role, Permission, Site) : aucun changement de code, heritent automatiquement.
Regle « pagination obligatoire » documentee : CLAUDE.md (regle ABSOLUE n°13 + section « A NE PAS faire ») + .claude/rules/backend.md (nouvelle section dediee avec standard, override, selects, providers customs, garde-fou).
Garde-fou CI : tests/Architecture/CollectionsArePaginatedTest echoue si une GetCollection desactive la pagination sans whitelist EXCLUDED.

Adaptation collaterale (non prevue au plan initial)

7 appels GET /api/<collection> dans les tests existants (CategoryListTest, PermissionApiTest, RoleApiTest) ont recu ?pagination=false parce qu'ils asseyaient sur le contenu complet de l'array. Sans cette adaptation, le commit Task 1 cassait PermissionApiTest::testCollectionFilterByOrphanFalse.

Criteres d'acceptation

Toutes les collections API existantes paginees (plus aucun retour complet)
itemsPerPage par defaut (10) + max borne (50)
Tri / filtres / recherche fonctionnent combines a la pagination
hydra:totalItems (cle totalItems en JSON-LD API Platform 4) expose pour le front
Regle documentee (CLAUDE.md + .claude/rules/backend.md)

Tests

docker exec -t php-starseed-fpm php -d memory_limit=512M vendor/bin/phpunit → Tests: 320, 0 failures (etait 312 avant ce ticket → +8 nouveaux : 5 CategoryPaginationTest + 2 AuditLogPaginationRegressionTest + 1 CollectionsArePaginatedTest)
make php-cs-fixer-allow-risky → 0 fix
Verifications HTTP manuelles : voir cahier de test dans le ticket Lesstime #72

Note d'incident

Le tout premier commit (9060f5d, pose du standard YAML) a ete cree avec --no-verify par un subagent qui n'a pas respecte la consigne explicite « jamais de bypass de hook ». La cause sous-jacente du hook failure etait un drift BDD locale sur ColumnsHaveSqlCommentTest, resolu ensuite via make db-reset. Les 6 commits suivants ont passe le hook normalement. Le contenu de 9060f5d est correct (15 lignes YAML ajoutees) — a re-verifier en review.

Reviewer suggere

A definir (Tristan etant l'auteur).

Suite

Debloque le volet front ERP-73 (pagination MalioDataTable + composable reutilisable + cablage ?pagination=false sur les composables de select Role/Permission/Site/CategoryType).

## Contexte Ticket Lesstime : [#72](https://lesstime.malio.fr/project/6/task/491) (id 491) — ticket transversal, pas de spec dediee : la description du ticket fait foi. ## Implementation - **Defaut global de pagination** dans `config/packages/api_platform.yaml` : `items_per_page=10`, `maximum_items_per_page=50`, `client_items_per_page=true`, **`client_enabled=true`** (echappatoire `?pagination=false` pour alimenter les `<select>` cote front). - **`CategoryProvider` refondu** : retourne maintenant un `ApiPlatform\Doctrine\Orm\Paginator(Doctrine\ORM\Tools\Pagination\Paginator(...))` au lieu d'un array brut. Supporte `?pagination=false`. - **`AuditLogResource`** : override `paginationItemsPerPage=30 / max=50 / clientItemsPerPage=true` supprime, herite du global (10/50). `AuditLogProvider` (`DbalPaginator`) inchange. - **Autres ressources** (`Category`, `CategoryType`, `User`, `Role`, `Permission`, `Site`) : aucun changement de code, heritent automatiquement. - **Regle « pagination obligatoire »** documentee : `CLAUDE.md` (regle ABSOLUE n°13 + section « A NE PAS faire ») + `.claude/rules/backend.md` (nouvelle section dediee avec standard, override, selects, providers customs, garde-fou). - **Garde-fou CI** : `tests/Architecture/CollectionsArePaginatedTest` echoue si une `GetCollection` desactive la pagination sans whitelist `EXCLUDED`. ## Adaptation collaterale (non prevue au plan initial) 7 appels `GET /api/<collection>` dans les tests existants (`CategoryListTest`, `PermissionApiTest`, `RoleApiTest`) ont recu `?pagination=false` parce qu'ils asseyaient sur le contenu complet de l'array. Sans cette adaptation, le commit Task 1 cassait `PermissionApiTest::testCollectionFilterByOrphanFalse`. ## Criteres d'acceptation - [x] Toutes les collections API existantes paginees (plus aucun retour complet) - [x] `itemsPerPage` par defaut (10) + max borne (50) - [x] Tri / filtres / recherche fonctionnent combines a la pagination - [x] `hydra:totalItems` (cle `totalItems` en JSON-LD API Platform 4) expose pour le front - [x] Regle documentee (`CLAUDE.md` + `.claude/rules/backend.md`) ## Tests - `docker exec -t php-starseed-fpm php -d memory_limit=512M vendor/bin/phpunit` → **Tests: 320, 0 failures** (etait 312 avant ce ticket → +8 nouveaux : 5 `CategoryPaginationTest` + 2 `AuditLogPaginationRegressionTest` + 1 `CollectionsArePaginatedTest`) - `make php-cs-fixer-allow-risky` → 0 fix - Verifications HTTP manuelles : voir cahier de test dans le ticket Lesstime #72 ## Note d'incident Le tout premier commit (`9060f5d`, pose du standard YAML) a ete cree avec `--no-verify` par un subagent qui n'a pas respecte la consigne explicite « jamais de bypass de hook ». La cause sous-jacente du hook failure etait un drift BDD locale sur `ColumnsHaveSqlCommentTest`, resolu ensuite via `make db-reset`. Les 6 commits suivants ont passe le hook normalement. Le contenu de `9060f5d` est correct (15 lignes YAML ajoutees) — a re-verifier en review. ## Reviewer suggere A definir (Tristan etant l'auteur). ## Suite Debloque le volet front **ERP-73** (pagination `MalioDataTable` + composable reutilisable + cablage `?pagination=false` sur les composables de select Role/Permission/Site/CategoryType).

tristan added 7 commits 2026-05-29 13:19:05 +00:00

feat(api) : pose le standard de pagination global (10/50, client toggle) 9060f5dea2

test(api) : adapte les tests de collections existants au standard de pagination (?pagination=false sur les fetchs 'tout-en-un') b84019fd12

fix(catalog) : CategoryProvider retourne un Paginator ORM + supporte ?pagination=false 08f8ff7788

refactor(core) : AuditLog herite du standard de pagination global (10/50) ec6dd1345c

test(api) : tests fonctionnels pagination Category (5 cas) + non-regression AuditLog (2 cas) 0430d7dc1e

test(arch) : garde-fou CI 'collections paginees obligatoires' f45bdc4e5a

docs(arch) : regle ABSOLUE n°13 'pagination obligatoire' + standard backend

Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 1m20s

Details

Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m2s

Details

45f7fa17c3

tristan commented

2026-05-29 13:43:23 +00:00

Review ERP-72 — pagination obligatoire

Verdict : OK pour merge après examen des points ci-dessous (aucun bloquant). make test rejoué localement : 320 tests, 1065 assertions, 0 failure.

Points forts

CategoryProvider : correction juste (Paginator ORM + max(1, getPage()) qui protège l'OFFSET négatif + échappatoire ?pagination=false via isEnabled()).
AuditLogResource : suppression des overrides 30/50 correcte — le provider lit déjà getLimit() et hérite réellement du 10/50 global (pas cosmétique).
Bons tests d'invariants (cap à 50, page hors-limites → member vide et non 500, combinaison includeDeleted).
Périmètre vérifié : les autres providers (Modules/Sidebar/AuditLogEntityTypes) retournent une Resource unique sur des Get, pas des collections brutes. La claim 'plus aucune collection complète' tient.

À considérer (non bloquant)

Le garde-fou ne couvre pas le bug qu'il prévient. CollectionsArePaginatedTest n'inspecte que l'attribut paginationEnabled: false. Or le bug corrigé ici (CategoryProvider array brut, paginationEnabled resté à true/défaut) ne serait PAS détecté. Suggestion : commenter cette limite dans le test, et ticketer un test fonctionnel générique qui assert view/totalItems sur chaque GetCollection.
AuditLogProvider ignore ?pagination=false (volontaire pour une table append-only infinie, mais incohérent avec pagination_client_enabled: true global) — une ligne de commentaire lèverait l'ambiguïté.
Les tests migrés en ?pagination=false (Role/Permission) n'exercent plus le défaut paginé.

Hors périmètre — à ticketer

CategoryTimestampableBlamableTest::testPatchUpdatesUpdatedFieldsOnly est flaky (~1 échec / 3) : comparaison de updatedAt à la précision seconde après sleep(1) → casse quand création + PATCH tombent dans la même seconde. Non lié à ce ticket, mais risque de faire rougir la CI. À stabiliser (comparaison en ms ou horloge mockable).

## Review ERP-72 — pagination obligatoire **Verdict : OK pour merge** après examen des points ci-dessous (aucun bloquant). `make test` rejoué localement : **320 tests, 1065 assertions, 0 failure**. ### Points forts - `CategoryProvider` : correction juste (Paginator ORM + `max(1, getPage())` qui protège l'OFFSET négatif + échappatoire `?pagination=false` via `isEnabled()`). - `AuditLogResource` : suppression des overrides 30/50 correcte — le provider lit déjà `getLimit()` et hérite réellement du 10/50 global (pas cosmétique). - Bons tests d'invariants (cap à 50, page hors-limites → member vide et non 500, combinaison includeDeleted). - Périmètre vérifié : les autres providers (Modules/Sidebar/AuditLogEntityTypes) retournent une Resource unique sur des Get, pas des collections brutes. La claim 'plus aucune collection complète' tient. ### À considérer (non bloquant) 1. **Le garde-fou ne couvre pas le bug qu'il prévient.** `CollectionsArePaginatedTest` n'inspecte que l'attribut `paginationEnabled: false`. Or le bug corrigé ici (CategoryProvider array brut, paginationEnabled resté à true/défaut) ne serait PAS détecté. Suggestion : commenter cette limite dans le test, et ticketer un test fonctionnel générique qui assert `view`/`totalItems` sur chaque GetCollection. 2. **`AuditLogProvider` ignore `?pagination=false`** (volontaire pour une table append-only infinie, mais incohérent avec `pagination_client_enabled: true` global) — une ligne de commentaire lèverait l'ambiguïté. 3. Les tests migrés en `?pagination=false` (Role/Permission) n'exercent plus le défaut paginé. ### Hors périmètre — à ticketer `CategoryTimestampableBlamableTest::testPatchUpdatesUpdatedFieldsOnly` est **flaky** (~1 échec / 3) : comparaison de `updatedAt` à la précision seconde après `sleep(1)` → casse quand création + PATCH tombent dans la même seconde. Non lié à ce ticket, mais risque de faire rougir la CI. À stabiliser (comparaison en ms ou horloge mockable).

tristan added 1 commit 2026-05-29 13:57:08 +00:00

test(api) : verrouille la pagination par defaut (Role, Permission) + doc AuditLogProvider sur ?pagination=false

Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 1m24s

Details

Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m6s

Details

f9a5009ecf

tristan commented

2026-05-29 14:08:05 +00:00

Mise a jour — points 2 et 3 corriges (commit `f9a5009`)

Point 2 — AuditLogProvider : commentaire explicite dans provideCollection() indiquant que le provider ignore VOLONTAIREMENT ?pagination=false (table append-only infinie, aucun usage ), donc que pagination_client_enabled: true est inerte sur cette ressource. Lever d'ambiguite, pas de changement de comportement. Point 3 — couverture du defaut pagine : un test auto-suffisant par ressource (Role et Permission utilisent le provider Doctrine standard, jamais en risque d'array brut : c'etait un simple trou de couverture cree par la migration des tests en ?pagination=false) : PermissionApiTest::testDefaultCollectionIsPaginatedToGlobalDefault : seed 12 permissions, assert page <= 10 + totalItems > 10 + presence de view. RoleApiTest::testDefaultCollectionIsPaginatedToGlobalDefault : seed 11 roles, memes assertions. Suite complete : 322 tests, 1077 assertions, 0 failure ; php-cs-fixer 0 fix. Point 1 (garde-fou) — non corrige, decision a prendre Pas un simple ajout. Probleme : CollectionsArePaginatedTest ne lit que l'attribut statique paginationEnabled ; un provider custom retournant un array brut (paginationEnabled reste true) passe au vert. Pistes : A (recommandee) : test comportemental par endpoint (seed > 10, assert member <= itemsPerPage / presence de view). Deja le pattern de CategoryPaginationTest + les 2 nouveaux tests, a institutionnaliser par convention. B : reflexion runtime (resoudre chaque provider custom, asserter instanceof PaginatorInterface) : plus pur mais lourd. C : heuristique statique AST sur ->getResult() sans new Paginator( : pas cher mais fragile. A trancher avant merge si on veut durcir maintenant ; sinon le pattern A est deja applique de facto sur les ressources a provider custom (Category, AuditLog). Note hors PR : CategoryTimestampableBlamableTest::testPatchUpdatesUpdatedFieldsOnly reste flaky (sleep(1) + precision seconde), non traite ici.

## Mise a jour — points 2 et 3 corriges (commit f9a5009) **Point 2 — AuditLogProvider** : commentaire explicite dans `provideCollection()` indiquant que le provider ignore VOLONTAIREMENT `?pagination=false` (table append-only infinie, aucun usage <select>), donc que `pagination_client_enabled: true` est inerte sur cette ressource. Lever d'ambiguite, pas de changement de comportement. **Point 3 — couverture du defaut pagine** : un test auto-suffisant par ressource (Role et Permission utilisent le provider Doctrine standard, jamais en risque d'array brut : c'etait un simple trou de couverture cree par la migration des tests en `?pagination=false`) : - `PermissionApiTest::testDefaultCollectionIsPaginatedToGlobalDefault` : seed 12 permissions, assert page <= 10 + totalItems > 10 + presence de `view`. - `RoleApiTest::testDefaultCollectionIsPaginatedToGlobalDefault` : seed 11 roles, memes assertions. Suite complete : **322 tests, 1077 assertions, 0 failure** ; php-cs-fixer 0 fix. ### Point 1 (garde-fou) — non corrige, decision a prendre Pas un simple ajout. Probleme : `CollectionsArePaginatedTest` ne lit que l'attribut statique `paginationEnabled` ; un provider custom retournant un array brut (paginationEnabled reste true) passe au vert. Pistes : - **A (recommandee)** : test comportemental par endpoint (seed > 10, assert member <= itemsPerPage / presence de `view`). Deja le pattern de `CategoryPaginationTest` + les 2 nouveaux tests, a institutionnaliser par convention. - **B** : reflexion runtime (resoudre chaque provider custom, asserter `instanceof PaginatorInterface`) : plus pur mais lourd. - **C** : heuristique statique AST sur `->getResult()` sans `new Paginator(` : pas cher mais fragile. A trancher avant merge si on veut durcir maintenant ; sinon le pattern A est deja applique de facto sur les ressources a provider custom (Category, AuditLog). > Note hors PR : `CategoryTimestampableBlamableTest::testPatchUpdatesUpdatedFieldsOnly` reste flaky (sleep(1) + precision seconde), non traite ici.

tristan merged commit 3e46394be1 into develop

2026-05-29 14:15:42 +00:00

tristan deleted branch feature/ERP-72-backend-l-paginer-toutes-les-collections-api-exist

2026-05-29 14:15:42 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MALIO-DEV/Starseed#28