added support to duplicate existing connections#208
Merged
Conversation
📝 WalkthroughWalkthroughAdds "duplicate connection" capability: new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Connections UI
participant Router as Router
participant Form as Connection Form
participant Storage as SecureStorage
User->>UI: Click "Duplicate" on connection
UI->>UI: generateDuplicateConnectionName(name, existingNames)
UI->>Router: Navigate to add-connection with {duplicateFrom, suggestedName}
Router->>Form: Mount with duplicateFrom & suggestedName
Form->>Storage: Fetch credentials for source connection id
Storage-->>Form: Return stored credentials
Form->>Form: Initialize fields from duplicateFrom / suggestedName
Form-->>User: Show pre-filled form
sequenceDiagram
participant Form as Connection Form
participant Effect as useEffect Hook
participant State as Form State
Form->>Effect: mount (props: initialValues, duplicateFrom, suggestedName)
Effect->>Effect: determine sourceConnection = initialValues || duplicateFrom
Effect->>State: set formData (name: suggestedName || initialValues.name, provider: duplicateFrom.provider || 'gcs', ...)
Effect->>State: fetch provider credentials (try/catch, isMounted guard)
State-->>Form: rendered pre-populated form
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/renderer/components/connections/snowflake.tsx (1)
68-68: Add source-name fallback for duplicate mode.Line 68 should include
duplicateConnection?.namebefore the generic default to keep duplicate initialization resilient ifsuggestedNameis not provided.Suggested patch
- name: existingConnection?.name ?? suggestedName ?? 'Snowflake Connection', + name: + existingConnection?.name ?? + suggestedName ?? + duplicateConnection?.name ?? + 'Snowflake Connection',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/connections/snowflake.tsx` at line 68, The name fallback for initializing a Snowflake connection misses using duplicateConnection?.name, so update the object property that sets name (where you currently use existingConnection?.name ?? suggestedName ?? 'Snowflake Connection') to include duplicateConnection?.name before the generic default; i.e., change the expression to existingConnection?.name ?? suggestedName ?? duplicateConnection?.name ?? 'Snowflake Connection' so duplicate-mode initialization preserves the source name when suggestedName is absent.src/renderer/components/connections/redshift.tsx (1)
78-78: Add a duplicate-name fallback in initialization.Line 78 skips
duplicateConnection?.name, so duplicate flows withoutsuggestedNamefall back to a generic label. Add the duplicated source name before the hardcoded default.Suggested patch
- name: existingConnection?.name ?? suggestedName ?? 'Redshift Connection', + name: + existingConnection?.name ?? + suggestedName ?? + duplicateConnection?.name ?? + 'Redshift Connection',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/connections/redshift.tsx` at line 78, The name fallback currently sets name: existingConnection?.name ?? suggestedName ?? 'Redshift Connection' but omits duplicateConnection?.name; update the initialization to include duplicateConnection?.name before the generic default so the name becomes existingConnection?.name ?? suggestedName ?? duplicateConnection?.name ?? 'Redshift Connection', ensuring duplicate flows without a suggestedName use the duplicated source name; locate this in the Redshift connection initialization where the name property is set.src/renderer/components/connections/duckdb.tsx (1)
66-66: Harden duplicate name fallback.Line 66 should also consider
duplicateConnection?.namewhensuggestedNameis missing, so duplicate mode still starts with a meaningful name.Suggested patch
- name: existingConnection?.name ?? suggestedName ?? 'DuckDB Connection', + name: + existingConnection?.name ?? + suggestedName ?? + duplicateConnection?.name ?? + 'DuckDB Connection',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/connections/duckdb.tsx` at line 66, The name fallback on the DuckDB connection form currently uses existingConnection?.name then suggestedName then a static 'DuckDB Connection'; update the fallback to also check duplicateConnection?.name so duplication mode pre-fills a meaningful name. Locate the object creating the name (the property using existingConnection?.name ?? suggestedName ?? 'DuckDB Connection' in src/renderer/components/connections/duckdb.tsx) and insert duplicateConnection?.name into the nullish-coalescing chain (so the sequence includes existingConnection?.name, duplicateConnection?.name, suggestedName, then the static default).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/cloudExplorer/ConnectionForm.tsx`:
- Around line 116-117: The form's name field fallbacks currently use
initialValues?.name || suggestedName || '' but omit the duplicated connection's
own name; update both initializations (the object that sets name and the later
similar block around lines referencing suggestedName) to use initialValues?.name
|| suggestedName || duplicateFrom?.name || '' so the duplicated connection's
name is used when suggestedName is absent; locate the same pattern where
provider/name are set (references: initialValues, suggestedName, duplicateFrom,
and the name property) and add duplicateFrom?.name before the final empty-string
fallback.
In `@src/renderer/components/connections/bigquery.tsx`:
- Around line 129-143: The effect that hydrates the service account key sets
formState.keyfile to the literal 'wtf' when no stored key exists, injecting
invalid JSON; update the fetchKey logic in the React.useEffect so it does not
apply that debug fallback — either only call setFormState when storedKey is
present (e.g., if (storedKey) setFormState(...)) or set keyfile to a safe empty
value (e.g., storedKey ?? '') instead of 'wtf'; reference the fetchKey function,
getBigQueryServiceAccountKey, setFormState, existingConnection and
duplicateConnection when making the change.
In `@src/renderer/components/connections/postgres.tsx`:
- Line 64: The name fallback currently uses existingConnection?.name ??
suggestedName ?? '' which can leave duplicate mode with an empty name; update
the fallback to include duplicateConnection?.name before the final empty string
(e.g., existingConnection?.name ?? suggestedName ?? duplicateConnection?.name ??
'') so that when duplicating a connection the duplicated connection's name is
used if suggestedName is absent.
In `@src/renderer/screens/addConnection/index.tsx`:
- Around line 98-101: The lookup for duplicate type accesses
duplicateData.connection.type directly and can throw if duplicateData exists but
has no connection; update the guard around that block (the conditional and the
lookup where you call baseItems.find) to verify duplicateData.connection is
present (e.g., check duplicateData && duplicateData.connection or use optional
chaining like duplicateData?.connection?.type) and handle the absent-connection
case by skipping the lookup or using a safe fallback so baseItems.find never
receives undefined.
---
Nitpick comments:
In `@src/renderer/components/connections/duckdb.tsx`:
- Line 66: The name fallback on the DuckDB connection form currently uses
existingConnection?.name then suggestedName then a static 'DuckDB Connection';
update the fallback to also check duplicateConnection?.name so duplication mode
pre-fills a meaningful name. Locate the object creating the name (the property
using existingConnection?.name ?? suggestedName ?? 'DuckDB Connection' in
src/renderer/components/connections/duckdb.tsx) and insert
duplicateConnection?.name into the nullish-coalescing chain (so the sequence
includes existingConnection?.name, duplicateConnection?.name, suggestedName,
then the static default).
In `@src/renderer/components/connections/redshift.tsx`:
- Line 78: The name fallback currently sets name: existingConnection?.name ??
suggestedName ?? 'Redshift Connection' but omits duplicateConnection?.name;
update the initialization to include duplicateConnection?.name before the
generic default so the name becomes existingConnection?.name ?? suggestedName ??
duplicateConnection?.name ?? 'Redshift Connection', ensuring duplicate flows
without a suggestedName use the duplicated source name; locate this in the
Redshift connection initialization where the name property is set.
In `@src/renderer/components/connections/snowflake.tsx`:
- Line 68: The name fallback for initializing a Snowflake connection misses
using duplicateConnection?.name, so update the object property that sets name
(where you currently use existingConnection?.name ?? suggestedName ?? 'Snowflake
Connection') to include duplicateConnection?.name before the generic default;
i.e., change the expression to existingConnection?.name ?? suggestedName ??
duplicateConnection?.name ?? 'Snowflake Connection' so duplicate-mode
initialization preserves the source name when suggestedName is absent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/renderer/components/cloudExplorer/ConnectionForm.tsxsrc/renderer/components/cloudExplorer/ExplorerConnections.tsxsrc/renderer/components/cloudExplorer/ExplorerNewConnection.tsxsrc/renderer/components/connections/bigquery.tsxsrc/renderer/components/connections/databricks.tsxsrc/renderer/components/connections/duckdb.tsxsrc/renderer/components/connections/kinetica.tsxsrc/renderer/components/connections/postgres.tsxsrc/renderer/components/connections/redshift.tsxsrc/renderer/components/connections/snowflake.tsxsrc/renderer/screens/addConnection/index.tsxsrc/renderer/screens/connections/index.tsxsrc/renderer/utils/connectionNaming.ts
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
src/renderer/components/connections/postgres.tsx (1)
64-64:⚠️ Potential issue | 🟡 MinorAdd duplicated connection name as fallback in duplicate mode.
Line 64 still allows an empty initial name when
suggestedNameis missing. In duplicate flow, fallback should includeduplicateConnection?.namebefore''.Suggested patch
- name: existingConnection?.name ?? suggestedName ?? '', + name: + existingConnection?.name ?? + suggestedName ?? + duplicateConnection?.name ?? + '',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/connections/postgres.tsx` at line 64, The initial name field can be empty when suggestedName is missing during the duplicate flow; update the name initialization (where name is set from existingConnection, suggestedName, or '') to include duplicateConnection?.name as the next fallback so it becomes: existingConnection?.name ?? suggestedName ?? duplicateConnection?.name ?? ''. Locate the assignment in the Postgres connection component (the object/initial form values where name: existingConnection?.name ?? suggestedName ?? '') and add the duplicateConnection?.name fallback to ensure duplicated connections get the original name.src/renderer/components/cloudExplorer/ConnectionForm.tsx (1)
116-117:⚠️ Potential issue | 🟡 MinorRestore duplicate-name fallback when
suggestedNameis absent.At Line 116 and Line 159, duplicate mode still falls back to
''instead of the source connection name, so the form can initialize with an empty name whensuggestedNameis missing.Suggested fix
- name: initialValues?.name || suggestedName || '', + name: initialValues?.name || suggestedName || duplicateFrom?.name || '',- name: initialValues?.name || suggestedName || '', + name: + initialValues?.name || suggestedName || sourceConnection.name || '',Also applies to: 159-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/cloudExplorer/ConnectionForm.tsx` around lines 116 - 117, The form initialization and duplicate-mode setup in ConnectionForm are not falling back to the source connection's name when suggestedName is absent; update both places that set the name (where initialValues, suggestedName and duplicateFrom are used) to include duplicateFrom?.name as the next fallback before ''. Specifically, in the component initialization and the duplicate-mode branch, change the name expression (currently initialValues?.name || suggestedName || '') to initialValues?.name || suggestedName || duplicateFrom?.name || '' so the form pre-fills with the source connection name when duplicating.
🧹 Nitpick comments (3)
src/renderer/components/cloudExplorer/ConnectionForm.tsx (1)
263-265: Avoid silently swallowing credential preload errors.The empty catch makes duplicate/edit prefill failures invisible and hard to debug. A lightweight warning log (or non-blocking UI hint) would improve reliability during troubleshooting.
Suggested fix
- } catch { - /* empty */ + } catch (error) { + // eslint-disable-next-line no-console + console.warn('[ConnectionForm] Failed to preload secure credentials:', error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/cloudExplorer/ConnectionForm.tsx` around lines 263 - 265, The empty catch in ConnectionForm that swallows credential preload errors should be replaced with a lightweight warning handler: catch and log the error (include the error object) using the app logger or console.warn and optionally set a non-blocking UI hint state (e.g., call setCredentialWarning or enqueue a snackbar) so users/devs see preload failures; locate the try/catch in the ConnectionForm component (the credential prefill/preload logic) and update that catch block to log a descriptive message plus the caught error and, if present, trigger a transient UI warning without blocking the flow.src/renderer/components/cloudExplorer/ExplorerConnections.tsx (1)
127-152:renderConnectionDetailstyping is correct; consider a type-guard alternative for stricter narrowing.The switch-on-
providerpattern makes each cast (config as GCSConfig, etc.) contextually safe today. IfCloudStorageConfigever becomes a discriminated union (e.g.,{ provider: 'gcs' } & GCSConfig), the explicit casts could be replaced with straightforward property access. No action needed now, but worth keeping in mind as the type evolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/cloudExplorer/ExplorerConnections.tsx` around lines 127 - 152, The renderConnectionDetails function currently narrows config via casts (config as GCSConfig / S3Config / AzureConfig); replace these casts with proper type guards or a discriminated union check to strictly narrow types: add predicate helpers (e.g., isGCSConnection, isS3Connection, isAzureConnection) that accept CloudConnection and return connection is CloudConnection & { provider: 'gcs' } (or similar) and then use those guards inside renderConnectionDetails so you can access Project ID, Region, and Account directly from connection.config without explicit casts; reference renderConnectionDetails, CloudConnection, GCSConfig, S3Config, and AzureConfig when implementing the guards.src/renderer/components/connections/kinetica.tsx (1)
136-168: Redundant outer guard beforefetchCredentials().The outer
if (existingConnection || duplicateConnection)at line 166 duplicates the identical check already insidefetchCredentials(line 140). The async function is invoked unconditionally; the inner guard is sufficient.♻️ Proposed simplification
React.useEffect(() => { let isMounted = true; const fetchCredentials = async () => { const sourceConnection = existingConnection || duplicateConnection; if (sourceConnection) { // ... credential fetch } }; - if (existingConnection || duplicateConnection) { - fetchCredentials(); - } + fetchCredentials(); return () => { isMounted = false; }; }, [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/connections/kinetica.tsx` around lines 136 - 168, Remove the redundant outer guard that checks existingConnection || duplicateConnection before invoking fetchCredentials; fetchCredentials itself already performs this check. In the useEffect containing fetchCredentials, always call fetchCredentials() (and keep the isMounted cleanup and the inner guard inside fetchCredentials unchanged). This uses the existing functions getDatabaseUsername/getDatabasePassword and state updater setFormState and avoids duplicating the connection existence check around fetchCredentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/connections/databricks.tsx`:
- Around line 55-66: The formState initializer for DatabricksConnection omits
duplicateConnection?.name and only runs once, causing empty names and stale
fields when props like duplicateConnection, existingConnection, or suggestedName
change; update the initializer to include duplicateConnection?.name in the name
fallback (i.e., name: existingConnection?.name ?? suggestedName ??
duplicateConnection?.name ?? ''), and add a React.useEffect that watches
duplicateConnection, existingConnection, and suggestedName to rehydrate
setFormState with the same field mappings (preserving token as ''), so the form
updates when navigation/duplicate props change.
In `@src/renderer/components/connections/kinetica.tsx`:
- Around line 58-61: The memoized cast of duplicateFrom?.connection to
KineticaConnection is unsafe; add a runtime type guard (e.g., function
isKineticaConnection(conn: ConnectionInput | undefined): conn is
KineticaConnection) that checks connection.type === 'kinetica' and/or required
Kinetica fields (host, port, apiKey) and use it inside the useMemo that defines
duplicateConnection so you only return the typed object when the guard passes
(otherwise return undefined and optionally surface/log an error); update
references to duplicateConnection accordingly so the form doesn't silently
receive an all-undefined object.
- Around line 172-177: The effect inside the connections Kinetica component is
listing unstable references getDatabaseUsername and getDatabasePassword in its
dependency array, causing re-render loops; remove getDatabaseUsername and
getDatabasePassword from the useEffect dependency array (leave
existingConnection and duplicateConnection) or alternatively memoize those
getters in useSecureStorage with useCallback, and ensure the effect uses
setFormState only with stable deps so it no longer retriggers every render.
In `@src/renderer/components/connections/redshift.tsx`:
- Around line 77-79: The connection "name" fallback currently uses
existingConnection?.name ?? suggestedName ?? 'Redshift Connection', which
ignores duplicateConnection?.name; update the name assignment (in the connection
object construction in redshift.tsx) to include duplicateConnection?.name as a
fallback (e.g., existingConnection?.name ?? duplicateConnection?.name ??
suggestedName ?? 'Redshift Connection') so duplicate drafts use the source name
when suggestedName is missing.
In `@src/renderer/components/connections/snowflake.tsx`:
- Around line 164-169: The effect watching
existingConnection/duplicateConnection unnecessarily re-runs because
getDatabaseUsername and getDatabasePassword from useSecureStorage are unstable;
either memoize those functions in useSecureStorage with useCallback (so they are
stable and can remain in the dependency array) or remove them from the
dependency array of the effect and rely on stable values (e.g., call
fetchCredentials without including those functions). Update useSecureStorage to
wrap getDatabaseUsername and getDatabasePassword in useCallback, or remove them
from the effect dependencies that call fetchCredentials to stop repeated
re-fetching.
- Around line 55-58: The current memo unconditionally casts
duplicateFrom.connection to SnowflakeConnection (duplicateConnection) which
bypasses type-safety; update the React.useMemo for duplicateConnection to
perform a runtime type guard on duplicateFrom.connection (e.g., check a
discriminator like connection.type === 'snowflake' or presence of
Snowflake-specific fields) and only return the casted SnowflakeConnection when
the guard passes, otherwise return null/undefined; apply the same guarded
pattern to other components that cast ConnectionModel.connection (e.g.,
Postgres, BigQuery, Redshift) to maintain consistency.
---
Duplicate comments:
In `@src/renderer/components/cloudExplorer/ConnectionForm.tsx`:
- Around line 116-117: The form initialization and duplicate-mode setup in
ConnectionForm are not falling back to the source connection's name when
suggestedName is absent; update both places that set the name (where
initialValues, suggestedName and duplicateFrom are used) to include
duplicateFrom?.name as the next fallback before ''. Specifically, in the
component initialization and the duplicate-mode branch, change the name
expression (currently initialValues?.name || suggestedName || '') to
initialValues?.name || suggestedName || duplicateFrom?.name || '' so the form
pre-fills with the source connection name when duplicating.
In `@src/renderer/components/connections/postgres.tsx`:
- Line 64: The initial name field can be empty when suggestedName is missing
during the duplicate flow; update the name initialization (where name is set
from existingConnection, suggestedName, or '') to include
duplicateConnection?.name as the next fallback so it becomes:
existingConnection?.name ?? suggestedName ?? duplicateConnection?.name ?? ''.
Locate the assignment in the Postgres connection component (the object/initial
form values where name: existingConnection?.name ?? suggestedName ?? '') and add
the duplicateConnection?.name fallback to ensure duplicated connections get the
original name.
---
Nitpick comments:
In `@src/renderer/components/cloudExplorer/ConnectionForm.tsx`:
- Around line 263-265: The empty catch in ConnectionForm that swallows
credential preload errors should be replaced with a lightweight warning handler:
catch and log the error (include the error object) using the app logger or
console.warn and optionally set a non-blocking UI hint state (e.g., call
setCredentialWarning or enqueue a snackbar) so users/devs see preload failures;
locate the try/catch in the ConnectionForm component (the credential
prefill/preload logic) and update that catch block to log a descriptive message
plus the caught error and, if present, trigger a transient UI warning without
blocking the flow.
In `@src/renderer/components/cloudExplorer/ExplorerConnections.tsx`:
- Around line 127-152: The renderConnectionDetails function currently narrows
config via casts (config as GCSConfig / S3Config / AzureConfig); replace these
casts with proper type guards or a discriminated union check to strictly narrow
types: add predicate helpers (e.g., isGCSConnection, isS3Connection,
isAzureConnection) that accept CloudConnection and return connection is
CloudConnection & { provider: 'gcs' } (or similar) and then use those guards
inside renderConnectionDetails so you can access Project ID, Region, and Account
directly from connection.config without explicit casts; reference
renderConnectionDetails, CloudConnection, GCSConfig, S3Config, and AzureConfig
when implementing the guards.
In `@src/renderer/components/connections/kinetica.tsx`:
- Around line 136-168: Remove the redundant outer guard that checks
existingConnection || duplicateConnection before invoking fetchCredentials;
fetchCredentials itself already performs this check. In the useEffect containing
fetchCredentials, always call fetchCredentials() (and keep the isMounted cleanup
and the inner guard inside fetchCredentials unchanged). This uses the existing
functions getDatabaseUsername/getDatabasePassword and state updater setFormState
and avoids duplicating the connection existence check around fetchCredentials.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/renderer/components/cloudExplorer/ConnectionForm.tsxsrc/renderer/components/cloudExplorer/ExplorerConnections.tsxsrc/renderer/components/connections/bigquery.tsxsrc/renderer/components/connections/databricks.tsxsrc/renderer/components/connections/kinetica.tsxsrc/renderer/components/connections/postgres.tsxsrc/renderer/components/connections/redshift.tsxsrc/renderer/components/connections/snowflake.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/components/connections/bigquery.tsx
Nuri1977
approved these changes
Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit