Fixing recent connections in connection dialog

Author: Aasim Khan

extensions/mssql/src/models/connectionStore.ts

@@ -689,13 +689,15 @@ export class ConnectionStore {
             connResults = connResults.concat(recentConnections);
         }
 
-        // Deduplicate connections by ID
+        // Deduplicate connections by ID and profile source.
         const uniqueConnections = new Map<string, IConnectionProfileWithSource>();
         let dupeCount = 0;
 
         for (const conn of connResults) {
-            if (!uniqueConnections.has(conn.id)) {
-                uniqueConnections.set(conn.id, conn);
+            const key = conn.id + "-" + conn.profileSource; // Use both ID and source as key to allow same profile in both lists
+            const storedConn = uniqueConnections.get(key);
+            if (storedConn === undefined || storedConn?.profileSource !== conn.profileSource) {
+                uniqueConnections.set(key, conn);
             } else {
                 dupeCount++;
                 this._logger.verbose(

extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionsListContainer.tsx

@@ -144,7 +144,7 @@ export const ConnectionsListContainer = () => {
             <div className={styles.listScrollArea}>
                 <div className={styles.paneTitle}>
                     <Text weight="semibold" className={styles.paneTitle}>
-                        {locConstants.connectionDialog.savedConnections}
+                        {locConstants.connectionDialog.recentConnections}
                     </Text>
                     <Button
                         icon={<ArrowClockwise16Filled />}
@@ -153,27 +153,28 @@ export const ConnectionsListContainer = () => {
                         title={locConstants.common.refresh}
                     />
                 </div>
-                <div className={styles.main}>
-                    {savedConnections?.map((connection, index) => {
+                <Tree>
+                    {// state may not be initialized yet due to async loading of context
+                    recentConnections?.map((connection, index) => {
                         return (
                             <ConnectionCard
                                 connection={connection}
-                                key={"saved" + index}
+                                key={"mru" + index}
                                 actionButton={{
                                     icon: <Delete16Regular />,
                                     onClick: (e) => {
-                                        context.deleteSavedConnection(connection);
+                                        context.removeRecentConnection(connection);
                                         e.stopPropagation();
                                     },
-                                    tooltip: locConstants.connectionDialog.deleteSavedConnection,
+                                    tooltip: locConstants.connectionDialog.removeRecentConnection,
                                 }}
                             />
                         );
                     })}
-                </div>
+                </Tree>
                 <div className={styles.paneTitle}>
                     <Text weight="semibold" className={styles.paneTitle}>
-                        {locConstants.connectionDialog.recentConnections}
+                        {locConstants.connectionDialog.savedConnections}
                     </Text>
                     <Button
                         icon={<ArrowClockwise16Filled />}
@@ -182,25 +183,24 @@ export const ConnectionsListContainer = () => {
                         title={locConstants.common.refresh}
                     />
                 </div>
-                <Tree>
-                    {// state may not be initialized yet due to async loading of context
-                    recentConnections?.map((connection, index) => {
+                <div className={styles.main}>
+                    {savedConnections?.map((connection, index) => {
                         return (
                             <ConnectionCard
                                 connection={connection}
-                                key={"mru" + index}
+                                key={"saved" + index}
                                 actionButton={{
                                     icon: <Delete16Regular />,
                                     onClick: (e) => {
-                                        context.removeRecentConnection(connection);
+                                        context.deleteSavedConnection(connection);
                                         e.stopPropagation();
                                     },
-                                    tooltip: locConstants.connectionDialog.removeRecentConnection,
+                                    tooltip: locConstants.connectionDialog.deleteSavedConnection,
                                 }}
                             />
                         );
                     })}
-                </Tree>
+                </div>
             </div>
         </div>
     );

extensions/mssql/test/unit/connectionStore.test.ts

@@ -11,7 +11,11 @@ import { CredentialStore } from "../../src/credentialstore/credentialstore";
 import { Logger } from "../../src/models/logger";
 import { ConnectionConfig } from "../../src/connectionconfig/connectionconfig";
 import VscodeWrapper from "../../src/controllers/vscodeWrapper";
-import { IConnectionProfile, IConnectionProfileWithSource } from "../../src/models/interfaces";
+import {
+    CredentialsQuickPickItemType,
+    IConnectionProfile,
+    IConnectionProfileWithSource,
+} from "../../src/models/interfaces";
 import { MatchScore } from "../../src/models/utils";
 import { Deferred } from "../../src/protocol";
 import { azureAuthConn, sqlAuthConn, connStringConn } from "./utils.test";
@@ -36,7 +40,7 @@ suite("ConnectionStore Tests", () => {
         mockLogger = sandbox.createStubInstance(Logger);
 
         mockCredentialStore = sandbox.createStubInstance(CredentialStore);
-        mockCredentialStore.readCredential.resolves(undefined as any);
+        mockCredentialStore.readCredential.resolves(undefined);
         mockCredentialStore.saveCredential.resolves(true);
         mockCredentialStore.deleteCredential.resolves();
 
@@ -138,4 +142,39 @@ suite("ConnectionStore Tests", () => {
             score: MatchScore.NotMatch,
         });
     });
+
+    test("readAllConnections preserves saved and recent entries that share the same id", async () => {
+        const sharedId = "shared-connection-id";
+        const savedConnection = {
+            id: sharedId,
+            profileSource: CredentialsQuickPickItemType.Profile,
+            server: "saved-server",
+            database: "saved-db",
+        } as IConnectionProfileWithSource;
+        const recentConnection = {
+            id: sharedId,
+            profileSource: CredentialsQuickPickItemType.Mru,
+            server: "recent-server",
+            database: "recent-db",
+        } as IConnectionProfileWithSource;
+
+        mockConnectionConfig.getConnections.resolves([savedConnection]);
+
+        connectionStore = new ConnectionStore(
+            mockContext,
+            mockCredentialStore,
+            mockLogger,
+            mockConnectionConfig,
+            mockVscodeWrapper,
+        );
+
+        await connectionStore.initialized;
+        sandbox.stub(connectionStore, "getRecentlyUsedConnections").returns([recentConnection]);
+
+        const connections = await connectionStore.readAllConnections(true);
+
+        expect(connections).to.have.lengthOf(2);
+        expect(connections).to.deep.include(savedConnection);
+        expect(connections).to.deep.include(recentConnection);
+    });
 });