fix(editor): UX Improvements to RBAC feature set (#9683)

This commit is contained in:
Csaba Tuncsik
2024-07-18 14:17:27 +02:00
committed by GitHub
parent 5b440a7679
commit 028a8a2c75
32 changed files with 337 additions and 112 deletions

View File

@@ -7,6 +7,7 @@
:filters="filters"
:additional-filters-handler="onFilter"
:type-props="{ itemSize: 77 }"
:loading="loading"
@click:add="addCredential"
@update:filters="filters = $event"
>
@@ -79,8 +80,6 @@ import ProjectTabs from '@/components/Projects/ProjectTabs.vue';
import useEnvironmentsStore from '@/stores/environments.ee.store';
import { useSettingsStore } from '@/stores/settings.store';
type IResourcesListLayoutInstance = InstanceType<typeof ResourcesListLayout>;
export default defineComponent({
name: 'CredentialsView',
components: {
@@ -96,6 +95,7 @@ export default defineComponent({
type: '',
},
sourceControlStoreUnsubscribe: () => {},
loading: false,
};
},
computed: {
@@ -133,9 +133,6 @@ export default defineComponent({
},
},
watch: {
'filters.type'() {
this.sendFiltersTelemetry('type');
},
'$route.params.projectId'() {
void this.initialize();
},
@@ -161,6 +158,7 @@ export default defineComponent({
});
},
async initialize() {
this.loading = true;
const isVarsEnabled = useSettingsStore().isEnterpriseFeatureEnabled(
EnterpriseEditionFeature.Variables,
);
@@ -176,6 +174,7 @@ export default defineComponent({
];
await Promise.all(loadPromises);
this.loading = false;
},
onFilter(
resource: ICredentialsResponse,
@@ -199,9 +198,6 @@ export default defineComponent({
return matches;
},
sendFiltersTelemetry(source: string) {
(this.$refs.layout as IResourcesListLayoutInstance).sendFiltersTelemetry(source);
},
},
});
</script>

View File

@@ -0,0 +1,126 @@
import { within } from '@testing-library/vue';
import { createPinia, setActivePinia } from 'pinia';
import userEvent from '@testing-library/user-event';
import { createComponentRenderer } from '@/__tests__/render';
import { getDropdownItems } from '@/__tests__/utils';
import { useRouter } from 'vue-router';
import ProjectSettings from '@/views/ProjectSettings.vue';
import { useProjectsStore } from '@/stores/projects.store';
import { VIEWS } from '@/constants';
import { useUsersStore } from '@/stores/users.store';
import { createProjectListItem } from '@/__tests__/data/projects';
import { useSettingsStore } from '@/stores/settings.store';
import type { IN8nUISettings } from 'n8n-workflow';
vi.mock('vue-router', () => {
const params = {};
const push = vi.fn();
return {
useRoute: () => ({
params,
}),
useRouter: () => ({
push,
}),
RouterLink: vi.fn(),
};
});
const renderComponent = createComponentRenderer(ProjectSettings);
const teamProjects = Array.from({ length: 3 }, () => createProjectListItem('team'));
let router: ReturnType<typeof useRouter>;
let projectsStore: ReturnType<typeof useProjectsStore>;
let usersStore: ReturnType<typeof useUsersStore>;
let settingsStore: ReturnType<typeof useSettingsStore>;
describe('ProjectSettings', () => {
beforeEach(() => {
const pinia = createPinia();
setActivePinia(pinia);
router = useRouter();
projectsStore = useProjectsStore();
usersStore = useUsersStore();
settingsStore = useSettingsStore();
vi.spyOn(usersStore, 'fetchUsers').mockImplementation(async () => await Promise.resolve());
vi.spyOn(projectsStore, 'getAllProjects').mockImplementation(
async () => await Promise.resolve(),
);
vi.spyOn(projectsStore, 'teamProjects', 'get').mockReturnValue(teamProjects);
vi.spyOn(settingsStore, 'settings', 'get').mockReturnValue({
enterprise: {
projects: {
team: {
limit: -1,
},
},
},
} as IN8nUISettings);
projectsStore.setCurrentProject({
id: '123',
type: 'team',
name: 'Test Project',
relations: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
scopes: [],
});
});
it('should show confirmation modal before deleting project and delete with transfer', async () => {
const deleteProjectSpy = vi
.spyOn(projectsStore, 'deleteProject')
.mockImplementation(async () => {});
const { getByTestId, getByRole } = renderComponent();
const deleteButton = getByTestId('project-settings-delete-button');
await userEvent.click(deleteButton);
expect(deleteProjectSpy).not.toHaveBeenCalled();
const modal = getByRole('dialog');
expect(modal).toBeVisible();
const confirmButton = getByTestId('project-settings-delete-confirm-button');
expect(confirmButton).toBeDisabled();
await userEvent.click(within(modal).getAllByRole('radio')[0]);
const projectSelect = getByTestId('project-sharing-select');
const projectSelectDropdownItems = await getDropdownItems(projectSelect);
await userEvent.click(projectSelectDropdownItems[0]);
expect(confirmButton).toBeEnabled();
await userEvent.click(confirmButton);
expect(deleteProjectSpy).toHaveBeenCalledWith('123', expect.any(String));
expect(router.push).toHaveBeenCalledWith({ name: VIEWS.HOMEPAGE });
});
it('should show confirmation modal before deleting project and deleting without transfer', async () => {
const deleteProjectSpy = vi
.spyOn(projectsStore, 'deleteProject')
.mockImplementation(async () => {});
const { getByTestId, getByRole } = renderComponent();
const deleteButton = getByTestId('project-settings-delete-button');
await userEvent.click(deleteButton);
expect(deleteProjectSpy).not.toHaveBeenCalled();
const modal = getByRole('dialog');
expect(modal).toBeVisible();
const confirmButton = getByTestId('project-settings-delete-confirm-button');
expect(confirmButton).toBeDisabled();
await userEvent.click(within(modal).getAllByRole('radio')[1]);
const input = within(modal).getByRole('textbox');
await userEvent.type(input, 'delete all ');
expect(confirmButton).toBeDisabled();
await userEvent.type(input, 'data');
expect(confirmButton).toBeEnabled();
await userEvent.click(confirmButton);
expect(deleteProjectSpy).toHaveBeenCalledWith('123', undefined);
expect(router.push).toHaveBeenCalledWith({ name: VIEWS.HOMEPAGE });
});
});

View File

@@ -0,0 +1,426 @@
<script lang="ts" setup>
import { computed, ref, watch, onBeforeMount, onMounted, nextTick } from 'vue';
import { useRouter } from 'vue-router';
import { deepCopy } from 'n8n-workflow';
import { N8nFormInput } from 'n8n-design-system';
import { useUsersStore } from '@/stores/users.store';
import type { IUser } from '@/Interface';
import { useI18n } from '@/composables/useI18n';
import { useProjectsStore } from '@/stores/projects.store';
import ProjectTabs from '@/components/Projects/ProjectTabs.vue';
import type { Project, ProjectRelation } from '@/types/projects.types';
import { useToast } from '@/composables/useToast';
import { VIEWS } from '@/constants';
import ProjectDeleteDialog from '@/components/Projects/ProjectDeleteDialog.vue';
import ProjectRoleUpgradeDialog from '@/components/Projects/ProjectRoleUpgradeDialog.vue';
import { useRolesStore } from '@/stores/roles.store';
import type { ProjectRole } from '@/types/roles.types';
import { useCloudPlanStore } from '@/stores/cloudPlan.store';
import { useTelemetry } from '@/composables/useTelemetry';
type FormDataDiff = {
name?: string;
role?: ProjectRelation[];
memberAdded?: ProjectRelation[];
memberRemoved?: ProjectRelation[];
};
const usersStore = useUsersStore();
const locale = useI18n();
const projectsStore = useProjectsStore();
const rolesStore = useRolesStore();
const cloudPlanStore = useCloudPlanStore();
const toast = useToast();
const router = useRouter();
const telemetry = useTelemetry();
const dialogVisible = ref(false);
const upgradeDialogVisible = ref(false);
const isDirty = ref(false);
const isValid = ref(false);
const formData = ref<Pick<Project, 'name' | 'relations'>>({
name: '',
relations: [],
});
const projectRoleTranslations = ref<{ [key: string]: string }>({
'project:editor': locale.baseText('projects.settings.role.editor'),
'project:admin': locale.baseText('projects.settings.role.admin'),
});
const nameInput = ref<InstanceType<typeof N8nFormInput> | null>(null);
const usersList = computed(() =>
usersStore.allUsers.filter((user: IUser) => {
const isAlreadySharedWithUser = (formData.value.relations || []).find(
(r: ProjectRelation) => r.id === user.id,
);
return !isAlreadySharedWithUser;
}),
);
const projects = computed(() =>
projectsStore.teamProjects.filter((project) => project.id !== projectsStore.currentProjectId),
);
const projectRoles = computed(() =>
rolesStore.processedProjectRoles.map((role) => ({
...role,
name: projectRoleTranslations.value[role.role],
})),
);
const firstLicensedRole = computed(() => projectRoles.value.find((role) => role.licensed)?.role);
const onAddMember = (userId: string) => {
isDirty.value = true;
const user = usersStore.usersById[userId];
if (!user) return;
const { id, firstName, lastName, email } = user;
const relation = { id, firstName, lastName, email } as ProjectRelation;
if (firstLicensedRole.value) {
relation.role = firstLicensedRole.value;
}
formData.value.relations.push(relation);
};
const onRoleAction = (user: Partial<IUser>, role: string) => {
isDirty.value = true;
const index = formData.value.relations.findIndex((r: ProjectRelation) => r.id === user.id);
if (role === 'remove') {
formData.value.relations.splice(index, 1);
} else {
formData.value.relations[index].role = role as ProjectRole;
}
};
const onNameInput = () => {
isDirty.value = true;
};
const onCancel = () => {
formData.value.relations = projectsStore.currentProject?.relations
? deepCopy(projectsStore.currentProject.relations)
: [];
formData.value.name = projectsStore.currentProject?.name ?? '';
isDirty.value = false;
};
const makeFormDataDiff = (): FormDataDiff => {
const diff: FormDataDiff = {};
if (!projectsStore.currentProject) {
return diff;
}
if (formData.value.name !== projectsStore.currentProject.name) {
diff.name = formData.value.name ?? '';
}
if (formData.value.relations.length !== projectsStore.currentProject.relations.length) {
diff.memberAdded = formData.value.relations.filter(
(r: ProjectRelation) => !projectsStore.currentProject?.relations.find((cr) => cr.id === r.id),
);
diff.memberRemoved = projectsStore.currentProject.relations.filter(
(cr: ProjectRelation) => !formData.value.relations.find((r) => r.id === cr.id),
);
}
diff.role = formData.value.relations.filter((r: ProjectRelation) => {
const currentRelation = projectsStore.currentProject?.relations.find((cr) => cr.id === r.id);
return currentRelation?.role !== r.role && !diff.memberAdded?.find((ar) => ar.id === r.id);
});
return diff;
};
const sendTelemetry = (diff: FormDataDiff) => {
if (diff.name) {
telemetry.track('User changed project name', {
project_id: projectsStore.currentProject?.id,
name: diff.name,
});
}
if (diff.memberAdded) {
diff.memberAdded.forEach((r) => {
telemetry.track('User added member to project', {
project_id: projectsStore.currentProject?.id,
target_user_id: r.id,
role: r.role,
});
});
}
if (diff.memberRemoved) {
diff.memberRemoved.forEach((r) => {
telemetry.track('User removed member from project', {
project_id: projectsStore.currentProject?.id,
target_user_id: r.id,
});
});
}
if (diff.role) {
diff.role.forEach((r) => {
telemetry.track('User changed member role on project', {
project_id: projectsStore.currentProject?.id,
target_user_id: r.id,
role: r.role,
});
});
}
};
const onSubmit = async () => {
try {
if (isDirty.value && projectsStore.currentProject) {
const diff = makeFormDataDiff();
await projectsStore.updateProject({
id: projectsStore.currentProject.id,
name: formData.value.name,
relations: formData.value.relations.map((r: ProjectRelation) => ({
userId: r.id,
role: r.role,
})),
});
sendTelemetry(diff);
isDirty.value = false;
toast.showMessage({
title: locale.baseText('projects.settings.save.successful.title', {
interpolate: { projectName: formData.value.name ?? '' },
}),
type: 'success',
});
}
} catch (error) {
toast.showError(error, locale.baseText('projects.settings.save.error.title'));
}
};
const onDelete = async () => {
await projectsStore.getAllProjects();
dialogVisible.value = true;
};
const onConfirmDelete = async (transferId?: string) => {
try {
if (projectsStore.currentProject) {
const projectName = projectsStore.currentProject?.name ?? '';
await projectsStore.deleteProject(projectsStore.currentProject.id, transferId);
await router.push({ name: VIEWS.HOMEPAGE });
toast.showMessage({
title: locale.baseText('projects.settings.delete.successful.title', {
interpolate: { projectName },
}),
type: 'success',
});
dialogVisible.value = true;
}
} catch (error) {
toast.showError(error, locale.baseText('projects.settings.delete.error.title'));
}
};
const selectProjectNameIfMatchesDefault = () => {
if (formData.value.name === locale.baseText('projects.settings.newProjectName')) {
nameInput.value?.inputRef?.focus();
nameInput.value?.inputRef?.select();
}
};
watch(
() => projectsStore.currentProject,
async () => {
formData.value.name = projectsStore.currentProject?.name ?? '';
formData.value.relations = projectsStore.currentProject?.relations
? deepCopy(projectsStore.currentProject.relations)
: [];
await nextTick();
selectProjectNameIfMatchesDefault();
},
{ immediate: true },
);
onBeforeMount(async () => {
await usersStore.fetchUsers();
});
onMounted(() => {
selectProjectNameIfMatchesDefault();
});
</script>
<template>
<div :class="$style.projectSettings">
<div :class="$style.header">
<ProjectTabs />
</div>
<form @submit.prevent="onSubmit">
<fieldset>
<label for="projectName">{{ locale.baseText('projects.settings.name') }}</label>
<N8nFormInput
id="projectName"
ref="nameInput"
v-model="formData.name"
label=""
type="text"
name="name"
required
data-test-id="project-settings-name-input"
@input="onNameInput"
@validate="isValid = $event"
/>
</fieldset>
<fieldset>
<label for="projectMembers">{{
locale.baseText('projects.settings.projectMembers')
}}</label>
<N8nUserSelect
id="projectMembers"
class="mb-s"
size="large"
:users="usersList"
:current-user-id="usersStore.currentUser?.id"
:placeholder="$locale.baseText('workflows.shareModal.select.placeholder')"
data-test-id="project-members-select"
@update:model-value="onAddMember"
>
<template #prefix>
<N8nIcon icon="search" />
</template>
</N8nUserSelect>
<N8nUsersList
:actions="[]"
:users="formData.relations"
:current-user-id="usersStore.currentUser?.id"
:delete-label="$locale.baseText('workflows.shareModal.list.delete')"
>
<template #actions="{ user }">
<div :class="$style.buttons">
<N8nSelect
class="mr-2xs"
:model-value="user?.role || projectRoles[0].role"
size="small"
@update:model-value="onRoleAction(user, $event)"
>
<N8nOption
v-for="role in projectRoles"
:key="role.role"
:value="role.role"
:label="role.name"
:disabled="!role.licensed"
>
{{ role.name
}}<span
v-if="!role.licensed"
:class="$style.upgrade"
@click="upgradeDialogVisible = true"
>
&nbsp;-&nbsp;{{ locale.baseText('generic.upgrade') }}
</span>
</N8nOption>
</N8nSelect>
<N8nButton
type="tertiary"
native-type="button"
square
icon="trash"
data-test-id="project-user-remove"
@click="onRoleAction(user, 'remove')"
/>
</div>
</template>
</N8nUsersList>
</fieldset>
<fieldset :class="$style.buttons">
<div>
<small v-if="isDirty" class="mr-2xs">{{
locale.baseText('projects.settings.message.unsavedChanges')
}}</small>
<N8nButton
:disabled="!isDirty"
type="secondary"
native-type="button"
class="mr-2xs"
data-test-id="project-settings-cancel-button"
@click.stop.prevent="onCancel"
>{{ locale.baseText('projects.settings.button.cancel') }}</N8nButton
>
</div>
<N8nButton
:disabled="!isDirty || !isValid"
type="primary"
data-test-id="project-settings-save-button"
>{{ locale.baseText('projects.settings.button.save') }}</N8nButton
>
</fieldset>
<fieldset>
<hr class="mb-2xl" />
<h3 class="mb-xs">{{ locale.baseText('projects.settings.danger.title') }}</h3>
<small>{{ locale.baseText('projects.settings.danger.message') }}</small>
<br />
<N8nButton
type="tertiary"
native-type="button"
class="mt-s"
data-test-id="project-settings-delete-button"
@click.stop.prevent="onDelete"
>{{ locale.baseText('projects.settings.danger.deleteProject') }}</N8nButton
>
</fieldset>
</form>
<ProjectDeleteDialog
v-model="dialogVisible"
:current-project="projectsStore.currentProject"
:projects="projects"
@confirm-delete="onConfirmDelete"
/>
<ProjectRoleUpgradeDialog
v-model="upgradeDialogVisible"
:limit="projectsStore.teamProjectsLimit"
:plan-name="cloudPlanStore.currentPlanData?.displayName"
/>
</div>
</template>
<style lang="scss" module>
.projectSettings {
display: grid;
width: 100%;
justify-items: center;
grid-auto-rows: max-content;
form {
width: 100%;
max-width: 1280px;
padding: 0 var(--spacing-2xl);
fieldset {
padding-bottom: var(--spacing-2xl);
label {
display: block;
margin-bottom: var(--spacing-xs);
font-size: var(--font-size-xl);
}
}
}
}
.header {
width: 100%;
max-width: 1280px;
padding: var(--spacing-l) var(--spacing-2xl) 0;
}
.upgrade {
cursor: pointer;
}
.buttons {
display: flex;
justify-content: flex-end;
align-items: center;
}
</style>

View File

@@ -109,7 +109,7 @@ export default defineComponent({
}
if (forceRedirectedHere) {
await this.$router.push({ name: VIEWS.NEW_WORKFLOW });
await this.$router.push({ name: VIEWS.HOMEPAGE });
} else {
await this.$router.push({ name: VIEWS.USERS_SETTINGS });
}

View File

@@ -130,7 +130,7 @@ async function onSubmit(values: { [key: string]: string | boolean }) {
} catch {}
}
await router.push({ name: VIEWS.NEW_WORKFLOW });
await router.push({ name: VIEWS.HOMEPAGE });
} catch (error) {
toast.showError(error, i18n.baseText('auth.signup.setupYourAccountError'));
}

View File

@@ -38,12 +38,6 @@ describe('VariablesView', () => {
server.shutdown();
});
it('should render loading state', () => {
const wrapper = renderComponent({ pinia });
expect(wrapper.container.querySelectorAll('.n8n-loading')).toHaveLength(3);
});
describe('should render empty state', () => {
it('when feature is disabled and logged in user is not owner', async () => {
settingsStore.settings.enterprise[EnterpriseEditionFeature.Variables] = false;

View File

@@ -38,7 +38,7 @@ const TEMPORARY_VARIABLE_UID_BASE = '@tmpvar';
const allVariables = ref<EnvironmentVariable[]>([]);
const editMode = ref<Record<string, boolean>>({});
const loading = ref(false);
const permissions = getVariablesPermissions(usersStore.currentUser);
const isFeatureEnabled = computed(() =>
@@ -132,9 +132,11 @@ const environmentVariableToResource = (data: EnvironmentVariable): IResource =>
async function initialize() {
if (!isFeatureEnabled.value) return;
loading.value = true;
await environmentsStore.fetchAllVariables();
allVariables.value = [...environmentsStore.variables];
loading.value = false;
}
function addTemporaryVariable() {
@@ -260,6 +262,7 @@ onBeforeUnmount(() => {
:show-filters-dropdown="false"
type="datatable"
:type-props="{ columns: datatableColumns }"
:loading="loading"
@sort="resetNewVariablesList"
@click:add="addTemporaryVariable"
>

View File

@@ -73,15 +73,13 @@ describe('WorkflowsView', () => {
});
it('should filter workflows by tags', async () => {
const { container, getByTestId, getAllByTestId, queryByTestId } = renderComponent({
const { getByTestId, getAllByTestId, queryByTestId } = renderComponent({
pinia,
});
expect(container.querySelectorAll('.n8n-loading')).toHaveLength(3);
expect(queryByTestId('resources-list')).not.toBeInTheDocument();
await waitFor(() => {
expect(container.querySelectorAll('.n8n-loading')).toHaveLength(0);
// There are 5 workflows defined in server fixtures
expect(getAllByTestId('resources-list-item')).toHaveLength(5);
});

View File

@@ -9,11 +9,12 @@
:shareable="isShareable"
:initialize="initialize"
:disabled="readOnlyEnv"
:loading="loading"
@click:add="addWorkflow"
@update:filters="onFiltersUpdated"
>
<template #header>
<ProjectTabs v-if="showProjectTabs" />
<ProjectTabs />
</template>
<template #add-button="{ disabled }">
<n8n-tooltip :disabled="!readOnlyEnv">
@@ -162,8 +163,6 @@ import { useProjectsStore } from '@/stores/projects.store';
import ProjectTabs from '@/components/Projects/ProjectTabs.vue';
import { useTemplatesStore } from '@/stores/templates.store';
type IResourcesListLayoutInstance = InstanceType<typeof ResourcesListLayout>;
interface Filters {
search: string;
homeProject: string;
@@ -194,6 +193,7 @@ const WorkflowsView = defineComponent({
tags: [],
} as Filters,
sourceControlStoreUnsubscribe: () => {},
loading: false,
};
},
computed: {
@@ -255,13 +255,6 @@ const WorkflowsView = defineComponent({
}
return ['Sales', 'sales-and-marketing'].includes(this.userRole);
},
showProjectTabs() {
return (
!!this.$route.params.projectId ||
!!this.allWorkflows.length ||
this.projectsStore.myProjects.length > 1
);
},
addWorkflowButtonText() {
return this.projectsStore.currentProject
? this.$locale.baseText('workflows.project.add')
@@ -275,9 +268,6 @@ const WorkflowsView = defineComponent({
this.saveFiltersOnQueryString();
},
},
'filters.tags'() {
this.sendFiltersTelemetry('tags');
},
'$route.params.projectId'() {
void this.initialize();
},
@@ -323,11 +313,13 @@ const WorkflowsView = defineComponent({
});
},
async initialize() {
this.loading = true;
await Promise.all([
this.usersStore.fetchUsers(),
this.workflowsStore.fetchAllWorkflows(this.$route?.params?.projectId as string | undefined),
this.workflowsStore.fetchActiveWorkflows(),
]);
this.loading = false;
},
onClickTag(tagId: string) {
if (!this.filters.tags.includes(tagId)) {
@@ -357,9 +349,6 @@ const WorkflowsView = defineComponent({
return matches;
},
sendFiltersTelemetry(source: string) {
(this.$refs.layout as IResourcesListLayoutInstance).sendFiltersTelemetry(source);
},
saveFiltersOnQueryString() {
const query: { [key: string]: string } = {};