[ERP-62] Page Répertoire clients (datatable + Ajouter / Exporter) #44
Reference in New Issue
Block a user
Delete Branch "feature/ERP-62-page-repertoire-clients"
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?
ERP-62 — Page Répertoire clients (datatable + Ajouter / Exporter)
Tâche Lesstime #480. Stacke sur ERP-61 (clés i18n
commercial.clients.*) — non encore mergé : la diff versdevelopinclut le commit ERP-61 tant qu'il n'est pas mergé.Front
/clients(route à plat) :MalioDataTable6 colonnes (Nom entreprise / Contact / Téléphone formaté / Email / codes Catégories / badges Site(s)), toggle « Voir les archivés » (état 100 % local), boutons + Ajouter (visible sicommercial.clients.manage) et Exporter (visible siview, téléchargeclients/export.xlsxviauseApi), clic ligne →/clients/{id}, empty state.useClientsRepository= wrapper deusePaginatedList<Client>({ url: '/clients' })+ toggleincludeArchived(repasse page 1).formatPhoneFR(signature cible à coordonner avec ERP-66 / 1.13) + clé i18nshowArchived.Back — ⚠️ MAJ contrat de sérialisation (incluse dans cette MR)
Le
GET /api/clientsn'exposait ni les codes catégories ni les sites en liste (le bloc Lesstime l'affirmait à tort). Corrigé :Client:category:read+site:readajoutés auxnormalizationContext(GetCollection/Get/Post/Patch) + accesseur agrégégetSites()(#[Groups(client:read)]).DoctrineClientRepository::createListQueryBuilder: jointures +addSelect(categories / addresses / sites) anti N+1.Tests
ClientApiTest(codes catégories + sites name/color en liste).make test✅ 454.useClientsRepository.spec.ts+phone.test.ts.vitest✅ 111.nuxi typecheck✅ (mes fichiers).Non couvert
Golden path navigateur non joué : dev-nuxt (conteneur) cassé (résolution
@malio/layer-ui/tailwind.config.ts) + BDD sans clients démo (nécessitemake db-reset). Aspects front restants traités séparément.Review ERP-62 — Page Répertoire clients
Bon travail : code propre et bien commenté (FR), tests back + front présents, conventions Starseed respectées (
usePaginatedList, composantsMalio*, état toggle 100 % local, pagination serveur). Route/api/clients/export.xlsx, clés i18ncommercial.clients.*et eventsMalioDataTable(row-clickable/@row-click) vérifiés ✅.Un point de coordination bloquant + quelques remarques.
🔴 1. BLOQUANT — Conflit de contrat de sérialisation avec la PR #45
Cette MR ajoute
category:read+site:readà laGetCollectiondeClient+ un accesseur agrégégetSites()sousclient:read→ la liste embarque catégories + sites.La PR #45 (
fix(commercial) : corrige le contrat de sérialisation, ERP-80/81/82/83, en parallèle) prend la décision inverse : laGetCollectionreste['client:read','default:read'](catégories/sites hors liste, embarqués uniquement sur le détail via les adresses, sansclient.sitesagrégé).Conséquences :
Client.php(mêmesnormalizationContext).client.sites(agrégé), #45 exposeaddress.sites(par adresse). Le front d'ERP-62 dépend degetSites().➡️ À arbitrer avant merge : la liste doit-elle embarquer catégories + sites ? Si oui, c'est #45 qu'il faut aligner ; sinon, ERP-62 doit agréger côté front.
🟠 2. Les 3 fetch-joins to-many impactent l'export et
?pagination=falsecreateListQueryBuilderest partagé avecClientExportController(->getQuery()->getResult(), sans pagination) et avec l'échappatoire?pagination=false. LesleftJoin('c.categories')/leftJoin('c.addresses')/leftJoin('addr.sites')avecaddSelectproduisent un produit cartésien hydraté en mémoire sur tout le référentiel dans ces deux chemins. La liste paginée est bornée (OK grâce àfetchJoinCollection: true), mais l'export hérite des joins → à valider sur volumétrie réelle (envisager de réserver lesaddSelectau chemin paginé).🟡 3. Mineur —
loadingnon câbléusePaginatedListexposeloading, non branché surMalioDataTable→ pas d'indicateur pendant le fetch.🟡 4. Mineur — toast d'erreur export
exportXlsxutilise la même clécommercial.clients.toast.errorpourtitleetmessage(redondant). Un titre dédié serait préférable.✅ Point bloquant #1 — RÉSOLU
J'ai intégré
develop(qui contient désormais le fix de contrat de sérialisation #45 / ERP-80/81/82/83) dans cette branche et résolu le conflit surClient.php.Constat : contrairement à ce qu'on pensait, le contrat mergé dans
developn'embarquait PAS catégories/sites sur laGetCollectionni d'accesseurgetSites()(ils n'étaient ajoutés que sur le détailGet). Le delta de cette MR était donc bien nécessaire et a été conservé, sans dupliquer ni concurrencer le contrat.Résolution (un seul endroit, sur le contrat de
develop) :GetCollection: ajout decategory:read+site:read+ accesseur agrégégetSites()→ la liste ERP-62 expose biencode(catégories) etname/color(sites).Get(détail) :client_rib:readnon réintroduit — le fix sécurité de #45 est préservé (contenu RIB gaté parclient:read:accounting, plus de fuite IBAN/BIC vers la Commerciale).Post/Patch:category:read+site:readconservés pour une shape de réponse cohérente avec la liste/détail après création/édition.categories/addresses/sitessont conservés (anti N+1 maintenant que ces relations sont embarquées en liste) et coexistent avec le filtre catégorie en sous-requête dedevelop.Vérifications (post-merge) :
make test✅ 461 tests (dontClientApiTest::testListEmbedsCategoryCodesAndAggregatedSites+ leClientSerializationContractTestde #45).make php-cs-fixer-allow-risky✅ 0 correction.make nuxt-test✅ 111 tests.La MR est de nouveau mergeable sur
develop(headfc9746d).Code review ERP-62 — PR solide et bien testée (contrats de sérialisation #80/#81/#82 et gating RIB propres). Aucun bug bloquant de correction fonctionnelle. Retours ci-dessous : surtout perf et propreté, posés inline. Points vérifiés OK : pagination liste (fetchJoinCollection → COUNT/LIMIT corrects), sérialisation des params multi (categoryCode[]/siteId[]), unification single/multi des filtres, clés i18n présentes, état tableau 100 % local.
@@ -0,0 +228,4 @@return ''}return new Date(value).toLocaleDateString('fr-FR')[Robustesse]
formatLastActivityne se protège pas d'une date invalide.Context : rendu de la colonne « Dernière activité » depuis
updatedAt.Cause : si
updatedAtarrive mal formé,new Date(value)produitInvalid Dateet la cellule affiche littéralementInvalid Date— leif (!value)ne couvre que null/vide.Reco : garder le résultat derrière
Number.isNaN(date.getTime())→ retour'', cohérent avec la cellule vide déjà prévue.@@ -0,0 +12,4 @@* ou pointee, ex: `06.12.34.56.78` ou `0612345678`).* - Retourne une chaine vide si la valeur est vide/nulle (cellule vide propre).*/export function formatPhoneFR(value: string | null | undefined): string {[Code mort / scope]
formatPhoneFRajouté mais jamais consommé.Context : la page Répertoire n'a pas de colonne téléphone (4 colonnes : Nom / Catégories / Site / Dernière activité).
Cause :
formatPhoneFRn'est référencé par aucune source.vue/.ts(seuls les stubs d'auto-import générés sous.nuxt/le citent). Le docblock annonce lui-même que ERP-66 livrera la vraie version.Reco : retirer
phone.ts+ son test de cette PR et le livrer avec son consommateur réel (ERP-66), ou justifier l'introduction anticipée. On évite de merger un util sans usage réel.@@ -40,0 +45,4 @@// requete par client, puis par adresse). Le Paginator ORM// (fetchJoinCollection: true, cf. ClientProvider) gere le COUNT// malgre ces jointures to-many.->leftJoin('c.categories', 'cat')->addSelect('cat')[Altitude] Le « QueryBuilder de liste » porte désormais des préoccupations d'hydratation.
Context : la méthode impose 3 fetch-joins to-many à tout appelant.
Cause : un futur consommateur qui ne veut que filtrer/compter (ou récupérer des ids) paiera le coût du cartésien. L'optimisation N+1 est posée au mauvais niveau.
Reco : séparer un builder « filtres seuls » (réutilisable, c'est le contrat documenté de l'interface) d'une étape d'hydratation optionnelle, appelée explicitement par la liste paginée. Lié au commentaire perf ci-dessous.
@@ -40,0 +47,4 @@// malgre ces jointures to-many.->leftJoin('c.categories', 'cat')->addSelect('cat')->leftJoin('c.addresses', 'addr')->addSelect('addr')->leftJoin('addr.sites', 'site')->addSelect('site')[Perf] Produit cartésien sur l'export non paginé.
Context : ces 3
addSelectto-many imbriqués (categories × addresses × sites) servent un QB partagé entre la liste paginée (ClientProvider) et l'export XLSX (ClientExportController→getResult()avecpagination=false).Cause : sur la liste c'est borné à 10 clients (OK). Sur l'export on hydrate le produit cartésien de TOUT le répertoire en une requête : 1 client à 5 cat × 4 adresses × 3 sites = 60 lignes SQL, × N clients → explosion lignes/mémoire à l'échelle.
Reco : découpler l'hydratation de la sélection. Pour l'export, charger les collections par requêtes séparées (
WHERE client_id IN (...)) ou via projection DTO dédiée au XLSX. Au minimum valider sur 1000+ clients avant prod.@@ -99,0 +166,4 @@** @return list<int>*/private function normalizeIntList(array $values): array[Robustesse]
normalizeIntListpeut lever unTypeErrorstrict.Context : la closure type son paramètre
int, mais la signature publique de l'interface estarray $siteIds(éléments non typés).Cause : aujourd'hui sauvé car
ClientProvider::readIntListcaste en amont. Mais un appelant direct (test futur, autre module) passant['1','2'](strings) déclencherait unTypeErrorenstrict_types— fragile pour une méthode censée justement normaliser.Reco : caster dans la closure (
(int) $v > 0aprèsis_numeric) plutôt que de typer le paramètre, pour une normalisation réellement défensive.Réponses aux retours de review
Merci pour les relectures 🙏 Récap point par point. Corrections dans le commit
93aa225.✅ Corrigé (
93aa225)commercial.clients.toast.exportErrordistinct dutitle.formatLastActivitydate invalide : garde-fouNumber.isNaN(date.getTime())→ cellule vide (plus de « Invalid Date »).normalizeIntList/normalizeStringListTypeError strict : repassées enforeachdéfensif (is_numeric+ cast, paramarray<mixed>). Un appelant direct passant['1','2']ne casse plus.✅ Conservé volontairement
formatPhoneFR« code mort » : gardé. Introduction anticipée assumée — util téléphone transverse (présent partout : clients, contacts, fournisseurs…), signature stable coordonnée avec ERP-66. Docblock reformulé en ce sens dans93aa225.✅ Résolu (rebase)
GetCollectionconservecategory:read+site:readetgetSites()est présent. Décision actée : la liste embarque catégories + sites (client.sitesagrégé), nécessaire aux colonnes ERP-62 ; le détail garde en plusaddress.sitespar adresse (#45). Les deux shapes coexistent pour deux usages.📌 Tickets de suivi (hors périmètre ERP-62)
createListQueryBuilder= filtres seuls ; export en 2ᵉ passe batchée / DTO). Volumétrie M1 faible → non urgent, benchmark 1000+ avant prod.loadingnon câblé → Malio UI #40 : ajouter une proploadingsurMalioDataTable(+ affichage du total de lignes).MalioDataTablen'expose pas de prop de chargement aujourd'hui, donc à traiter côté lib.Prêt à merger de mon côté.