mirror of
https://github.com/MaksTinyWorkshop/_Assistant_Lead_Tech
synced 2026-04-06 13:31:43 +02:00
capitalisation: guard admin NestJS silencieux + patterns Epic 4 app-alexandrie
- Anti-pattern: @UseGuards(AdminRoleGuard) sans @RequireAdminRole() → guard inefficace - Anti-pattern: code erreur générique sur statut HTTP sémantique (ALIAS_ALREADY_RESOLVED) - Anti-pattern: guard de rôle via return conditionnel dans render React Native - Pattern: tests e2e scénarios d'autorisation alternatifs avec buildApp isolé - Anti-pattern: méthodes store Zustand qui avalent les erreurs sans rethrow - Anti-pattern: regex globale singleton pour transformation de contenu
This commit is contained in:
@@ -100,3 +100,138 @@ Ce mécanisme permet :
|
||||
- d'éviter la pollution de la base de connaissance
|
||||
- de capitaliser progressivement l'expérience des projets
|
||||
- de garder `Lead_tech` cohérent et fiable.
|
||||
|
||||
---
|
||||
|
||||
2026-03-24 — app-alexandrie
|
||||
|
||||
FILE_UPDATE_PROPOSAL
|
||||
Fichier cible : 10_backend_risques_et_vigilance.md
|
||||
|
||||
Pourquoi :
|
||||
Code review story 4.7 — pattern récurrent : utiliser `VALIDATION_ERROR` comme code d'erreur sur un 409 rend les erreurs indistinguables côté client et monitoring. Chaque scénario métier doit avoir son propre code.
|
||||
|
||||
Proposition :
|
||||
**Anti-pattern : code d'erreur générique sur statut HTTP sémantique**
|
||||
Ne jamais utiliser `VALIDATION_ERROR` ou `INTERNAL_ERROR` sur un 409 CONFLICT. Chaque cas métier doit avoir un code dédié dans `error-code.ts` (ex: `ALIAS_ALREADY_RESOLVED`, `HANDLE_ALREADY_TAKEN`). Raison : les clients (mobile, monitoring, tests) ne peuvent pas distinguer les cas sans un code sémantique. Règle : 1 scénario métier distinct = 1 code d'erreur distinct.
|
||||
|
||||
---
|
||||
|
||||
2026-03-24 — app-alexandrie
|
||||
|
||||
FILE_UPDATE_PROPOSAL
|
||||
Fichier cible : 10_frontend_risques_et_vigilance.md
|
||||
|
||||
Pourquoi :
|
||||
Code review story 4.7 — guard d'accès admin implémenté avec un return conditionnel inline dans le corps du composant React Native, causant un flash et une UX instable quand `user` est encore `null` au montage.
|
||||
|
||||
Proposition :
|
||||
**Anti-pattern : guard de rôle via return conditionnel dans le render**
|
||||
Ne jamais faire `if (user?.role !== 'ADMIN') return <AccessDenied />` directement dans le corps d'un composant. Pendant le chargement du store auth, `user` peut être `null`, ce qui cause un flash du contenu "Accès refusé" suivi d'un re-render. Pattern correct : `useEffect(() => { if (user !== null && user.role !== 'ADMIN') router.replace('/(tabs)'); }, [user, router])` + rendu vide (`return <View />`) tant que `user === null || user.role !== 'ADMIN'`.
|
||||
|
||||
---
|
||||
|
||||
2026-03-24 — app-alexandrie
|
||||
|
||||
FILE_UPDATE_PROPOSAL
|
||||
Fichier cible : 10_backend_risques_et_vigilance.md
|
||||
|
||||
Pourquoi :
|
||||
Code review story 4.7 — tests e2e manquants pour le scénario non-abonné 403 sur un endpoint protégé par entitlements. Le mock d'entitlements partagé dans le `beforeAll` du describe rendait ce test impossible sans un `buildApp` isolé.
|
||||
|
||||
Proposition :
|
||||
**Pattern : tests e2e de scénarios d'autorisation alternatifs avec buildApp isolé**
|
||||
Quand un describe e2e partage un `buildApp` avec des entitlements actifs dans `beforeAll`, les tests de scénarios non-abonnés / inactifs doivent créer leur propre instance via `buildApp({ getEntitlementsForUser: jest.fn().mockResolvedValue({ subscription: { isActive: false, ... } }) })` avec `await app.close()` en fin de test. Ne pas tenter de surcharger le mock partagé (risque de pollution entre tests).
|
||||
|
||||
---
|
||||
|
||||
2026-03-24 — app-alexandrie
|
||||
|
||||
FILE_UPDATE_PROPOSAL
|
||||
Fichier cible : 10_frontend_risques_et_vigilance.md
|
||||
|
||||
Pourquoi :
|
||||
Pattern récurrent dans les stores Zustand mobiles : les méthodes `update*` (updateThread, updateComment) omettent de relancer (`throw`) les erreurs catchées, ce qui avale silencieusement des erreurs métier (ex: UNSAFE_LINK) et empêche l'écran d'afficher un feedback à l'utilisateur.
|
||||
|
||||
Proposition :
|
||||
**Anti-pattern : méthodes store Zustand qui avalent les erreurs sans rethrow**
|
||||
|
||||
```typescript
|
||||
// ❌ MAUVAIS — l'erreur est avalée, l'écran ne sait pas que ça a échoué
|
||||
async updateThread(forumSlug, threadId, body) {
|
||||
await communityService.updateThread(accessToken, forumSlug, threadId, body);
|
||||
// Si une erreur est lancée, elle disparaît silencieusement
|
||||
},
|
||||
|
||||
// ✅ BON — l'erreur est propagée pour que l'écran puisse réagir
|
||||
async updateThread(forumSlug, threadId, body) {
|
||||
try {
|
||||
await communityService.updateThread(accessToken, forumSlug, threadId, body);
|
||||
} catch (e) {
|
||||
const err = e as Error & { code?: string };
|
||||
throw err; // Le code d'erreur (ex: UNSAFE_LINK) est préservé sur l'objet
|
||||
}
|
||||
},
|
||||
```
|
||||
|
||||
Règle : toute méthode store qui appelle le service réseau DOIT soit (1) relancer l'erreur enrichie avec `throw err`, soit (2) la stocker dans le state (`set({ error: err.message })`). Jamais les deux à la fois sans rethrow si l'écran doit réagir au catch.
|
||||
|
||||
---
|
||||
|
||||
FILE_UPDATE_PROPOSAL
|
||||
Fichier cible : 10_frontend_risques_et_vigilance.md
|
||||
|
||||
Pourquoi :
|
||||
Les regex globales (`/g`) définies comme constantes au niveau module en TypeScript/Node.js maintiennent un état `lastIndex`. Bien que `String.prototype.replace()` réinitialise ce `lastIndex`, l'utilisation future avec `.test()` ou `.exec()` (ex: lors d'un refactor) introduira un bug stateful difficile à détecter.
|
||||
|
||||
Proposition :
|
||||
**Anti-pattern : regex globale singleton pour transformation de contenu**
|
||||
|
||||
```typescript
|
||||
// ❌ RISQUÉ — regex globale partagée entre tous les appels
|
||||
const LINK_PATTERN = /https?:\/\/\S+/gi;
|
||||
function processLinks(content: string) {
|
||||
return content.replace(LINK_PATTERN, ...); // OK today
|
||||
// Mais si quelqu'un ajoute LINK_PATTERN.test(x) ailleurs → bug lastIndex
|
||||
}
|
||||
|
||||
// ✅ SÛR — nouvelle instance à chaque appel, aucun état partagé
|
||||
function makeLinkPattern(): RegExp {
|
||||
return /https?:\/\/\S+/gi;
|
||||
}
|
||||
function processLinks(content: string) {
|
||||
return content.replace(makeLinkPattern(), ...);
|
||||
}
|
||||
```
|
||||
|
||||
Règle : les regex avec flag `/g` ou `/y` utilisées pour la transformation de strings → toujours créer via une factory, jamais en singleton.
|
||||
|
||||
---
|
||||
|
||||
2026-03-24 — app-alexandrie
|
||||
|
||||
FILE_UPDATE_PROPOSAL
|
||||
Fichier cible : 10_backend_risques_et_vigilance.md
|
||||
|
||||
Pourquoi :
|
||||
Story 4.5 Epic 4 — `@UseGuards(AdminRoleGuard)` sans `@RequireAdminRole()` laisse passer tous les utilisateurs silencieusement. Bug de sécurité non évident, détecté uniquement en review adversariale.
|
||||
|
||||
Proposition :
|
||||
**Anti-pattern : guard admin NestJS sans son décorateur couplé**
|
||||
|
||||
`AdminRoleGuard.canActivate()` lit la metadata `REQUIRE_ADMIN_ROLE_KEY` posée par `@RequireAdminRole()`. Sans le décorateur, `requiresAdmin = false` → le guard approuve tout le monde.
|
||||
|
||||
```typescript
|
||||
// ❌ DANGEREUX — guard actif mais inefficace
|
||||
@UseGuards(AdminRoleGuard)
|
||||
@Get('admin/something')
|
||||
handler() {}
|
||||
|
||||
// ✅ CORRECT — toujours les deux ensemble
|
||||
@RequireAdminRole()
|
||||
@UseGuards(AdminRoleGuard)
|
||||
@Get('admin/something')
|
||||
handler() {}
|
||||
```
|
||||
|
||||
Règle : tout endpoint admin = `@RequireAdminRole()` + `@UseGuards(AdminRoleGuard)` obligatoirement ensemble. S'applique à tout guard NestJS basé sur la reflection de metadata.
|
||||
|
||||
Reference in New Issue
Block a user