diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 594b952b7bc..f89129c8ec1 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3058,6 +3058,7 @@ "variables.add.unavailable": "Upgrade plan to keep using variables", "variables.add.unavailable.empty": "Upgrade plan to start using variables", "variables.add.onlyOwnerCanCreate": "Only owner can create variables", + "variables.add.button.label": "Create variable", "variables.empty.heading": "{name}, let's set up a variable", "variables.empty.heading.userNotSetup": "Set up a variable", "variables.empty.description": "Variables can be used to store data that can be referenced easily across multiple workflows.", diff --git a/packages/frontend/editor-ui/src/constants/navigation.ts b/packages/frontend/editor-ui/src/constants/navigation.ts index c12fbe22acc..f412a5a150d 100644 --- a/packages/frontend/editor-ui/src/constants/navigation.ts +++ b/packages/frontend/editor-ui/src/constants/navigation.ts @@ -9,7 +9,6 @@ export const enum VIEWS { TEMPLATE_SETUP = 'TemplatesWorkflowSetupView', TEMPLATES = 'TemplatesSearchView', CREDENTIALS = 'CredentialsView', - VARIABLES = 'VariablesView', NEW_WORKFLOW = 'NodeViewNew', WORKFLOW = 'NodeViewExisting', DEMO = 'WorkflowDemo', diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectHeader.vue b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectHeader.vue index 90ba870352d..37fd85b8c89 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectHeader.vue +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectHeader.vue @@ -9,7 +9,7 @@ import { useProjectsStore } from '../projects.store'; import ProjectTabs from './ProjectTabs.vue'; import ProjectIcon from './ProjectIcon.vue'; import { getResourcePermissions } from '@n8n/permissions'; -import { VIEWS } from '@/constants'; +import { EnterpriseEditionFeature, VIEWS } from '@/constants'; import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store'; import ProjectCreateResource from './ProjectCreateResource.vue'; import { useSettingsStore } from '@/stores/settings.store'; @@ -23,6 +23,9 @@ import { PROJECT_DATA_TABLES } from '@/features/core/dataTable/constants'; import ReadyToRunV2Button from '@/experiments/readyToRunWorkflowsV2/components/ReadyToRunV2Button.vue'; import { N8nButton, N8nHeading, N8nText, N8nTooltip } from '@n8n/design-system'; +import { VARIABLE_MODAL_KEY } from '@/features/settings/environments.ee/environments.constants'; +import { useTelemetry } from '@/composables/useTelemetry'; +import { useUsersStore } from '@/features/settings/users/users.store'; const route = useRoute(); const router = useRouter(); const i18n = useI18n(); @@ -30,6 +33,8 @@ const projectsStore = useProjectsStore(); const sourceControlStore = useSourceControlStore(); const settingsStore = useSettingsStore(); const uiStore = useUIStore(); +const telemetry = useTelemetry(); +const usersStore = useUsersStore(); const projectPages = useProjectPages(); @@ -72,6 +77,9 @@ const projectName = computed(() => { const projectPermissions = computed( () => getResourcePermissions(projectsStore.currentProject?.scopes).project, ); +const globalPermissions = computed( + () => getResourcePermissions(usersStore.currentUser?.globalScopes).variable, +); const showSettings = computed( () => @@ -115,6 +123,7 @@ const ACTION_TYPES = { CREDENTIAL: 'credential', FOLDER: 'folder', DATA_TABLE: 'dataTable', + VARIABLE: 'variable', } as const; type ActionTypes = (typeof ACTION_TYPES)[keyof typeof ACTION_TYPES]; @@ -148,6 +157,16 @@ const createDataTableButton = computed(() => ({ !getResourcePermissions(homeProject.value?.scopes)?.dataTable?.create, })); +const createVariableButton = computed(() => ({ + value: ACTION_TYPES.VARIABLE, + label: i18n.baseText('variables.add.button.label'), + icon: sourceControlStore.preferences.branchReadOnly ? ('lock' as IconName) : undefined, + size: 'mini' as const, + disabled: + sourceControlStore.preferences.branchReadOnly || + (!projectPermissions.value.create && !globalPermissions.value.create), +})); + const selectedMainButtonType = computed(() => props.mainButton ?? ACTION_TYPES.WORKFLOW); const mainButtonConfig = computed(() => { @@ -156,6 +175,8 @@ const mainButtonConfig = computed(() => { return createCredentialButton.value; case ACTION_TYPES.DATA_TABLE: return createDataTableButton.value; + case ACTION_TYPES.VARIABLE: + return createVariableButton.value; case ACTION_TYPES.WORKFLOW: default: return createWorkflowButton.value; @@ -187,6 +208,19 @@ const menu = computed(() => { }); } + if ( + selectedMainButtonType.value !== ACTION_TYPES.VARIABLE && + settingsStore.isEnterpriseFeatureEnabled[EnterpriseEditionFeature.Variables] + ) { + items.push({ + value: ACTION_TYPES.VARIABLE, + label: i18n.baseText('variables.add.button.label'), + disabled: + sourceControlStore.preferences.branchReadOnly || + !getResourcePermissions(homeProject.value?.scopes).projectVariable.create, + }); + } + if (showFolders.value) { items.push({ value: ACTION_TYPES.FOLDER, @@ -283,6 +317,10 @@ const actions: Record void> = { params: { projectId, new: 'new' }, }); }, + [ACTION_TYPES.VARIABLE]: () => { + uiStore.openModalWithData({ name: VARIABLE_MODAL_KEY, data: { mode: 'new' } }); + telemetry.track('User clicked header add variable button'); + }, } as const; const pageType = computed(() => { diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.test.ts b/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.test.ts index 1d91e10463a..547ed142181 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.test.ts +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.test.ts @@ -15,6 +15,10 @@ import type { IUser } from '@n8n/rest-api-client/api/users'; import type { Scope } from '@n8n/permissions'; import type { EnvironmentVariable } from '@/features/settings/environments.ee/environments.types'; import useEnvironmentsStore from '@/features/settings/environments.ee/environments.store'; +import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; +import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store'; +import type { Project } from '@/features/collaboration/projects/projects.types'; +import type { SourceControlPreferences } from '@/features/integrations/sourceControl.ee/sourceControl.types'; const router = createRouter({ history: createWebHistory(), @@ -113,7 +117,28 @@ describe('ProjectVariables', () => { const uiStore = mockedStore(useUIStore); - return { usersStore, settingsStore, environmentsStore, uiStore }; + // Mock project store so that project header renders correctly + const projectsStore = mockedStore(useProjectsStore); + projectsStore.personalProject = { + id: 'personal-project-id', + name: 'Current Project', + scopes: ['projectVariable:create', 'projectVariable:read'], + } as Project; + projectsStore.currentProject = projectsStore.personalProject; + + const sourceControlStore = mockedStore(useSourceControlStore); + sourceControlStore.preferences = { + branchReadOnly: false, + } as SourceControlPreferences; + + return { + usersStore, + settingsStore, + environmentsStore, + uiStore, + projectsStore, + sourceControlStore, + }; }; it('should render variable entries', async () => { @@ -239,9 +264,9 @@ describe('ProjectVariables', () => { ]); const { getByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); + await waitFor(() => expect(getByTestId('add-resource-variable')).toBeVisible()); - await userEvent.click(getByTestId('resources-list-add')); + await userEvent.click(getByTestId('add-resource-variable')); expect(uiStore.openModalWithData).toHaveBeenCalledWith({ name: VARIABLE_MODAL_KEY, @@ -261,7 +286,7 @@ describe('ProjectVariables', () => { const { uiStore } = userWithPrivileges(variables); const { getByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); + await waitFor(() => expect(getByTestId('variables-row')).toBeVisible()); await userEvent.hover(getByTestId('variables-row')); expect(getByTestId('variable-row-edit-button')).toBeVisible(); @@ -298,10 +323,8 @@ describe('ProjectVariables', () => { userWithPrivileges(variables); - const { getByTestId, queryAllByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(2); + const { queryAllByTestId } = renderComponent(); + await waitFor(() => expect(queryAllByTestId('variables-row')).toHaveLength(2)); await userEvent.hover(queryAllByTestId('variables-row')[0]); expect(queryAllByTestId('variable-row-delete-button')[0]).toBeVisible(); @@ -324,13 +347,11 @@ describe('ProjectVariables', () => { ]); const { getByTestId, queryAllByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(2); + await waitFor(() => expect(queryAllByTestId('variables-row')).toHaveLength(2)); await userEvent.click(getByTestId('variable-filter-incomplete')); - expect(queryAllByTestId('variables-row').length).toBe(1); + expect(queryAllByTestId('variables-row')).toHaveLength(1); }); }); @@ -350,9 +371,8 @@ describe('ProjectVariables', () => { ]); const { getByTestId, queryAllByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); + await waitFor(() => expect(queryAllByTestId('variables-row')).toHaveLength(2)); - expect(queryAllByTestId('variables-row').length).toBe(2); // Default sort should be ascending expect(queryAllByTestId('variables-row')[0].querySelector('td')?.textContent).toBe('ALPHA'); @@ -418,7 +438,7 @@ describe('ProjectVariables', () => { ]; const { getByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); + await waitFor(() => expect(getByTestId('variables-row')).toBeVisible()); await userEvent.hover(getByTestId('variables-row')); const editButton = getByTestId('variable-row-edit-button'); @@ -442,7 +462,8 @@ describe('ProjectVariables', () => { ]; const { getByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); + + await waitFor(() => expect(getByTestId('variables-row')).toBeVisible()); await userEvent.hover(getByTestId('variables-row')); const deleteButton = getByTestId('variable-row-delete-button'); diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.vue b/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.vue index ebbc6781ad2..5b071085d33 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.vue +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/views/ProjectVariables.vue @@ -65,9 +65,12 @@ const { showError, showMessage } = useToast(); const projectId = route.params.projectId; -const permissions = computed( +const globalPermissions = computed( () => getResourcePermissions(usersStore.currentUser?.globalScopes).variable, ); +const projectPermissions = computed( + () => getResourcePermissions(projectsStore.currentProject?.scopes).projectVariable, +); const { isLoading, execute } = useAsyncState(environmentsStore.fetchAllVariables, [], { immediate: true, @@ -104,7 +107,10 @@ const variables = computed(() => const globalVariables = computed(() => environmentsStore.variables.filter((v) => !v.project)); -const canCreateVariables = computed(() => isFeatureEnabled.value && permissions.value.create); +const canCreateVariables = computed( + () => + isFeatureEnabled.value && (globalPermissions.value.create ?? projectPermissions.value.create), +); const columns = computed(() => { const cols: DatatableColumn[] = [ @@ -308,7 +314,7 @@ onMounted(() => { @click:add="openCreateVariableModal" > - - + {{ i18n.baseText('variables.row.button.delete') }} diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.utils.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.utils.ts index 452b6edff58..0b986f91fcb 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.utils.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/sourceControl.utils.ts @@ -47,7 +47,7 @@ export const getPushPriorityByStatus = (status: SourceControlledFileStatus) => pushStatusPriority[status] ?? 0; const createVariablesToast = (router: Router) => { - const route = { name: VIEWS.VARIABLES, query: { incomplete: 'true' } }; + const route = { name: VIEWS.PROJECTS_VARIABLES, query: { incomplete: 'true' } }; const { href } = router.resolve(route); return { diff --git a/packages/frontend/editor-ui/src/features/settings/environments.ee/views/VariablesView.test.ts b/packages/frontend/editor-ui/src/features/settings/environments.ee/views/VariablesView.test.ts deleted file mode 100644 index 56920ae336f..00000000000 --- a/packages/frontend/editor-ui/src/features/settings/environments.ee/views/VariablesView.test.ts +++ /dev/null @@ -1,319 +0,0 @@ -import VariablesView from './VariablesView.vue'; -import { useSettingsStore } from '@/stores/settings.store'; -import { useUsersStore } from '@/features/settings/users/users.store'; -import { useRBACStore } from '@/stores/rbac.store'; -import { useEnvironmentsStore } from '../environments.store'; -import { createComponentRenderer } from '@/__tests__/render'; -import { EnterpriseEditionFeature } from '@/constants'; -import { STORES } from '@n8n/stores'; -import { createTestingPinia } from '@pinia/testing'; -import { mockedStore, SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils'; -import { createRouter, createWebHistory } from 'vue-router'; -import userEvent from '@testing-library/user-event'; -import { waitFor, within } from '@testing-library/vue'; -import type { EnvironmentVariable } from '../environments.types'; -import type { IUser } from '@n8n/rest-api-client/api/users'; -import type { Scope } from '@n8n/permissions'; - -const router = createRouter({ - history: createWebHistory(), - routes: [ - { - path: '/', - component: { template: '
' }, - }, - ], -}); - -const renderComponent = createComponentRenderer(VariablesView, { global: { plugins: [router] } }); - -const fullAccessScopes: Scope[] = [ - 'variable:create', - 'variable:read', - 'variable:update', - 'variable:delete', - 'variable:list', -]; - -describe('VariablesView', () => { - beforeEach(async () => { - createTestingPinia({ initialState: { [STORES.SETTINGS]: SETTINGS_STORE_DEFAULT_STATE } }); - await router.push('/'); - await router.isReady(); - }); - - describe('should render empty state', () => { - it('when feature is disabled and logged in user is not owner', async () => { - const settingsStore = mockedStore(useSettingsStore); - settingsStore.settings.enterprise[EnterpriseEditionFeature.Variables] = false; - const rbacStore = mockedStore(useRBACStore); - rbacStore.globalScopes = ['variable:read', 'variable:list']; - - const { queryByTestId } = renderComponent(); - - await waitFor(() => { - expect(queryByTestId('empty-resources-list')).not.toBeInTheDocument(); - expect(queryByTestId('unavailable-resources-list')).toBeVisible(); - expect(queryByTestId('cannot-create-variables')).not.toBeInTheDocument(); - }); - }); - - it('when feature is disabled and logged in user is owner', async () => { - const settingsStore = mockedStore(useSettingsStore); - settingsStore.settings.enterprise[EnterpriseEditionFeature.Variables] = false; - const rbacStore = mockedStore(useRBACStore); - rbacStore.globalScopes = fullAccessScopes; - - const { queryByTestId } = renderComponent(); - - await waitFor(() => { - expect(queryByTestId('empty-resources-list')).not.toBeInTheDocument(); - expect(queryByTestId('unavailable-resources-list')).toBeVisible(); - expect(queryByTestId('cannot-create-variables')).not.toBeInTheDocument(); - }); - }); - - it('when feature is enabled and logged in user is owner', async () => { - const settingsStore = mockedStore(useSettingsStore); - settingsStore.settings.enterprise[EnterpriseEditionFeature.Variables] = false; - const rbacStore = mockedStore(useRBACStore); - rbacStore.globalScopes = [ - 'variable:create', - 'variable:read', - 'variable:update', - 'variable:delete', - 'variable:list', - ]; - - const { queryByTestId } = renderComponent(); - - await waitFor(() => { - expect(queryByTestId('empty-resources-list')).not.toBeInTheDocument(); - expect(queryByTestId('unavailable-resources-list')).not.toBeInTheDocument(); - expect(queryByTestId('cannot-create-variables')).not.toBeInTheDocument(); - }); - }); - - it('when feature is enabled and logged in user is not owner', async () => { - const settingsStore = mockedStore(useSettingsStore); - settingsStore.settings.enterprise[EnterpriseEditionFeature.Variables] = true; - const rbacStore = mockedStore(useRBACStore); - rbacStore.globalScopes = ['variable:read', 'variable:list']; - - const { queryByTestId } = renderComponent(); - - await waitFor(() => { - expect(queryByTestId('empty-resources-list')).not.toBeInTheDocument(); - expect(queryByTestId('unavailable-resources-list')).not.toBeInTheDocument(); - expect(queryByTestId('cannot-create-variables')).toBeVisible(); - }); - }); - }); - - const userWithPrivileges = (variables: EnvironmentVariable[]) => { - const userStore = mockedStore(useUsersStore); - userStore.currentUser = { globalScopes: fullAccessScopes } as IUser; - const settingsStore = mockedStore(useSettingsStore); - settingsStore.settings.enterprise[EnterpriseEditionFeature.Variables] = true; - const environmentsStore = mockedStore(useEnvironmentsStore); - environmentsStore.variables = variables; - - return { userStore, settingsStore, environmentsStore }; - }; - - it('should render variable entries', async () => { - userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'a', - }, - { - id: '2', - key: 'b', - value: 'b', - }, - { - id: '3', - key: 'c', - value: 'c', - }, - ]); - - const wrapper = renderComponent(); - - const table = await wrapper.findByTestId('resources-table'); - expect(table).toBeVisible(); - expect(wrapper.container.querySelectorAll('tr')).toHaveLength(4); - }); - - it('should truncate long variable values', async () => { - userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'This is a very long variable value that should be truncated', - }, - ]); - - const { findByTestId, getByText, queryByText } = renderComponent(); - - const table = await findByTestId('resources-table'); - expect(table).toBeVisible(); - expect(queryByText('This is a very long variable value that should be truncated')).toBeNull(); - expect(getByText('This is a very long ...')).toBeVisible(); - }); - - describe('CRUD', () => { - it('should create variables', async () => { - const { environmentsStore } = userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'a', - }, - ]); - - const { getByTestId, queryAllByTestId, getByPlaceholderText } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(1); - - await userEvent.click(getByTestId('resources-list-add')); - - expect(queryAllByTestId('variables-row').length).toBe(2); - - const newVariable = { key: 'b', value: 'b' }; - - await userEvent.type(getByPlaceholderText('Enter a name'), newVariable.key); - await userEvent.type(getByPlaceholderText('Enter a value'), newVariable.value); - - await userEvent.click(getByTestId('variable-row-save-button')); - - expect(environmentsStore.createVariable).toHaveBeenCalledWith(newVariable); - }); - - it('should delete variables', async () => { - const { environmentsStore } = userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'a', - }, - { - id: '2', - key: 'b', - value: 'b', - }, - ]); - - const { getByTestId, queryAllByTestId, getByLabelText, queryAllByLabelText } = - renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(2); - - await userEvent.hover(queryAllByTestId('variables-row')[0]); - expect(queryAllByTestId('variable-row-delete-button')[0]).toBeVisible(); - await userEvent.click(queryAllByTestId('variable-row-delete-button')[0]); - - // Cancel - expect(getByLabelText('Delete variable')).toBeVisible(); - await userEvent.click(within(getByLabelText('Delete variable')).getByText('Cancel')); - expect(environmentsStore.deleteVariable).not.toHaveBeenCalled(); - - await userEvent.hover(queryAllByTestId('variables-row')[0]); - expect(queryAllByTestId('variable-row-delete-button')[0]).toBeVisible(); - await userEvent.click(queryAllByTestId('variable-row-delete-button')[0]); - - // Delete - const dialog = queryAllByLabelText('Delete variable').at(-1); - expect(dialog).toBeVisible(); - await userEvent.click(within(dialog as HTMLElement).getByText('Delete')); - - expect(environmentsStore.deleteVariable).toHaveBeenCalledWith(environmentsStore.variables[0]); - }); - - it('should update variable', async () => { - const { environmentsStore } = userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'a', - }, - ]); - - const { getByTestId, queryAllByTestId, getByPlaceholderText } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(1); - - await userEvent.hover(getByTestId('variables-row')); - - expect(getByTestId('variable-row-edit-button')).toBeVisible(); - await userEvent.click(getByTestId('variable-row-edit-button')); - - const newVariable = { id: '1', key: 'ab', value: 'ab' }; - - await userEvent.type(getByPlaceholderText('Enter a name'), 'b'); - await userEvent.type(getByPlaceholderText('Enter a value'), 'b'); - - await userEvent.click(getByTestId('variable-row-save-button')); - - expect(environmentsStore.updateVariable).toHaveBeenCalledWith(newVariable); - }); - }); - - describe('filter', () => { - it('should filter by incomplete', async () => { - userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'a', - }, - { - id: '2', - key: 'b', - value: '', - }, - ]); - - const { getByTestId, queryAllByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(2); - await userEvent.click(getByTestId('variable-filter-incomplete')); - - expect(queryAllByTestId('variables-row').length).toBe(1); - }); - }); - - describe('sorting', () => { - it('should sort by name (asc | desc)', async () => { - userWithPrivileges([ - { - id: '1', - key: 'a', - value: 'a', - }, - { - id: '2', - key: 'b', - value: 'b', - }, - ]); - - const { getByTestId, queryAllByTestId } = renderComponent(); - await waitFor(() => expect(getByTestId('resources-list-add')).toBeVisible()); - - expect(queryAllByTestId('variables-row').length).toBe(2); - expect(queryAllByTestId('variables-row')[0].querySelector('td')?.textContent).toBe('a'); - - await userEvent.click(getByTestId('resources-list-sort')); - await userEvent.click(queryAllByTestId('resources-list-sort-item')[1]); - - expect(queryAllByTestId('variables-row')[0].querySelector('td')?.textContent).toBe('b'); - }); - }); -}); diff --git a/packages/frontend/editor-ui/src/features/settings/environments.ee/views/VariablesView.vue b/packages/frontend/editor-ui/src/features/settings/environments.ee/views/VariablesView.vue deleted file mode 100644 index e86c11eb54f..00000000000 --- a/packages/frontend/editor-ui/src/features/settings/environments.ee/views/VariablesView.vue +++ /dev/null @@ -1,421 +0,0 @@ - - - - - diff --git a/packages/frontend/editor-ui/src/features/shared/commandBar/composables/useGenericCommands.ts b/packages/frontend/editor-ui/src/features/shared/commandBar/composables/useGenericCommands.ts index 0694df8a56a..4571d03a27f 100644 --- a/packages/frontend/editor-ui/src/features/shared/commandBar/composables/useGenericCommands.ts +++ b/packages/frontend/editor-ui/src/features/shared/commandBar/composables/useGenericCommands.ts @@ -68,7 +68,7 @@ export function useGenericCommands(): CommandGroup { title: i18n.baseText('mainSidebar.variables'), section: i18n.baseText('commandBar.sections.general'), handler: () => { - void router.push({ name: VIEWS.VARIABLES }); + void router.push({ name: VIEWS.HOME_VARIABLES }); }, icon: { component: N8nIcon, diff --git a/packages/frontend/editor-ui/src/router.ts b/packages/frontend/editor-ui/src/router.ts index 52b482cf77e..5888a9ad021 100644 --- a/packages/frontend/editor-ui/src/router.ts +++ b/packages/frontend/editor-ui/src/router.ts @@ -67,8 +67,6 @@ const SetupWorkflowFromTemplateView = async () => await import('@/features/workflows/templates/views/SetupWorkflowFromTemplateView.vue'); const TemplatesSearchView = async () => await import('@/features/workflows/templates/views/TemplatesSearchView.vue'); -const VariablesView = async () => - await import('@/features/settings/environments.ee/views/VariablesView.vue'); const SettingsUsageAndPlan = async () => await import('@/features/settings/usage/views/SettingsUsageAndPlan.vue'); const SettingsSso = async () => await import('@/features/settings/sso/views/SettingsSso.vue'); @@ -243,15 +241,6 @@ export const routes: RouteRecordRaw[] = [ } }, }, - { - path: '/variables', - name: VIEWS.VARIABLES, - components: { - default: VariablesView, - sidebar: MainSidebar, - }, - meta: { middleware: ['authenticated'] }, - }, { path: '/workflow/:name/debug/:executionId', name: VIEWS.EXECUTION_DEBUG, diff --git a/packages/testing/playwright/pages/VariablesPage.ts b/packages/testing/playwright/pages/VariablesPage.ts index df44aeb7744..11498536459 100644 --- a/packages/testing/playwright/pages/VariablesPage.ts +++ b/packages/testing/playwright/pages/VariablesPage.ts @@ -1,8 +1,11 @@ import { expect, type Locator } from '@playwright/test'; import { BasePage } from './BasePage'; +import { VariableModal } from './components/VariableModal'; export class VariablesPage extends BasePage { + readonly variableModal = new VariableModal(this.page.getByTestId('variableModal-modal')); + getUnavailableResourcesList() { return this.page.getByTestId('unavailable-resources-list'); } @@ -16,7 +19,7 @@ export class VariablesPage extends BasePage { } getEmptyResourcesListNewVariableButton() { - return this.getEmptyResourcesList().locator('button'); + return this.page.getByRole('button', { name: 'Add first variable' }); } getSearchBar() { @@ -24,17 +27,13 @@ export class VariablesPage extends BasePage { } getCreateVariableButton() { - return this.page.getByTestId('resources-list-add'); + return this.page.getByTestId('add-resource-variable'); } getVariablesRows() { return this.page.getByTestId('variables-row'); } - getVariablesEditableRows() { - return this.page.getByTestId('variables-row').filter({ has: this.page.locator('input') }); - } - getVariableRow(key: string) { return this.getVariablesRows().filter({ hasText: key }); } @@ -47,22 +46,33 @@ export class VariablesPage extends BasePage { return row.getByTestId('variable-row-save-button'); } - async createVariable(key: string, value: string) { - await this.getCreateVariableButton().click(); + /** + * Create a variable with the key, + * @param key - The key of the variable + * @param value - The value of the variable + */ - const editingRow = this.getVariablesEditableRows().first(); - await this.setRowValue(editingRow, 'key', key); - await this.setRowValue(editingRow, 'value', value); - await this.saveRowEditing(editingRow); + async createVariableFromModal( + key: string, + value: string, + { shouldSave }: { shouldSave: boolean } = { shouldSave: true }, + ) { + await this.variableModal.waitForModal(); + await this.variableModal.addVariable(key, value, { shouldSave }); } async createVariableFromEmptyState(key: string, value: string) { await this.getEmptyResourcesListNewVariableButton().click(); + await this.createVariableFromModal(key, value); + } - const editingRow = this.getVariablesEditableRows().first(); - await this.setRowValue(editingRow, 'key', key); - await this.setRowValue(editingRow, 'value', value); - await this.saveRowEditing(editingRow); + async createVariable( + key: string, + value: string, + { shouldSave }: { shouldSave: boolean } = { shouldSave: true }, + ) { + await this.getCreateVariableButton().click(); + await this.createVariableFromModal(key, value, { shouldSave }); } async deleteVariable(key: string) { @@ -75,22 +85,13 @@ export class VariablesPage extends BasePage { await modal.locator('.btn--confirm').click(); } - async editRow(key: string) { + async editVariable( + key: string, + newValue: string, + { shouldSave }: { shouldSave: boolean } = { shouldSave: true }, + ) { const row = this.getVariableRow(key); await row.getByTestId('variable-row-edit-button').click(); - } - - async setRowValue(row: Locator, field: 'key' | 'value', value: string) { - const input = row.getByTestId(`variable-row-${field}-input`).locator('input, textarea'); - await input.selectText(); - await input.fill(value); - } - - async saveRowEditing(row: Locator) { - await this.getEditableRowSaveButton(row).click(); - } - - async cancelRowEditing(row: Locator) { - await this.getEditableRowCancelButton(row).click(); + await this.createVariableFromModal(key, newValue, { shouldSave }); } } diff --git a/packages/testing/playwright/pages/components/VariableModal.ts b/packages/testing/playwright/pages/components/VariableModal.ts new file mode 100644 index 00000000000..0cf3bcfe524 --- /dev/null +++ b/packages/testing/playwright/pages/components/VariableModal.ts @@ -0,0 +1,66 @@ +import type { Locator } from '@playwright/test'; + +/** + * Variable modal component for canvas and variables interactions. + * Used within VariablesPage as `n8n.variables.modal.*` + * + * @example + * // Access via canvas page or variables page + * await n8n.variables.modal.addVariable(); + * await expect(n8n.variables.modal.getModal()).toBeVisible(); + */ +export class VariableModal { + constructor(private root: Locator) {} + + getModal(): Locator { + return this.root; + } + + getKeyInput(): Locator { + return this.root.getByTestId('variable-modal-key-input').getByRole('textbox'); + } + + getValueInput(): Locator { + return this.root.getByTestId('variable-modal-value-input').getByRole('textbox'); + } + + async waitForModal(): Promise { + await this.root.waitFor({ state: 'visible' }); + } + + getSaveButton(): Locator { + return this.root.getByTestId('variable-modal-save-button'); + } + + async save(): Promise { + const saveBtn = this.getSaveButton(); + await saveBtn.click(); + } + + async close(): Promise { + const closeBtn = this.root.locator('.el-dialog__close').first(); + if (await closeBtn.isVisible()) { + await closeBtn.click(); + } + } + + /** + * Add a variable to the modal + * @param key - The variable key + * @param value - The variable value + * @param options - The options to pass to the modal + * @param options.closeDialog - Whether to close the modal after saving + */ + async addVariable( + key: string, + value: string, + { shouldSave }: { shouldSave: boolean } = { shouldSave: true }, + ): Promise { + await this.getKeyInput().fill(key); + await this.getValueInput().fill(value); + if (shouldSave) { + console.log('Saving variable from modal'); + await this.save(); + } + } +} diff --git a/packages/testing/playwright/tests/ui/23-variables.spec.ts b/packages/testing/playwright/tests/ui/23-variables.spec.ts index 8a14dfa03cd..4e4473fdf60 100644 --- a/packages/testing/playwright/tests/ui/23-variables.spec.ts +++ b/packages/testing/playwright/tests/ui/23-variables.spec.ts @@ -64,22 +64,17 @@ test.describe('Variables', () => { `ENV_BASE_${generateValidId()}`, 'base_value', ); + await expect(n8n.variables.getVariablesRows()).toHaveCount(1); const initialCount = await n8n.variables.getVariablesRows().count(); const key = `ENV_VAR_INVALID_${generateValidId()}$`; // Invalid key with special character const value = 'test_value'; - await n8n.variables.getCreateVariableButton().click(); - const editingRow = n8n.variables.getVariablesEditableRows().first(); - await n8n.variables.setRowValue(editingRow, 'key', key); - await n8n.variables.setRowValue(editingRow, 'value', value); - await n8n.variables.saveRowEditing(editingRow); + await n8n.variables.createVariable(key, value, { shouldSave: false }); + const saveButton = n8n.variables.variableModal.getSaveButton(); + await expect(saveButton).toBeDisabled(); + await n8n.variables.variableModal.close(); - await expect(editingRow).toContainText( - 'This field may contain only letters, numbers, and underscores', - ); - - await n8n.variables.cancelRowEditing(editingRow); await expect(n8n.variables.getVariablesRows()).toHaveCount(initialCount); }); @@ -90,10 +85,7 @@ test.describe('Variables', () => { const newValue = 'updated_value'; - await n8n.variables.editRow(key); - const editingRow = n8n.variables.getVariablesEditableRows().first(); - await n8n.variables.setRowValue(editingRow, 'value', newValue); - await n8n.variables.saveRowEditing(editingRow); + await n8n.variables.editVariable(key, newValue, { shouldSave: true }); const variableRow = n8n.variables.getVariableRow(key); await expect(variableRow).toContainText(newValue); @@ -105,6 +97,7 @@ test.describe('Variables', () => { const value = 'delete_test_value'; await n8n.variables.createVariableFromEmptyState(key, value); + await expect(n8n.variables.getVariablesRows()).toHaveCount(1); const initialCount = await n8n.variables.getVariablesRows().count(); await n8n.variables.deleteVariable(key);