From 16bb11361499472f316d58b3e7d5029aa0cf1788 Mon Sep 17 00:00:00 2001
From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com>
Date: Thu, 26 Mar 2026 16:12:33 -0700
Subject: [PATCH] fix: add rollback on rename failure and update PR description

- Roll back user migration if updateRoleByName returns null during a
  rename (race: role deleted between existence check and update)
- Add test verifying rollback calls updateUsersByRole in reverse
- Update PR #12400 description to reflect current test counts (56
  handler tests, 40 data-layer tests) and safety features
---
 packages/api/src/admin/roles.spec.ts | 20 ++++++++++++++++++++
 packages/api/src/admin/roles.ts      |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/packages/api/src/admin/roles.spec.ts b/packages/api/src/admin/roles.spec.ts
index 309f9e84b4..20675f8ac6 100644
--- a/packages/api/src/admin/roles.spec.ts
+++ b/packages/api/src/admin/roles.spec.ts
@@ -467,6 +467,26 @@ describe('createAdminRolesHandlers', () => {
       expect(json).toHaveBeenCalledWith({ error: 'Role not found' });
     });
 
+    it('rolls back user migration when rename fails', async () => {
+      const deps = createDeps({
+        getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null),
+        updateRoleByName: jest.fn().mockResolvedValue(null),
+      });
+      const handlers = createAdminRolesHandlers(deps);
+      const { req, res, status, json } = createReqRes({
+        params: { name: 'editor' },
+        body: { name: 'new-name' },
+      });
+
+      await handlers.updateRole(req, res);
+
+      expect(status).toHaveBeenCalledWith(404);
+      expect(json).toHaveBeenCalledWith({ error: 'Role not found' });
+      expect(deps.updateUsersByRole).toHaveBeenCalledTimes(2);
+      expect(deps.updateUsersByRole).toHaveBeenNthCalledWith(1, 'editor', 'new-name');
+      expect(deps.updateUsersByRole).toHaveBeenNthCalledWith(2, 'new-name', 'editor');
+    });
+
     it('returns 500 on unexpected error', async () => {
       const deps = createDeps({
         getRoleByName: jest.fn().mockResolvedValue(mockRole()),
diff --git a/packages/api/src/admin/roles.ts b/packages/api/src/admin/roles.ts
index 9efef1a586..7b22dde5e0 100644
--- a/packages/api/src/admin/roles.ts
+++ b/packages/api/src/admin/roles.ts
@@ -163,6 +163,9 @@ export function createAdminRolesHandlers(deps: AdminRolesDeps) {
 
       const role = await updateRoleByName(name, updates);
       if (!role) {
+        if (isRename) {
+          await updateUsersByRole(trimmedName, name);
+        }
         return res.status(404).json({ error: 'Role not found' });
       }