-
Notifications
You must be signed in to change notification settings - Fork 6k
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: Ado 1296 spike credential setup in templates #7786
Conversation
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.
Looks really solid! 👏
I left a few comments, but mostly about refactoring.
packages/editor-ui/src/components/CredentialPicker/CredentialPicker.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/components/CredentialPicker/CredentialPicker.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/components/CredentialPicker/CredentialPicker.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/components/CredentialPicker/CredentialPicker.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue
Outdated
Show resolved
Hide resolved
packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue
Outdated
Show resolved
Hide resolved
@MiloradFilipovic addressed all the comments. Please have another look 🙏 |
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.
@@ -0,0 +1,132 @@ | |||
<template> |
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.
Nitpick, but we started writing <template>
after <script setup>
in out components. I think we should stick to that standard.
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.
Check! I was actually wondering which is the correct way to go with, as I've seen both. Will change!
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.
Looking good
2 flaky tests on run #3043 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
Canvas Node Manipulation and Navigation > should add merge node and test connections |
Screenshots
Video
|
26-resource-locator.cy.ts • 1 flaky test
Test | Artifacts | |
---|---|---|
Resource Locator > should retrieve list options when other params throw errors |
Screenshots
Video
|
Review all test suite changes for PR #7786 ↗︎
✅ All Cypress E2E specs passed |
Part of #7786 Adds telemetry for the setup template credentials feature. Spec: https://www.notion.so/n8n/Handoff-a1150c38f6e042db91fdf61c56900967?pvs=4#e495f63ebb56415b8eb5af3ec537e8e0
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.
Re-approving after merge
✅ All Cypress E2E specs passed |
# [1.19.0](https://github.com/n8n-io/n8n/compare/n8n@1.18.0...n8n@1.19.0) (2023-11-29) ### Bug Fixes * **core:** Ensure member and admin cannot be promoted to owner ([#7830](#7830)) ([9b87a59](9b87a59)), closes [/linear.app/n8n/issue/PAY-985/add-user-role-modification-endpoint#comment-62355f6](https://github.com//linear.app/n8n/issue/PAY-985/add-user-role-modification-endpoint/issues/comment-62355f6) * **core:** Prevent error messages due to statistics about data loading ([#7824](#7824)) ([847f6ac](847f6ac)) * **core:** Tighten checks for multi-main setup usage ([#7788](#7788)) ([fdb2c18](fdb2c18)) * **core:** Use AbortController to notify nodes to abort execution ([#6141](#6141)) ([d2c18c5](d2c18c5)) * **editor:** Add telemetry to workflow history ([#7811](#7811)) ([d497041](d497041)) * **editor:** Allow owners and admins to share workflows and credentials they don't own ([#7833](#7833)) ([3ab3ec9](3ab3ec9)) * **editor:** Disable context menu actions in read-only mode ([#7789](#7789)) ([902beff](902beff)) * **editor:** Fix cloud plan data loading on instance ([#7841](#7841)) ([8b99384](8b99384)) * **editor:** Fix credential icon for old node type version ([#7843](#7843)) ([4074107](4074107)) * **editor:** Fix icon for unknown node type ([#7842](#7842)) ([28ac5a7](28ac5a7)) * **editor:** Fix mouse position in workflow previews ([#7853](#7853)) ([c063398](c063398)) * **editor:** Show nice error when environment is not set up ([#7778](#7778)) ([5835e05](5835e05)) * **editor:** Suppress dev server websocket messages in workflow view ([#7808](#7808)) ([685ffd7](685ffd7)) * **Google Sheets Node:** Read operation execute for each item ([#7800](#7800)) ([d548872](d548872)) * **HTTP Request Node:** Enable expressions for binary input data fields ([#7782](#7782)) ([6208af0](6208af0)) * **Microsoft SQL Node:** Prevent double escaping table name ([#7801](#7801)) ([73ec753](73ec753)) ### Features * Add AI tool building capabilities ([#7336](#7336)) ([87def60](87def60)) * Add initial scope checks via decorators ([#7737](#7737)) ([a37f1cb](a37f1cb)) * Ado 1296 spike credential setup in templates ([#7786](#7786)) ([aae45b0](aae45b0)) * **core:** Add Support for custom CORS origins for webhooks ([#7455](#7455)) ([99a9ea4](99a9ea4)) * **core:** Allow user role modification ([#7797](#7797)) ([7a86d36](7a86d36)) * **core:** Set up endpoint for all existing roles with license flag ([#7834](#7834)) ([2356fb0](2356fb0)) * **editor:** Add node name and version to NDV node settings ([#7731](#7731)) ([da85198](da85198)) * **editor:** Add routing middleware, permission checks, RBAC store, RBAC component ([#7702](#7702)) ([67a8891](67a8891)) * **editor:** Replace middleware for Role checks with Scope checks ([#7847](#7847)) ([72852a6](72852a6)) * **editor:** Show avatars for users currently working on the same workflow ([#7763](#7763)) ([77bc8ec](77bc8ec)) * **Notion Node:** Option to simplify output in getChildBlocks operation ([#7791](#7791)) ([d667bca](d667bca)) * **Slack Node:** Add support for getting the profile of a user ([#7829](#7829)) ([90bb6ba](90bb6ba)) Co-authored-by: ivov <ivov@users.noreply.github.com>
Reminder: If a PR title doesn't contain |
Got released with |
Introduces following changes:
Linear ticket: https://linear.app/n8n/issue/ADO-1296/feature-credential-setup-in-templates
Handoff notes: https://www.notion.so/n8n/Handoff-a1150c38f6e042db91fdf61c56900967
Stacked PRs:
Kapture.2023-11-23.at.10.49.08.mp4