-
Notifications
You must be signed in to change notification settings - Fork 9.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
base: master
Are you sure you want to change the base?
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 #8491
Run Properties:
|
Project |
n8n
|
Branch Review |
master
|
Run status |
Passed #8491
|
Run duration | 04m 42s |
Commit |
870f8640c7: 🌳 master 🖥️ browsers:node18.12.0-chrome107 🤖 PR User 🗃️ e2e/*
|
Committer | कारतोफ्फेलस्क्रिप्ट™ |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
1
|
Pending |
0
|
Skipped |
0
|
Passing |
484
|
View all changes introduced in this branch ↗︎ |
|
1 similar comment
|
…edits-in-the-users-instance-v2
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)