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
This commit is contained in:
Dustin Healy 2026-03-26 16:12:33 -07:00
parent 94fdb3cd93
commit 16bb113614
2 changed files with 23 additions and 0 deletions

View file

@ -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()),

View file

@ -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' });
}