diff --git a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.test.ts b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.test.ts index bfc61757d35..d3322367586 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.test.ts +++ b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.test.ts @@ -206,28 +206,36 @@ describe('useRoleMappingRules', () => { }); describe('save', () => { - it('should send creates without an order field', async () => { + it('should send the local order on creates so user-intended position is preserved', async () => { + vi.mocked(roleMappingRuleApi.listRoleMappingRules).mockResolvedValue([ + makeRule({ id: 'persisted-1', type: 'instance', order: 0 }), + ]); vi.mocked(roleMappingRuleApi.createRoleMappingRule).mockResolvedValue(makeRule()); + vi.mocked(roleMappingRuleApi.updateRoleMappingRule).mockResolvedValue(makeRule()); + await composable.loadRules(); composable.addRule('instance'); - composable.updateRule(composable.instanceRules.value[0].id, { + const localId = composable.instanceRules.value[1].id; + composable.updateRule(localId, { expression: '$claims.admin', role: 'global:member', }); + // Drag the new local rule above the persisted one. + await composable.reorder('instance', 1, 0); await composable.save(); expect(roleMappingRuleApi.createRoleMappingRule).toHaveBeenCalledTimes(1); const [, payload] = vi.mocked(roleMappingRuleApi.createRoleMappingRule).mock.calls[0]; - expect(payload).not.toHaveProperty('order'); expect(payload).toMatchObject({ expression: '$claims.admin', role: 'global:member', type: 'instance', + order: 0, }); }); - it('should serialize create calls to preserve user-intended order', async () => { + it('should serialize create calls to avoid concurrent temp-order collisions', async () => { const callOrder: string[] = []; vi.mocked(roleMappingRuleApi.createRoleMappingRule).mockImplementation( async (_ctx, input) => { diff --git a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.ts b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.ts index f6493e5bce7..fdcb964672f 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.ts +++ b/packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.ts @@ -123,7 +123,7 @@ export function useRoleMappingRules() { const allLocalRules = [...instanceRules.value, ...projectRules.value]; const localRuleIds = new Set(allLocalRules.map((r) => r.id)); - const updatePayload = (r: RoleMappingRuleResponse) => ({ + const rulePayload = (r: RoleMappingRuleResponse) => ({ expression: r.expression, role: r.role, type: r.type, @@ -131,16 +131,6 @@ export function useRoleMappingRules() { projectIds: r.projectIds, }); - // New rules rely on the backend to append them in arrival order, so - // `order` is intentionally omitted — otherwise stale local values can - // conflict with the DB's unique (type, order) constraint. - const createPayload = (r: RoleMappingRuleResponse) => ({ - expression: r.expression, - role: r.role, - type: r.type, - projectIds: r.projectIds, - }); - const deleteIds = [...serverRuleIds].filter((id) => !localRuleIds.has(id)); const updateRules = allLocalRules.filter( (r) => !r.id.startsWith('local-') && serverRuleIds.has(r.id), @@ -148,14 +138,15 @@ export function useRoleMappingRules() { const createRules = allLocalRules.filter((r) => r.id.startsWith('local-')); // Deletes and updates can run concurrently. Creates must be sequential - // so the backend appends them in the user-intended order. + // because the backend reshuffles orders on each create, and race + // conditions between concurrent creates can collide on temp orders. await Promise.all([ ...deleteIds.map(async (id) => await api.deleteRule(id)), - ...updateRules.map(async (r) => await api.updateRule(r.id, updatePayload(r))), + ...updateRules.map(async (r) => await api.updateRule(r.id, rulePayload(r))), ]); for (const rule of createRules) { - await api.createRule(createPayload(rule)); + await api.createRule(rulePayload(rule)); } await loadRules();