From 281ce2cc011cab8535e194b3f6bab58cd932d654 Mon Sep 17 00:00:00 2001 From: Jan Kalkan Date: Fri, 23 Oct 2026 12:25:21 +0000 Subject: [PATCH 01/19] fix(API): Accept expressions in number, boolean, and options credential fields (#41832) Co-authored-by: Claude Opus 5.5 --- .../dto/credentials/credential-public.dto.ts | 6 ++++- packages/cli/src/public-api/types.ts | 4 ++- .../__tests__/credentials.utils.test.ts | 26 +++++++++++++++++++ .../handlers/credentials/credentials.utils.ts | 8 ++++++ .../spec/paths/createCredential.generated.yml | 1 + .../spec/paths/updateCredential.generated.yml | 2 +- .../public-api/credentials.test.ts | 11 ++++++++ 7 files changed, 55 insertions(+), 3 deletions(-) diff --git a/packages/@n8n/api-types/src/dto/credentials/credential-public.dto.ts b/packages/@n8n/api-types/src/dto/credentials/credential-public.dto.ts index 3f0f03f2e5b..aab85e1ce34 100644 --- a/packages/@n8n/api-types/src/dto/credentials/credential-public.dto.ts +++ b/packages/@n8n/api-types/src/dto/credentials/credential-public.dto.ts @@ -63,6 +63,8 @@ export class CreateCredentialPublicDto extends Z.class({ data: z.record(z.string(), z.unknown()).openapi({ writeOnly: true, example: { accessToken: 'ada612vad6fa5df4adf5a5dsf4389adsf76da7s' }, + description: + 'The credential data. Fields also accept n8n expressions, which are strings that start with "=".', }), isResolvable: z .boolean() @@ -106,7 +108,9 @@ export class UpdateCredentialPublicDto extends Z.class({ .openapi({ writeOnly: true, example: { accessToken: 'new_token_value' }, - description: 'The credential data. Required when changing credential type.', + description: + 'The credential data. Required when changing credential type. ' + + 'Fields also accept n8n expressions, which are strings that start with "=".', }), isGlobal: z.boolean().optional().openapi({ example: false, diff --git a/packages/cli/src/public-api/types.ts b/packages/cli/src/public-api/types.ts index 4a8696b8ddb..47963a5ea30 100644 --- a/packages/cli/src/public-api/types.ts +++ b/packages/cli/src/public-api/types.ts @@ -1,3 +1,5 @@ +import type { Schema } from 'jsonschema'; + type PaginationBase = { limit: number }; export type PaginationOffsetDecoded = PaginationBase & { offset: number }; @@ -19,7 +21,7 @@ export interface IDependency { export interface IJsonSchema { additionalProperties: false; type: 'object'; - properties: { [key: string]: { type: string } }; + properties: { [key: string]: Schema }; allOf?: IDependency[]; required: string[]; } diff --git a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts index da973917ae1..a3aa6720337 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts @@ -156,6 +156,32 @@ describe('credentials.utils', () => { ); }); + describe('typed fields', () => { + const credentialsHelper = credentialsHelperWithProperties([ + { name: 'port', type: 'number', displayName: 'Port', default: 21 }, + { name: 'secure', type: 'boolean', displayName: 'Secure', default: false }, + ...CONDITIONAL_PROPERTIES, + ]); + + it.each([ + ['number', { port: '={{ $vars.PORT }}' }], + ['boolean', { secure: '={{ $vars.SECURE }}' }], + ['options', { authType: '={{ $vars.AUTH_TYPE }}' }], + ])('accepts an expression in a %s field', (_, data) => { + expect(() => validateCredentialData(credentialsHelper, 'someType', data)).not.toThrow(); + }); + + it.each([ + ['number', { port: '21' }], + ['boolean', { secure: 'true' }], + ['options', { authType: 'other' }], + ])('rejects a non-expression value of the wrong type in a %s field', (_, data) => { + expect(() => validateCredentialData(credentialsHelper, 'someType', data)).toThrow( + /request\.body\.data/, + ); + }); + }); + it('enforces conditionally-required fields when partialData is not set', () => { const credentialsHelper = credentialsHelperWithProperties(CONDITIONAL_PROPERTIES); diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts index ef24cad284c..1dd7645270a 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts @@ -5,6 +5,7 @@ import { type DisplayCondition, type INodeProperties, type INodePropertyOptions, + isExpression, } from 'n8n-workflow'; import type { CredentialsHelper } from '@/credentials-helper'; @@ -72,6 +73,13 @@ export function validateCredentialData( delete schema.allOf; } + // An expression resolves at runtime, so validate it only as a string. + for (const [name, value] of Object.entries(data)) { + if (isExpression(value) && Object.hasOwn(schema.properties, name)) { + schema.properties[name] = { type: 'string' }; + } + } + const { valid, errors } = validate(data, schema, { nestedErrors: true }); if (!valid) { diff --git a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/createCredential.generated.yml b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/createCredential.generated.yml index 26524e5949f..5667f474540 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/createCredential.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/createCredential.generated.yml @@ -35,6 +35,7 @@ requestBody: data: type: object additionalProperties: {} + description: The credential data. Fields also accept n8n expressions, which are strings that start with "=". writeOnly: true example: accessToken: ada612vad6fa5df4adf5a5dsf4389adsf76da7s diff --git a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/updateCredential.generated.yml b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/updateCredential.generated.yml index e53558daeaa..9a7fe1482af 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/updateCredential.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/updateCredential.generated.yml @@ -35,7 +35,7 @@ requestBody: data: type: object additionalProperties: {} - description: The credential data. Required when changing credential type. + description: The credential data. Required when changing credential type. Fields also accept n8n expressions, which are strings that start with "=". writeOnly: true example: accessToken: new_token_value diff --git a/packages/cli/test/integration/public-api/credentials.test.ts b/packages/cli/test/integration/public-api/credentials.test.ts index 137981335c6..ba62bedf9e5 100644 --- a/packages/cli/test/integration/public-api/credentials.test.ts +++ b/packages/cli/test/integration/public-api/credentials.test.ts @@ -148,6 +148,17 @@ describe('POST /credentials', () => { } }); + test('should accept an expression in a number field', async () => { + const data = { host: 'ftp.example.com', port: '={{ $vars.PORT }}' }; + + const response = await authOwnerAgent + .post('/credentials') + .send({ name: 'ftp credential', type: 'ftp', data }); + + expect(response.statusCode).toBe(200); + expect(await getDecryptedCredentialData(response.body.id)).toEqual(data); + }); + test('should fail with an unknown credential type', async () => { const response = await authOwnerAgent.post('/credentials').send({ name: 'test credential', From ff95b93143e361295a587233710d6f2c1490ad71 Mon Sep 17 00:00:00 2001 From: Rob Hough Date: Fri, 23 Oct 2026 12:41:05 +0000 Subject: [PATCH 02/19] fix(editor): Use consistent confirmation modals in Agents (#41922) Co-authored-by: Bob Hough <15361982+heymynameisrob@users.noreply.github.com> Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com> --- .../AgentRegistryMcpConfigForm.test.ts | 16 +++++++---- .../AgentSessionTimelineView.spec.ts | 2 ++ .../__tests__/AgentSessionsListView.spec.ts | 8 ------ .../__tests__/useAgentBuilderSession.test.ts | 25 ++++++++++------- .../teams/AgentChannelTeamsEditView.test.ts | 8 +++--- .../AgentChannelTeamsManagedSetup.test.ts | 8 +++--- .../teams/AgentChannelTeamsSetup.test.ts | 8 +++--- .../channels/teams/useTeamsSetupRestart.ts | 19 ++++++------- .../features/agents/components/AgentCard.vue | 1 + .../components/AgentConfirmationModal.vue | 8 +++++- .../components/AgentRegistryMcpConfigForm.vue | 27 +++++++++---------- .../composables/useAgentBuilderSession.ts | 20 +++++++------- .../agents/views/AgentBuilderView.vue | 1 + .../agents/views/AgentSessionsListView.vue | 20 +++++++------- 14 files changed, 91 insertions(+), 80 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentRegistryMcpConfigForm.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentRegistryMcpConfigForm.test.ts index b7aa5574522..3116e78981f 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentRegistryMcpConfigForm.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentRegistryMcpConfigForm.test.ts @@ -1,4 +1,6 @@ import { defineComponent, ref } from 'vue'; +import { createPinia, setActivePinia } from 'pinia'; +import { useUIStore } from '@/app/stores/ui.store'; import { mount } from '@vue/test-utils'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { @@ -22,10 +24,6 @@ vi.mock('../composables/useAgentRegistryMcpConfig', () => ({ useAgentRegistryMcpConfig: useAgentRegistryMcpConfigMock, })); -vi.mock('@/app/composables/useMessage', () => ({ - useMessage: () => ({ confirm: confirmRemove }), -})); - vi.mock('@n8n/i18n', () => { const i18n = { baseText: baseTextMock }; return { useI18n: () => i18n }; @@ -163,6 +161,13 @@ async function remove(wrapper: ReturnType): Promise { describe('AgentRegistryMcpConfigForm', () => { beforeEach(() => { vi.clearAllMocks(); + setActivePinia(createPinia()); + vi.spyOn(useUIStore(), 'openModalWithData').mockImplementation(({ data }) => { + void confirmRemove(data).then((result: string) => { + const onResult = result === MODAL_CONFIRM ? data.onConfirm : data.onCancel; + if (typeof onResult === 'function') onResult(); + }); + }); saveMock.mockReturnValue(true); confirmRemove.mockResolvedValue(MODAL_CANCEL); }); @@ -233,8 +238,9 @@ describe('AgentRegistryMcpConfigForm', () => { await expect(remove(wrapper)).resolves.toBe(false); expect(confirmRemove).toHaveBeenCalledWith( - 'tools.connection.settings.removeConfirm.description', expect.objectContaining({ + description: 'tools.connection.settings.removeConfirm.description', + isDestructive: true, confirmButtonText: 'tools.connection.settings.removeConfirm.confirmButton', }), ); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionTimelineView.spec.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionTimelineView.spec.ts index a48a41f55f9..49af8d47606 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionTimelineView.spec.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionTimelineView.spec.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { shallowMount, flushPromises } from '@vue/test-utils'; import { nextTick, reactive } from 'vue'; +import { createPinia, setActivePinia } from 'pinia'; import type { PushMessage } from '@n8n/api-types'; import { N8nEmptyState } from '@n8n/design-system'; import type * as AgentBuilderSessionModule from '@/features/agents/composables/useAgentBuilderSession'; @@ -147,6 +148,7 @@ async function renderPrivateTimeline() { describe('AgentSessionTimelineView', () => { beforeEach(() => { + setActivePinia(createPinia()); localStorage.removeItem('N8N_AGENT_PREVIEW_OPEN:p1:a1'); agentPermissions.canUpdate.value = true; deleteSession.mockClear(); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionsListView.spec.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionsListView.spec.ts index 3239032eeb8..3d8ea01a657 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionsListView.spec.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentSessionsListView.spec.ts @@ -150,10 +150,6 @@ vi.mock('../components/AgentSessionsFilter.vue', () => ({ }, })); -vi.mock('@/app/composables/useMessage', () => ({ - useMessage: () => ({ confirm: vi.fn() }), -})); - vi.mock('@n8n/composables/useToast', () => ({ useToast: () => ({ showError: vi.fn(), showMessage: vi.fn() }), })); @@ -163,10 +159,6 @@ vi.mock('../utils/thread-title', () => ({ thread.title ?? thread.firstMessage ?? 'Session', })); -vi.mock('@/app/constants', () => ({ - MODAL_CONFIRM: 'modal-confirm', -})); - vi.mock('@/app/utils/formatters/dateFormatter', () => ({ convertToDisplayDate: () => ({ date: '2026-07-20', time: '10:05' }), })); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentBuilderSession.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentBuilderSession.test.ts index 3cfc8d000fe..ec2b52657a7 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentBuilderSession.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentBuilderSession.test.ts @@ -1,5 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { effectScope, nextTick, reactive, ref } from 'vue'; +import { createPinia, setActivePinia } from 'pinia'; +import { useUIStore } from '@/app/stores/ui.store'; import { MODAL_CANCEL, MODAL_CONFIRM } from '@/app/constants/modals'; @@ -39,10 +41,6 @@ vi.mock('@n8n/i18n', () => ({ useI18n: () => ({ baseText: (key: string) => key }), })); -vi.mock('@/app/composables/useMessage', () => ({ - useMessage: () => ({ confirm }), -})); - vi.mock('@n8n/composables/useToast', () => ({ useToast: () => ({ showError, showMessage }), })); @@ -64,6 +62,13 @@ function createSession(routeBacked = ref(false)) { describe('useAgentBuilderSession', () => { beforeEach(() => { vi.clearAllMocks(); + setActivePinia(createPinia()); + vi.spyOn(useUIStore(), 'openModalWithData').mockImplementation(({ data }) => { + void confirm(data).then((result: string) => { + const onResult = result === MODAL_CONFIRM ? data.onConfirm : data.onCancel; + if (typeof onResult === 'function') onResult(); + }); + }); route.query = {}; sessionsStore.threads = []; sessionsStore.previewThreads = []; @@ -261,13 +266,13 @@ describe('useAgentBuilderSession', () => { await expect(session.deleteSession('thread-1')).resolves.toBe(true); expect(confirm).toHaveBeenCalledExactlyOnceWith( - 'agentSessions.deleteConfirm.message', - 'agentSessions.deleteConfirm.headline', - { - type: 'warning', + expect.objectContaining({ + title: 'agentSessions.deleteConfirm.headline', + description: 'agentSessions.deleteConfirm.message', + isDestructive: true, confirmButtonText: 'agentSessions.deleteConfirm.confirmButtonText', - cancelButtonText: '', - }, + cancelButtonText: 'generic.cancel', + }), ); expect(sessionsStore.deleteThread).toHaveBeenCalledExactlyOnceWith( 'project-1', diff --git a/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsEditView.test.ts b/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsEditView.test.ts index 5d65bf6d0d3..50d0715017a 100644 --- a/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsEditView.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsEditView.test.ts @@ -28,7 +28,9 @@ vi.mock('@n8n/composables/useToast', () => ({ vi.mock('file-saver', () => ({ saveAs: vi.fn() })); const confirm = vi.fn(); -vi.mock('@/app/composables/useMessage', () => ({ useMessage: () => ({ confirm }) })); +vi.mock('../../composables/useAgentConfirmationModal', () => ({ + useAgentConfirmationModal: () => ({ openAgentConfirmationModal: confirm }), +})); vi.mock('./api', () => ({ checkTeamsCredential: vi.fn(), @@ -160,8 +162,8 @@ describe('AgentChannelTeamsEditView', () => { await waitFor(() => expect(emitted().restart).toEqual([[{ credentialId: 'cred-2' }]])); }); - it('keeps the saved credential when the user cancels', async () => { - confirm.mockResolvedValue('cancel'); + it.each(['cancel', 'close'])('keeps the saved credential on %s', async (result) => { + confirm.mockResolvedValue(result); const { getByTestId, emitted } = renderView({ props: viewProps }); await fireEvent.click(getByTestId('pick-credential')); diff --git a/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsManagedSetup.test.ts b/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsManagedSetup.test.ts index 09aea3f939b..4559f5fbe04 100644 --- a/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsManagedSetup.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsManagedSetup.test.ts @@ -21,7 +21,9 @@ import { AGENT_PERSONALISATION_KEY } from '../agentPersonalisation'; vi.mock('file-saver', () => ({ saveAs: vi.fn() })); const confirm = vi.fn(); -vi.mock('@/app/composables/useMessage', () => ({ useMessage: () => ({ confirm }) })); +vi.mock('../../composables/useAgentConfirmationModal', () => ({ + useAgentConfirmationModal: () => ({ openAgentConfirmationModal: confirm }), +})); vi.mock('./api', () => ({ fetchTeamsAppPackage: vi.fn().mockResolvedValue(new Blob()), @@ -288,8 +290,8 @@ describe('AgentChannelTeamsManagedSetup', () => { expect(emitted()).not.toHaveProperty('update:modelValue'); }); - it('keeps the sign-in when the user cancels', async () => { - confirm.mockResolvedValue('cancel'); + it.each(['cancel', 'close'])('keeps the sign-in on %s', async (result) => { + confirm.mockResolvedValue(result); const { getByTestId, emitted } = renderSaved(); await fireEvent.click(getByTestId('pick-sign-in')); diff --git a/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsSetup.test.ts b/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsSetup.test.ts index 6e1c60df15f..e74bb2172bc 100644 --- a/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsSetup.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/channels/teams/AgentChannelTeamsSetup.test.ts @@ -40,7 +40,9 @@ vi.mock('@n8n/composables/useToast', () => ({ vi.mock('@/features/credentials/credentials.api'); const confirm = vi.hoisted(() => vi.fn()); -vi.mock('@/app/composables/useMessage', () => ({ useMessage: () => ({ confirm }) })); +vi.mock('../../composables/useAgentConfirmationModal', () => ({ + useAgentConfirmationModal: () => ({ openAgentConfirmationModal: confirm }), +})); vi.mock('./api', () => ({ checkTeamsCredential: vi.fn(), @@ -578,9 +580,9 @@ describe('AgentChannelTeamsSetup', () => { expect(emitted()['update:modelValue']?.at(-1)).toEqual(['cred-1']); }); - it('keeps the credential of a connected channel when the restart is cancelled', async () => { + it.each(['cancel', 'close'])('keeps the connected credential on %s', async (result) => { withBot(); - confirm.mockResolvedValue('cancel'); + confirm.mockResolvedValue(result); // A listener makes the model controlled, as under v-model in the hosts. const connected = { savedCredentialId: 'cred-1', diff --git a/packages/frontend/editor-ui/src/features/agents/channels/teams/useTeamsSetupRestart.ts b/packages/frontend/editor-ui/src/features/agents/channels/teams/useTeamsSetupRestart.ts index 95a76d49606..929fc2276c1 100644 --- a/packages/frontend/editor-ui/src/features/agents/channels/teams/useTeamsSetupRestart.ts +++ b/packages/frontend/editor-ui/src/features/agents/channels/teams/useTeamsSetupRestart.ts @@ -1,7 +1,7 @@ import { useI18n } from '@n8n/i18n'; import { watch, type Ref } from 'vue'; -import { useMessage } from '@/app/composables/useMessage'; +import { useAgentConfirmationModal } from '../../composables/useAgentConfirmationModal'; import { MODAL_CONFIRM } from '@/app/constants'; import type { AgentChannelRestart } from '../types'; @@ -9,18 +9,15 @@ import type { AgentChannelRestart } from '../types'; /** Setting up again takes the agent offline in Teams until the new setup is done. */ export function useTeamsSetupRestart() { const i18n = useI18n(); - const message = useMessage(); + const { openAgentConfirmationModal } = useAgentConfirmationModal(); async function confirmRestart() { - const confirmed = await message.confirm( - i18n.baseText('agents.channels.teams.restart.confirmMessage'), - i18n.baseText('agents.channels.teams.restart.confirmTitle'), - { - type: 'warning', - confirmButtonText: i18n.baseText('agents.channels.teams.restart.button'), - cancelButtonText: i18n.baseText('generic.cancel'), - }, - ); + const confirmed = await openAgentConfirmationModal({ + title: i18n.baseText('agents.channels.teams.restart.confirmTitle'), + description: i18n.baseText('agents.channels.teams.restart.confirmMessage'), + confirmButtonText: i18n.baseText('agents.channels.teams.restart.button'), + cancelButtonText: i18n.baseText('generic.cancel'), + }); return confirmed === MODAL_CONFIRM; } diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentCard.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentCard.vue index d41b37c8498..45a00af74e9 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentCard.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentCard.vue @@ -164,6 +164,7 @@ async function onAction(action: string) { description: locale.baseText('agents.delete.modal.description', { interpolate: { name: props.agent.name }, }), + isDestructive: true, confirmButtonText: locale.baseText('agents.delete.modal.button.delete'), cancelButtonText: locale.baseText('generic.cancel'), }); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentConfirmationModal.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentConfirmationModal.vue index 44b7822878b..ac194852bf2 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentConfirmationModal.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentConfirmationModal.vue @@ -9,6 +9,7 @@ export type AgentConfirmationModalData = { description: string; confirmButtonText: string; cancelButtonText: string; + isDestructive?: boolean; onConfirm?: () => unknown | Promise; onCancel?: () => unknown | Promise; onClose?: () => unknown | Promise; @@ -71,7 +72,12 @@ async function onOpenChange(open: boolean) { {{ props.data.cancelButtonText }} - + {{ props.data.confirmButtonText }} diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentRegistryMcpConfigForm.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentRegistryMcpConfigForm.vue index 8f93ca0681b..ee47a6e014b 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentRegistryMcpConfigForm.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentRegistryMcpConfigForm.vue @@ -5,7 +5,7 @@ import type { McpToolPermissions } from '@n8n/api-types'; import { computed, ref, watch } from 'vue'; import { MODAL_CONFIRM } from '@/app/constants'; -import { useMessage } from '@/app/composables/useMessage'; +import { useAgentConfirmationModal } from '../composables/useAgentConfirmationModal'; import McpConnectionStatusCallout from '@/features/shared/toolsConnection/McpConnectionStatusCallout.vue'; import McpDetailBody from '@/features/shared/toolsConnection/McpDetailBody.vue'; import McpToolPermissionsEditor from '@/features/shared/toolsConnection/McpToolPermissionsEditor.vue'; @@ -30,7 +30,7 @@ const emit = defineEmits<{ }>(); const i18n = useI18n(); -const message = useMessage(); +const { openAgentConfirmationModal } = useAgentConfirmationModal(); const modalData = computed(() => props.data); const config = useAgentRegistryMcpConfig(modalData, () => emit('credential-deleted')); const item = config.item; @@ -124,20 +124,19 @@ function confirm(): boolean { async function remove(): Promise { if (!props.data.onRemove) return false; - const confirmed = await message.confirm( - i18n.baseText('tools.connection.settings.removeConfirm.description', { + const confirmed = await openAgentConfirmationModal({ + title: i18n.baseText('tools.connection.settings.removeConfirm.title', { + interpolate: { name: title.value }, + }), + description: i18n.baseText('tools.connection.settings.removeConfirm.description', { interpolate: { item: 'tool', service: title.value }, }), - { - title: i18n.baseText('tools.connection.settings.removeConfirm.title', { - interpolate: { name: title.value }, - }), - confirmButtonText: i18n.baseText('tools.connection.settings.removeConfirm.confirmButton', { - interpolate: { item: 'tool' }, - }), - cancelButtonText: i18n.baseText('generic.cancel'), - }, - ); + isDestructive: true, + confirmButtonText: i18n.baseText('tools.connection.settings.removeConfirm.confirmButton', { + interpolate: { item: 'tool' }, + }), + cancelButtonText: i18n.baseText('generic.cancel'), + }); if (confirmed !== MODAL_CONFIRM) return false; props.data.onRemove(); return true; diff --git a/packages/frontend/editor-ui/src/features/agents/composables/useAgentBuilderSession.ts b/packages/frontend/editor-ui/src/features/agents/composables/useAgentBuilderSession.ts index d49c7e2c195..9bd1f35533c 100644 --- a/packages/frontend/editor-ui/src/features/agents/composables/useAgentBuilderSession.ts +++ b/packages/frontend/editor-ui/src/features/agents/composables/useAgentBuilderSession.ts @@ -5,13 +5,13 @@ import { truncate } from '@n8n/utils/string/truncate'; import { useRoute, useRouter } from 'vue-router'; import type { LocationQueryRaw } from 'vue-router'; -import { useMessage } from '@/app/composables/useMessage'; import { MODAL_CONFIRM } from '@/app/constants'; import { useAgentSessionsStore } from '../agentSessions.store'; import type { AgentExecutionThread } from './useAgentThreadsApi'; import { CONTINUE_SESSION_ID_PARAM, NEW_SESSION_PARAM } from '../constants'; import { useThreadTitle } from '../utils/thread-title'; +import { useAgentConfirmationModal } from './useAgentConfirmationModal'; /** * Max chars for session-name display in the preview breadcrumb dropdown trigger @@ -240,7 +240,7 @@ function useSessionDeletion( ) { const sessionsStore = useAgentSessionsStore(); const i18n = useI18n(); - const message = useMessage(); + const { openAgentConfirmationModal } = useAgentConfirmationModal(); const toast = useToast(); const isDeletingSession = ref(false); let isDisposed = false; @@ -256,15 +256,13 @@ function useSessionDeletion( isDeletingSession.value = true; try { - const confirmed = await message.confirm( - i18n.baseText('agentSessions.deleteConfirm.message'), - i18n.baseText('agentSessions.deleteConfirm.headline'), - { - type: 'warning', - confirmButtonText: i18n.baseText('agentSessions.deleteConfirm.confirmButtonText'), - cancelButtonText: '', - }, - ); + const confirmed = await openAgentConfirmationModal({ + title: i18n.baseText('agentSessions.deleteConfirm.headline'), + description: i18n.baseText('agentSessions.deleteConfirm.message'), + isDestructive: true, + confirmButtonText: i18n.baseText('agentSessions.deleteConfirm.confirmButtonText'), + cancelButtonText: i18n.baseText('generic.cancel'), + }); if (confirmed !== MODAL_CONFIRM || !isTargetCurrent()) return false; await sessionsStore.deleteThread(targetProjectId, targetAgentId, sessionId); diff --git a/packages/frontend/editor-ui/src/features/agents/views/AgentBuilderView.vue b/packages/frontend/editor-ui/src/features/agents/views/AgentBuilderView.vue index 56d98356f81..26f02afb5ad 100644 --- a/packages/frontend/editor-ui/src/features/agents/views/AgentBuilderView.vue +++ b/packages/frontend/editor-ui/src/features/agents/views/AgentBuilderView.vue @@ -2669,6 +2669,7 @@ async function onHeaderAction(action: string) { description: locale.baseText('agents.delete.modal.description', { interpolate: { name: agentName.value }, }), + isDestructive: true, confirmButtonText: locale.baseText('agents.delete.modal.button.delete'), cancelButtonText: locale.baseText('generic.cancel'), }); diff --git a/packages/frontend/editor-ui/src/features/agents/views/AgentSessionsListView.vue b/packages/frontend/editor-ui/src/features/agents/views/AgentSessionsListView.vue index d3bd1952593..b559c914f2f 100644 --- a/packages/frontend/editor-ui/src/features/agents/views/AgentSessionsListView.vue +++ b/packages/frontend/editor-ui/src/features/agents/views/AgentSessionsListView.vue @@ -1,5 +1,5 @@ + + diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.test.ts b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.test.ts index 1287ce3c03e..c686da42f79 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.test.ts +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.test.ts @@ -40,6 +40,31 @@ const preview = (count: number): PromotionPreview => ({ })), }); +describe('usePromotionChangeCount blocker', () => { + beforeEach(() => { + vi.mocked(api.getPromotionPreview).mockReset(); + }); + + it('should keep the blocker of the preview and clear it on the next check', async () => { + const blocker = { code: 'reviewOpen' as const, reviewId: 'review-1' }; + vi.mocked(api.getPromotionPreview) + .mockResolvedValueOnce({ ...preview(0), blocker }) + .mockResolvedValueOnce(preview(1)); + const { + blocker: current, + changeCount, + refetch, + } = usePromotionChangeCount(ref('project-a'), 'promote', ref(true)); + + await waitAllPromises(); + expect(current.value).toEqual(blocker); + expect(changeCount.value).toBe(0); + + await refetch(); + expect(current.value).toBeNull(); + }); +}); + describe('usePromotionChangeCount', () => { // One entry per request, in call order, so a test can answer them out of order. let pending: Pending[]; diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.ts b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.ts index b92dbe571c8..d584debfa31 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.ts +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionChangeCount.ts @@ -1,5 +1,5 @@ import { ref, watch, type Ref } from 'vue'; -import type { PromotionDirection } from '@n8n/api-types'; +import type { PromotionBlocker, PromotionDirection } from '@n8n/api-types'; import { useRootStore } from '@n8n/stores/useRootStore'; import { useLatestFetch } from '@/app/composables/useLatestFetch'; import { getPromotionPreview } from '../promotions.api'; @@ -17,6 +17,8 @@ export function usePromotionChangeCount( const changeCount = ref(0); /** True when the last check failed, so the badge can show the failure. */ const failed = ref(false); + /** Set when an open review already holds the changes. The preview then lists none. */ + const blocker = ref(null); async function fetchCount() { // Only the newest request may write, so a slow answer for a project the user left @@ -25,6 +27,7 @@ export function usePromotionChangeCount( const requestedProjectId = projectId.value; changeCount.value = 0; failed.value = false; + blocker.value = null; if (!enabled.value || !requestedProjectId) { return; } @@ -36,6 +39,7 @@ export function usePromotionChangeCount( ); if (!isLatest()) return; changeCount.value = totalChangeCount(buildSelectionGraph(preview, direction)); + blocker.value = preview.blocker ?? null; } catch { if (isLatest()) failed.value = true; } @@ -43,5 +47,5 @@ export function usePromotionChangeCount( watch([projectId, enabled], fetchCount, { immediate: true }); - return { changeCount, failed, refetch: fetchCount }; + return { changeCount, failed, blocker, refetch: fetchCount }; } diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionPreview.ts b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionPreview.ts index 560b4ab62d0..be482ffb78a 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionPreview.ts +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/composables/usePromotionPreview.ts @@ -16,6 +16,8 @@ type PromotionPreviewState = /** The instance has no environment for this direction. */ | 'disconnected' | 'error' + /** An open review holds the changes, so the preview lists none. */ + | 'blocked' | 'empty' | 'ready'; @@ -80,7 +82,7 @@ export function usePromotionPreview(projectId: Ref, direction: Ref { expect(getByTestId('promotion-preview-tree')).toBeInTheDocument(); }); + describe('with an open review', () => { + const blocked = () => + billingPreview({ + resources: [], + changes: [], + blocker: { code: 'reviewOpen', reviewId: 'review-1' }, + }); + + async function renderBlocked(canReadReviews: boolean, inboxEnabled = true) { + vi.mocked(api.getPromotionPreview).mockResolvedValue(blocked()); + await router.push('/projects/p1/promotions/promote'); + const pinia = createTestingPinia(); + mockedStore(useRBACStore).hasScope.mockReturnValue(canReadReviews); + const settingsStore = mockedStore(useSettingsStore); + settingsStore.settings = { + ...settingsStore.settings, + inbox: { enabled: inboxEnabled, availableTypes: [], failedTypes: [] }, + }; + const result = renderComponent({ pinia, props: { projectId: 'p1', direction: 'promote' } }); + await waitAllPromises(); + return result; + } + + it('should show the blocker with a link to the Inbox and no changes to promote', async () => { + const { getByTestId, queryByTestId, getByText } = await renderBlocked(true); + + expect(getByTestId('promotion-review-blocker')).toHaveTextContent('A review is open'); + expect(queryByTestId('promotion-preview-tree')).not.toBeInTheDocument(); + expect(queryByTestId('promotion-preview-empty')).not.toBeInTheDocument(); + expect(getByTestId('promotion-preview-submit')).toBeDisabled(); + + await userEvent.click(getByText('Open the Inbox')); + await waitFor(() => expect(router.currentRoute.value.name).toBe(INBOX_VIEW)); + }); + + it('should ask for an admin when the user cannot read reviews', async () => { + const { getByTestId, queryByText } = await renderBlocked(false); + + expect(getByTestId('promotion-review-blocker')).toHaveTextContent( + 'Ask an admin to merge or close the review.', + ); + expect(queryByText('Open the Inbox')).not.toBeInTheDocument(); + }); + + it('should not link to the Inbox when the Inbox is off', async () => { + const { getByTestId, queryByText } = await renderBlocked(true, false); + + expect(getByTestId('promotion-review-blocker')).toHaveTextContent( + 'Ask an admin to merge or close the review.', + ); + expect(queryByText('Open the Inbox')).not.toBeInTheDocument(); + }); + }); + it('should show the error when the refresh of an empty preview fails', async () => { vi.mocked(api.getPromotionPreview).mockResolvedValueOnce(billingPreview({ resources: [] })); const { getByTestId } = await render(); diff --git a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/views/PromotionPreviewView.vue b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/views/PromotionPreviewView.vue index 39faf6cf6ee..fd8c4ab7443 100644 --- a/packages/frontend/editor-ui/src/features/integrations/promotions.ee/views/PromotionPreviewView.vue +++ b/packages/frontend/editor-ui/src/features/integrations/promotions.ee/views/PromotionPreviewView.vue @@ -28,6 +28,7 @@ import { useProjectsStore } from '@/features/collaboration/projects/projects.sto import PromotionBlockers from '../components/preview/PromotionBlockers.vue'; import PromotionChangeCards from '../components/preview/PromotionChangeCards.vue'; import PromotionResourceTree from '../components/preview/PromotionResourceTree.vue'; +import PromotionReviewBlocker from '../components/preview/PromotionReviewBlocker.vue'; import { usePromotionOperation } from '../composables/usePromotionOperation'; import { usePromotionResourceDetails } from '../composables/usePromotionResourceDetails'; import { usePromotionPreview } from '../composables/usePromotionPreview'; @@ -365,6 +366,7 @@ const treeResizer = useResizablePanel({ data-test-id="promotion-preview-error" @click:button="refresh" /> +