Elgato_dark/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n synced 2026-04-21 15:47:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

fix(editor): Send order on create to preserve reorder of local rules

Browse source 
Omitting order on create broke reordering when a new local rule was
dragged above existing persisted rules — the backend would append
instead of respecting the user-intended position. The backend now
safely shifts existing rules on order collision, so the frontend can
always send the local order. Creates remain sequential to avoid
concurrent temp-order collisions.
This commit is contained in:
Csaba Tuncsik 2026-04-21 06:03:01 +02:00
parent 0988890bcf
commit 88c2f6830b
No known key found for this signature in database
2 changed files with 17 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.test.ts
View file

@ -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) => {

19
packages/frontend/editor-ui/src/features/settings/sso/provisioning/composables/useRoleMappingRules.ts
View file

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