-
Notifications
You must be signed in to change notification settings - Fork 54.3k
refactor(editor): Update connection handling to use providerKey #25061
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
base: master
Are you sure you want to change the base?
refactor(editor): Update connection handling to use providerKey #25061
Conversation
…ad of connectionId - enhance UI components with data-test attributes for improved testing - removes unused copy
Bundle ReportChanges will decrease total bundle size by 657 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: editor-ui-esmAssets Changed:
Files in
Files in
Files in
|
|
|
||
| async function getConnection(connectionId: string): Promise<SecretProviderConnection> { | ||
| async function getConnection(providerKey: string): Promise<SecretProviderConnection> { | ||
| console.log('getConnection', providerKey); |
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.
console.log
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.
Maybe we should lint this :D
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ecrets provider connection
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.
2 issues found across 13 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/frontend/editor-ui/src/features/integrations/secretsProviders.ee/composables/useSecretsProviderConnection.ee.ts">
<violation number="1" location="packages/frontend/editor-ui/src/features/integrations/secretsProviders.ee/composables/useSecretsProviderConnection.ee.ts:59">
P2: Remove the debug console.log to avoid leaking provider identifiers in production UI code.</violation>
</file>
<file name="packages/frontend/editor-ui/src/features/integrations/secretsProviders.ee/composables/useConnectionModal.ee.ts">
<violation number="1" location="packages/frontend/editor-ui/src/features/integrations/secretsProviders.ee/composables/useConnectionModal.ee.ts:35">
P2: `providerKey` is a Ref in the options, but the new code copies its current value into a fresh ref. This breaks reactivity with the caller and can leave the modal stuck on a stale provider key when the input ref changes. Use the provided ref directly (or a computed getter/setter) to keep it reactive.
(Based on your team's feedback about keeping composable parameters reactive.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| async function getConnection(connectionId: string): Promise<SecretProviderConnection> { | ||
| async function getConnection(providerKey: string): Promise<SecretProviderConnection> { | ||
| console.log('getConnection', providerKey); |
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.
P2: Remove the debug console.log to avoid leaking provider identifiers in production UI code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/editor-ui/src/features/integrations/secretsProviders.ee/composables/useSecretsProviderConnection.ee.ts, line 59:
<comment>Remove the debug console.log to avoid leaking provider identifiers in production UI code.</comment>
<file context>
@@ -55,13 +55,14 @@ export function useSecretsProviderConnection(options?: { useMockApi?: boolean })
- async function getConnection(connectionId: string): Promise<SecretProviderConnection> {
+ async function getConnection(providerKey: string): Promise<SecretProviderConnection> {
+ console.log('getConnection', providerKey);
// GET /rest/secret-providers/connections/:providerKey
isLoading.value = true;
</file context>
|
|
||
| // State | ||
| const connectionId = ref(options.connectionId); | ||
| const providerKey = ref<string | undefined>(options.providerKey?.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.
P2: providerKey is a Ref in the options, but the new code copies its current value into a fresh ref. This breaks reactivity with the caller and can leave the modal stuck on a stale provider key when the input ref changes. Use the provided ref directly (or a computed getter/setter) to keep it reactive.
(Based on your team's feedback about keeping composable parameters reactive.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/editor-ui/src/features/integrations/secretsProviders.ee/composables/useConnectionModal.ee.ts, line 35:
<comment>`providerKey` is a Ref in the options, but the new code copies its current value into a fresh ref. This breaks reactivity with the caller and can leave the modal stuck on a stale provider key when the input ref changes. Use the provided ref directly (or a computed getter/setter) to keep it reactive.
(Based on your team's feedback about keeping composable parameters reactive.) </comment>
<file context>
@@ -32,11 +32,12 @@ export function useConnectionModal(options: UseConnectionModalOptions) {
// State
- const connectionId = ref(options.connectionId);
+ const providerKey = ref<string | undefined>(options.providerKey?.value);
+ const connectionId = ref<string | undefined>(undefined);
const connectionName = ref('');
</file context>
| const providerKey = ref<string | undefined>(options.providerKey?.value); | |
| const providerKey = options.providerKey ?? ref<string | undefined>(undefined); |
konstantintieber
left a comment
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.

Summary
Update connection handling to use providerKey instead of connectionId
Also
Related Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)