-
Notifications
You must be signed in to change notification settings - Fork 10.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(editor): Add free AI credits CTA #12365
feat(editor): Add free AI credits CTA #12365
Conversation
@@ -1061,7 +1070,9 @@ function resetCredentialData(): void { | |||
<InlineNameEdit | |||
:model-value="credentialName" | |||
:subtitle="credentialType ? credentialType.displayName : ''" | |||
:readonly="!credentialPermissions.update || !credentialType" | |||
:readonly=" |
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 prevents users from editing the credential name
@@ -597,6 +602,10 @@ function scrollToBottom() { | |||
} | |||
|
|||
async function retestCredential() { | |||
if (isEditingManagedCredential.value) { |
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.
Do not send the request as there is nothing to test on managed credentials
@@ -235,7 +236,10 @@ watch(showOAuthSuccessBanner, (newValue, oldValue) => { | |||
</script> | |||
|
|||
<template> | |||
<div> | |||
<n8n-callout v-if="isManaged" theme="warning" icon="exclamation-triangle"> |
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.
Prevent user from seeing the credential values
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
Nicely done. A few small comments.
beforeEach(() => { | ||
vi.clearAllMocks(); | ||
|
||
(useSettingsStore as any).mockReturnValue({ |
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.
It would be type-safer to avoid using any
here. You can find other examples of places where we have mocked the settings store, for example here packages/editor-ui/src/composables/useDebugInfo.test.ts
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.
Will address in a follow up PR
7f40bd3
to
fdbc530
Compare
also validates that managed credentials cannot be edited
145007d
to
e38df6f
Compare
n8n Run #8494
Run Properties:
|
Project |
n8n
|
Branch Review |
ado-2994-n8n-fe-add-cta-to-generate-the-credits-in-the-users-instance-v2
|
Run status |
Passed #8494
|
Run duration | 04m 35s |
Commit |
d7164166c2: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 RicardoE105 🗃️ e2e/*
|
Committer | Ricardo Espinoza |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
3
|
Pending |
0
|
Skipped |
0
|
Passing |
484
|
View all changes introduced in this branch ↗︎ |
|
1 similar comment
|
…edits-in-the-users-instance-v2
✅ All Cypress E2E specs passed |
Got released with |
1 similar comment
Got released with |
Got released with |
1 similar comment
Got released with |
Summary
Builds on top of #12363
Demo: https://share.cleanshot.com/gcK464Kn
Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/ADO-2994/[n8n-fe]-add-cta-to-generate-the-credits-in-the-users-instance
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)