From dfc0ce638c1ecd024f72980d08bfbea35fc602d3 Mon Sep 17 00:00:00 2001 From: Alex Ma Date: Wed, 6 Sep 2023 09:40:39 -0700 Subject: [PATCH] Added fix to drag and drop (#24252) * Added fix to drag and drop by updating check and adding notification upon failed drag * update to connectionConfig test --- .../connection/common/connectionConfig.ts | 20 +++++----- .../test/common/connectionConfig.test.ts | 37 +++++++++---------- .../browser/asyncServerTreeDragAndDrop.ts | 4 +- .../browser/dragAndDropController.ts | 11 ++++++ .../asyncServerTreeDragAndDrop.test.ts | 3 +- .../browser/dragAndDropController.test.ts | 3 +- 6 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/sql/platform/connection/common/connectionConfig.ts b/src/sql/platform/connection/common/connectionConfig.ts index 379dcaf2aa..bc1dd55ef9 100644 --- a/src/sql/platform/connection/common/connectionConfig.ts +++ b/src/sql/platform/connection/common/connectionConfig.ts @@ -363,15 +363,17 @@ export class ConnectionConfig { */ public canChangeConnectionConfig(profile: ConnectionProfile, newGroupID: string): boolean { let profiles = this.getIConnectionProfileStores(true); - let existingProfile = profiles.find(p => - p.providerName === profile.providerName && - this.checkIfAuthenticationOptionsMatch(p, profile) && - p.options.databaseName === profile.options.databaseName && - p.options.serverName === profile.options.serverName && - p.options.userName === profile.options.userName && - p.options.connectionName === profile.options.connectionName && - p.groupId === newGroupID && - this.checkIfNonDefaultOptionsMatch(p, profile)); + let existingProfile = profiles.find(p => { + let authenticationCheck = this.checkIfAuthenticationOptionsMatch(p, profile); + let nonDefaultCheck = this.checkIfNonDefaultOptionsMatch(p, profile); + let basicOptionCheck = p.providerName === profile.providerName && + p.options.database === profile.options.database && + p.options.server === profile.options.server && + p.options.user === profile.options.user && + p.options.connectionName === profile.options.connectionName && + p.groupId === newGroupID; + return authenticationCheck && nonDefaultCheck && basicOptionCheck; + }); return existingProfile === undefined; } diff --git a/src/sql/platform/connection/test/common/connectionConfig.test.ts b/src/sql/platform/connection/test/common/connectionConfig.test.ts index 283ba00777..b131cb5697 100644 --- a/src/sql/platform/connection/test/common/connectionConfig.test.ts +++ b/src/sql/platform/connection/test/common/connectionConfig.test.ts @@ -78,9 +78,9 @@ suite('ConnectionConfig', () => { const testConnections: IConnectionProfileStore[] = deepFreeze([ { options: { - serverName: 'server1', - databaseName: 'database', - userName: 'user', + server: 'server1', + database: 'database', + user: 'user', password: 'password', authenticationType: 'SqlLogin' }, @@ -91,9 +91,9 @@ suite('ConnectionConfig', () => { }, { options: { - serverName: 'server2', - databaseName: 'database', - userName: 'user', + server: 'server2', + database: 'database', + user: 'user', password: 'password', authenticationType: 'SqlLogin' }, @@ -104,9 +104,9 @@ suite('ConnectionConfig', () => { }, { options: { - serverName: 'server3', - databaseName: 'database', - userName: 'user', + server: 'server3', + database: 'database', + user: 'user', password: 'password', authenticationType: 'SqlLogin' }, @@ -123,7 +123,7 @@ suite('ConnectionConfig', () => { let connectionProvider: azdata.ConnectionProviderOptions = { options: [ { - name: 'serverName', + name: 'server', displayName: undefined!, description: undefined!, groupName: undefined!, @@ -135,7 +135,7 @@ suite('ConnectionConfig', () => { valueType: ServiceOptionType.string }, { - name: 'databaseName', + name: 'database', displayName: undefined!, description: undefined!, groupName: undefined!, @@ -147,7 +147,7 @@ suite('ConnectionConfig', () => { valueType: ServiceOptionType.string }, { - name: 'userName', + name: 'user', displayName: undefined!, description: undefined!, groupName: undefined!, @@ -326,9 +326,9 @@ suite('ConnectionConfig', () => { test('addConnection should not add the new profile to user settings if already exists', async () => { let existingConnection = testConnections[0]; let newProfile: IConnectionProfile = { - serverName: existingConnection.options['serverName'], - databaseName: existingConnection.options['databaseName'], - userName: existingConnection.options['userName'], + serverName: existingConnection.options['server'], + databaseName: existingConnection.options['database'], + userName: existingConnection.options['user'], password: existingConnection.options['password'], authenticationType: existingConnection.options['authenticationType'], groupId: existingConnection.groupId, @@ -350,7 +350,6 @@ suite('ConnectionConfig', () => { configurationService.updateValue('datasource.connections', deepClone(testConnections), ConfigurationTarget.USER); let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile); - connectionProfile.options['databaseDisplayName'] = existingConnection.options['databaseName']; let config = new ConnectionConfig(configurationService, capabilitiesService.object); await config.addConnection(connectionProfile); @@ -413,7 +412,7 @@ suite('ConnectionConfig', () => { test('getConnections should return connections with a valid id', () => { let workspaceConnections = deepClone(testConnections).map(c => { - c.id = c.options['serverName']; + c.id = c.options['server']; return c; }); let userConnections = deepClone(testConnections).map(c => { @@ -428,12 +427,12 @@ suite('ConnectionConfig', () => { let allConnections = config.getConnections(false); assert.strictEqual(allConnections.length, testConnections.length); allConnections.forEach(connection => { - let userConnection = testConnections.find(u => u.options['serverName'] === connection.serverName); + let userConnection = testConnections.find(u => u.options['server'] === connection.serverName); if (userConnection !== undefined) { assert.notStrictEqual(connection.id, connection.getOptionsKey()); assert.ok(!!connection.id); } else { - let workspaceConnection = workspaceConnections.find(u => u.options['serverName'] === connection.serverName); + let workspaceConnection = workspaceConnections.find(u => u.options['server'] === connection.serverName); assert.notStrictEqual(connection.id, connection.getOptionsKey()); assert.strictEqual(workspaceConnection!.id, connection.id); } diff --git a/src/sql/workbench/services/objectExplorer/browser/asyncServerTreeDragAndDrop.ts b/src/sql/workbench/services/objectExplorer/browser/asyncServerTreeDragAndDrop.ts index 69bffe278d..e7d24759c8 100644 --- a/src/sql/workbench/services/objectExplorer/browser/asyncServerTreeDragAndDrop.ts +++ b/src/sql/workbench/services/objectExplorer/browser/asyncServerTreeDragAndDrop.ts @@ -11,6 +11,7 @@ import { IDragAndDropData } from 'vs/base/browser/dnd'; import { ITreeDragAndDrop, ITreeDragOverReaction, TreeDragOverReactions } from 'vs/base/browser/ui/tree/tree'; import { ServerTreeDragAndDrop } from 'sql/workbench/services/objectExplorer/browser/dragAndDropController'; import { ServerTreeElement, AsyncServerTree } from 'sql/workbench/services/objectExplorer/browser/asyncServerTree'; +import { INotificationService } from 'vs/platform/notification/common/notification'; /** * Implements drag and drop for the server tree @@ -22,8 +23,9 @@ export class AsyncServerTreeDragAndDrop implements ITreeDragAndDrop { undefined, // configuration service new TestCapabilitiesService(), // capabilities service ); - serverTreeDragAndDrop = new AsyncServerTreeDragAndDrop(mockConnectionManagementService.object); + serverTreeDragAndDrop = new AsyncServerTreeDragAndDrop(mockConnectionManagementService.object, new TestNotificationService()); capabilitiesService = new TestCapabilitiesService(); capabilitiesService.capabilities[mssqlProviderName] = { connection: msSQLCapabilities }; diff --git a/src/sql/workbench/services/objectExplorer/test/browser/dragAndDropController.test.ts b/src/sql/workbench/services/objectExplorer/test/browser/dragAndDropController.test.ts index 6ba346a9e0..967d624fc0 100644 --- a/src/sql/workbench/services/objectExplorer/test/browser/dragAndDropController.test.ts +++ b/src/sql/workbench/services/objectExplorer/test/browser/dragAndDropController.test.ts @@ -10,6 +10,7 @@ import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/t import { IStorageService } from 'vs/platform/storage/common/storage'; import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { ServerTreeDragAndDrop } from 'sql/workbench/services/objectExplorer/browser/dragAndDropController'; import { TestTree } from 'sql/workbench/test/browser/parts/tree/treeMock'; import { ConnectionProviderProperties } from 'sql/platform/capabilities/common/capabilitiesService'; @@ -90,7 +91,7 @@ suite('SQL Drag And Drop Controller tests', () => { undefined, // configuration service new TestCapabilitiesService(), // capabilities service ); - serverTreeDragAndDrop = new ServerTreeDragAndDrop(mockConnectionManagementService.object); + serverTreeDragAndDrop = new ServerTreeDragAndDrop(mockConnectionManagementService.object, new TestNotificationService()); capabilitiesService = new TestCapabilitiesService(); capabilitiesService.capabilities[mssqlProviderName] = { connection: msSQLCapabilities };