Skip to content

Fixing recent connections in connection dialog#21646

Merged
aasimkhan30 merged 14 commits intomainfrom
aasim/fix/recentConnections
Apr 22, 2026
Merged

Fixing recent connections in connection dialog#21646
aasimkhan30 merged 14 commits intomainfrom
aasim/fix/recentConnections

Conversation

@aasimkhan30
Copy link
Copy Markdown
Contributor

@aasimkhan30 aasimkhan30 commented Mar 17, 2026

Description

Fixes: #21540

  1. Previously recent connections were not being highlighted because the dedup logic in read connections got rid of any connections with the same id.
  2. Also fixing the order of recent and saved connections in sidebar
Now Before
image image

Code Changes Checklist

  • New or updated unit tests added
  • All existing tests pass (npm run test)
  • Code follows contributing guidelines
  • Telemetry/logging updated if relevant
  • No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes connection dialog hydration/display of recent connections by ensuring saved and recent connections with the same connection ID aren’t deduplicated away, and updates the sidebar list ordering to show recent connections first.

Changes:

  • Update ConnectionStore.readAllConnections(true) deduplication to key by (id, profileSource) so saved+MRU entries with the same ID are preserved.
  • Swap the UI ordering in the Connection Dialog sidebar to display “Recent connections” above “Saved connections”, with the correct delete/remove actions per section.
  • Add a unit test to validate that saved and recent entries sharing the same ID are both returned.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
extensions/mssql/src/models/connectionStore.ts Adjusts deduplication to preserve both saved and MRU entries with the same connection ID.
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionsListContainer.tsx Reorders recent/saved connection sections and wires the corresponding remove/delete actions.
extensions/mssql/test/unit/connectionStore.test.ts Adds coverage to ensure readAllConnections(true) keeps both saved and recent entries when IDs collide.

Comment thread extensions/mssql/src/models/connectionStore.ts Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 17, 2026

PR Changes

Category Target Branch PR Branch Difference
vscode-mssql VSIX 78022 KB 78023 KB ⚪ 1 KB ( 0% )
sql-database-projects VSIX 6212 KB 6212 KB ⚪ 0 KB ( 0% )
data-workspace VSIX 535 KB 535 KB ⚪ 0 KB ( 0% )
keymap VSIX 7 KB 7 KB ⚪ 0 KB ( 0% )

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 91.16022% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.65%. Comparing base (b4164b9) to head (9294000).

Files with missing lines Patch % Lines
...nectionconfig/connectionDialogWebviewController.ts 90.58% 8 Missing ⚠️
extensions/mssql/src/models/connectionStore.ts 88.52% 7 Missing ⚠️
...tensions/mssql/src/webviews/common/locConstants.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #21646      +/-   ##
==========================================
+ Coverage   74.54%   74.65%   +0.11%     
==========================================
  Files         391      392       +1     
  Lines      117193   117355     +162     
  Branches     6650     6710      +60     
==========================================
+ Hits        87358    87617     +259     
+ Misses      29835    29738      -97     
Flag Coverage Δ
data-workspace 77.10% <ø> (ø)
mssql 74.30% <91.16%> (+0.13%) ⬆️
sqlproj 77.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...iews/pages/ConnectionDialog/connectionCardUtils.ts 100.00% <100.00%> (ø)
...tensions/mssql/src/webviews/common/locConstants.ts 29.26% <0.00%> (ø)
extensions/mssql/src/models/connectionStore.ts 56.31% <88.52%> (+13.27%) ⬆️
...nectionconfig/connectionDialogWebviewController.ts 65.88% <90.58%> (+1.18%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kburtram
kburtram previously approved these changes Mar 17, 2026
lewis-sanchez
lewis-sanchez previously approved these changes Mar 17, 2026
@aasimkhan30 aasimkhan30 dismissed stale reviews from lewis-sanchez and kburtram via abb9a5c March 17, 2026 21:41
@Benjin
Copy link
Copy Markdown
Contributor

Benjin commented Mar 18, 2026

Let's discuss what the intended behavior should be for MRU connections. I've added some comments in #21540

Copilot AI review requested due to automatic review settings March 20, 2026 19:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the Connection Dialog’s “Recent Connections” behavior by preventing recent entries from being dropped during connection list loading, and updates the sidebar list ordering to show “Recent Connections” before “Saved Connections”.

Changes:

  • Update ConnectionStore.readAllConnections() deduplication to consider both connection id and profile source (saved vs MRU).
  • Reorder the Connection Dialog sidebar sections so Recent Connections appears above Saved Connections.
  • Add a unit test to ensure saved and recent entries with the same id are both preserved.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
extensions/mssql/src/models/connectionStore.ts Adjusts deduplication key to preserve both saved and recent entries that share an id.
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionsListContainer.tsx Reorders recent/saved sections and updates list rendering keys + actions accordingly.
extensions/mssql/test/unit/connectionStore.test.ts Adds a regression test for preserving saved+recent entries with the same id.
Comments suppressed due to low confidence (1)

extensions/mssql/src/models/connectionStore.ts:704

  • The dedupe key is now id + profileSource, but the verbose log message still says "Duplicate connection ID found". This is misleading because duplicates are now defined by the composite key (and the duplicate might only exist within the same source). Update the message to reflect the actual dedupe criteria (e.g. include profileSource or the composite key).
            const key = `${conn.id}-${conn.profileSource}`; // Use both ID and source as key to allow same profile in both lists
            if (!uniqueConnections.has(key)) {
                uniqueConnections.set(key, conn);
            } else {
                dupeCount++;
                this._logger.verbose(
                    `Duplicate connection ID found: ${conn.id}. Ignoring duplicate connection.`,
                );

Copilot AI review requested due to automatic review settings April 14, 2026 20:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.

Files not reviewed (2)
  • extensions/mssql/package-lock.json: Language not supported
  • extensions/sql-database-projects/package-lock.json: Language not supported

Comment thread extensions/mssql/src/models/connectionStore.ts Outdated
Comment thread extensions/mssql/test/unit/connectionStore.test.ts
…d ensuring unique entries based on database differences
Copilot AI review requested due to automatic review settings April 22, 2026 18:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

extensions/mssql/src/models/connectionStore.ts:723

  • The verbose log message in the de-dupe loop is now misleading: duplicates are detected via Utils.isSameProfile / isSameRecentConnectionEntry, not necessarily by connection ID, and conn.id may be undefined (MRU entries). Update the message to describe what was actually considered a duplicate and include a more reliable identifier (e.g., source + server/db/auth/user/accountId) rather than labeling it as an ID duplicate.
            } else {
                dupeCount++;
                this._logger.verbose(
                    `Duplicate connection ID found: ${conn.id}. Ignoring duplicate connection.`,
                );

Comment thread extensions/mssql/test/unit/connectionStore.test.ts
Comment thread extensions/mssql/src/models/connectionStore.ts
Comment thread extensions/mssql/src/webviews/pages/ConnectionDialog/connectionsListContainer.tsx Outdated
Copilot AI review requested due to automatic review settings April 22, 2026 18:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

extensions/mssql/src/models/connectionStore.ts:723

  • readAllConnections no longer deduplicates strictly by ID, but the verbose log message still says "Duplicate connection ID found" and logs ${conn.id} (which may be undefined or not the reason for dedupe). Update the log message to reflect the actual dedupe criteria (e.g., duplicate connection within same source by identity), and consider including the relevant identity fields in the log for troubleshooting.
                this._logger.verbose(
                    `Duplicate connection ID found: ${conn.id}. Ignoring duplicate connection.`,
                );

Comment thread extensions/mssql/src/models/connectionStore.ts
Comment thread extensions/mssql/src/webviews/pages/ConnectionDialog/connectionsListContainer.tsx Outdated
Copilot AI review requested due to automatic review settings April 22, 2026 18:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Comment thread extensions/mssql/src/models/connectionStore.ts
Comment thread extensions/mssql/src/models/connectionStore.ts
Comment thread extensions/mssql/src/models/connectionStore.ts
@aasimkhan30 aasimkhan30 merged commit 8164973 into main Apr 22, 2026
7 checks passed
@aasimkhan30 aasimkhan30 deleted the aasim/fix/recentConnections branch April 22, 2026 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Connection Recent Connections doesn't populate

6 participants