refactor(user): migrate admin routes from server by shikanime · Pull Request #2738 · cloud-pi-native/console

shikanime · 2026-09-17T09:59:36Z

Issues liées

Quel est le comportement actuel ?

Les routes d'administration des utilisateurs (/api/v1/users) sont servies par l'ancienne application Fastify apps/server.

Quel est le nouveau comportement ?

Migration du module user vers apps/server-nestjs :

UserController, UserService, UserModule.
user-queries.utils.ts : sélections Prisma typées.
Enregistrement du module dans main.module.ts.
Parité des contrats et codes HTTP contre apps/server/src/resources/user/ (400 sur rôle admin inconnu, take: 5 sur la recherche, émission user.upsert par rôle).

Cette PR introduit-elle un breaking change ?

Non.

Autres informations

Recréée pour #1889 (remplace #2498, non réouvrable après suppression de branche).

shikanime

Verdict : Changements demandés

Migration fidèle sur le plan requêtes (where, take: 5, rejet de rôle inconnu, union before/after des rôles impactés — la sémantique legacy est respectée). Point positif : la logique where est copiée sans « amélioration » silencieuse, ce qui rend la parité vérifiable ligne à ligne. Un point bloquant de pont événements → plugins, détaillé inline.

shikanime · 2026-09-18T08:52:55Z

+      user.adminRoleIds?.forEach(roleId => impactedRoleIds.add(roleId))
+    }
+    for (const roleId of impactedRoleIds) {
+      await this.eventEmitter.emitAsync('adminRole.upsert', { roleId })


[🔴 Bloquant] emitAsync('adminRole.upsert') n'a aucun consommateur @OnEvent('adminRole.upsert') dans server-nestjs : à la bascule (apps/server sert encore la route aujourd'hui, donc rien de cassé en prod), la synchro Keycloak/GitLab des groupes admin est silencieusement perdue. Suggestion : ajouter le pont dans keycloak.service.ts / gitlab.service.ts sur le modèle de project.upsert → capturePluginResult, ou tracer un ticket de suivi bloquant la bascule.

Vérifié : le consommateur existe désormais. La branche de base #2749 (pr/admin-role-event-bridge, tête 01467dc713) ajoute @OnEvent('adminRole.upsert') dans keycloak.service.ts:59 et gitlab.service.ts:87, tous deux via capturePluginResult, avec admin-role-bridge.spec.ts de part et d'autre : le pont demandé existe, plus de perte silencieuse à la bascule.

⚠️ En revanche le contrat de payload diverge et doit être unifié dans #2749, sinon le no-op silencieux revient par la forme : keycloak lit un roleId: string (parité legacy hook.adminRole.upsert(roleId)), gitlab lit un objet AdminRoleEventPayload (app-events.service.ts:38) et admin-role.service.ts:45/124 de #2749 émet cet objet — donc le handler keycloak de #2749 ne matche déjà plus ses propres événements (roles.find(({ id }) => id === roleId)), et user.service.ts:64 qui émet { roleId } ne satisfait ni l'un ni l'autre.

Je laisse ce fil ouvert : la forme canonique doit être tranchée dans #2749 (base), puis #2738 s'alignera dessus. Hors périmètre de #2738 seul.

shikanime

And remove // -- comments

Aligne le module user sur le pattern établi par le module zone (#2487) : - `user-queries.utils.ts` : suppression du `userSelect`/`UserRecord` morts, signatures sur `Prisma.TransactionClient`, `createUser` purement DB (la validation du doublon email monte dans le service en `ConflictException`). - `user.service.ts` : extraction de `resolveAdminRoleIds`, `patchUsersInTx`, `emitImpactedRoleEvents` ; patch des rôles sous transaction. - `user.module.ts` : imports repliés sur `InfrastructureModule` (déjà exporté par celui-ci). Refs #2738 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I3bd632f95dd466e585698eba738efcd86a6a6964

github-actions · 2026-09-24T12:45:58Z

🤖 Hey !

A preview of the application is available at : https://console-pr-2738.dso.cpin-hp.numerique-interieur.fr

Please be patient, deployment may take a few minutes.

shikanime · 2026-09-29T07:46:05Z

+    const users = [makeUser()]
+    service.getAllUsers.mockResolvedValue(users)
+
+    const result = await controller.getAllUsers({ adminRoleIds: ['r1'], relationType: 'OR' } as never)


Avoid the use of as never

Corrigé : voir 4131482912 — plus aucun as never, entrées parsées par les schémas partagés.

shikanime · 2026-09-29T07:46:57Z

+    const relationType = query.relationType ?? 'AND'
+    const { relationType: _, ...listQuery } = query
+    const users = await this.userService.getAllUsers(listQuery, relationType)
+    return users.map(toContractUser)


Just pass query at this point

Corrigé : le contrôleur passe query tel quel ; le défaut relationType ?? 'AND' vit dans buildAllUsersWhere.

shikanime · 2026-09-29T07:47:57Z

+    users: PatchUsersBody,
+  ): Promise<User[]> {
+    const usersBefore = await getUsers(this.prisma, { id: { in: users.map(({ id }) => id) } })
+    await this.prisma.$transaction(tx => patchUsers(tx, users))


Probably useless ? Just pass this.prisma ? Or wrap usersBefore in transaction

Conservé tel quel : usersBefore doit être lu avant la transaction, la lecture dans la transaction ne réduirait pas la fenêtre de course.

StephaneTrebel

🔴 Changements demandés. Le payload adminRole.upsert émis par cette PR est incompatible avec les consommateurs Keycloak et GitLab de sa base #2749, ce qui laisse les groupes externes non synchronisés après un patch utilisateur. Les échappatoires as never des tests restent interdites ; la CI du head est verte, mais une preuve de bout en bout de la chaîne événementielle manque.

StephaneTrebel · 2026-09-29T08:52:45Z

+      user.adminRoleIds?.forEach(roleId => impactedRoleIds.add(roleId))
+    }
+    for (const roleId of impactedRoleIds) {
+      await this.eventEmitter.emitAsync('adminRole.upsert', { roleId })


🔴 blocking: Cette émission { roleId } ne correspond à aucun consommateur de la branche de base #2749 : KeycloakService.handleAdminRoleUpsert attend un string, tandis que GitlabService.handleAdminRoleUpsert attend un AdminRoleEventPayload (id, oidcGroup, members). Après un PATCH, Keycloak ne trouve donc aucun rôle et GitLab ne peut associer aucun groupe : la synchronisation legacy est perdue silencieusement. Trancher le payload canonique dans #2749, aligner ses deux consommateurs puis émettre exactement cette forme ici, avec un test de chaîne PATCH → événement → plugins.

Corrigé : emitImpactedRoleEvents charge désormais les rôles (id, oidcGroup) et leurs membres, puis émet le payload canonique { id, oidcGroup, members } attendu par KeycloakService.handleAdminRoleUpsert et GitlabService.handleAdminRoleUpsert.

StephaneTrebel · 2026-09-29T08:52:45Z

+    service.patchUsers.mockResolvedValue(users)
+    const body = [{ id: users[0].id, adminRoleIds: ['r1'] }]
+
+    expect(await controller.patchUsers(body as never)).toEqual(users.map(toContractUser))


🟠 important: as never est interdit dans ce dépôt : il coupe la preuve de type du test et masque ici la compatibilité réelle de PatchUsersBody. Le même échappatoire demeure à la ligne 41. Construire des entrées typées par les schémas partagés, sans assertion.

Corrigé : les deux as never disparaissent — entrées typées via AllUsersQuerySchema.parse / PatchUsersBodySchema.parse dans le spec contrôleur.

shikanime

Verdict : Commentaire

L'extraction des schémas de contrat dans packages/shared est mécanique et les deux côtés client/serveur restent alignés. Le pont d'événements adminRole.upsert émet désormais le payload complet {id, oidcGroup, members} attendu par les consommateurs — la forme { roleId } signalée en review est corrigée. Rien de bloquant côté diff.

shikanime · 2026-09-30T23:17:38Z

+      select: { id: true, email: true, firstName: true, lastName: true, adminRoleIds: true },
+    })
+    for (const role of roles) {
+      await this.eventEmitter.emitAsync('adminRole.upsert', {


[✨ Éloge] emitImpactedRoleEvents calcule l'union des rôles avant/après puis relit les membres depuis la base après commit du $transaction : payload idempotent au replay et aligné sur le contrat des consommateurs Keycloak/GitLab — c'est le patron d'émission post-commit qu'il faut généraliser.

…ion semantics Co-authored-by: Automata <automata@shikanime.studio>

Signed-off-by: Shikanime Deva <22115108+shikanime@users.noreply.github.com>

Aligne le module user sur le pattern établi par le module zone (#2487) : - `user-queries.utils.ts` : suppression du `userSelect`/`UserRecord` morts, signatures sur `Prisma.TransactionClient`, `createUser` purement DB (la validation du doublon email monte dans le service en `ConflictException`). - `user.service.ts` : extraction de `resolveAdminRoleIds`, `patchUsersInTx`, `emitImpactedRoleEvents` ; patch des rôles sous transaction. - `user.module.ts` : imports repliés sur `InfrastructureModule` (déjà exporté par celui-ci). Refs #2738 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I3bd632f95dd466e585698eba738efcd86a6a6964

`AllUsersQuerySchema` étant exporté, l'alias `AllUsersQuery` est redondant ; les consommateurs dérivent le type par `z.infer`. `LettersQuery` est conservé (utilisé par le client). Refs #1889 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I5d954715680b8fbcd866edf4bb300dc36a6a6964

Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>

- supprimer UserService.createUser et les helpers createUser/getUserByEmail (aucun appelant ni route au contrat : chemin mort signalé en revue) - supprimer l'import faker inutilisé de user.controller.spec.ts - verrouiller par spec : 400 (BadRequestException) sur adminRole inconnue, take:5 sur la recherche par lettres, émission union avant ∪ après Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>

Reflect.getMetadata assertions pass whether or not the guard is enforced and add no behavioural coverage; the guard logic stays covered in user.guard.spec.ts and the controller delegation tests are untouched. Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>

…nputs Emit the full { id, oidcGroup, members } payload that the 2749 event bridge consumers expect, drop as never from the controller spec by parsing inputs through the shared schemas, and move the relationType default into buildAllUsersWhere so the controller passes the query straight through. Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I28676aa618050a7042ed24dfa9e66b896a6a6964

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: Ieb2ee298f5d0547bf988787721ed029a6a6a6964

cloud-pi-native-sonarqube · 2026-10-01T15:42:56Z

Quality Gate passed

Issues

Measures

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

github-code-quality Bot found potential problems Sep 17, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/user/user.service.ts Fixed

github-actions Bot added the built label Sep 17, 2026

shikanime self-assigned this Sep 17, 2026

shikanime added this to the 9.27.0 milestone Sep 17, 2026