# 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 :

```php
// 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** :

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

```yaml
# 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) :

```typescript
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** :

```php
// 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** :

```php
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 :

```php
// 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 :

```vue
<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.