Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(editor): Update ownership pills #11155

Merged
merged 10 commits into from
Oct 28, 2024
Merged

feat(editor): Update ownership pills #11155

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cc1a5ea
chore: add icon
r00gm Oct 8, 2024
1ba6f1c
wip update badges
r00gm Oct 8, 2024
b4de3ed
Merge branch 'master' into pay-1803-update-ownership-pills-on-workflo…
r00gm Oct 17, 2024
db45ac4
update tooltip
r00gm Oct 17, 2024
99fec3d
Merge branch 'master' into pay-1803-update-ownership-pills-on-workflo…
r00gm Oct 23, 2024
3efaf60
fix e2e
r00gm Oct 23, 2024
c43142e
Merge branch 'master' into pay-1803-update-ownership-pills-on-workflo…
r00gm Oct 25, 2024
2e12f53
address feedback
r00gm Oct 25, 2024
e46d571
fix e2e
r00gm Oct 25, 2024
444a596
Merge branch 'master' into pay-1803-update-ownership-pills-on-workflo…
r00gm Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cypress/e2e/17-sharing.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ describe('Sharing', { disableAutoLogin: true }, () => {
credentialsPage.getters
.credentialCards()
.should('have.length', 2)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('have.length', 1);
});
});
Expand Down
14 changes: 7 additions & 7 deletions cypress/e2e/39-projects.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,12 @@ describe('Projects', { disableAutoLogin: true }, () => {
workflowsPage.getters.newWorkflowButtonCard().click();
projects.createWorkflow('Test_workflow_1.json', 'Workflow in Project 2');

// Move the workflow owned by me from Home to Project 1
// Move the workflow (You) from Home to Project 1
projects.getHomeButton().click();
workflowsPage.getters
.workflowCards()
.should('have.length', 3)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('exist');
workflowsPage.getters.workflowCardActions('Workflow in Home project').click();
workflowsPage.getters.workflowMoveButton().click();
Expand All @@ -505,7 +505,7 @@ describe('Projects', { disableAutoLogin: true }, () => {
workflowsPage.getters
.workflowCards()
.should('have.length', 3)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('not.exist');

// Move the workflow from Project 1 to Project 2
Expand Down Expand Up @@ -574,7 +574,7 @@ describe('Projects', { disableAutoLogin: true }, () => {
workflowsPage.getters
.workflowCards()
.should('have.length', 3)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('have.length', 1);

// Move the credential from Project 1 to Project 2
Expand Down Expand Up @@ -657,7 +657,7 @@ describe('Projects', { disableAutoLogin: true }, () => {
credentialsPage.getters
.credentialCards()
.should('have.length', 3)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('have.length', 2);

// Move the credential from admin user back to its original project (Project 1)
Expand Down Expand Up @@ -716,7 +716,7 @@ describe('Projects', { disableAutoLogin: true }, () => {
workflowsPage.getters
.workflowCards()
.should('have.length', 1)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('exist');
workflowsPage.getters.workflowCardActions('My workflow').click();
workflowsPage.getters.workflowMoveButton().click();
Expand All @@ -737,7 +737,7 @@ describe('Projects', { disableAutoLogin: true }, () => {
workflowsPage.getters
.workflowCards()
.should('have.length', 1)
.filter(':contains("Owned by me")')
.filter(':contains("(You)")')
.should('not.exist');

//Log out with instance owner and log in with the member user
Expand Down
25 changes: 23 additions & 2 deletions packages/editor-ui/src/components/Projects/ProjectCardBadge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('ProjectCardBadge', () => {
},
});

expect(getByText('Owned by me')).toBeVisible();
expect(getByText('(You)')).toBeVisible();
});

it('should show "Owned by me" badge if homeProject ID equals personalProject ID', () => {
Expand All @@ -22,6 +22,7 @@ describe('ProjectCardBadge', () => {
resource: {
homeProject: {
id: '1',
name: 'John',
},
},
resourceType: 'workflow',
Expand All @@ -31,7 +32,27 @@ describe('ProjectCardBadge', () => {
},
});

expect(getByText('Owned by me')).toBeVisible();
expect(getByText('John (You)')).toBeVisible();
});

it('should show shared with count', () => {
const { getByText } = renderComponent({
props: {
resource: {
sharedWithProjects: [{}, {}, {}],
homeProject: {
id: '1',
name: 'John',
},
},
resourceType: 'workflow',
personalProject: {
id: '1',
},
},
});

expect(getByText('+ 3')).toBeVisible();
});

test.each([
Expand Down
67 changes: 55 additions & 12 deletions packages/editor-ui/src/components/Projects/ProjectCardBadge.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,28 @@ const projectState = computed(() => {
}
return ProjectState.Unknown;
});

const numberOfMembersInHomeTeamProject = computed(
() => props.resource.sharedWithProjects?.length ?? 0,
);
Comment on lines +56 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cstuncsik please double check this is the correct source of truth

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the intention is here (maybe the variable name is confusing)

If we want to show how many users the resource is shared with, then it's correct.
If we want to show how many users are there in the home project of the resource, then it's not
If the latter you need to get the home project of the resource (by the homeProject.id) and check the length of its relations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to the extend of my knowledge.. we want to share how many users the resource is shared with, i'll ask in the ticket to double check

Copy link
Contributor

@cstuncsik cstuncsik Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the easier, the other one could only be problematic in the Home project because the resources are listed from all available projects and you would need to get all the relations for all the projects

In a team project you just get it from the store (projectsStore.currentProject.relations)


const badgeText = computed(() => {
const { name, email } = splitName(props.resource.homeProject?.name ?? '');
const display = name ?? email ?? '';
if (
projectState.value === ProjectState.Owned ||
projectState.value === ProjectState.SharedOwned
) {
return i18n.baseText('generic.ownedByMe');
return [display, i18n.baseText('generic.ownedByMe')].filter(Boolean).join(' ');
} else {
const { name, email } = splitName(props.resource.homeProject?.name ?? '');
return name ?? email ?? '';
return display;
}
});
const badgeIcon = computed(() => {
switch (projectState.value) {
case ProjectState.SharedPersonal:
case ProjectState.SharedOwned:
return 'user-friends';
case ProjectState.Team:
case ProjectState.SharedTeam:
return 'archive';
return 'layer-group';
default:
return '';
}
Expand All @@ -81,13 +84,15 @@ const badgeTooltip = computed(() => {
return i18n.baseText('projects.badge.tooltip.sharedOwned', {
interpolate: {
resourceTypeLabel: props.resourceTypeLabel,
count: numberOfMembersInHomeTeamProject.value,
},
});
case ProjectState.SharedPersonal:
return i18n.baseText('projects.badge.tooltip.sharedPersonal', {
interpolate: {
resourceTypeLabel: props.resourceTypeLabel,
name: badgeText.value,
count: numberOfMembersInHomeTeamProject.value,
},
});
case ProjectState.Personal:
Expand All @@ -109,6 +114,7 @@ const badgeTooltip = computed(() => {
interpolate: {
resourceTypeLabel: props.resourceTypeLabel,
name: badgeText.value,
count: numberOfMembersInHomeTeamProject.value,
},
});
default:
Expand All @@ -118,14 +124,51 @@ const badgeTooltip = computed(() => {
</script>
<template>
<N8nTooltip :disabled="!badgeTooltip" placement="top">
<N8nBadge v-if="badgeText" class="mr-xs" theme="tertiary" bold data-test-id="card-badge">
<span v-n8n-truncate:20>{{ badgeText }}</span>
<N8nIcon v-if="badgeIcon" :icon="badgeIcon" size="small" class="ml-5xs" />
</N8nBadge>
<div class="mr-xs">
<N8nBadge
v-if="badgeText"
:class="$style.badge"
theme="tertiary"
bold
data-test-id="card-badge"
>
<N8nIcon v-if="badgeIcon" :icon="badgeIcon" size="small" class="mr-3xs" />
<span v-n8n-truncate:20>{{ badgeText }}</span>
</N8nBadge>
<N8nBadge
v-if="numberOfMembersInHomeTeamProject"
:class="[$style.badge, $style.countBadge]"
theme="tertiary"
bold
>
+ {{ numberOfMembersInHomeTeamProject }}
</N8nBadge>
</div>
<template #content>
{{ badgeTooltip }}
</template>
</N8nTooltip>
</template>

<style lang="scss" module></style>
<style lang="scss" module>
.badge {
padding: var(--spacing-4xs) var(--spacing-2xs);
background-color: var(--color-background-xlight);
border-color: var(--color-foreground-base);

z-index: 1;
position: relative;
:global(.n8n-text) {
color: var(--color-text-base);
}
}

.countBadge {
margin-left: -5px;
z-index: 0;
position: relative;
:global(.n8n-text) {
color: var(--color-text-light);
}
}
</style>
8 changes: 4 additions & 4 deletions packages/editor-ui/src/plugins/i18n/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"generic.seePlans": "See plans",
"generic.loading": "Loading",
"generic.and": "and",
"generic.ownedByMe": "Owned by me",
"generic.ownedByMe": "(You)",
"generic.moreInfo": "More info",
"generic.next": "Next",
"about.aboutN8n": "About n8n",
Expand Down Expand Up @@ -2542,11 +2542,11 @@
"projects.move.resource.success.message": "{resourceName} {resourceTypeLabel} was moved to {targetProjectName}. {workflow} {link}",
"projects.move.resource.success.message.workflow": "Please double check any credentials this workflow is using are also shared with {targetProjectName}.",
"projects.move.resource.success.link": "View {targetProjectName}",
"projects.badge.tooltip.sharedOwned": "This {resourceTypeLabel} is owned by you and shared with one or more projects or users",
"projects.badge.tooltip.sharedPersonal": "This {resourceTypeLabel} is owned by {name} and shared with one or more projects or users",
"projects.badge.tooltip.sharedOwned": "This {resourceTypeLabel} is owned by you and shared with {count} users",
"projects.badge.tooltip.sharedPersonal": "This {resourceTypeLabel} is owned by {name} and shared with {count} users",
"projects.badge.tooltip.personal": "This {resourceTypeLabel} is owned by {name}",
"projects.badge.tooltip.team": "This {resourceTypeLabel} is owned and accessible by the {name} project.",
"projects.badge.tooltip.sharedTeam": "This {resourceTypeLabel} is owned and accessible by the {name} project and shared with one or more projects or users",
"projects.badge.tooltip.sharedTeam": "This {resourceTypeLabel} is owned by the {name} project and accessible by {count} users",
"mfa.setup.invalidAuthenticatorCode": "{code} is not a valid number",
"mfa.setup.invalidCode": "Two-factor code failed. Please try again.",
"mfa.code.modal.title": "Two-factor authentication",
Expand Down