From 9946cfe901b538ec5ca29e85cde3ec2044d33d00 Mon Sep 17 00:00:00 2001 From: Jan Kalkan Date: Thu, 8 Oct 2026 14:06:33 +0000 Subject: [PATCH] feat(core): Allow confirming destructive data table changes when continuing an apply (#40491) Co-authored-by: Claude Opus 5.5 --- .../promotion-operations.dto.test.ts | 22 +++++ .../promotion-binding-preflight.dto.ts | 5 +- .../promotions/promotion-operations.dto.ts | 19 +++- packages/@n8n/cli/docs/commands/promotions.md | 14 ++- packages/@n8n/cli/skills/n8n-cli/SKILL.md | 1 + .../@n8n/cli/src/__tests__/client.test.ts | 14 +++ .../promotion-connection-operations.test.ts | 86 ++++++++++++++++++- packages/@n8n/cli/src/client.ts | 9 +- .../promotion-connection/apply-command.ts | 5 ++ .../promotion-connection/apply-continue.ts | 15 +++- .../apply-selection-continue.ts | 8 +- .../promotions-package.integration.test.ts | 71 ++++++++++++++- .../promotions.ee/promotions.service.ts | 42 +++++++-- .../promotions.public.controller.ts | 4 +- .../paths/continueApplyPackage.generated.yml | 5 +- ...ontinueApplyProjectSelection.generated.yml | 5 +- .../PromotionBindingsDialog.test.ts | 28 ++++++ .../composables/usePromotionBindings.ts | 3 + 18 files changed, 333 insertions(+), 23 deletions(-) diff --git a/packages/@n8n/api-types/src/dto/promotions/__tests__/promotion-operations.dto.test.ts b/packages/@n8n/api-types/src/dto/promotions/__tests__/promotion-operations.dto.test.ts index 68a4c9b9c64..e2118adc767 100644 --- a/packages/@n8n/api-types/src/dto/promotions/__tests__/promotion-operations.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/promotions/__tests__/promotion-operations.dto.test.ts @@ -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', () => { diff --git a/packages/@n8n/api-types/src/dto/promotions/promotion-binding-preflight.dto.ts b/packages/@n8n/api-types/src/dto/promotions/promotion-binding-preflight.dto.ts index 17f8f03c601..da5a29a3e03 100644 --- a/packages/@n8n/api-types/src/dto/promotions/promotion-binding-preflight.dto.ts +++ b/packages/@n8n/api-types/src/dto/promotions/promotion-binding-preflight.dto.ts @@ -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') }), diff --git a/packages/@n8n/api-types/src/dto/promotions/promotion-operations.dto.ts b/packages/@n8n/api-types/src/dto/promotions/promotion-operations.dto.ts index b39f6729385..f9ac92ca779 100644 --- a/packages/@n8n/api-types/src/dto/promotions/promotion-operations.dto.ts +++ b/packages/@n8n/api-types/src/dto/promotions/promotion-operations.dto.ts @@ -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 }, ) {} diff --git a/packages/@n8n/cli/docs/commands/promotions.md b/packages/@n8n/cli/docs/commands/promotions.md index c67a10cee90..2a91f5d4f20 100644 --- a/packages/@n8n/cli/docs/commands/promotions.md +++ b/packages/@n8n/cli/docs/commands/promotions.md @@ -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. diff --git a/packages/@n8n/cli/skills/n8n-cli/SKILL.md b/packages/@n8n/cli/skills/n8n-cli/SKILL.md index 0b9b1f6bb94..5e04000567b 100644 --- a/packages/@n8n/cli/skills/n8n-cli/SKILL.md +++ b/packages/@n8n/cli/skills/n8n-cli/SKILL.md @@ -234,6 +234,7 @@ n8n-cli promotion-connection apply \ # then run the apply-continue command that apply printed: n8n-cli promotion-connection apply-continue \ --expected-config-id= --expected-branch= --expected-commit-sha= +# To also apply destructive data table changes, add --confirm-destructive-changes. ``` Connection JSON for step 2: diff --git a/packages/@n8n/cli/src/__tests__/client.test.ts b/packages/@n8n/cli/src/__tests__/client.test.ts index aa1afb43405..a61e8d22814 100644 --- a/packages/@n8n/cli/src/__tests__/client.test.ts +++ b/packages/@n8n/cli/src/__tests__/client.test.ts @@ -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' })); diff --git a/packages/@n8n/cli/src/__tests__/promotion-connection-operations.test.ts b/packages/@n8n/cli/src/__tests__/promotion-connection-operations.test.ts index 1241916b7bd..38ace4d3510 100644 --- a/packages/@n8n/cli/src/__tests__/promotion-connection-operations.test.ts +++ b/packages/@n8n/cli/src/__tests__/promotion-connection-operations.test.ts @@ -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, + ); + }); }); diff --git a/packages/@n8n/cli/src/client.ts b/packages/@n8n/cli/src/client.ts index e2fa434a97a..a880b39d58c 100644 --- a/packages/@n8n/cli/src/client.ts +++ b/packages/@n8n/cli/src/client.ts @@ -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(`/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(`/promotions/projects/${projectId}/apply/continue`, { workflowIds, expectedSource, + confirmDestructiveChanges, }); } diff --git a/packages/@n8n/cli/src/commands/promotion-connection/apply-command.ts b/packages/@n8n/cli/src/commands/promotion-connection/apply-command.ts index 26fbf1240f4..9413c357d8f 100644 --- a/packages/@n8n/cli/src/commands/promotion-connection/apply-command.ts +++ b/packages/@n8n/cli/src/commands/promotion-connection/apply-command.ts @@ -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'); } diff --git a/packages/@n8n/cli/src/commands/promotion-connection/apply-continue.ts b/packages/@n8n/cli/src/commands/promotion-connection/apply-continue.ts index 1a1370fc99a..bbcc9444ac9 100644 --- a/packages/@n8n/cli/src/commands/promotion-connection/apply-continue.ts +++ b/packages/@n8n/cli/src/commands/promotion-connection/apply-continue.ts @@ -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=', @@ -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)); }); } diff --git a/packages/@n8n/cli/src/commands/promotion-connection/apply-selection-continue.ts b/packages/@n8n/cli/src/commands/promotion-connection/apply-selection-continue.ts index 4dc7cba6b68..23ef8ee7fcb 100644 --- a/packages/@n8n/cli/src/commands/promotion-connection/apply-selection-continue.ts +++ b/packages/@n8n/cli/src/commands/promotion-connection/apply-selection-continue.ts @@ -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=', @@ -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, diff --git a/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts b/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts index b7873f21abf..6018fa716f9 100644 --- a/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts +++ b/packages/cli/src/modules/promotions.ee/__tests__/promotions-package.integration.test.ts @@ -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 => + 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 }) => { diff --git a/packages/cli/src/modules/promotions.ee/promotions.service.ts b/packages/cli/src/modules/promotions.ee/promotions.service.ts index 0bd545e071e..b5f6af02158 100644 --- a/packages/cli/src/modules/promotions.ee/promotions.service.ts +++ b/packages/cli/src/modules/promotions.ee/promotions.service.ts @@ -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 = { projectConflictPolicy: ProjectConflictPolicy.Overwrite, workflowConflictPolicy: WorkflowConflictPolicy.NewVersion, @@ -457,7 +458,12 @@ export class PromotionsService { actor: User, request: ContinueApplyPackageDto, ): Promise { - 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 { 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 { 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); diff --git a/packages/cli/src/public-api/v1/controllers/promotions.public.controller.ts b/packages/cli/src/public-api/v1/controllers/promotions.public.controller.ts index 8f9f0f779b9..3ed76221f5c 100644 --- a/packages/cli/src/public-api/v1/controllers/promotions.public.controller.ts +++ b/packages/cli/src/public-api/v1/controllers/promotions.public.controller.ts @@ -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) diff --git a/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyPackage.generated.yml b/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyPackage.generated.yml index 7655e3ca27b..5e8a25bf749 100644 --- a/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyPackage.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyPackage.generated.yml @@ -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 diff --git a/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyProjectSelection.generated.yml b/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyProjectSelection.generated.yml index e8719bde00b..a3421e1ecca 100644 --- a/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyProjectSelection.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/promotions/spec/paths/continueApplyProjectSelection.generated.yml @@ -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 diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/components/PromotionBindingsDialog.test.ts b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/components/PromotionBindingsDialog.test.ts index 55684969827..82717af83f8 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/components/PromotionBindingsDialog.test.ts +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/components/PromotionBindingsDialog.test.ts @@ -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) => { diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionBindings.ts b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionBindings.ts index cd4e1158be3..5717afa859d 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionBindings.ts +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionBindings.ts @@ -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);