diff --git a/.agents/skills/ui-design/SKILL.md b/.agents/skills/ui-design/SKILL.md index ea2d602bc2b..bef1818b66d 100644 --- a/.agents/skills/ui-design/SKILL.md +++ b/.agents/skills/ui-design/SKILL.md @@ -13,7 +13,18 @@ Comprehensive guide for building, styling, and using components in the frontend. - When reviewing UI changes, use `rules/web-interface-guidelines.md` ## Best practices -- ALWAYS use CSS variables for styles from `packages/frontend/@n8n/design-system/src/css/_tokens.scss` or `packages/frontend/@n8n/design-system/src/css/_primitives.scss`. Use hard-coded values only when no suitable tokens. +- ALWAYS use CSS variables from `_tokens.scss` or `_primitives.scss` in + `packages/frontend/@n8n/design-system/src/css/`. Look up every length, color, + font-size and duration before you write it as a literal. Each spacing token + carries its px value in a comment: `--spacing--5xs: 0.125rem; /** 2px **/`. + Grep that comment to find the token for a px value. The grep gives one hit for + each token family, so pick the family that matches the property: `--spacing--*` + for margin and padding, `--radius--*` for border-radius, `--height--*` for + height. The styleguide + `packages/frontend/@n8n/design-system/src/styleguide/spacing-size.mdx` lists the + same scale as a table. The spacing scale starts at 2px, so a small value is not + a reason to hardcode. Hardcode only if the lookup finds no token. Give the + reason in a comment. - ALWAYS prefer using existing components from `packages/frontend/@n8n/design-system/src/components`. Prefer components that aren't marked `@deprecated`. - If you need to add hover/active alpha behavior to solid components, prefer `color-mix()` with explicit percentages. - When working with animations or transitions, ALWAYS prefer using mixins from `packages/frontend/@n8n/design-system/src/css/mixins/motion.scss` diff --git a/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts b/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts index d998c3f224c..97f14e95c47 100644 --- a/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts +++ b/packages/@n8n/permissions/src/roles/__tests__/custom-role-scopes.test.ts @@ -104,6 +104,28 @@ describe('custom role scope whitelists', () => { } }); + it('exposes "Tags: View" as exactly the tag read/list pair', () => { + expect(GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.tag.View).toEqual(['tag:read', 'tag:list']); + }); + + it('keeps tag "Manage" a strict superset of tag "View"', () => { + // The editor's implied/downgrade arithmetic (SUPERSEDED_BY: View -> Manage) + // only holds while Manage contains everything View grants. + const view = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.tag.View; + const manage = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.tag.Manage; + expect(view.every((scope) => (manage as readonly string[]).includes(scope))).toBe(true); + expect(manage.length).toBeGreaterThan(view.length); + }); + + it('keeps "Tags: View" within GLOBAL_MEMBER_SCOPES', () => { + // "Tags: View" is granted to every instance role by default (see + // instanceRoleScopes.ts), so it must never exceed what the built-in Member + // role already has. + for (const scope of GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.tag.View) { + expect(GLOBAL_MEMBER_SCOPES).toContain(scope); + } + }); + it('exposes "Users: View" as exactly user:list, matching GLOBAL_MEMBER_SCOPES', () => { // "Users: View" is granted to every instance role by default (see // instanceRoleScopes.ts). It must never exceed what the built-in Member diff --git a/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts b/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts index 2e6039af463..3d6adfcef65 100644 --- a/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/custom-role-scopes.ee.ts @@ -161,9 +161,12 @@ export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = { 'Manage all': ['apiKey:create', 'apiKey:update', 'apiKey:manage'], }, tag: { - // read/list are bundled with write scopes: tags on workflows you can already - // read are always visible (they come embedded in the workflow response), so - // there is no meaningful "view-only" tier for tag definitions. + // Tags on a workflow you can read come embedded in the workflow response, and + // applying one rides on workflow:update, not a tag scope. read/list gate only + // the tag *picker* — listing every existing tag to choose from — which every + // role gets (see MANDATORY_INSTANCE_OPTIONS in instanceRoleScopes.ts). Manage + // keeps read/list so it stays a strict superset of View, matching `user`. + View: ['tag:read', 'tag:list'], Manage: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete'], }, project: { diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 18304d703b0..d3a01b27f15 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3040,6 +3040,7 @@ "instanceRoles.description.role.manage": "Create, edit, and delete all custom roles (instance and project).", "instanceRoles.description.apiKey.manageOwn": "Create and edit the user's own API keys", "instanceRoles.description.apiKey.manageAll": "View and delete other users' API keys.", + "instanceRoles.description.tag.view": "Users always see and can select any tags associated with workflows they have access to.", "instanceRoles.description.tag.manage": "Create, edit, and delete tags", "instanceRoles.description.project.create": "Create new projects", "instanceRoles.description.insights.view": "View instance-level insights", @@ -4408,6 +4409,7 @@ "tagsDropdown.createTag": "Create tag \"{filter}\"", "tagsDropdown.manageTags": "Manage tags", "tagsDropdown.noMatchingTagsExist": "No matching tags exist", + "tagsDropdown.noPermissionToCreate": "You don't have permission to create tags", "tagsDropdown.noTagsExist": "No tags exist", "tagsDropdown.showError.message": "A problem occurred when trying to create the '{name}' tag", "tagsDropdown.showError.title": "Could not create tag", diff --git a/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts b/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts index fc5554b1485..5b95da9a4fd 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/InstanceRoleView.test.ts @@ -42,7 +42,8 @@ const mockCustomRole = { displayName: 'Support', slug: 'support', description: 'A custom instance role', - scopes: ['user:list'], + // The mandatory options every instance role carries — see instanceRoleScopes.ts. + scopes: ['user:list', 'tag:read', 'tag:list'], licensed: true, systemRole: false, roleType: 'global' as const, @@ -95,8 +96,8 @@ describe('InstanceRoleView', () => { expect(rolesStore.createRole).toHaveBeenCalledWith({ displayName: 'Support', description: '', - // "Users: View" is mandatory on every instance role — see instanceRoleScopes.ts. - scopes: ['user:list'], + // Mandatory on every instance role — see instanceRoleScopes.ts. + scopes: ['user:list', 'tag:read', 'tag:list'], roleType: 'global', }); }); @@ -190,7 +191,7 @@ describe('InstanceRoleView', () => { expect(rolesStore.updateRole).toHaveBeenCalledWith('support', { displayName: 'Support 2', description: 'A custom instance role', - scopes: ['user:list'], + scopes: ['user:list', 'tag:read', 'tag:list'], }); }); expect(mockShowMessage).toHaveBeenCalledWith({ @@ -226,7 +227,7 @@ describe('InstanceRoleView', () => { expect(rolesStore.updateRole).toHaveBeenCalledWith('support', { displayName: 'Support 2', description: 'A custom instance role', - scopes: ['user:list'], + scopes: ['user:list', 'tag:read', 'tag:list'], }); }); }); @@ -302,7 +303,7 @@ describe('InstanceRoleView', () => { await waitFor(() => expect(rolesStore.updateRole).toHaveBeenCalled()); }); - it('does not mark a role unsaved when Users: View is already stored', async () => { + it('does not mark a role unsaved when the mandatory scopes are already stored', async () => { rolesStore.fetchRoleBySlug.mockResolvedValue(mockCustomRole); const { getByRole, container } = renderComponent({ props: { roleSlug: 'support' } }); @@ -318,7 +319,7 @@ describe('InstanceRoleView', () => { it('does not mark a role unsaved when only non-editor scopes were stripped', async () => { rolesStore.fetchRoleBySlug.mockResolvedValue({ ...mockCustomRole, - scopes: ['user:list', 'workflow:read'], + scopes: ['user:list', 'tag:read', 'tag:list', 'workflow:read'], }); const { getByRole, container } = renderComponent({ props: { roleSlug: 'support' } }); @@ -331,7 +332,7 @@ describe('InstanceRoleView', () => { expect(getByRole('button', { name: 'Save' })).toBeDisabled(); }); - it('enables save when a stored role is missing mandatory Users: View scopes', async () => { + it('enables save when a stored role is missing mandatory scopes', async () => { const legacyRole = { ...mockCustomRole, scopes: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete'], @@ -362,6 +363,38 @@ describe('InstanceRoleView', () => { }); }); + it('enables save for a role stored before Tags: View became mandatory', async () => { + // A custom role saved on an earlier release: it has Users: View but no + // tag scopes at all. The editor shows Tags: View checked and offers Save, + // which is what writes the scopes — there is no backfill migration. + const preTagViewRole = { ...mockCustomRole, scopes: ['user:list'] }; + rolesStore.fetchRoleBySlug.mockResolvedValue(preTagViewRole); + rolesStore.updateRole.mockResolvedValueOnce({ + ...preTagViewRole, + scopes: ['user:list', 'tag:read', 'tag:list'], + }); + + const { getByRole, getByTestId } = renderComponent({ props: { roleSlug: 'support' } }); + + await waitFor(() => { + expect(getByTestId('scope-option-tag-view').getAttribute('aria-checked')).toBe('true'); + }); + expect(getByTestId('scope-option-tag-view').hasAttribute('disabled')).toBe(true); + + const save = getByRole('button', { name: 'Save' }); + expect(save).toBeEnabled(); + + await userEvent.click(save); + + await waitFor(() => { + expect(rolesStore.updateRole).toHaveBeenCalledWith('support', { + displayName: 'Support', + description: 'A custom instance role', + scopes: ['user:list', 'tag:read', 'tag:list'], + }); + }); + }); + it('falls back to the count from page load when the pre-save fetch fails', async () => { rolesStore.fetchRoleBySlug .mockResolvedValueOnce({ ...mockCustomRole, usedByUsers: 8 }) diff --git a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts index ee576b14429..bc4e0caf4da 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.test.ts @@ -4,7 +4,9 @@ import userEvent from '@testing-library/user-event'; import { waitFor } from '@testing-library/vue'; import ScopeGroupSelector from './ScopeGroupSelector.vue'; import { INSTANCE_SCOPE_GROUP_LIST, INSTANCE_SCOPE_GROUPS } from '../instanceRoleScopes'; +import { GLOBAL_MEMBER_SCOPES } from '@n8n/permissions'; import { CUSTOM_ROLES_DOCS_URL } from '@/app/constants'; +import { getTooltip } from '@/__tests__/utils'; const totalOptions = INSTANCE_SCOPE_GROUP_LIST.reduce((sum, g) => sum + g.options.length, 0); @@ -53,7 +55,7 @@ describe('ScopeGroupSelector', () => { expect(scopes).toHaveLength(5); }); - it('emits the option scopes removed when toggling a checked option off', async () => { + it('downgrades to "View" when toggling a checked option off', async () => { const { getByTestId, emitted } = renderComponent(ScopeGroupSelector, { props: { modelValue: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete', 'user:read'], @@ -64,7 +66,7 @@ describe('ScopeGroupSelector', () => { await waitFor(() => expect(emitted()['update:modelValue']).toBeTruthy()); const [scopes] = emitted()['update:modelValue'][0] as [string[]]; - expect(scopes).toEqual(['user:read']); + expect(scopes).toEqual(['user:read', 'tag:read', 'tag:list']); }); it('completes the full resolved set when toggling an indeterminate option', async () => { @@ -321,29 +323,60 @@ describe('ScopeGroupSelector', () => { }); }); - describe('mandatory "Users: View" option', () => { - // The caller (InstanceRoleView's `withMandatoryInstanceScopes`) is what - // guarantees these scopes are always in `modelValue` — the selector itself - // stays a pure function of its props, same as every other option. - const withUserView = [...INSTANCE_SCOPE_GROUPS.user.View]; - + // The caller (InstanceRoleView's `withMandatoryInstanceScopes`) is what + // guarantees these scopes are always in `modelValue` — the selector itself + // stays a pure function of its props, same as every other option. + describe.each([ + ['Users: View', 'scope-option-user-view', [...INSTANCE_SCOPE_GROUPS.user.View]], + ['Tags: View', 'scope-option-tag-view', [...INSTANCE_SCOPE_GROUPS.tag.View]], + ])('mandatory "%s" option', (_label, testId, scopes) => { it('renders checked and disabled', () => { const { getByTestId } = renderComponent(ScopeGroupSelector, { - props: { modelValue: withUserView }, + props: { modelValue: scopes }, }); - const userView = getByTestId('scope-option-user-view'); - expect(userView.getAttribute('aria-checked')).toBe('true'); - expect(userView.hasAttribute('disabled')).toBe(true); + const option = getByTestId(testId); + expect(option.getAttribute('aria-checked')).toBe('true'); + expect(option.hasAttribute('disabled')).toBe(true); }); it('does not emit an update when clicked', async () => { const { getByTestId, emitted } = renderComponent(ScopeGroupSelector, { - props: { modelValue: withUserView }, + props: { modelValue: scopes }, }); - await userEvent.click(getByTestId('scope-option-user-view')); + await userEvent.click(getByTestId(testId)); expect(emitted()['update:modelValue']).toBeFalsy(); }); }); + + describe('tag View/Manage tiering', () => { + it('keeps the mandatory tooltip on "Tags: View" even when "Manage" is checked', async () => { + // Without the mandatory-first ordering in `optionTooltip`, this reads + // "Included in Manage" — which implies unchecking Manage would remove + // View, and View can never be removed. + const { getByTestId } = renderComponent(ScopeGroupSelector, { + props: { modelValue: [...INSTANCE_SCOPE_GROUPS.tag.Manage] }, + }); + + await userEvent.hover(getByTestId('scope-option-tag-view')); + + await waitFor(() => + expect(getTooltip()).toHaveTextContent( + 'Users always see and can select any tags associated with workflows they have access to.', + ), + ); + }); + + it('shows the built-in Member role as Tags View checked and Tags Manage mixed', () => { + // Member holds 4 of the 5 tag scopes (no tag:delete), so Manage stays + // half-checked. That gap is IAM-1383's, not this View split's — pinned + // here so nobody closes it by accident. + const { getByTestId } = renderComponent(ScopeGroupSelector, { + props: { modelValue: [...GLOBAL_MEMBER_SCOPES] }, + }); + expect(getByTestId('scope-option-tag-view').getAttribute('aria-checked')).toBe('true'); + expect(getByTestId('scope-option-tag-manage').getAttribute('aria-checked')).toBe('mixed'); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue index 04a4835a76d..9747050d532 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue +++ b/packages/frontend/editor-ui/src/features/roles/instance/components/ScopeGroupSelector.vue @@ -9,6 +9,7 @@ import { getEscalationWarningKey, isOptionImplied, isOptionMandatory, + mandatoryOptionTooltipKey, resolveOptionState, toggleOptionInGroup, type InstanceResource, @@ -48,23 +49,24 @@ function impliedTooltip(option: InstanceScopeOption, groupOptions: InstanceScope } /** - * Tooltip shown for a permission option. When the option is implied by another - * (e.g. "Manage own" under a checked "Manage all") the "Included in …" note - * takes precedence; a mandatory option (granted to every role, see - * `isOptionMandatory`) explains why it can't be turned off; otherwise it - * explains what the permission grants. + * Tooltip shown for a permission option. A mandatory option (granted to every + * role, see `isOptionMandatory`) explains why it can't be turned off; an option + * implied by another (e.g. "Manage own" under a checked "Manage all") shows the + * "Included in …" note; otherwise it explains what the permission grants. */ function optionTooltip( resource: InstanceResource, option: InstanceScopeOption, groupOptions: InstanceScopeOption[], ): string { + // Mandatory wins over "Included in …": for an option that can never be unchecked, + // saying it is included in another implies unchecking that other one would remove + // it, which is false. + const mandatoryKey = mandatoryOptionTooltipKey(resource, option); + if (mandatoryKey) return i18n.baseText(mandatoryKey); if (isOptionImplied(option, groupOptions, props.modelValue)) { return impliedTooltip(option, groupOptions); } - if (isOptionMandatory(resource, option)) { - return i18n.baseText('instanceRoles.option.mandatory'); - } return option.descriptionKey ? i18n.baseText(option.descriptionKey) : ''; } diff --git a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts index dd25972dcb7..41dc30dbaa3 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.test.ts @@ -8,6 +8,7 @@ import { getEscalationWarningKey, isOptionImplied, isOptionMandatory, + mandatoryOptionTooltipKey, resolveOptionState, toggleOptionInGroup, withMandatoryInstanceScopes, @@ -80,7 +81,7 @@ describe('instanceRoleScopes config', () => { it('exposes the configured option labels per resource', () => { expect(Object.keys(INSTANCE_SCOPE_GROUPS.apiKey)).toEqual(['Manage own', 'Manage all']); - expect(Object.keys(INSTANCE_SCOPE_GROUPS.tag)).toEqual(['Manage']); + expect(Object.keys(INSTANCE_SCOPE_GROUPS.tag)).toEqual(['View', 'Manage']); expect(Object.keys(INSTANCE_SCOPE_GROUPS.role)).toEqual(['Manage project roles', 'Manage']); expect(Object.keys(INSTANCE_SCOPE_GROUPS.project)).toEqual(['Create']); expect(Object.keys(INSTANCE_SCOPE_GROUPS.settings)).toEqual([ @@ -136,7 +137,7 @@ describe('isOptionImplied', () => { expect(isOptionImplied(manageOwn, apiKeyGroup.options, allScopes)).toBe(true); }); - it('returns false for options in groups with no superseding relationships', () => { + it('returns false for a superseding option, even with its own scopes fully present', () => { const tagGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'tag')!; const tagManage = tagGroup.options.find((o) => o.key === 'Manage')!; expect( @@ -191,10 +192,15 @@ describe('resolveOptionState', () => { expect(resolveOptionState(manageAll, apiKeyGroup.options, partialOwn)).toBe('indeterminate'); }); - it('does not affect unrelated groups', () => { + it('applies the same View/Manage arithmetic to tags', () => { const tagGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'tag')!; const tagManage = tagGroup.options.find((o) => o.key === 'Manage')!; + // 1 of View's 2 scopes — View isn't fully checked, so this stays a genuine partial. expect(resolveOptionState(tagManage, tagGroup.options, ['tag:read'])).toBe('indeterminate'); + // View fully checked: "Manage" would read 2/5, but both scopes are View's. + expect(resolveOptionState(tagManage, tagGroup.options, ['tag:read', 'tag:list'])).toBe( + 'unchecked', + ); expect( resolveOptionState(tagManage, tagGroup.options, [ 'tag:read', @@ -290,6 +296,17 @@ describe('toggleOptionInGroup', () => { }); it('removes the whole scope set when the option has no subordinate', () => { + const insightsGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'insights')!; + const insightsView = insightsGroup.options.find((o) => o.key === 'View')!; + const result = toggleOptionInGroup( + ['user:read', ...insightsView.scopes], + insightsView, + insightsGroup.options, + ); + expect(result).toEqual(['user:read']); + }); + + it('downgrades to "View" when unchecking tag "Manage", keeping the mandatory scopes', () => { const tagGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'tag')!; const manageTags = tagGroup.options.find((o) => o.key === 'Manage')!; const result = toggleOptionInGroup( @@ -297,7 +314,7 @@ describe('toggleOptionInGroup', () => { manageTags, tagGroup.options, ); - expect(result).toEqual(['user:read']); + expect(result).toEqual(['user:read', 'tag:read', 'tag:list']); }); it('does not mutate the input array', () => { @@ -359,30 +376,66 @@ describe('mandatory instance options', () => { const userView = userGroup.options.find((o) => o.key === 'View')!; const userManage = userGroup.options.find((o) => o.key === 'Manage')!; - it('flags "Users: View" as mandatory and every other option as not', () => { + const tagGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'tag')!; + const tagView = tagGroup.options.find((o) => o.key === 'View')!; + const tagManage = tagGroup.options.find((o) => o.key === 'Manage')!; + + const MANDATORY = [ + ['user', 'View'], + ['tag', 'View'], + ] as const; + + it('flags "Users: View" and "Tags: View" as mandatory and every other option as not', () => { expect(isOptionMandatory('user', userView)).toBe(true); + expect(isOptionMandatory('tag', tagView)).toBe(true); expect(isOptionMandatory('user', userManage)).toBe(false); + expect(isOptionMandatory('tag', tagManage)).toBe(false); for (const group of INSTANCE_SCOPE_GROUP_LIST) { for (const option of group.options) { - if (group.resource === 'user' && option.key === 'View') continue; + if (MANDATORY.some(([r, k]) => r === group.resource && k === option.key)) continue; expect(isOptionMandatory(group.resource, option)).toBe(false); } } }); - it('withMandatoryInstanceScopes adds "Users: View" scopes to an empty list', () => { - expect(new Set(withMandatoryInstanceScopes([]))).toEqual(new Set(userView.scopes)); + it('resolves every mandatory entry to a real option in its group (typo guard)', () => { + for (const [resource, key] of MANDATORY) { + const group = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === resource); + expect(group?.options.some((o) => o.key === key)).toBe(true); + } + }); + + it('gives a mandatory option its override tooltip, or its own description as fallback', () => { + expect(mandatoryOptionTooltipKey('user', userView)).toBe('instanceRoles.option.mandatory'); + expect(mandatoryOptionTooltipKey('tag', tagView)).toBe('instanceRoles.description.tag.view'); + }); + + it('returns no mandatory tooltip key for an optional option', () => { + expect(mandatoryOptionTooltipKey('user', userManage)).toBeUndefined(); + expect(mandatoryOptionTooltipKey('tag', tagManage)).toBeUndefined(); + }); + + it("withMandatoryInstanceScopes adds every mandatory option's scopes to an empty list", () => { + expect(withMandatoryInstanceScopes([])).toEqual(['user:list', 'tag:read', 'tag:list']); }); it('withMandatoryInstanceScopes does not duplicate scopes already present', () => { const withDuplicate = withMandatoryInstanceScopes(['user:list', 'tag:read']); expect(withDuplicate.filter((s) => s === 'user:list')).toHaveLength(1); + expect(withDuplicate.filter((s) => s === 'tag:read')).toHaveLength(1); expect(withDuplicate).toEqual(expect.arrayContaining(['user:list', 'tag:read'])); }); it('withMandatoryInstanceScopes preserves unrelated scopes untouched', () => { - const result = withMandatoryInstanceScopes(['tag:read', 'tag:list']); - expect(result).toEqual(expect.arrayContaining(['tag:read', 'tag:list', ...userView.scopes])); + const result = withMandatoryInstanceScopes(['insights:read', 'insights:list']); + expect(result).toEqual( + expect.arrayContaining([ + 'insights:read', + 'insights:list', + ...userView.scopes, + ...tagView.scopes, + ]), + ); }); }); diff --git a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts index 5c7866c130d..5e0a5ab1bfd 100644 --- a/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts +++ b/packages/frontend/editor-ui/src/features/roles/instance/instanceRoleScopes.ts @@ -91,7 +91,10 @@ export const INSTANCE_OPTION_DESCRIPTION_KEYS: Partial< 'Manage own': 'instanceRoles.description.apiKey.manageOwn', 'Manage all': 'instanceRoles.description.apiKey.manageAll', }, - tag: { Manage: 'instanceRoles.description.tag.manage' }, + tag: { + View: 'instanceRoles.description.tag.view', + Manage: 'instanceRoles.description.tag.manage', + }, project: { Create: 'instanceRoles.description.project.create' }, insights: { View: 'instanceRoles.description.insights.view' }, }; @@ -155,18 +158,51 @@ export const ALL_INSTANCE_SCOPES: Scope[] = [ ...new Set(INSTANCE_SCOPE_GROUP_LIST.flatMap((g) => g.options.flatMap((o) => o.scopes))), ]; +type MandatoryInstanceOption = { + resource: InstanceResource; + option: string; + /** Overrides the option's own description where the "why it's locked" wording differs. */ + tooltipKey?: BaseTextKey; +}; + /** - * "Users: View" is baseline behavior every instance role carries — the default - * Member role already has it — not something a custom role can opt out of. - * Rendered checked and disabled in the editor. `withMandatoryInstanceScopes` - * is applied to the form (and on save), not the persisted snapshot, so a - * stored role that is missing these scopes stays unsaved until the next save. + * Options every instance role carries regardless of what is saved — the default + * Member role already grants them, so they are baseline behaviour rather than + * something a custom role opts into. Rendered checked and disabled. + * `withMandatoryInstanceScopes` is applied to the form (and on save), not the + * persisted snapshot, so a stored role missing these scopes stays unsaved until + * the next save. */ -export function isOptionMandatory(resource: InstanceResource, option: InstanceScopeOption) { - return resource === 'user' && option.key === 'View'; +const MANDATORY_INSTANCE_OPTIONS: readonly MandatoryInstanceOption[] = [ + { resource: 'user', option: 'View', tooltipKey: 'instanceRoles.option.mandatory' }, + { resource: 'tag', option: 'View' }, // falls back to instanceRoles.description.tag.view +]; + +/** Tooltip key for a mandatory option, or undefined when the option is not mandatory. */ +export function mandatoryOptionTooltipKey( + resource: InstanceResource, + option: InstanceScopeOption, +): BaseTextKey | undefined { + const entry = MANDATORY_INSTANCE_OPTIONS.find( + (m) => m.resource === resource && m.option === option.key, + ); + return entry ? (entry.tooltipKey ?? option.descriptionKey) : undefined; } -const MANDATORY_INSTANCE_SCOPES: readonly Scope[] = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.user.View; +export function isOptionMandatory(resource: InstanceResource, option: InstanceScopeOption) { + return MANDATORY_INSTANCE_OPTIONS.some((m) => m.resource === resource && m.option === option.key); +} + +/** Derived from the group list so a typo in the table contributes nothing silently. */ +const MANDATORY_INSTANCE_SCOPES: readonly Scope[] = [ + ...new Set( + INSTANCE_SCOPE_GROUP_LIST.flatMap((group) => + group.options + .filter((option) => isOptionMandatory(group.resource, option)) + .flatMap((option) => option.scopes), + ), + ), +]; /** Unions in the mandatory scopes (see `isOptionMandatory`) on top of an already-filtered scope list. */ export function withMandatoryInstanceScopes(scopes: readonly string[]): string[] { diff --git a/packages/frontend/editor-ui/src/features/shared/tags/components/AnnotationTagsDropdown.ee.test.ts b/packages/frontend/editor-ui/src/features/shared/tags/components/AnnotationTagsDropdown.ee.test.ts new file mode 100644 index 00000000000..8a94ffdf059 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/shared/tags/components/AnnotationTagsDropdown.ee.test.ts @@ -0,0 +1,62 @@ +import { createTestingPinia } from '@pinia/testing'; +import { renderComponent } from '@/__tests__/render'; +import AnnotationTagsDropdown from './AnnotationTagsDropdown.ee.vue'; +import { mockedStore } from '@/__tests__/utils'; +import { useAnnotationTagsStore } from '../tags.store'; + +const hasPermission = vi.hoisted(() => + vi.fn<(_checks: unknown, opts?: { rbac?: { scope?: string } }) => boolean>(() => true), +); + +vi.mock('@/app/utils/rbac/permissions', () => ({ + hasPermission, +})); + +// Stub the inner TagsDropdown so we can inspect the props the wrapper passes to it +// without triggering Element Plus teleport/select complexity. +const capturedProps: Array> = []; +vi.mock('./TagsDropdown.vue', () => ({ + default: { + name: 'TagsDropdown', + props: { + createEnabled: Boolean, + createBlockedI18nKey: String, + manageEnabled: Boolean, + allTags: Array, + isLoading: Boolean, + tagsById: Object, + modelValue: Array, + placeholder: String, + eventBus: Object, + createTag: Function, + }, + setup(props: Record) { + capturedProps.push({ ...props }); + }, + template: '
', + }, +})); + +describe('AnnotationTagsDropdown', () => { + beforeEach(() => { + createTestingPinia(); + capturedProps.length = 0; + + const store = mockedStore(useAnnotationTagsStore); + store.fetchAll = vi.fn().mockResolvedValue([]); + store.allTags = []; + store.isLoading = false; + store.tagsById = {}; + }); + + it('never passes createBlockedI18nKey, whatever the annotation tag scopes are', () => { + // The permission note is worded for workflow tags and driven by `tag:create`. + // Annotation tags have their own scopes, so wiring them up is a separate change. + hasPermission.mockReturnValue(false); + renderComponent(AnnotationTagsDropdown, { props: { createEnabled: true } }); + + expect(capturedProps).toHaveLength(1); + expect(capturedProps.at(-1)?.createEnabled).toBe(false); + expect(capturedProps.at(-1)?.createBlockedI18nKey).toBeUndefined(); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.emptyState.test.ts b/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.emptyState.test.ts new file mode 100644 index 00000000000..232892db6c8 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.emptyState.test.ts @@ -0,0 +1,108 @@ +import { createTestingPinia } from '@pinia/testing'; +import userEvent from '@testing-library/user-event'; +import { waitFor } from '@testing-library/vue'; + +import { renderComponent } from '@/__tests__/render'; +import TagsDropdown from './TagsDropdown.vue'; +import type { ITag } from '@n8n/rest-api-client/api/tags'; +import type { BaseTextKey } from '@n8n/i18n'; + +/** + * Covers the dropdown's empty state — the message shown in place of the option + * list, and the note explaining why "Create tag …" is missing. + * + * `createBlockedI18nKey` is opt-in per consumer: TagsDropdown is also the model + * picker on the Chat settings page and the tag *filter* on the workflow list, + * neither of which has a permission problem to report. + */ +const ALL_TAGS: ITag[] = [ + { id: '1', name: 'alpha', createdAt: '', updatedAt: '' }, + { id: '2', name: 'beta', createdAt: '', updatedAt: '' }, +]; + +const NO_PERMISSION_NOTE = "You don't have permission to create tags"; + +type Overrides = { + allTags?: ITag[]; + createEnabled?: boolean; + createBlockedI18nKey?: BaseTextKey; +}; + +const renderDropdown = (overrides: Overrides = {}) => + renderComponent(TagsDropdown, { + pinia: createTestingPinia(), + props: { + placeholder: 'pick tags', + modelValue: [], + eventBus: null, + allTags: [], + isLoading: false, + tagsById: {}, + manageEnabled: false, + createEnabled: false, + ...overrides, + }, + }); + +/** The dropdown panel is teleported out of the component's own container. */ +const dropdownText = () => document.querySelector('.tags-dropdown')?.textContent ?? ''; + +const typeFilter = async (container: Element, text: string) => { + const input = container.querySelector('input'); + if (!input) throw new Error('Found no filter input'); + await userEvent.type(input, text); +}; + +describe('TagsDropdown - empty state', () => { + it('shows "No tags exist" and the note when the instance has no listable tags', async () => { + renderDropdown({ createBlockedI18nKey: 'tagsDropdown.noPermissionToCreate' }); + + await waitFor(() => expect(dropdownText()).toContain('No tags exist')); + expect(dropdownText()).toContain(NO_PERMISSION_NOTE); + }); + + it('keeps the note while a filter is typed and no tags are listable', async () => { + const { container } = renderDropdown({ + createBlockedI18nKey: 'tagsDropdown.noPermissionToCreate', + }); + + await typeFilter(container, 'anything'); + + await waitFor(() => expect(dropdownText()).toContain('No tags exist')); + expect(dropdownText()).toContain(NO_PERMISSION_NOTE); + }); + + it('shows "No matching tags exist" and the note when the filter matches nothing', async () => { + const { container } = renderDropdown({ + allTags: ALL_TAGS, + createBlockedI18nKey: 'tagsDropdown.noPermissionToCreate', + }); + + await typeFilter(container, 'zzz'); + + await waitFor(() => expect(dropdownText()).toContain('No matching tags exist')); + expect(dropdownText()).toContain(NO_PERMISSION_NOTE); + }); + + it('shows no note when creation is off and no key is passed (workflow-list filter)', async () => { + renderDropdown(); + + await waitFor(() => expect(dropdownText()).toContain('No tags exist')); + expect(dropdownText()).not.toContain(NO_PERMISSION_NOTE); + }); + + it('keeps the create prompt, and never the note, when the user can create tags', async () => { + const { container } = renderDropdown({ + createEnabled: true, + createBlockedI18nKey: 'tagsDropdown.noPermissionToCreate', + }); + + await waitFor(() => expect(dropdownText()).toContain('Type to create a tag')); + expect(dropdownText()).not.toContain(NO_PERMISSION_NOTE); + + await typeFilter(container, 'gamma'); + + await waitFor(() => expect(dropdownText()).toContain('Create tag "gamma"')); + expect(dropdownText()).not.toContain(NO_PERMISSION_NOTE); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.vue b/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.vue index 0f4fcde8bed..62f5d957454 100644 --- a/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.vue +++ b/packages/frontend/editor-ui/src/features/shared/tags/components/TagsDropdown.vue @@ -22,6 +22,13 @@ interface TagsDropdownProps { createTag?: (name: string) => Promise; multipleLimit?: number; createTagI18nKey?: BaseTextKey; + /** + * i18n key for a note explaining why creating a tag isn't available, shown under + * "No tags exist". Set it only when the reason is a missing permission — leave it + * undefined when the consumer disabled creation itself (e.g. the workflow-list + * filter), or the note is a non-sequitur. + */ + createBlockedI18nKey?: BaseTextKey; } const i18n = useI18n(); @@ -37,6 +44,7 @@ const props = withDefaults(defineProps(), { createTag: undefined, multipleLimit: 0, createTagI18nKey: 'tagsDropdown.createTag', + createBlockedI18nKey: undefined, }); const emit = defineEmits<{ @@ -243,12 +251,18 @@ onClickOutside( {{ i18n.baseText(props.createTagI18nKey, { interpolate: { filter } }) }} - + {{ i18n.baseText('tagsDropdown.typeToCreateATag') }} - {{ - i18n.baseText('tagsDropdown.noMatchingTagsExist') - }} - {{ i18n.baseText('tagsDropdown.noTagsExist') }} + diff --git a/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.test.ts b/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.test.ts index 81474087bec..d7e4fff1252 100644 --- a/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.test.ts +++ b/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.test.ts @@ -21,6 +21,7 @@ vi.mock('./TagsDropdown.vue', () => ({ name: 'TagsDropdown', props: { createEnabled: Boolean, + createBlockedI18nKey: String, manageEnabled: Boolean, allTags: Array, isLoading: Boolean, @@ -81,6 +82,31 @@ describe('WorkflowTagsDropdown — scope-derived props', () => { }); }); + describe('createBlockedI18nKey', () => { + it('is set when the consumer wants creation but tag:create is denied', () => { + withScopes('tag:update', 'tag:delete'); + renderComponent(WorkflowTagsDropdown, { props: { createEnabled: true } }); + + expect(capturedProps.at(-1)?.createBlockedI18nKey).toBe('tagsDropdown.noPermissionToCreate'); + }); + + it('is undefined when tag:create is granted', () => { + withScopes('tag:create'); + renderComponent(WorkflowTagsDropdown, { props: { createEnabled: true } }); + + expect(capturedProps.at(-1)?.createBlockedI18nKey).toBeUndefined(); + }); + + it('is undefined when the consumer turned creation off itself', () => { + // The workflow-list tag filter passes createEnabled: false deliberately; + // a permission note there would be a non-sequitur. + withScopes(); + renderComponent(WorkflowTagsDropdown, { props: { createEnabled: false } }); + + expect(capturedProps.at(-1)?.createBlockedI18nKey).toBeUndefined(); + }); + }); + describe('manageEnabled', () => { it('is true when the user has tag:create', () => { withScopes('tag:create'); diff --git a/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.vue b/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.vue index 714df6f6974..d8eca14e928 100644 --- a/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.vue +++ b/packages/frontend/editor-ui/src/features/shared/tags/components/WorkflowTagsDropdown.vue @@ -6,6 +6,7 @@ import { useTagsStore } from '../tags.store'; import { TAGS_MANAGER_MODAL_KEY } from '../tags.constants'; import type { EventBus } from '@n8n/utils/event-bus'; import { useTagPermissions } from '../useTagPermissions'; +import type { BaseTextKey } from '@n8n/i18n'; interface TagsDropdownWrapperProps { placeholder?: string; @@ -42,6 +43,13 @@ const tagsById = computed(() => tagsStore.tagsById); const isCreateEnabled = computed(() => props.createEnabled && canCreate.value); const isManageEnabled = computed(() => canManage.value); +// Only when the user wanted to create but lacks the scope — a consumer that +// turned creation off itself (e.g. the workflow-list filter) gets no note. +const isCreateBlocked = computed(() => props.createEnabled && !canCreate.value); + +const createBlockedI18nKey = computed(() => + isCreateBlocked.value ? 'tagsDropdown.noPermissionToCreate' : undefined, +); async function createTag(name: string) { return await tagsStore.create(name); @@ -68,6 +76,7 @@ void tagsStore.fetchAll(); v-model="selectedTags" :placeholder="placeholder" :create-enabled="isCreateEnabled" + :create-blocked-i18n-key="createBlockedI18nKey" :manage-enabled="isManageEnabled" :event-bus="eventBus" :all-tags="allTags"