Aller au contenu principal

ENG-001.5H — Migration des Collectes municipales vers Municipal Management — Rapport d'implémentation

MissionENG-001.5H — Migration des Collectes vers Municipal Management
Rattaché àENG-001.5 — Extraction de Municipal Management ; Recalibrage de roadmap §4 (PR Municipal-H) ; ENG-001.5A ; ENG-001.5B ; ENG-001.5C ; ENG-001.5G
Dépôtdmv_api
Branchefeature/eng-001-5h-municipal-collectes (basée sur main, PR #15 — Municipal-A/B/C/G — déjà mergée)
StatutImplémenté
Comportement fonctionnelInchangé pour toutes les routes publiques (URLs, méthodes, payloads, formats de réponse identiques), sauf deux corrections de bugs pré-existants sur le chemin d'écriture Mairie (voir §10)

1. État initial observé

Audit exhaustif mené avant toute écriture de code (contrôleurs, services, modèles, DTO, validations, routes, middleware, jobs, commandes, Import, frontends dmv-workspace/dmv-public/dmv-backoffice/dmv_backoffice).

  • Modèle Eloquent Territory\Models\CommuneCollecte (table commune_collectes), physiquement dans Territory, utilisé par trois lecteurs différents avant cette PR : MairieReadService::listCollectes() (Eloquent), TerritoryService::getCommuneCollectes() (Eloquent, lecture publique) et AdminCommuneInfoController (SQL brut DB::table()).
  • Écriture Mairie : MairieCommuneController::storeCollecte/updateCollecte/destroyCollecteMairieWriteService::createCollecte/updateCollecte/deleteCollecte, en DB::table() brut, aucune validation explicite ($request->all() transmis tel quel), 4 routes protégées par le middleware commune.manager (EnsureCommuneManager, vérifie le rôle municipal_manager + la permission manage_collectes + le module collectes).
  • Écriture Admin : AdminCommuneInfoController::storeCollecte/updateCollecte/destroyCollecte, en DB::table() brut, validation Laravel explicite (nom/jour requis, max:255/max:100, etc.), 3 routes protégées par l'authentification Admin. AdminCommuneInfoController::fullCommune() agrège aussi les collectes dans sa réponse.
  • Lecture publique Territory (GET /api/v1/communes/{id}/collectes) : hors périmètre, jamais d'écriture, confirmé inchangé.
  • Frontend : dmv-workspace (services/mairie/collectes.ts, components/mairie/collectes/collecte-modal.tsx) consomme les routes Mairie en lecture/écriture complètes. dmv-backoffice (src/pages/Communes.jsx) consomme les routes Admin. dmv-public consomme uniquement la route publique Territory. dmv_backoffice (legacy, écriture directe Supabase) : aucune référence à commune_collectes trouvée dans son code source — confirmé non concerné.
  • Jobs/Commandes/Import : recherche exhaustive (grep sur commune_collectes/CommuneCollecte dans app/Console, app/Modules/Import) — aucun résultat.
  • Couverture de tests avant cette PR : zéro test sur le chemin d'écriture Mairie (confirmé — tests/Feature/Mairie/MairieTest.php ne contient aucun test « collecte »), conformément au constat déjà fait par ENG-001-5-roadmap-recalibration.md §3.1. Admin et Territory étaient couverts (AdminCommuneInfoTest.php, TerritoryTest.php).

2. Chemins d'écriture confirmés

Exactement les deux chemins annoncés par la mission — aucun troisième chemin découvert :

  1. Mairie (MairieCommuneController + MairieWriteService).
  2. Admin (AdminCommuneInfoController).

Territory ne fait que lire (TerritoryService::getCommuneCollectes()), confirmé par lecture du code (aucune méthode d'écriture équivalente à refreshElus()/refreshInfos() de CommuneMairieDataRefreshService n'existe pour les collectes). Le legacy dmv_backoffice (écriture Supabase directe) ne touche pas cette ressource. Aucun Job, aucune Commande Artisan, aucun module Import ne référence commune_collectes.


3. Contrats utilisés

Exclusivement MunicipalManagementReader/MunicipalManagementWriter, déjà stubés depuis Municipal-A avec des signatures suffisantes — aucune extension de signature nécessaire :

MunicipalManagementReader::getCollectes(string $communeId): Collection
MunicipalManagementWriter::createCollecte(string $communeId, array $data): MunicipalCollecteDTO
MunicipalManagementWriter::updateCollecte(string $communeId, string $collecteId, array $data): MunicipalCollecteDTO
MunicipalManagementWriter::deleteCollecte(string $communeId, string $collecteId): void

Une seule extension a été nécessaire, sur le DTO (pas sur une signature de contrat) : MunicipalCollecteDTO ne portait pas created_at, un champ pourtant réellement présent dans le schéma (0000_00_00_000007_create_commune_collectes_table.php) et déjà inclus dans les réponses JSON historiques de Mairie (sérialisation Eloquent) et d'Admin (DB::table()->first()). Ce DTO n'avait encore aucun consommateur réel avant cette PR (méthodes toujours LogicException depuis Municipal-A) : complété avant sa première publication effective, même raisonnement de gouvernance que les deux extensions de contrat documentées par ENG-001-5B-implementation-report.md §4 (« correction avant publication, pas modification d'un contrat déjà stabilisé »). Documenté par transparence, pas traité comme une divergence de Niveau 3 puisqu'aucune signature de méthode n'a changé.


4. Logique déplacée

AvantAprès
MairieCommuneController::indexCollectesMairieReadService::listCollectes() (Eloquent CommuneCollecte)MairieCommuneController::indexCollectesMunicipalManagementReader::getCollectes()
MairieCommuneController::storeCollecteMairieWriteService::createCollecte() (DB::table() brut)MairieCommuneController::storeCollecte (résout ses propres défauts historiques, corrigés — voir §10) → MunicipalManagementWriter::createCollecte()
MairieCommuneController::updateCollecte/destroyCollecteMairieWriteService::updateCollecte/deleteCollecteMunicipalManagementWriter::updateCollecte/deleteCollecte directement (plus d'intermédiaire MairieWriteService, même schéma que Municipal-G pour les services)
AdminCommuneInfoController::fullCommune()DB::table('commune_collectes') brutMunicipalManagementReader::getCollectes()
AdminCommuneInfoController::storeCollecte/updateCollecte/destroyCollecteDB::table('commune_collectes') brutMunicipalManagementWriter::createCollecte/updateCollecte/deleteCollecte ; la vérification d'existence préalable (updateCollecte/destroyCollecte, message 'Collecte introuvable.') passe désormais par MunicipalManagementReader::getCollectes()->contains(...) plutôt que par un DB::table() direct — élimine toute trace de SQL sur commune_collectes dans le contrôleur Admin, y compris en lecture

MunicipalManagementReadService::getCollectes() et MunicipalManagementWriteService::createCollecte/updateCollecte/deleteCollecte implémentent réellement ces opérations contre commune_collectes, via DB::table() (pas Eloquent), conformément à ADR-014 §11/§13.


5. Logique supprimée

  • MairieReadService::listCollectes() — supprimée (0 appelant restant).
  • MairieWriteService::createCollecte(), updateCollecte(), deleteCollecte() — supprimées (0 appelant restant).
  • Dans AdminCommuneInfoController::updateCollecte()/destroyCollecte() : le filtrage manuel array_intersect_key($request->all(), array_flip($allowed)) a été retiré — devenu redondant, MunicipalManagementWriteService::updateCollecte() applique désormais exactement la même liste blanche en interne (COLLECTE_CHAMPS_MODIFIABLES).

Aucune autre suppression : élus, infos pratiques, commune_info_sections, commune (description/image), publications, services municipaux et alertes restent strictement hors périmètre et inchangés.

Le modèle Eloquent Territory\Models\CommuneCollecte n'a pas été supprimé — contrairement au précédent Alertes/Services (Municipal-C/G), où les modèles Mairie correspondants avaient pu être supprimés faute d'autre consommateur. Ici, Territory\Services\TerritoryService::getCommuneCollectes() (lecture publique, explicitement hors périmètre de cette mission) en dépend toujours et n'a pas été touché. Vérifié par grep exhaustif avant toute décision de suppression, conformément à l'instruction de la mission (« ne jamais supprimer un fichier uniquement parce qu'il semble dupliqué »).


6. Façades conservées

  • MairieCommuneController (portion collectes) : validation (aucune explicite, préservée à l'identique — voir §3.2 de la roadmap), vérification d'existence de la commune (abort_unless, message inchangé), résolution des valeurs par défaut historiques Mairie, autorisation (middleware commune.manager, inchangé), délégation. Aucune logique métier, aucun accès direct à commune_collectes.
  • AdminCommuneInfoController (portion collectes) : validation Laravel explicite inchangée, vérification d'existence (désormais via MunicipalManagementReader, message historique préservé), délégation. Aucune logique métier, aucun accès direct (lecture ou écriture) à commune_collectes.

7. Routes préservées

Aucune route créée, supprimée ou modifiée.

Mairie (4 routes, inchangées) :

GET /api/v1/mairie/communes/{communeId}/collectes
POST /api/v1/mairie/communes/{communeId}/collectes
PATCH /api/v1/mairie/communes/{communeId}/collectes/{collecteId}
DELETE /api/v1/mairie/communes/{communeId}/collectes/{collecteId}

Admin (3 routes, inchangées, + /full qui agrège les collectes sans être une route dédiée) :

POST /api/v1/admin/communes/{id}/collectes
PATCH /api/v1/admin/communes/{id}/collectes/{collecteId}
DELETE /api/v1/admin/communes/{id}/collectes/{collecteId}
GET /api/v1/admin/communes/{id}/full

Vérifié par php artisan route:list avant/après (URIs, méthodes, contrôleurs strictement identiques) et par le garde-fou automatisé (§9).


8. Tests créés

tests/Feature/MunicipalManagement/MunicipalCollecteDelegationTest.php20 tests, 70 assertions :

  1. lecture_mairie_retourne_les_collectes_de_la_commune_triees_par_ordre (+ vérifie les clés exactes de la réponse)
  2. lecture_isole_les_collectes_dune_autre_commune
  3. creation_par_owner_avec_module_active_delegue_a_municipal_management
  4. creation_sans_couleurs_ne_leve_plus_derreur_et_applique_les_defauts_du_schema (non-régression bug §10)
  5. creation_sans_permission_manage_collectes_retourne_403
  6. creation_avec_module_collectes_desactive_retourne_403
  7. modification_par_owner_delegue_a_municipal_management
  8. modification_dune_collecte_inexistante_retourne_404
  9. modification_dune_collecte_dune_autre_commune_retourne_404 (non-régression bug §10, isolation inter-communes)
  10. suppression_par_owner_delegue_a_municipal_management
  11. suppression_dune_collecte_inexistante_reste_idempotente_204
  12. admin_creation_delegue_a_municipal_management
  13. admin_creation_sans_nom_est_rejetee_par_la_validation_existante
  14. admin_modification_delegue_a_municipal_management
  15. admin_modification_dune_collecte_inexistante_retourne_404_avec_message_historique
  16. admin_suppression_delegue_a_municipal_management
  17. admin_suppression_dune_collecte_inexistante_retourne_404_avec_message_historique
  18. admin_full_commune_inclut_les_collectes_via_municipal_management
  19. les_facades_mairie_et_admin_ne_contiennent_plus_de_logique_metier_collectes (garde-fou, §9)
  20. il_nexiste_plus_quune_seule_implementation_metier_des_collectes (garde-fou, §9)

tests/Feature/MunicipalManagement/MunicipalManagementFoundationTest.php — ajusté : getCollectes retiré de l'échantillon « lève LogicException » (implémentée), remplacé par getElus/getInfos uniquement — même schéma que l'ajustement fait par Municipal-B et Municipal-G sur ce même fichier.

Couverture explicitement demandée par la mission : lecture, création, modification, suppression, autorisation Mairie (permission + module), délégation Mairie, délégation Admin, isolation entre communes, ressource inexistante, formats de réponse, routes inchangées, garde-fou architectural — toutes présentes ci-dessus. Le réordonnancement n'a pas de test dédié : confirmé par audit qu'aucune route ni méthode de réordonnancement n'existe pour les collectes (contrairement aux services municipaux, commune_collectes n'a jamais eu de fonctionnalité de réordonnancement en masse — seul le champ ordre est modifiable individuellement via updateCollecte).


9. Garde-fous d'architecture

  1. les_facades_mairie_et_admin_ne_contiennent_plus_de_logique_metier_collectes — scanne le code source de MairieCommuneController et AdminCommuneInfoController, échoue si l'un des deux contient encore un DB::table('commune_collectes') ou une référence à CommuneCollecte::, vérifie la présence des deux contrats.
  2. il_nexiste_plus_quune_seule_implementation_metier_des_collectes — vérifie que MairieReadService/MairieWriteService n'exposent plus aucune méthode collectes, que les routes Mairie (4) et Admin (3) existent toujours à l'identique, et que seul MunicipalManagementReadService/WriteService implémentent réellement getCollectes/createCollecte/updateCollecte/deleteCollecte.

Contrairement aux garde-fous Alertes/Services (ENG-001.5C/5G), celui-ci ne vérifie pas l'absence du modèle Eloquent CommuneCollecte — ce modèle reste légitimement utilisé par Territory (§5). La non-duplication est donc vérifiée au niveau des façades (grep du code source) plutôt qu'au niveau de l'existence globale du modèle.


10. Divergences

Conformément à IA-GOUVERNANCE.md, chaque point est documenté ; aucun n'a nécessité l'arrêt de la mission (aucune décision d'ownership, de contrat au sens signature, ou d'architecture n'a été prise — uniquement des corrections de bugs Niveau 1 et une complétion de DTO non consommé, voir §3).

10.1 — Extension du DTO MunicipalCollecteDTO (champ created_at)

Voir §3. Documentée avant implémentation, pas après.

10.2 — Bug corrigé : bg_color/text_color nuls violant une contrainte NOT NULL (Mairie)

Constat. MairieWriteService::createCollecte() insérait explicitement 'bg_color' => $data['bg_color'] ?? null (et pareil pour text_color) quand absent. Les colonnes bg_color/text_color de commune_collectes sont NOT NULL avec une valeur par défaut au niveau du schéma (#e6f1fb/#185fa5) — la valeur par défaut du schéma ne s'applique que si la colonne est omise de l'INSERT, pas si elle reçoit une valeur NULL explicite. Confirmé déclenchable en production : dmv-workspace/components/mairie/collectes/collecte-modal.tsx:76-77 envoie explicitement bg_color: null/text_color: null quand l'utilisateur laisse ces champs vides dans le formulaire.

Pourquoi ce n'est pas resté une divergence bloquante. Aucune couverture de test n'existait sur ce chemin avant cette PR (§1) — le bug n'était donc jamais exercé automatiquement. La mission exige explicitement un test de « création » qui doit réussir, et l'exécution réelle du chemin de création avec des couleurs vides (cas d'usage réel côté frontend) aurait échoué avec une erreur SQL. Correction de bug (Niveau 1 de IA-GOUVERNANCE.md : « correction de bugs » relève de l'implémentation, pas de l'architecture) : MairieCommuneController::storeCollecte() résout désormais bg_color/text_color avec les valeurs par défaut du schéma au lieu de null quand absentes. MunicipalManagementWriteService::createCollecte() applique en complément le même filet de sécurité (defense in depth), pour protéger tout futur appelant du contrat sans jamais imposer de choix métier à la place des façades.

Ce n'est pas une unification Mairie/Admin. Admin appliquait déjà ces mêmes valeurs de secours ('#e6f1fb'/'#185fa5') — mais par coïncidence du fait que les deux proviennent de la même source de vérité (le schéma), pas parce qu'une décision a été prise d'aligner Mairie sur Admin. Les autres défauts historiques (icône '🗑️' Mairie vs '🗑' Admin, absence de validation côté Mairie) restent strictement divergents et préservés tels quels par façade — voir §10.4.

10.3 — Bug corrigé : isolation inter-communes non garantie sur updateCollecte() (Mairie)

Constat. MairieWriteService::updateCollecte() scopait l'UPDATE par id et commune_id, mais le SELECT de retour uniquement par id (DB::table('commune_collectes')->where('id', $collecteId)->firstOrFail()). Conséquence : un gestionnaire municipal de la commune A qui envoie un PATCH sur une collecte appartenant à la commune B voit l'UPDATE ne toucher aucune ligne (correctement filtré), mais reçoit tout de même une réponse 200 avec les données inchangées de la collecte de la commune B — au lieu d'un 404. Fuite d'information mineure (existence + contenu d'une collecte hors périmètre), jamais couverte par un test avant cette PR.

Pourquoi ce n'est pas resté une divergence bloquante. La mission demande explicitement un test d'« isolation entre communes » — ce bug est directement ce que ce test est censé vérifier. Corriger l'implémentation pour que ce test puisse légitimement passer relève de la correction de bug (Niveau 1), pas d'une décision d'architecture : MunicipalManagementWriteService::updateCollecte()/findCollecteOrFail() scopent désormais la lecture de retour par commune_id en plus de id, et lèvent une ModelNotFoundException (→ 404 via le gestionnaire d'exception Laravel par défaut, même mécanisme déjà utilisé par updateAlerte/updateService) si la collecte n'appartient pas à la commune. Le chemin Admin n'était pas concerné (vérification d'existence déjà correctement scopée avant cette PR).

10.4 — Différences de comportement Mairie/Admin préservées, non unifiées

Conformément à l'instruction explicite de la mission (« ne pas l'unifier silencieusement »), les divergences suivantes, déjà présentes avant cette PR, sont préservées à l'identique par façade :

AspectMairieAdmin
Validation à la créationAucune ($request->all() transmis tel quel)Explicite (nom/jour requis, max:255/max:100, etc.)
Défaut icon si absent'🗑️' (avec sélecteur de variation emoji)'🗑' (sans)
Défaut nom/jour si absents'' (chaîne vide, pas de rejet)Rejetés par la validation (422)
Réponse DELETE204 sans corps, idempotente (aucune vérification d'existence préalable)200 avec {'ok': true}, 404 si la collecte n'existe pas

Aucune de ces différences n'a été modifiée par cette PR — seule leur implémentation sous-jacente a changé (délégation à un contrat commun au lieu d'un accès direct à la table), pas leur comportement observable.


11. Décisions encore ouvertes

Aucune nouvelle décision d'architecture n'a été soulevée par cette mission. Les décisions déjà identifiées par les documents antérieurs restent inchangées et non tranchées ici :

  • 13.C (ENG-001.5) — ownership de commune_info_sections : sans rapport avec les collectes, toujours ouverte, bloquante pour une future PR Municipal-J (infos pratiques).
  • 13.D (ENG-001.5) — moment de déplacement de EnsureMairieAccess/EnsureCommuneManager : ces deux middleware continuent de protéger les routes collectes sans avoir été déplacés, conformément à la recommandation déjà actée (option 2 : attendre l'extraction du Platform Service Authorization). Non rouverte par cette mission.

Aucune décision nouvelle n'a été nécessaire pour les collectes elles-mêmes : les deux bugs corrigés (§10.2, §10.3) relèvent de la correction technique (Niveau 1), pas d'un arbitrage produit ou architectural.


12. Enseignements pour ENG-001.5I (Élus)

  • Le modèle Eloquent Territory peut devoir survivre à la migration si Territory (ou un autre consommateur hors périmètre) en dépend en lecture — ne pas supposer, comme pour Alertes/Services, que le modèle peut être supprimé en fin de PR. Pour les élus : TerritoryService::getCommuneElus() existe déjà et dépend très probablement de Territory\Models\CommuneElu de la même manière — à vérifier explicitement dès l'audit préalable de Municipal-I.
  • L'absence de test sur le chemin d'écriture Mairie peut cacher des bugs réels, pas seulement un manque de couverture — deux bugs concrets (contrainte NOT NULL, isolation inter-communes) ont été découverts uniquement parce que la mission exigeait de construire cette couverture à partir de zéro. Le risque signalé par ENG-001-5-roadmap-recalibration.md §8 pour Élus (« logique de réconciliation SIRENE non triviale, aucun test actuel sur le chemin Mairie ») doit être pris au moins aussi au sérieux : auditer le comportement réel (y compris les cas limites de valeurs nulles/absentes) avant d'écrire le moindre test de non-régression, pas seulement lire le code une fois.
  • Élus a trois chemins d'écriture concurrents (Mairie, Admin, Territory-SIRENE via CommuneMairieDataRefreshService::refreshElus()), contre deux pour les collectes — la logique de réconciliation SIRENE (correspondance par nom normalisé, désactivation, insertion) devra être préservée à l'identique, probablement en la faisant déléguer au contrat Municipal Management sans en changer le comportement observable (mêmes compteurs added/updated/disabled), sur le modèle de ce que cette PR a fait pour les deux chemins Mairie/Admin des collectes.
  • Un DTO stub jamais consommé peut être complété sans arbitrage (voir §3, §10.1) — vérifier systématiquement, avant d'implémenter une ressource, que son DTO Municipal-A porte bien tous les champs réellement présents dans le schéma de la table cible (comparer au fichier de migration, pas seulement au modèle Eloquent existant).

Références