mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(core): Allow confirming destructive data table changes when continuing an apply (#40491)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan Kalkan
GitHub
parent
4c0902f6bf
commit
9946cfe901
|
|
@ -40,6 +40,11 @@ describe('ContinueApplyPackageDto', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('accepts a confirmation of destructive data table changes', () => {
|
||||
const body = { expectedSource, confirmDestructiveChanges: true };
|
||||
expect(ContinueApplyPackageDto.parse(body)).toEqual(body);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{},
|
||||
{ expectedSource: {} },
|
||||
|
|
@ -82,6 +87,7 @@ describe('ApplyPackageDto', () => {
|
|||
{ force: true },
|
||||
{ expectedSource: {} },
|
||||
{ expectedSource: { ...expectedSource, commitSha: 'HEAD' } },
|
||||
{ expectedSource, confirmDestructiveChanges: true },
|
||||
])('rejects unsupported or incomplete fields: %j', (body) => {
|
||||
expect(ApplyPackageDto.safeParse(body).success).toBe(false);
|
||||
});
|
||||
|
|
@ -142,12 +148,28 @@ describe('ApplySelectionDto', () => {
|
|||
const workflowIds = ['workflow1'];
|
||||
expect(ApplySelectionDto.parse({ workflowIds })).toEqual({ workflowIds });
|
||||
});
|
||||
|
||||
it('rejects a confirmation of destructive data table changes', () => {
|
||||
expect(
|
||||
ApplySelectionDto.safeParse({ workflowIds: ['workflow1'], confirmDestructiveChanges: true })
|
||||
.success,
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ContinueApplySelectionDto', () => {
|
||||
it('requires the reviewed source', () => {
|
||||
expect(ContinueApplySelectionDto.safeParse({ workflowIds: ['workflow1'] }).success).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts a confirmation of destructive data table changes', () => {
|
||||
const body = {
|
||||
workflowIds: ['workflow1'],
|
||||
expectedSource: { configId: 'config1', branchName: 'main', commitSha: 'a'.repeat(40) },
|
||||
confirmDestructiveChanges: true,
|
||||
};
|
||||
expect(ContinueApplySelectionDto.parse(body)).toEqual(body);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ApplyPackageResultDto', () => {
|
||||
|
|
|
|||
|
|
@ -128,7 +128,10 @@ const dataTableChangeSchema = z.discriminatedUnion('kind', [
|
|||
}),
|
||||
]);
|
||||
|
||||
/** Conflicts need a package or target correction before creation can proceed. */
|
||||
/**
|
||||
* Continue can confirm a `destructive-change` conflict.
|
||||
* The other conflicts need a package or target correction before creation can proceed.
|
||||
*/
|
||||
export const promotionBindingConflictSchema = z.discriminatedUnion('code', [
|
||||
credentialConflictSchema.extend({ code: z.literal('missing-id') }),
|
||||
credentialConflictSchema.extend({ code: z.literal('conflicting-types') }),
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import '../../openapi-extend';
|
||||
|
||||
import { z } from 'zod';
|
||||
|
||||
import { n8nIdSchema } from '../../schemas/id.schema';
|
||||
|
|
@ -98,9 +100,23 @@ export class ApplyPackageDto extends Z.class(
|
|||
{ strict: true },
|
||||
) {}
|
||||
|
||||
const confirmDestructiveChangesSchema = z
|
||||
.boolean()
|
||||
.optional()
|
||||
.openapi({
|
||||
description:
|
||||
'Also applies destructive data table changes, which a `destructive-change` conflict reports: ' +
|
||||
'removed, renamed, or retyped columns. The values in those columns are removed. ' +
|
||||
'Rows and other columns are kept. The confirmation covers every such change when Continue runs, ' +
|
||||
'including changes that are not in the reviewed result.',
|
||||
});
|
||||
|
||||
/** Continue must name the source that the paused Apply reported. */
|
||||
export class ContinueApplyPackageDto extends Z.class(
|
||||
{ expectedSource: expectedSourceSchema },
|
||||
{
|
||||
expectedSource: expectedSourceSchema,
|
||||
confirmDestructiveChanges: confirmDestructiveChangesSchema,
|
||||
},
|
||||
{ strict: true },
|
||||
) {}
|
||||
|
||||
|
|
@ -124,6 +140,7 @@ export class ContinueApplySelectionDto extends Z.class(
|
|||
{
|
||||
workflowIds: selectedWorkflowIdsSchema,
|
||||
expectedSource: expectedSourceSchema,
|
||||
confirmDestructiveChanges: confirmDestructiveChangesSchema,
|
||||
},
|
||||
{ strict: true },
|
||||
) {}
|
||||
|
|
|
|||
|
|
@ -294,7 +294,7 @@ none.
|
|||
|--------|-----------|---------------|
|
||||
| `applied` | `0` | The package was imported. JSON output includes `counts` and `warnings`. |
|
||||
| `source-changed` | `3` | The configuration, branch, or commit is not the one you reviewed. Nothing was imported. Review the changes again. |
|
||||
| `blocked` | `4` | Preflight found missing bindings, access requirements, or conflicts. Nothing was imported. The output shows the `apply-continue` command to run after you resolve them. |
|
||||
| `blocked` | `4` | Preflight found missing bindings, access requirements, or conflicts. A conflict can be a destructive data table change. Nothing was imported. The output shows the `apply-continue` command to run after you resolve them. |
|
||||
|
||||
With `--json`, the output is the full result for each status. For `blocked`, the
|
||||
`preflight` object lists each missing project, missing binding, access
|
||||
|
|
@ -319,6 +319,12 @@ for `apply`: `blocked` again means that something still blocks, and
|
|||
`source-changed` means that you must review again. The server keeps no session,
|
||||
so run the command again when it exits `4`.
|
||||
|
||||
Add `--confirm-destructive-changes` to also apply destructive data table
|
||||
changes: removed, renamed, or retyped columns. The values in those columns are
|
||||
removed. Rows and other columns are kept. The flag applies all such changes when
|
||||
the command runs. This includes changes that the blocked Apply did not report.
|
||||
Without the flag, a destructive change blocks the command, and it exits `4`.
|
||||
|
||||
## Apply a reviewed commit
|
||||
|
||||
```bash
|
||||
|
|
@ -390,6 +396,12 @@ reported. The command runs the preflight again. The results and exit codes are
|
|||
the same as for `apply-selection`. The server keeps no session, so run the
|
||||
command again when it exits `4`.
|
||||
|
||||
Add `--confirm-destructive-changes` to also apply destructive data table
|
||||
changes: removed, renamed, or retyped columns. The values in those columns are
|
||||
removed. Rows and other columns are kept. The flag applies all such changes when
|
||||
the command runs. This includes changes that the blocked Apply did not report.
|
||||
Without the flag, a destructive change blocks the command, and it exits `4`.
|
||||
|
||||
## `promotion-connection list-projects` / `add-project` / `remove-project`
|
||||
|
||||
Link team projects to a `projects`-scoped connection.
|
||||
|
|
|
|||
|
|
@ -234,6 +234,7 @@ n8n-cli promotion-connection apply <id> \
|
|||
# then run the apply-continue command that apply printed:
|
||||
n8n-cli promotion-connection apply-continue <id> \
|
||||
--expected-config-id=<configId> --expected-branch=<branch> --expected-commit-sha=<full sha>
|
||||
# To also apply destructive data table changes, add --confirm-destructive-changes.
|
||||
```
|
||||
|
||||
Connection JSON for step 2:
|
||||
|
|
|
|||
|
|
@ -103,6 +103,12 @@ describe('N8nClient packages', () => {
|
|||
path: 'apply/continue',
|
||||
body: { expectedSource },
|
||||
},
|
||||
{
|
||||
title: 'posts continue with the confirmation of destructive data table changes',
|
||||
send: async () => await client.continueApplyPackage('conn-1', expectedSource, true),
|
||||
path: 'apply/continue',
|
||||
body: { expectedSource, confirmDestructiveChanges: true },
|
||||
},
|
||||
])('$title', async ({ send, path, body }) => {
|
||||
fetchMock.mockResolvedValue(jsonResponse(200, { status: 'applied' }));
|
||||
|
||||
|
|
@ -135,6 +141,14 @@ describe('N8nClient packages', () => {
|
|||
path: 'apply/continue',
|
||||
body: { workflowIds: ['wf-1'], expectedSource },
|
||||
},
|
||||
{
|
||||
title:
|
||||
'continues a selection apply with the confirmation of destructive data table changes',
|
||||
send: async () =>
|
||||
await client.continueApplyProjectSelection('proj-1', ['wf-1'], expectedSource, true),
|
||||
path: 'apply/continue',
|
||||
body: { workflowIds: ['wf-1'], expectedSource, confirmDestructiveChanges: true },
|
||||
},
|
||||
])('$title', async ({ send, path, body }) => {
|
||||
fetchMock.mockResolvedValue(jsonResponse(200, { status: 'source-changed' }));
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { Config } from '@oclif/core';
|
||||
import { Config } from '@oclif/core';
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
|
||||
import type { N8nClient } from '../client';
|
||||
|
|
@ -197,6 +197,26 @@ describe('promotion-connection apply command', () => {
|
|||
expect(message).toContain(
|
||||
`n8n-cli promotion-connection apply-continue conn-1 --expected-config-id=cfg-2 --expected-branch=release --expected-commit-sha=${SHA}`,
|
||||
);
|
||||
expect(message).not.toContain('--confirm-destructive-changes');
|
||||
expect(exit).toHaveBeenCalledWith(4);
|
||||
});
|
||||
|
||||
it('exits 4 and tells how to confirm a destructive data table change', async () => {
|
||||
const { run, logToStderr, exit } = runApply({
|
||||
...BLOCKED_RESULT,
|
||||
preflight: {
|
||||
...BLOCKED_RESULT.preflight,
|
||||
conflicts: [{ kind: 'data-table', code: 'destructive-change' }],
|
||||
},
|
||||
});
|
||||
|
||||
await run();
|
||||
|
||||
const message = logToStderr.mock.calls[0][0];
|
||||
expect(message).toContain(
|
||||
`n8n-cli promotion-connection apply-continue conn-1 --expected-config-id=cfg-2 --expected-branch=release --expected-commit-sha=${SHA}\n` +
|
||||
'To also apply data table changes that remove, rename, or retype columns, add --confirm-destructive-changes. The values in those columns are removed.',
|
||||
);
|
||||
expect(exit).toHaveBeenCalledWith(4);
|
||||
});
|
||||
|
||||
|
|
@ -257,9 +277,33 @@ describe('promotion-connection apply-continue command', () => {
|
|||
|
||||
await command.run();
|
||||
|
||||
expect(continueApplyPackage).toHaveBeenCalledWith('conn-1', EXPECTED_SOURCE);
|
||||
expect(continueApplyPackage).toHaveBeenCalledWith('conn-1', EXPECTED_SOURCE, undefined);
|
||||
expect(exit).toHaveBeenCalledWith(3);
|
||||
});
|
||||
|
||||
it('sends the confirmation of destructive data table changes', async () => {
|
||||
const command = new PromotionConnectionApplyContinue(
|
||||
[
|
||||
'conn-1',
|
||||
`--expected-config-id=${EXPECTED_SOURCE.configId}`,
|
||||
`--expected-branch=${EXPECTED_SOURCE.branchName}`,
|
||||
`--expected-commit-sha=${SHA}`,
|
||||
'--confirm-destructive-changes',
|
||||
'--format=table',
|
||||
],
|
||||
await Config.load(process.cwd()),
|
||||
);
|
||||
const internals = command as unknown as ApplyInternals;
|
||||
const continueApplyPackage = vi.fn().mockResolvedValue(APPLY_RESULT);
|
||||
vi.spyOn(internals, 'getClient').mockReturnValue({
|
||||
continueApplyPackage,
|
||||
} as unknown as N8nClient);
|
||||
vi.spyOn(internals, 'succeed').mockImplementation(() => {});
|
||||
|
||||
await command.run();
|
||||
|
||||
expect(continueApplyPackage).toHaveBeenCalledWith('conn-1', EXPECTED_SOURCE, true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('expected config ID and branch flags', () => {
|
||||
|
|
@ -470,7 +514,43 @@ describe('promotion-connection apply-selection-continue command', () => {
|
|||
|
||||
await command.run();
|
||||
|
||||
expect(continueApplyProjectSelection).toHaveBeenCalledWith('proj-1', ['wf-1'], EXPECTED_SOURCE);
|
||||
expect(continueApplyProjectSelection).toHaveBeenCalledWith(
|
||||
'proj-1',
|
||||
['wf-1'],
|
||||
EXPECTED_SOURCE,
|
||||
undefined,
|
||||
);
|
||||
expect(exit).toHaveBeenCalledWith(3);
|
||||
});
|
||||
|
||||
it('sends the confirmation of destructive data table changes', async () => {
|
||||
const command = new PromotionConnectionApplySelectionContinue(
|
||||
[
|
||||
'proj-1',
|
||||
'-w',
|
||||
'wf-1',
|
||||
`--expected-config-id=${EXPECTED_SOURCE.configId}`,
|
||||
`--expected-branch=${EXPECTED_SOURCE.branchName}`,
|
||||
`--expected-commit-sha=${SHA}`,
|
||||
'--confirm-destructive-changes',
|
||||
'--format=table',
|
||||
],
|
||||
await Config.load(process.cwd()),
|
||||
);
|
||||
const internals = command as unknown as ApplyInternals;
|
||||
const continueApplyProjectSelection = vi.fn().mockResolvedValue(APPLY_RESULT);
|
||||
vi.spyOn(internals, 'getClient').mockReturnValue({
|
||||
continueApplyProjectSelection,
|
||||
} as unknown as N8nClient);
|
||||
vi.spyOn(internals, 'succeed').mockImplementation(() => {});
|
||||
|
||||
await command.run();
|
||||
|
||||
expect(continueApplyProjectSelection).toHaveBeenCalledWith(
|
||||
'proj-1',
|
||||
['wf-1'],
|
||||
EXPECTED_SOURCE,
|
||||
true,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -440,9 +440,14 @@ export class N8nClient {
|
|||
);
|
||||
}
|
||||
|
||||
async continueApplyPackage(id: string, expectedSource: PromotionExpectedSource) {
|
||||
async continueApplyPackage(
|
||||
id: string,
|
||||
expectedSource: PromotionExpectedSource,
|
||||
confirmDestructiveChanges?: boolean,
|
||||
) {
|
||||
return await this.post<ApplyPackageResult>(`/promotions/connections/${id}/apply/continue`, {
|
||||
expectedSource,
|
||||
confirmDestructiveChanges,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -481,10 +486,12 @@ export class N8nClient {
|
|||
projectId: string,
|
||||
workflowIds: string[],
|
||||
expectedSource: PromotionExpectedSource,
|
||||
confirmDestructiveChanges?: boolean,
|
||||
) {
|
||||
return await this.post<ApplyPackageResult>(`/promotions/projects/${projectId}/apply/continue`, {
|
||||
workflowIds,
|
||||
expectedSource,
|
||||
confirmDestructiveChanges,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -75,6 +75,11 @@ function blockedMessage(
|
|||
` Conflicts: ${preflight.conflicts.length}`,
|
||||
'Run with --json for the details. After you resolve them, run:',
|
||||
` ${continueCommand(result)}`,
|
||||
...(preflight.conflicts.some(({ code }) => code === 'destructive-change')
|
||||
? [
|
||||
'To also apply data table changes that remove, rename, or retype columns, add --confirm-destructive-changes. The values in those columns are removed.',
|
||||
]
|
||||
: []),
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Args } from '@oclif/core';
|
||||
import { Args, Flags } from '@oclif/core';
|
||||
|
||||
import { PromotionApplyCommand, connectionContinueCommand } from './apply-command';
|
||||
import { expectedSourceFlags, toExpectedSource } from './expected-source';
|
||||
|
|
@ -6,7 +6,7 @@ import { BaseCommand } from '../../base-command';
|
|||
|
||||
export default class PromotionConnectionApplyContinue extends PromotionApplyCommand {
|
||||
static override description =
|
||||
'Continue an Apply that was blocked on bindings, after they are set up. Pass the config ID, branch, and commit SHA that the blocked Apply reported. Exits 3 when the source changed since then and 4 when bindings are still missing; nothing is imported in either case.';
|
||||
'Continue an Apply that was blocked on bindings, after they are set up. Pass the config ID, branch, and commit SHA that the blocked Apply reported. Exits 3 when the source changed since then and 4 when bindings are still missing or a destructive data table change is not confirmed with --confirm-destructive-changes; nothing is imported in either case.';
|
||||
|
||||
static override examples = [
|
||||
'<%= config.bin %> promotion-connection apply-continue conn-1 --expected-config-id=cfg-1 --expected-branch=main --expected-commit-sha=<full sha>',
|
||||
|
|
@ -17,6 +17,11 @@ export default class PromotionConnectionApplyContinue extends PromotionApplyComm
|
|||
static override flags = {
|
||||
...BaseCommand.baseFlags,
|
||||
...expectedSourceFlags({ required: true }),
|
||||
confirmDestructiveChanges: Flags.boolean({
|
||||
description:
|
||||
'Also apply data table changes that remove, rename, or retype columns. The values in those columns are removed.',
|
||||
aliases: ['confirm-destructive-changes'],
|
||||
}),
|
||||
};
|
||||
|
||||
async run() {
|
||||
|
|
@ -25,7 +30,11 @@ export default class PromotionConnectionApplyContinue extends PromotionApplyComm
|
|||
// oclif enforces the required flags, so this only narrows the type.
|
||||
if (!expectedSource) this.error('Pass all three --expected-* flags');
|
||||
await this.execute(async () => {
|
||||
const result = await this.getClient(flags).continueApplyPackage(args.id, expectedSource);
|
||||
const result = await this.getClient(flags).continueApplyPackage(
|
||||
args.id,
|
||||
expectedSource,
|
||||
flags.confirmDestructiveChanges,
|
||||
);
|
||||
this.reportApplyResult(result, flags, connectionContinueCommand(args.id));
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import { BaseCommand } from '../../base-command';
|
|||
|
||||
export default class PromotionConnectionApplySelectionContinue extends PromotionApplyCommand {
|
||||
static override description =
|
||||
'Continue a selection Apply that was blocked on bindings, after they are set up. Resend the same workflow ids, and pass the config ID, branch, and commit SHA that the blocked Apply reported. Exits 3 when the source changed since then and 4 when bindings are still missing; nothing is imported in either case.';
|
||||
'Continue a selection Apply that was blocked on bindings, after they are set up. Resend the same workflow ids, and pass the config ID, branch, and commit SHA that the blocked Apply reported. Exits 3 when the source changed since then and 4 when bindings are still missing or a destructive data table change is not confirmed with --confirm-destructive-changes; nothing is imported in either case.';
|
||||
|
||||
static override examples = [
|
||||
'<%= config.bin %> promotion-connection apply-selection-continue proj-abc -w wf-1 --expected-config-id=cfg-1 --expected-branch=main --expected-commit-sha=<full sha>',
|
||||
|
|
@ -23,6 +23,11 @@ export default class PromotionConnectionApplySelectionContinue extends Promotion
|
|||
required: true,
|
||||
}),
|
||||
...expectedSourceFlags({ required: true }),
|
||||
confirmDestructiveChanges: Flags.boolean({
|
||||
description:
|
||||
'Also apply data table changes that remove, rename, or retype columns. The values in those columns are removed.',
|
||||
aliases: ['confirm-destructive-changes'],
|
||||
}),
|
||||
};
|
||||
|
||||
async run() {
|
||||
|
|
@ -35,6 +40,7 @@ export default class PromotionConnectionApplySelectionContinue extends Promotion
|
|||
args.projectId,
|
||||
flags.workflow,
|
||||
expectedSource,
|
||||
flags.confirmDestructiveChanges,
|
||||
);
|
||||
this.reportApplyResult(
|
||||
result,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,8 @@
|
|||
import { applyPackageResultSchema } from '@n8n/api-types';
|
||||
import {
|
||||
applyPackageResultSchema,
|
||||
type ApplyPackageResultDto,
|
||||
type ContinueApplyPackageDto,
|
||||
} from '@n8n/api-types';
|
||||
import { LicenseState } from '@n8n/backend-common';
|
||||
import {
|
||||
createTeamProject,
|
||||
|
|
@ -1454,6 +1458,15 @@ describe('Apply a project selection', () => {
|
|||
|
||||
expect(result.status).toBe('applied');
|
||||
expect(await dataTableService.getColumns(orders.id, project.id)).toEqual(columnsBefore);
|
||||
|
||||
const confirmed = await service.continueApplyProjectSelection(project.id, owner, {
|
||||
workflowIds: [plainWorkflow.id],
|
||||
expectedSource: { configId: result.configId, ...result.git },
|
||||
confirmDestructiveChanges: true,
|
||||
});
|
||||
|
||||
expect(confirmed.status).toBe('applied');
|
||||
expect(await dataTableService.getColumns(orders.id, project.id)).toEqual(columnsBefore);
|
||||
});
|
||||
|
||||
it('returns source-changed for a stale commit without importing workflows', async () => {
|
||||
|
|
@ -1730,6 +1743,62 @@ describe('Apply data table changes', () => {
|
|||
},
|
||||
);
|
||||
|
||||
it.each([{ flow: 'full' as const }, { flow: 'selection' as const }])(
|
||||
'applies a $flow column removal only after Continue confirms it and keeps the rows',
|
||||
async ({ flow }) => {
|
||||
const promoted = await promoteDataTableWorkflows();
|
||||
const { connection, project, workflow, orders, dataTableService } = promoted;
|
||||
await dataTableService.addColumn(orders.id, project.id, { name: 'extra', type: 'string' });
|
||||
await dataTableService.insertRows(orders.id, project.id, [
|
||||
{ email: 'a@example.com', extra: 'keep me' },
|
||||
]);
|
||||
await dataTableService.updateDataTable(orders.id, project.id, { name: 'Local orders' });
|
||||
const continueFlow = async (
|
||||
expectedSource: ContinueApplyPackageDto['expectedSource'],
|
||||
confirmDestructiveChanges?: boolean,
|
||||
): Promise<ApplyPackageResultDto> =>
|
||||
flow === 'full'
|
||||
? (
|
||||
await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.post(`/promotions/connections/${connection.id}/apply/continue`)
|
||||
.send({ expectedSource, confirmDestructiveChanges })
|
||||
.expect(200)
|
||||
).body
|
||||
: await service.continueApplyProjectSelection(project.id, owner, {
|
||||
workflowIds: [workflow.id],
|
||||
expectedSource,
|
||||
confirmDestructiveChanges,
|
||||
});
|
||||
|
||||
const blocked = await applyFlow(flow, promoted);
|
||||
assert(blocked.status === 'blocked');
|
||||
const expectedSource = { configId: blocked.configId, ...blocked.git };
|
||||
|
||||
expect(await continueFlow(expectedSource)).toEqual(blocked);
|
||||
const { data: rowsBefore } = await dataTableService.getManyRowsAndCount(
|
||||
orders.id,
|
||||
project.id,
|
||||
{},
|
||||
);
|
||||
expect(rowsBefore).toEqual([expect.objectContaining({ extra: 'keep me' })]);
|
||||
|
||||
const result = await continueFlow(expectedSource, true);
|
||||
|
||||
assert(result.status === 'applied', JSON.stringify(result));
|
||||
expect(result.counts.dataTables.updated).toBe(1);
|
||||
expect(await dataTableService.getOne(orders.id, project.id)).toMatchObject({
|
||||
name: 'Orders',
|
||||
});
|
||||
expect(
|
||||
(await dataTableService.getColumns(orders.id, project.id)).map(({ name }) => name),
|
||||
).toEqual(['email', 'note']);
|
||||
const { data } = await dataTableService.getManyRowsAndCount(orders.id, project.id, {});
|
||||
expect(data).toEqual([expect.objectContaining({ email: 'a@example.com' })]);
|
||||
expect(data[0]).not.toHaveProperty('extra');
|
||||
},
|
||||
);
|
||||
|
||||
it.each([{ flow: 'full' as const }, { flow: 'selection' as const }])(
|
||||
'applies a $flow data table change that removes no column values, counts the table as updated and recreates a missing table',
|
||||
async ({ flow }) => {
|
||||
|
|
|
|||
|
|
@ -77,7 +77,8 @@ import { WorkingCopyUpdater, type SelectivePushOptions } from './working-copy-up
|
|||
|
||||
type ProjectReconciliationResult = { deletedProjectIds: string[] };
|
||||
|
||||
// Apply treats the package as source of truth; callers cannot override this policy.
|
||||
// Apply treats the package as source of truth. Callers cannot override this policy,
|
||||
// but a confirmed Continue uses `overwrite` for data tables.
|
||||
const IMPORT_POLICY: Omit<ImportRequest, 'user'> = {
|
||||
projectConflictPolicy: ProjectConflictPolicy.Overwrite,
|
||||
workflowConflictPolicy: WorkflowConflictPolicy.NewVersion,
|
||||
|
|
@ -457,7 +458,12 @@ export class PromotionsService {
|
|||
actor: User,
|
||||
request: ContinueApplyPackageDto,
|
||||
): Promise<ApplyPackageResultDto> {
|
||||
return await this.applyFromSource(connectionId, actor, request.expectedSource);
|
||||
return await this.applyFromSource(
|
||||
connectionId,
|
||||
actor,
|
||||
request.expectedSource,
|
||||
request.confirmDestructiveChanges === true,
|
||||
);
|
||||
}
|
||||
|
||||
async applyProjectSelection(
|
||||
|
|
@ -483,6 +489,7 @@ export class PromotionsService {
|
|||
actor,
|
||||
request.workflowIds,
|
||||
request.expectedSource,
|
||||
request.confirmDestructiveChanges === true,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -542,6 +549,7 @@ export class PromotionsService {
|
|||
actor: User,
|
||||
workflowIds: string[],
|
||||
expectedSource?: ApplySelectionDto['expectedSource'],
|
||||
confirmDestructiveChanges = false,
|
||||
): Promise<ApplyPackageResultDto> {
|
||||
const input = await this.resolver.resolveForProject(projectId, 'apply');
|
||||
await this.assertCheckoutReady(input, 'applying');
|
||||
|
|
@ -591,10 +599,13 @@ export class PromotionsService {
|
|||
selectedWorkflowIds: selection.selectedWorkflowIds,
|
||||
},
|
||||
});
|
||||
const blockingConflicts = confirmDestructiveChanges
|
||||
? preflight.conflicts.filter(({ code }) => code !== 'destructive-change')
|
||||
: preflight.conflicts;
|
||||
if (
|
||||
preflight.missingBindings.length > 0 ||
|
||||
preflight.accessRequirements.length > 0 ||
|
||||
preflight.conflicts.length > 0
|
||||
blockingConflicts.length > 0
|
||||
) {
|
||||
this.logger.info('Apply blocked by unresolved bindings', {
|
||||
status: 'blocked',
|
||||
|
|
@ -614,13 +625,16 @@ export class PromotionsService {
|
|||
projectId,
|
||||
selectedCount: selection.selectedWorkflowIds.length,
|
||||
deletedCount: selection.deletedWorkflowIds?.length ?? 0,
|
||||
confirmDestructiveChanges,
|
||||
});
|
||||
|
||||
const result = await this.n8nPackagesService.importPackageSelectionFromDirectory(
|
||||
{
|
||||
user: actor,
|
||||
overwriteDeletionPolicy: OverwriteDeletionPolicy.HardDelete,
|
||||
dataTableSchemaConflictPolicy: IMPORT_POLICY.dataTableSchemaConflictPolicy,
|
||||
dataTableSchemaConflictPolicy: confirmDestructiveChanges
|
||||
? DataTableSchemaConflictPolicy.Overwrite
|
||||
: IMPORT_POLICY.dataTableSchemaConflictPolicy,
|
||||
},
|
||||
{ sourceDir: packageFolder },
|
||||
selection,
|
||||
|
|
@ -641,6 +655,7 @@ export class PromotionsService {
|
|||
connectionId: string,
|
||||
actor: User,
|
||||
expectedSource?: ApplyPackageDto['expectedSource'],
|
||||
confirmDestructiveChanges = false,
|
||||
): Promise<ApplyPackageResultDto> {
|
||||
const input = await this.resolver.resolveForConnection(connectionId, 'apply');
|
||||
this.assertInstanceScope(input, 'Apply');
|
||||
|
|
@ -685,10 +700,13 @@ export class PromotionsService {
|
|||
const preflight = await this.bindingPreflight.checkDirectory({
|
||||
sourceDir: packageFolder,
|
||||
});
|
||||
const blockingConflicts = confirmDestructiveChanges
|
||||
? preflight.conflicts.filter(({ code }) => code !== 'destructive-change')
|
||||
: preflight.conflicts;
|
||||
if (
|
||||
preflight.missingBindings.length > 0 ||
|
||||
preflight.accessRequirements.length > 0 ||
|
||||
preflight.conflicts.length > 0
|
||||
blockingConflicts.length > 0
|
||||
) {
|
||||
this.logger.info('Apply blocked by unresolved bindings', {
|
||||
status: 'blocked',
|
||||
|
|
@ -703,10 +721,20 @@ export class PromotionsService {
|
|||
return { status: 'blocked', ...identity, preflight };
|
||||
}
|
||||
|
||||
this.logger.info('Importing a package', { connectionId, configId: input.configId });
|
||||
this.logger.info('Importing a package', {
|
||||
connectionId,
|
||||
configId: input.configId,
|
||||
confirmDestructiveChanges,
|
||||
});
|
||||
|
||||
const result = await this.n8nPackagesService.importPackageFromDirectory(
|
||||
{ user: actor, ...IMPORT_POLICY },
|
||||
{
|
||||
user: actor,
|
||||
...IMPORT_POLICY,
|
||||
dataTableSchemaConflictPolicy: confirmDestructiveChanges
|
||||
? DataTableSchemaConflictPolicy.Overwrite
|
||||
: IMPORT_POLICY.dataTableSchemaConflictPolicy,
|
||||
},
|
||||
{ sourceDir: packageFolder },
|
||||
);
|
||||
const importedProjectIds = result.projects.map((project) => project.localId);
|
||||
|
|
|
|||
|
|
@ -566,7 +566,7 @@ export class PromotionsPublicController {
|
|||
@GlobalScope('gitConnection:pull')
|
||||
@ApiSummary('Continue Apply after binding setup')
|
||||
@ApiDescription(
|
||||
'Rechecks the configured source and current target bindings. Send expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details without import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Existing target variable values are preserved. Requires the same cloned instance connection and permissions as Apply. The importer checks user write permissions; granular API-key write scopes are not passed to it. No server session or exactly-once guarantee is provided.',
|
||||
'Rechecks the configured source and current target bindings. Send expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. Send `confirmDestructiveChanges: true` to also apply destructive data table changes. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details without import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Existing target variable values are preserved. Requires the same cloned instance connection and permissions as Apply. The importer checks user write permissions; granular API-key write scopes are not passed to it. No server session or exactly-once guarantee is provided.',
|
||||
)
|
||||
@ApiTags(tags)
|
||||
@ApiResponse(200, ApplyPackageResultDto)
|
||||
|
|
@ -685,7 +685,7 @@ export class PromotionsPublicController {
|
|||
@GlobalScope('gitConnection:pull')
|
||||
@ApiSummary('Continue applying a selection after binding setup')
|
||||
@ApiDescription(
|
||||
"Rechecks the project's Apply source and the current bindings for the selection. Resend the full selection in workflowIds. Send the required expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. The server does not keep a session. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details before import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Unselected content stays unchanged. Requires the same cloned Apply direction and permissions as the initial selection apply. The API key needs the gitConnection:pull scope. The importer checks user write permissions.",
|
||||
"Rechecks the project's Apply source and the current bindings for the selection. Resend the full selection in workflowIds. Send the required expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. Send `confirmDestructiveChanges: true` to also apply destructive data table changes. The server does not keep a session. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details before import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Unselected content stays unchanged. Requires the same cloned Apply direction and permissions as the initial selection apply. The API key needs the gitConnection:pull scope. The importer checks user write permissions.",
|
||||
)
|
||||
@ApiTags(tags)
|
||||
@ApiResponse(200, ApplyPackageResultDto)
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ operationId: continueApplyPackage
|
|||
tags:
|
||||
- Promotions
|
||||
summary: Continue Apply after binding setup
|
||||
description: Rechecks the configured source and current target bindings. Send expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details without import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Existing target variable values are preserved. Requires the same cloned instance connection and permissions as Apply. The importer checks user write permissions; granular API-key write scopes are not passed to it. No server session or exactly-once guarantee is provided.
|
||||
description: 'Rechecks the configured source and current target bindings. Send expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. Send `confirmDestructiveChanges: true` to also apply destructive data table changes. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details without import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Existing target variable values are preserved. Requires the same cloned instance connection and permissions as Apply. The importer checks user write permissions; granular API-key write scopes are not passed to it. No server session or exactly-once guarantee is provided.'
|
||||
x-required-scope: gitConnection:pull
|
||||
x-eov-operation-id: unreachable
|
||||
x-eov-operation-handler: v1/handlers/decorator-routed.handler
|
||||
|
|
@ -39,6 +39,9 @@ requestBody:
|
|||
- branchName
|
||||
- commitSha
|
||||
additionalProperties: false
|
||||
confirmDestructiveChanges:
|
||||
type: boolean
|
||||
description: 'Also applies destructive data table changes, which a `destructive-change` conflict reports: removed, renamed, or retyped columns. The values in those columns are removed. Rows and other columns are kept. The confirmation covers every such change when Continue runs, including changes that are not in the reviewed result.'
|
||||
required:
|
||||
- expectedSource
|
||||
additionalProperties: false
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ operationId: continueApplyProjectSelection
|
|||
tags:
|
||||
- Promotions
|
||||
summary: Continue applying a selection after binding setup
|
||||
description: Rechecks the project's Apply source and the current bindings for the selection. Resend the full selection in workflowIds. Send the required expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. The server does not keep a session. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details before import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Unselected content stays unchanged. Requires the same cloned Apply direction and permissions as the initial selection apply. The API key needs the gitConnection:pull scope. The importer checks user write permissions.
|
||||
description: "Rechecks the project's Apply source and the current bindings for the selection. Resend the full selection in workflowIds. Send the required expectedSource with the configId, branchName, and full commitSha from the reviewed Apply result. Send `confirmDestructiveChanges: true` to also apply destructive data table changes. The server does not keep a session. Status `source-changed` requires a new Apply review. Status `blocked` returns fresh binding details before import writes. Status `applied` includes counts and warnings. Inspect status before reading counts. Unselected content stays unchanged. Requires the same cloned Apply direction and permissions as the initial selection apply. The API key needs the gitConnection:pull scope. The importer checks user write permissions."
|
||||
x-required-scope: gitConnection:pull
|
||||
x-eov-operation-id: unreachable
|
||||
x-eov-operation-handler: v1/handlers/decorator-routed.handler
|
||||
|
|
@ -46,6 +46,9 @@ requestBody:
|
|||
- branchName
|
||||
- commitSha
|
||||
additionalProperties: false
|
||||
confirmDestructiveChanges:
|
||||
type: boolean
|
||||
description: 'Also applies destructive data table changes, which a `destructive-change` conflict reports: removed, renamed, or retyped columns. The values in those columns are removed. Rows and other columns are kept. The confirmation covers every such change when Continue runs, including changes that are not in the reviewed result.'
|
||||
required:
|
||||
- workflowIds
|
||||
- expectedSource
|
||||
|
|
|
|||
|
|
@ -178,6 +178,11 @@ it('asks to confirm data deletion when only destructive changes block apply', as
|
|||
expect(apply).toBeEnabled();
|
||||
await userEvent.click(apply);
|
||||
expect(continueApplyPromotion).toHaveBeenCalledTimes(1);
|
||||
expect(continueApplyPromotion).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.any(String),
|
||||
expect.objectContaining({ confirmDestructiveChanges: true }),
|
||||
);
|
||||
expect(emitted('applied')).toEqual([[applied]]);
|
||||
});
|
||||
|
||||
|
|
@ -214,6 +219,11 @@ it('shows destructive changes above the bindings and continues with data deletio
|
|||
await userEvent.click(getByRole('button', { name: 'Apply data table changes' }));
|
||||
|
||||
expect(continueApplyPromotion).toHaveBeenCalledTimes(1);
|
||||
expect(continueApplyPromotion).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.any(String),
|
||||
expect.objectContaining({ confirmDestructiveChanges: true }),
|
||||
);
|
||||
expect(emitted('applied')).toBeUndefined();
|
||||
expect(getByRole('heading', { level: 2, name: 'Resolve bindings' })).toBeInTheDocument();
|
||||
expect(getByRole('heading', { name: 'Destructive changes' })).toBeInTheDocument();
|
||||
|
|
@ -281,6 +291,24 @@ it('forwards continueWith so Continue resumes the selection', async () => {
|
|||
expect(emitted('applied')).toEqual([[applied]]);
|
||||
});
|
||||
|
||||
it('confirms destructive changes when it resumes a selection', async () => {
|
||||
vi.mocked(continueApplyProjectSelection).mockResolvedValue(applied);
|
||||
const { getByRole } = await renderDialog({
|
||||
props: {
|
||||
open: true,
|
||||
blockedResult: blocked({ missingBindings: [], conflicts: [destructiveChange] }),
|
||||
createBinding: vi.fn(),
|
||||
continueWith: { kind: 'selection', projectId: 'team-a', workflowIds: ['wf-a'] },
|
||||
},
|
||||
});
|
||||
await userEvent.click(getByRole('button', { name: 'Apply data table changes' }));
|
||||
expect(continueApplyProjectSelection).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'team-a',
|
||||
expect.objectContaining({ confirmDestructiveChanges: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it.each(['Close', 'Close dialog', 'Back', 'Escape'] as const)(
|
||||
'keeps saved resources when dismissed with %s',
|
||||
async (action) => {
|
||||
|
|
|
|||
|
|
@ -220,6 +220,7 @@ export function usePromotionBindings() {
|
|||
isSubmitting.value = true;
|
||||
error.value = null;
|
||||
try {
|
||||
const confirmDestructiveChanges = destructiveChanges.value.length > 0 || undefined;
|
||||
const result =
|
||||
continueTarget.kind === 'selection'
|
||||
? await continueApplyProjectSelection(
|
||||
|
|
@ -228,10 +229,12 @@ export function usePromotionBindings() {
|
|||
{
|
||||
workflowIds: continueTarget.workflowIds,
|
||||
expectedSource: { ...expectedSource },
|
||||
confirmDestructiveChanges,
|
||||
},
|
||||
)
|
||||
: await continueApplyPromotion(rootStore.publicApiContext, connectionId, {
|
||||
expectedSource: { ...expectedSource },
|
||||
confirmDestructiveChanges,
|
||||
});
|
||||
if (currentSession !== session) return;
|
||||
if (result.status === 'blocked') reconcile(result);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user