Coltura/doc/review-PR1-ERP7-modular-monolith.md

Code Review — PR #1 [ERP-7] Mise en place du modular monolith

Branche : feature/ERP-7-mise-en-place-du-modular-monolith → develop
Auteur : Tristan
Date de review : 2026-04-09
Scope : 55 commits, 85 fichiers modifiés, ~30 000 lignes ajoutées

---

Résumé de la PR

Cette PR restructure Coltura (CRM/ERP) en architecture modulaire DDD (Domain-Driven Design) :

• Backend : introduction de bounded contexts (Module/Core, Module/Commercial) avec séparation Domain / Application / Infrastructure
• Shared : couche partagée (events, value objects, contracts, bus interfaces)
• Modules activables : config/modules.php comme source de vérité, activation/désactivation sans toucher au code
• Sidebar dynamique : config/sidebar.php déclare la navigation, le backend filtre selon les modules actifs, le frontend consomme via /api/sidebar
• Frontend : réorganisation en app/ (shell), shared/ (composables, stores, types), modules/ (Nuxt layers auto-détectés)
• Infra : migration Doctrine, Dockerfile prod, deploy.sh, nginx-proxy, maintenance mode

---

Issues trouvées

Issue 1 — CHANGELOG mentionne le mauvais projet

Sévérité	Critique
Fichier	CHANGELOG.md, ligne 3
Confiance	100/100

Constat : Le header du CHANGELOG dit :

Liste des évolutions du projet Ferme

Ce fichier appartient à Coltura, pas au projet Ferme. C'est une erreur de copier-coller lors du scaffolding initial.

Correction : Remplacer "Ferme" par "Coltura".

---

Issue 2 — Sidebar link /suppliers pointe vers une page inexistante (404)

Sévérité	Majeure
Fichier	config/sidebar.php, ligne 49
Confiance	75/100 (confirmé par 3 agents indépendants)

Constat : La sidebar déclare un lien vers /suppliers pour le module commercial :

// config/sidebar.php
[
    'label' => 'sidebar.commercial.suppliers',
    'icon'  => 'i-heroicons-truck',
    'to'    => '/suppliers',       // ← route inexistante
    'module' => 'commercial',
],

Mais la seule page du module commercial est frontend/modules/commercial/pages/commercial.vue, que Nuxt mappe sur la route /commercial (pas /suppliers).

Impact : Cliquer sur ce lien dans la sidebar donnera un 404.

Correction : Soit renommer la page en suppliers.vue, soit changer le 'to' en '/commercial'.

---

Issue 3 — Port PostgreSQL changé de 5436 à 5437

Sévérité	Majeure
Fichier	infra/dev/.env.docker
Règle violée	Workspace CLAUDE.md : "Coltura — 8083 / 3003 / 5436"
Confiance	75/100

Constat : Le fichier .env.docker définit POSTGRES_PORT=5437, alors que le port documenté pour Coltura est 5436.

Impact : Tout développeur qui suit les ports documentés (ou qui utilise des scripts basés sur ces ports) ne pourra pas se connecter à la base.

Correction : Revenir à 5436 ou mettre à jour les CLAUDE.md (workspace + projet).

---

Issue 4 — Port frontend changé de 3003 à 3004 sans mise à jour de la documentation

Sévérité	Majeure
Fichiers	frontend/nuxt.config.ts (ligne 40), docker-compose.yml (ligne 33)
Règle violée	Workspace CLAUDE.md : "Coltura — 8083 / 3003 / 5436" et CLAUDE.md projet : "make dev-nuxt # port 3003"
Confiance	75/100 (confirmé par 3 agents indépendants)

Constat :

// nuxt.config.ts
devServer: { port: 3004 }  // était 3003

# docker-compose.yml
ports:
  - "3004:3004"  # était 3004:3003

Les deux CLAUDE.md documentent toujours le port 3003.

Correction : Mettre à jour les deux CLAUDE.md pour refléter le nouveau port, ou revenir à 3003.

---

Issue 5 — useSidebar : state singleton jamais réinitialisé au logout

Sévérité	Majeure
Fichier	frontend/shared/composables/useSidebar.ts, lignes 3-5
Confiance	75/100

Constat : Les refs sections, disabledRoutes et loaded sont déclarées au niveau module (en dehors de la fonction composable) :

const sections = ref<SidebarSection[]>([])
const disabledRoutes = ref<string[]>([])
const loaded = ref(false)

Ce sont des singletons partagés sur toute la durée de vie de l'app. Après un logout + re-login :

1. loaded.value reste true
2. loadSidebar() n'est jamais rappelé
3. La sidebar affiche les données de la session précédente

Le middleware auth.global.ts ne recharge que si !loaded.value, et logout.vue ne reset jamais loaded.

Impact : Sidebar périmée après re-connexion. Si les modules changent côté serveur, le frontend ne le saura jamais sans un hard refresh.

Correction : Ajouter une fonction resetSidebar() appelée au logout, ou conditionner le rechargement autrement (ex: toujours recharger après login).

---

Issue 6 — UserOutput DTO : type mismatch int vs ?int + dead code

Sévérité	Moyenne
Fichier	src/Module/Core/Application/DTO/UserOutput.php, lignes 13 et 23
Confiance	75/100

Constat :

// Le constructeur attend un int non-nullable
public function __construct(
    public int $id,  // ← int, pas ?int
    // ...
)

// Mais User::getId() retourne ?int
public static function fromEntity(User $user): self
{
    return new self(
        id: $user->getId(),  // ← peut être null
        // ...
    );
}

Avec declare(strict_types=1), passer null à un paramètre int lève un TypeError.

De plus : Ce DTO n'est utilisé nulle part. MeProvider retourne directement l'entité User via $this->security->getUser(). Le DTO est du dead code.

Correction : Soit utiliser le DTO dans MeProvider (comme l'architecture le prévoit), soit le supprimer. Dans tous les cas, changer int $id en ?int $id.

---

Issue 7 — CreateUserCommand contourne UserRepositoryInterface

Sévérité	Moyenne
Fichier	src/Module/Core/Infrastructure/Console/CreateUserCommand.php, lignes 22-27
Règle violée	CLAUDE.md projet : "les repositories sont des interfaces"
Confiance	75/100 (confirmé par 2 agents indépendants)

Constat :

public function __construct(
    private EntityManagerInterface $em,  // ← injection directe de Doctrine
    private UserPasswordHasherInterface $hasher,
) {}

// ...
$this->em->persist($user);
$this->em->flush();

Le UserRepositoryInterface::save(User $user) existe et est implémenté par DoctrineUserRepository. La commande devrait l'utiliser :

// Correction attendue
public function __construct(
    private UserRepositoryInterface $userRepository,
    private UserPasswordHasherInterface $hasher,
) {}

// ...
$this->userRepository->save($user);

Impact : Viole le pattern DDD introduit dans cette même PR et crée un second chemin de persistance non contrôlé.

---

Issue 8 — auth.vue : indentation 2 espaces au lieu de 4

Sévérité	Mineure
Fichier	frontend/app/layouts/auth.vue
Règle violée	CLAUDE.md projet : "4 espaces d'indentation"
Confiance	75/100

Constat : Le fichier utilise 2 espaces d'indentation :

<template>
  <div class="min-h-screen">
    <slot />
  </div>
</template>

Tous les autres fichiers Vue de la PR (default.vue, login.vue, index.vue, logout.vue, commercial.vue) utilisent correctement 4 espaces.

Correction : Passer en 4 espaces.

---

Issues mineures non retenues (score < 75)

Pour information, ces points ont été identifiés mais jugés moins critiques :

• /api/sidebar et /api/modules en PUBLIC_ACCESS : intentionnel selon le CLAUDE.md qui documente ces endpoints comme publics
• doctrine.yaml ne mappe que le module Core : les entités de futurs modules ne seront pas détectées automatiquement (à documenter)
• Middleware modules.global.ts : boucle de redirection infinie possible si / est dans disabledRoutes (requiert une mauvaise config)
• Lien sidebar /admin : pointe vers une page inexistante (pré-existant sur develop)
• Labels hardcodés en français dans login.vue : "Nom d'utilisateur", "Mot de passe", "Se connecter" au lieu de $t('auth.username') etc. (les clés i18n existent dans fr.json)

---

Verdict

La PR pose de bonnes bases architecturales (DDD, modules activables, sidebar dynamique). Les issues principales sont :

• 2 bugs fonctionnels : sidebar /suppliers en 404 et state useSidebar jamais reset
• 3 incohérences config/doc : ports PG et frontend changés sans MAJ CLAUDE.md, CHANGELOG mauvais projet
• 2 incohérences architecturales : CreateUserCommand et UserOutput ne respectent pas les patterns introduits dans la PR
• 1 style : indentation auth.vue

Aucun de ces problèmes ne bloque le merge mais les 2 bugs fonctionnels (issues 2 et 5) devraient être corrigés avant.