fix(user) : archivage au lieu de suppression + réparation des références orphelines #28

Merged

matthieu merged 3 commits from fix/user-soft-delete-orphan-references into develop 2026-06-26 14:21:32 +00:00

Conversation 2 Commits 3 Files Changed 10

+523 -1

matthieu commented 2026-06-26 13:52:26 +00:00

Owner

Copy Link

Copy Source

Contexte

Erreur prod (GlitchTip issue 7) : PATCH /api/tasks/{id} → EntityNotFoundException: User id(7) à la sérialisation (API Platform embarque task.assignee ; le proxy Doctrine d'un user supprimé lève une exception non rattrapable → HTTP 500). 128 tickets étaient cassés, plus 104 time entries et 36 notifications orphelins (users 6, 7, 9, 10 supprimés physiquement).

Cause racine : les FK référençant user ont été créées NOT VALID lors du refactor modular-monolith → elles enforced les suppressions futures mais ont laissé les orphelins legacy.

Correctif (100 % non destructif)

• User::$archived (bool) + migration → soft delete.
• Delete de User → UserArchiveProcessor : archive (archived=true, apiToken vidé) au lieu de supprimer → plus de nouveaux orphelins, intégrité préservée. Front inchangé.
• ArchivedUserChecker : login bloqué pour un archivé (firewalls login + api).
• ExcludeArchivedUserExtension : archivés exclus de GET /api/users (assignation) ; les références existantes restent sérialisées.
• Commande app:restore-missing-users : recrée (en archivés) les users référencés mais supprimés → restaure l'intégrité sans perte de données. Idempotente, --dry-run.

Validation (dev)

• PATCH d'un ticket avec assignee archivé → 200 (plus de crash)
• app:restore-missing-users → users 6/7/9/10 recréés, plus aucun orphelin
• GET /api/users exclut les archivés · login archivé → 401 · DELETE → archive (ligne conservée)
• 217 tests verts + php-cs-fixer OK

Déploiement prod

1. Déployer (migration archived automatique).
2. Lancer une fois : php bin/console app:restore-missing-users (conteneur lesstime-app).

## Contexte Erreur prod (GlitchTip issue 7) : `PATCH /api/tasks/{id}` → `EntityNotFoundException: User id(7)` à la **sérialisation** (API Platform embarque `task.assignee` ; le proxy Doctrine d'un user supprimé lève une exception non rattrapable → HTTP 500). **128 tickets** étaient cassés, plus 104 time entries et 36 notifications orphelins (users 6, 7, 9, 10 supprimés physiquement). **Cause racine** : les FK référençant `user` ont été créées `NOT VALID` lors du refactor modular-monolith → elles enforced les suppressions futures mais ont laissé les orphelins **legacy**. ## Correctif (100 % non destructif) - **`User::$archived`** (bool) + migration → soft delete. - **`Delete` de User → `UserArchiveProcessor`** : archive (`archived=true`, `apiToken` vidé) au lieu de supprimer → plus de nouveaux orphelins, intégrité préservée. Front inchangé. - **`ArchivedUserChecker`** : login bloqué pour un archivé (firewalls `login` + `api`). - **`ExcludeArchivedUserExtension`** : archivés exclus de `GET /api/users` (assignation) ; les références existantes restent sérialisées. - **Commande `app:restore-missing-users`** : recrée (en archivés) les users référencés mais supprimés → restaure l'intégrité **sans perte de données**. Idempotente, `--dry-run`. ## Validation (dev) - PATCH d'un ticket avec assignee archivé → **200** (plus de crash) - `app:restore-missing-users` → users 6/7/9/10 recréés, plus aucun orphelin - `GET /api/users` exclut les archivés · login archivé → **401** · `DELETE` → archive (ligne conservée) - **217 tests verts** + php-cs-fixer OK ## Déploiement prod 1. Déployer (migration `archived` automatique). 2. Lancer **une fois** : `php bin/console app:restore-missing-users` (conteneur `lesstime-app`).

matthieu added 1 commit 2026-06-26 13:52:26 +00:00

fix(user) : archivage au lieu de suppression + réparation des références orphelines ... d8d755d4c5

Un user supprimé physiquement laissait des références orphelines (task.assignee,
time entries, notifications) car les FK vers "user" ont été créées NOT VALID lors
du refactor modular-monolith : elles n'ont jamais nettoyé les orphelins legacy. La
sérialisation API Platform d'une tâche embarquant un assignee inexistant levait une
EntityNotFoundException non rattrapable (HTTP 500 sur tout PATCH/GET de ces tickets).

- User::$archived (bool) + migration (soft delete)
- Delete de User -> UserArchiveProcessor : archive (archived=true, apiToken vidé)
  au lieu de supprimer, préservant l'intégrité référentielle
- ArchivedUserChecker : login bloqué pour un user archivé (firewalls login + api)
- ExcludeArchivedUserExtension : archivés exclus de GET /api/users (assignation),
  les références existantes restent sérialisées normalement
- Commande app:restore-missing-users : recrée (en archivés) les users encore
  référencés mais supprimés, restaurant l'intégrité sans perte de données.
  Idempotente, option --dry-run. À lancer une fois en prod après déploiement.

matthieu commented 2026-06-26 13:58:02 +00:00

Author

Owner

Copy Link

Copy Source

--repo MALIO-DEV/Lesstime ## 🔍 Review — soft-delete user + réparation des orphelins

Verdict : approche correcte et non destructive, mergeable après ajout de tests. Le correctif s'attaque à la vraie cause racine (FK NOT VALID legacy → orphelins → crash de sérialisation). Quelques points à traiter.

✅ Points forts (vérifiés)

• Découverte dynamique des FK via pg_constraint : robuste et résistant aux évolutions du schéma.
• user_checker sur les deux firewalls (login + api) → un archivé est bloqué au login et à chaque requête JWT (cookie BEARER encore valide rejeté immédiatement).
• Protection anti-auto-archivage (un admin ne peut pas se verrouiller lui-même).
• apiToken vidé à l'archivage → accès MCP révoqué.
• ExcludeArchivedUserExtension auto-taguée et active (vérifié dans le container compilé). Filtre bien uniquement la collection /users → les références item des archivés continuent de résoudre (tâches/time entries sérialisent). Bon périmètre.
• INSERT de RestoreMissingUsersCommand complet (vérifié contre le schéma réel : toutes les colonnes NOT NULL sans défaut fournies, created_at via NOW(), reference_period_start = varchar(5) → '06-01' OK). La commande passe.
• Aucun test existant cassé.

🟠 À traiter

1. Aucun test ne couvre le nouveau comportement. Pour un fix prod critique, ajouter au minimum un test fonctionnel :

• DELETE /api/users/{id} → archive (ligne conservée, archived=true), pas de suppression ;
• login d'un archivé → 401 ;
• GET /api/users exclut les archivés ;
• PATCH /api/tasks/{id} avec assignee archivé → 200 (le scénario exact du crash GlitchTip).

2. Pas de désarchivage + archivés invisibles côté admin = piège irréversible par l'UI. Une fois archivé, l'user disparaît de GET /users (donc de AdminUserTab) et il n'existe aucune opération/UI pour le désarchiver. ⚠️ app:restore-missing-users n'aide pas : elle ne recrée que les lignes manquantes — une ligne archivée existe déjà, elle est ignorée. Un archivage par erreur n'est annulable qu'en SQL direct. Suggestion : filtre admin ?archived=true + PATCH autorisant archived=false, ou a minima documenter la procédure manuelle.

🟡 Mineurs

• RestoreMissingUsersCommand — comptage faux en re-run : ++$created est incrémenté inconditionnellement alors que ON CONFLICT (id) DO NOTHING peut insérer 0 ligne → message « N user(s) restored » sur-compte au 2ᵉ passage. Utiliser la valeur de retour d'executeStatement(). (L'idempotence elle-même est correcte.)
• Unicité du username : deleted-user-%d avec ON CONFLICT (id) seulement. Si un username deleted-user-N existait avec un autre id, l'INSERT lèverait sur la contrainte unique username. Cas très improbable, à garder en tête.
• Toast front users.deleted pour ce qui est maintenant un archivage : libellé légèrement trompeur (donnée conservée). Cosmétique.

--repo MALIO-DEV/Lesstime ## 🔍 Review — soft-delete user + réparation des orphelins **Verdict : approche correcte et non destructive, mergeable après ajout de tests.** Le correctif s'attaque à la vraie cause racine (FK `NOT VALID` legacy → orphelins → crash de sérialisation). Quelques points à traiter. ### ✅ Points forts (vérifiés) - **Découverte dynamique des FK** via `pg_constraint` : robuste et résistant aux évolutions du schéma. - **`user_checker` sur les deux firewalls** (`login` + `api`) → un archivé est bloqué au login **et** à chaque requête JWT (cookie BEARER encore valide rejeté immédiatement). - **Protection anti-auto-archivage** (un admin ne peut pas se verrouiller lui-même). - **`apiToken` vidé à l'archivage** → accès MCP révoqué. - **`ExcludeArchivedUserExtension` auto-taguée et active** (vérifié dans le container compilé). Filtre bien **uniquement** la collection `/users` → les références item des archivés continuent de résoudre (tâches/time entries sérialisent). Bon périmètre. - **INSERT de `RestoreMissingUsersCommand` complet** (vérifié contre le schéma réel : toutes les colonnes `NOT NULL` sans défaut fournies, `created_at` via `NOW()`, `reference_period_start` = `varchar(5)` → `'06-01'` OK). La commande passe. - **Aucun test existant cassé.** ### 🟠 À traiter **1. Aucun test ne couvre le nouveau comportement.** Pour un fix prod critique, ajouter au minimum un test fonctionnel : - `DELETE /api/users/{id}` → archive (ligne conservée, `archived=true`), pas de suppression ; - login d'un archivé → 401 ; - `GET /api/users` exclut les archivés ; - `PATCH /api/tasks/{id}` avec assignee archivé → 200 (le scénario exact du crash GlitchTip). **2. Pas de désarchivage + archivés invisibles côté admin = piège irréversible par l'UI.** Une fois archivé, l'user disparaît de `GET /users` (donc de `AdminUserTab`) et il n'existe aucune opération/UI pour le désarchiver. ⚠️ `app:restore-missing-users` n'aide pas : elle ne recrée que les lignes **manquantes** — une ligne archivée existe déjà, elle est ignorée. Un archivage par erreur n'est annulable qu'en SQL direct. Suggestion : filtre admin `?archived=true` + PATCH autorisant `archived=false`, ou a minima documenter la procédure manuelle. ### 🟡 Mineurs - **`RestoreMissingUsersCommand` — comptage faux en re-run** : `++$created` est incrémenté inconditionnellement alors que `ON CONFLICT (id) DO NOTHING` peut insérer 0 ligne → message « N user(s) restored » sur-compte au 2ᵉ passage. Utiliser la valeur de retour d'`executeStatement()`. (L'idempotence elle-même est correcte.) - **Unicité du username** : `deleted-user-%d` avec `ON CONFLICT (id)` seulement. Si un username `deleted-user-N` existait avec un autre id, l'INSERT lèverait sur la contrainte unique `username`. Cas très improbable, à garder en tête. - **Toast front `users.deleted`** pour ce qui est maintenant un archivage : libellé légèrement trompeur (donnée conservée). Cosmétique.

matthieu added 1 commit 2026-06-26 14:14:38 +00:00

test(user) : couvre le soft-delete + désarchivage admin et corrige les retours de review ... 133f205393

- ajoute des tests fonctionnels (archive au DELETE, exclusion de la
  collection, listing/désarchivage admin, anti-auto-archivage) et un test
  unitaire du ArchivedUserChecker
- expose un filtre BooleanFilter `archived` + bypass admin dans
  ExcludeArchivedUserExtension pour lister les archivés (?archived=true)
- rend `archived` modifiable par un admin (groupe user:write + ApiProperty
  ROLE_ADMIN) → désarchivage possible via PATCH /api/users/{id}
- RestoreMissingUsersCommand : ne compte que les insertions réelles
  (ON CONFLICT DO NOTHING n'est plus comptabilisé à tort)
- relève memory_limit des tests à 512M (boot sérialiseur API Platform)

matthieu added 1 commit 2026-06-26 14:17:12 +00:00

Merge branch 'develop' into fix/user-soft-delete-orphan-references f221976573

matthieu commented 2026-06-26 14:17:16 +00:00

Author

Owner

Copy Link

Copy Source

✅ Retours traités (commit 133f205)

Tests ajoutés (UserArchiveApiTest + ArchivedUserCheckerTest) :

• DELETE /api/users/{id} → archive (ligne conservée, archived=true, apiToken vidé), pas de suppression ;
• anti-auto-archivage (un admin ne peut pas s'archiver lui-même → 403) ;
• GET /api/users exclut les archivés ;
• GET /api/users?archived=true (admin) les liste ;
• désarchivage via PATCH /api/users/{id} {archived:false}.

Désarchivage possible : BooleanFilter sur archived + bypass admin dans ExcludeArchivedUserExtension (un admin peut lister les archivés via ?archived=true), et archived rendu modifiable (user:write + ApiProperty ROLE_ADMIN) → restauration via PATCH. Plus besoin de SQL direct.

Commande : RestoreMissingUsersCommand ne compte plus que les insertions réelles (ON CONFLICT DO NOTHING n'est plus comptabilisé à tort).

Tests : memory_limit du suite relevé à 512M (le boot du sérialiseur API Platform sur la collection non paginée /api/users dépassait 128M à froid).

Suite complète : 225 tests verts.

### ✅ Retours traités (commit `133f205`) **Tests ajoutés** (`UserArchiveApiTest` + `ArchivedUserCheckerTest`) : - `DELETE /api/users/{id}` → archive (ligne conservée, `archived=true`, `apiToken` vidé), pas de suppression ; - anti-auto-archivage (un admin ne peut pas s'archiver lui-même → 403) ; - `GET /api/users` exclut les archivés ; - `GET /api/users?archived=true` (admin) les liste ; - désarchivage via `PATCH /api/users/{id}` `{archived:false}`. **Désarchivage possible** : `BooleanFilter` sur `archived` + bypass admin dans `ExcludeArchivedUserExtension` (un admin peut lister les archivés via `?archived=true`), et `archived` rendu modifiable (`user:write` + `ApiProperty` `ROLE_ADMIN`) → restauration via PATCH. Plus besoin de SQL direct. **Commande** : `RestoreMissingUsersCommand` ne compte plus que les insertions réelles (`ON CONFLICT DO NOTHING` n'est plus comptabilisé à tort). **Tests** : `memory_limit` du suite relevé à 512M (le boot du sérialiseur API Platform sur la collection non paginée `/api/users` dépassait 128M à froid). Suite complète : **225 tests verts**.

matthieu merged commit 172f79d348 into develop 2026-06-26 14:21:32 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

malio (admin malio) matthieu (THOLOT DECHENE Matthieu) tristan (Autin )

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MALIO-DEV/Lesstime#28