-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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.
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: Fetch user cloud role and pass it on in website links #8942
Conversation
role: this.usersStore.currentUserCloudInfo?.role, | ||
active_workflow_count: this.workflowsStore.activeWorkflows.length, | ||
}); | ||
window.open(url, '_blank'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use plain <a>
tag instead of navigating programmatically. That way all native browser interactions work as well (right click, cmd+click, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, agree. Will update but the requirement is to open this in new tab so the link will have target="_blank"
@@ -1758,6 +1758,7 @@ export declare namespace Cloud { | |||
username: string; | |||
email: string; | |||
hasEarlyAccess?: boolean; | |||
role?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be easy to confuse with the new RBAC roles. Where is this data defined? We can either rename the property or add a more strict type for it, e.g. CloudRole
or RoleAtCompany
type CloudRole = string;
export type UserAccount = {
...
role?: CloudRole
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, but this interface is already in the Cloud
namespace. Does this make it clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
<n8n-card | ||
hoverable | ||
data-test-id="browse-sales-templates-card" | ||
@click="trackCategoryLinkClick('Sales')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be on the <a>
tag, but fine as is
1 flaky test on run #4443 ↗︎
Details:
cypress/e2e/5-ndv.cy.ts • 1 flaky test
Review all test suite changes for PR #8942 ↗︎ |
✅ All Cypress E2E specs passed |
Got released with |
Summary
/me
reponse and passing it to website templates URLHow to test
Follow the steps from the Linear ticker
Related tickets and issues
https://linear.app/n8n/issue/ADO-2073/send-user-details-from-n8n-app
Review / Merge checklist
(no-changelog)
otherwise. (conventions)