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

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;
 	}
 

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);
 			}

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<ServerTreeEl
 
 	constructor(
 		@IConnectionManagementService connectionManagementService: IConnectionManagementService,
+		@INotificationService notificationService: INotificationService
 	) {
-		this._dragAndDrop = new ServerTreeDragAndDrop(connectionManagementService);
+		this._dragAndDrop = new ServerTreeDragAndDrop(connectionManagementService, notificationService);
 	}
 
 	/**

src/sql/workbench/services/objectExplorer/browser/dragAndDropController.ts

@@ -13,6 +13,8 @@ import { UNSAVED_GROUP_ID, mssqlProviderName, pgsqlProviderName } from 'sql/plat
 import { DataTransfers, IDragAndDropData } from 'vs/base/browser/dnd';
 import { TreeNode } from 'sql/workbench/services/objectExplorer/common/treeNode';
 import { AsyncServerTree } from 'sql/workbench/services/objectExplorer/browser/asyncServerTree';
+import { INotificationService } from 'vs/platform/notification/common/notification';
+import { localize } from 'vs/nls';
 
 export function supportsNodeNameDrop(nodeId: string): boolean {
 	if (nodeId === 'Table' || nodeId === 'Column' || nodeId === 'View' || nodeId === 'Function') {
@@ -52,9 +54,13 @@ function escapeString(input: string | undefined): string | undefined {
  */
 export class ServerTreeDragAndDrop implements IDragAndDrop {
 
+	private rejectDueToDupe: boolean;
+
 	constructor(
 		@IConnectionManagementService private _connectionManagementService: IConnectionManagementService,
+		@INotificationService private _notificationService: INotificationService
 	) {
+		this.rejectDueToDupe = false;
 	}
 
 	/**
@@ -195,6 +201,7 @@ export class ServerTreeDragAndDrop implements IDragAndDrop {
 			} else if (targetElement instanceof ConnectionProfile) {
 				canDragOver = source.groupId !== targetElement.groupId &&
 					this._connectionManagementService.canChangeConnectionConfig(source, targetElement.groupId);
+				this.rejectDueToDupe = !canDragOver;
 			} else if (targetElement instanceof TreeNode) {
 				canDragOver = source.groupId !== this.getTreeNodeParentGroup(targetElement).id;
 			}
@@ -263,6 +270,10 @@ export class ServerTreeDragAndDrop implements IDragAndDrop {
 	}
 
 	public dropAbort(tree: ITree, data: IDragAndDropData): void {
+		if (this.rejectDueToDupe) {
+			this.rejectDueToDupe = false;
+			this._notificationService.info(localize('objectExplorer.dragAndDropController.existingIdenticalProfile', 'Cannot drag profile into group: A profile with identical options already exists in the group.'));
+		}
 		TreeUpdateUtils.isInDragAndDrop = false;
 	}
 

src/sql/workbench/services/objectExplorer/test/browser/asyncServerTreeDragAndDrop.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 { ConnectionProviderProperties } from 'sql/platform/capabilities/common/capabilitiesService';
 import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
 import { mssqlProviderName } from 'sql/platform/connection/common/constants';
@@ -63,7 +64,7 @@ suite('AsyncServerTreeDragAndDrop', () => {
 			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 };

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 };