Fix schema compare include/exclude behavior (#8042)

* don't uncheck difference if unsuccessful

* changes after rebasing to get schema compare fix

* First cut of column checkbox checking reactive to include opteration

* handle blocking dependencies and affected dependencies

* Changing the checked property of table to be list

* Addressing comments

* add map to keep row number of diff entries

* remove findDifferenceRow() since it isn't needed anymore

* fix scrolling to the top when checking/unchecking and add info message

* change checked to updateCells

* improve warning cannot include/exclude message

extensions/mssql/src/mssql.d.ts

@@ -75,6 +75,11 @@ export interface SchemaCompareResult extends azdata.ResultStatus {
 	differences: DiffEntry[];
 }
 
+export interface SchemaCompareIncludeExcludeResult extends azdata.ResultStatus {
+	affectedDependencies: DiffEntry[];
+	blockingDependencies: DiffEntry[];
+}
+
 export interface SchemaCompareCompletionResult extends azdata.ResultStatus {
 	operationId: string;
 	areEqual: boolean;
@@ -91,6 +96,7 @@ export interface DiffEntry {
 	children: DiffEntry[];
 	sourceScript: string;
 	targetScript: string;
+	included: boolean;
 }
 
 export const enum SchemaUpdateAction {
@@ -290,7 +296,7 @@ export interface ISchemaCompareService {
 	schemaCompareGenerateScript(operationId: string, targetServerName: string, targetDatabaseName: string, taskExecutionMode: azdata.TaskExecutionMode): Thenable<azdata.ResultStatus>;
 	schemaComparePublishChanges(operationId: string, targetServerName: string, targetDatabaseName: string, taskExecutionMode: azdata.TaskExecutionMode): Thenable<azdata.ResultStatus>;
 	schemaCompareGetDefaultOptions(): Thenable<SchemaCompareOptionsResult>;
-	schemaCompareIncludeExcludeNode(operationId: string, diffEntry: DiffEntry, IncludeRequest: boolean, taskExecutionMode: azdata.TaskExecutionMode): Thenable<azdata.ResultStatus>;
+	schemaCompareIncludeExcludeNode(operationId: string, diffEntry: DiffEntry, IncludeRequest: boolean, taskExecutionMode: azdata.TaskExecutionMode): Thenable<SchemaCompareIncludeExcludeResult>;
 	schemaCompareOpenScmp(filePath: string): Thenable<SchemaCompareOpenScmpResult>;
 	schemaCompareSaveScmp(sourceEndpointInfo: SchemaCompareEndpointInfo, targetEndpointInfo: SchemaCompareEndpointInfo, taskExecutionMode: azdata.TaskExecutionMode, deploymentOptions: DeploymentOptions, scmpFilePath: string, excludedSourceObjects: SchemaCompareObjectId[], excludedTargetObjects: SchemaCompareObjectId[]): Thenable<azdata.ResultStatus>;
 	schemaCompareCancel(operationId: string): Thenable<azdata.ResultStatus>;

extensions/mssql/src/schemaCompare/schemaCompareService.ts

@@ -76,7 +76,7 @@ export class SchemaCompareService implements mssql.ISchemaCompareService {
 		);
 	}
 
-	public schemaCompareIncludeExcludeNode(operationId: string, diffEntry: mssql.DiffEntry, includeRequest: boolean, taskExecutionMode: azdata.TaskExecutionMode): Thenable<azdata.ResultStatus> {
+	public schemaCompareIncludeExcludeNode(operationId: string, diffEntry: mssql.DiffEntry, includeRequest: boolean, taskExecutionMode: azdata.TaskExecutionMode): Thenable<mssql.SchemaCompareIncludeExcludeResult> {
 		const params: contracts.SchemaCompareNodeParams = { operationId: operationId, diffEntry, includeRequest, taskExecutionMode: taskExecutionMode };
 		return this.client.sendRequest(contracts.SchemaCompareIncludeExcludeNodeRequest.type, params).then(
 			undefined,

extensions/schema-compare/src/schemaCompareMainWindow.ts

@@ -22,6 +22,8 @@ const generateScriptEnabledMessage = localize('schemaCompare.generateScriptEnabl
 const generateScriptNoChangesMessage = localize('schemaCompare.generateScriptNoChanges', "No changes to script");
 const applyEnabledMessage = localize('schemaCompare.applyButtonEnabledTitle', "Apply changes to target");
 const applyNoChangesMessage = localize('schemaCompare.applyNoChanges', "No changes to apply");
+const includeExcludeInfoMessage = localize('schemaCompare.includeExcludeInfoMessage', "Please note that include/exclude operations can take a moment to calculate affected dependencies");
+
 // Do not localize this, this is used to decide the icon for the editor.
 // TODO : In future icon should be decided based on language id (scmp) and not resource name
 const schemaCompareResourceName = 'Schema Compare';
@@ -69,6 +71,8 @@ export class SchemaCompareMainWindow {
 	private targetName: string;
 	private scmpSourceExcludes: mssql.SchemaCompareObjectId[];
 	private scmpTargetExcludes: mssql.SchemaCompareObjectId[];
+	private diffEntryRowMap = new Map<string, number>();
+	private showIncludeExcludeWaitingMessage: boolean = true;
 
 	public sourceEndpointInfo: mssql.SchemaCompareEndpointInfo;
 	public targetEndpointInfo: mssql.SchemaCompareEndpointInfo;
@@ -101,7 +105,6 @@ export class SchemaCompareMainWindow {
 		this.editor.registerContent(async view => {
 			this.differencesTable = view.modelBuilder.table().withProperties({
 				data: [],
-				height: 300,
 				title: localize('schemaCompare.differencesTableTitle', "Comparison between Source and Target")
 			}).component();
 
@@ -343,6 +346,11 @@ export class SchemaCompareMainWindow {
 		if (this.comparisonResult.differences.length > 0) {
 			this.flexModel.addItem(this.splitView);
 
+			// create a map of the differences to row numbers
+			for (let i = 0; i < data.length; ++i) {
+				this.diffEntryRowMap.set(this.createDiffEntryKey(this.comparisonResult.differences[i]), i);
+			}
+
 			// only enable generate script button if the target is a db
 			if (this.targetEndpointInfo.endpointType === mssql.SchemaCompareEndpointType.Database) {
 				this.generateScriptButton.enabled = true;
@@ -386,9 +394,54 @@ export class SchemaCompareMainWindow {
 		this.tablelistenersToDispose.push(this.differencesTable.onCellAction(async (rowState) => {
 			let checkboxState = <azdata.ICheckboxCellActionEventArgs>rowState;
 			if (checkboxState) {
+				// show an info notification the first time when trying to exclude to notify the user that it may take some time to calculate affected dependencies
+				if (this.showIncludeExcludeWaitingMessage) {
+					this.showIncludeExcludeWaitingMessage = false;
+					vscode.window.showInformationMessage(includeExcludeInfoMessage);
+				}
+
 				let diff = this.comparisonResult.differences[checkboxState.row];
-				await service.schemaCompareIncludeExcludeNode(this.comparisonResult.operationId, diff, checkboxState.checked, azdata.TaskExecutionMode.execute);
+				const result = await service.schemaCompareIncludeExcludeNode(this.comparisonResult.operationId, diff, checkboxState.checked, azdata.TaskExecutionMode.execute);
-				this.saveExcludeState(checkboxState);
+				let checkboxesToChange = [];
+				if (result.success) {
+					this.saveExcludeState(checkboxState);
+
+					// dependencies could have been included or excluded as a result, so save their exclude states
+					result.affectedDependencies.forEach(difference => {
+						// find the row of the difference and set its checkbox
+						const diffEntryKey = this.createDiffEntryKey(difference);
+						if (this.diffEntryRowMap.has(diffEntryKey)) {
+							const row = this.diffEntryRowMap.get(diffEntryKey);
+							checkboxesToChange.push({ row: row, column: 2, columnName: 'Include', checked: difference.included });
+							const dependencyCheckBoxState: azdata.ICheckboxCellActionEventArgs = {
+								checked: difference.included,
+								row: row,
+								column: 2,
+								columnName: undefined
+							};
+							this.saveExcludeState(dependencyCheckBoxState);
+						}
+					});
+				} else {
+					// failed because of dependencies
+					if (result.blockingDependencies) {
+						// show the first dependent that caused this to fail in the warning message
+						const diffEntryName = this.createName(diff.sourceValue ? diff.sourceValue : diff.targetValue);
+						const firstDependentName = this.createName(result.blockingDependencies[0].sourceValue ? result.blockingDependencies[0].sourceValue : result.blockingDependencies[0].targetValue);
+						const cannotExcludeMessage = localize('schemaCompare.cannotExcludeMessage', "Cannot exclude {0}. Included dependents exist such as {1}", diffEntryName, firstDependentName);
+						const cannotIncludeMessage = localize('schemaCompare.cannotIncludeMessage', "Cannot include {0}. Excluded dependents exist such as {1}", diffEntryName, firstDependentName);
+						vscode.window.showWarningMessage(checkboxState.checked ? cannotIncludeMessage : cannotExcludeMessage);
+					} else {
+						vscode.window.showWarningMessage(result.errorMessage);
+					}
+
+					// set checkbox back to previous state
+					checkboxesToChange.push({ row: checkboxState.row, column: checkboxState.column, columnName: 'Include', checked: !checkboxState.checked });
+				}
+
+				if (checkboxesToChange.length > 0) {
+					this.differencesTable.updateCells = checkboxesToChange;
+				}
 			}
 		}));
 	}
@@ -396,6 +449,7 @@ export class SchemaCompareMainWindow {
 	// save state based on source name if present otherwise target name (parity with SSDT)
 	private saveExcludeState(rowState: azdata.ICheckboxCellActionEventArgs) {
 		if (rowState) {
+			this.differencesTable.data[rowState.row][2] = rowState.checked;
 			let diff = this.comparisonResult.differences[rowState.row];
 			let key = (diff.sourceValue && diff.sourceValue.length > 0) ? this.createName(diff.sourceValue) : this.createName(diff.targetValue);
 			if (key) {
@@ -460,7 +514,6 @@ export class SchemaCompareMainWindow {
 
 	private removeExcludeEntry(collection: mssql.SchemaCompareObjectId[], entryName: string) {
 		if (collection) {
-			console.error('removing ' + entryName);
 			const index = collection.findIndex(e => this.createName(e.nameParts) === entryName);
 			collection.splice(index, 1);
 		}
@@ -491,6 +544,10 @@ export class SchemaCompareMainWindow {
 		return nameParts.join('.');
 	}
 
+	private createDiffEntryKey(entry: mssql.DiffEntry): string {
+		return `${this.createName(entry.sourceValue)}_${this.createName(entry.targetValue)}_${entry.updateAction}_${entry.name}`;
+	}
+
 	private getFormattedScript(diffEntry: mssql.DiffEntry, getSourceScript: boolean): string {
 		// if there is no entry, the script has to be \n because an empty string shows up as a difference but \n doesn't
 		if ((getSourceScript && diffEntry.sourceScript === null)
@@ -525,6 +582,7 @@ export class SchemaCompareMainWindow {
 		this.flexModel.removeItem(this.startText);
 		this.flexModel.addItem(this.loader, { CSSStyles: { 'margin-top': '30px' } });
 		this.flexModel.addItem(this.waitText, { CSSStyles: { 'margin-top': '30px', 'align-self': 'center' } });
+		this.showIncludeExcludeWaitingMessage = true;
 		this.diffEditor.updateProperties({
 			contentLeft: os.EOL,
 			contentRight: os.EOL,

extensions/schema-compare/src/test/testSchemaCompareService.ts

@@ -24,7 +24,7 @@ export class SchemaCompareTestService implements mssql.ISchemaCompareService {
 		return Promise.resolve(result);
 	}
 
-	schemaCompareIncludeExcludeNode(operationId: string, diffEntry: mssql.DiffEntry, IncludeRequest: boolean, taskExecutionMode: azdata.TaskExecutionMode): Thenable<azdata.ResultStatus> {
+	schemaCompareIncludeExcludeNode(operationId: string, diffEntry: mssql.DiffEntry, IncludeRequest: boolean, taskExecutionMode: azdata.TaskExecutionMode): Thenable<mssql.SchemaCompareIncludeExcludeResult> {
 		throw new Error('Method not implemented.');
 	}
 

src/sql/azdata.d.ts

@@ -3089,9 +3089,15 @@ declare module 'azdata' {
 		ariaColumnCount?: number;
 		ariaRole?: string;
 		focused?: boolean;
+		updateCells?: TableCell[];
 		moveFocusOutWithTab?: boolean; //accessibility requirement for tables with no actionable cells
 	}
 
+	export interface CheckBoxCell extends TableCell {
+		checked: boolean;
+		columnName: string;
+	}
+
 	export interface FileBrowserTreeProperties extends ComponentProperties {
 		ownerUri: string;
 	}

src/sql/base/browser/ui/table/plugins/checkboxSelectColumn.plugin.ts

@@ -189,7 +189,24 @@ export class CheckboxSelectColumn<T extends Slick.SlickData> implements Slick.Pl
 		}
 
 		//Ensure that the focus stays correct
-		this._grid.getActiveCellNode().focus();
+		if(this._grid.getActiveCellNode()) {
+			this._grid.getActiveCellNode().focus();
+		}
+
+		// set selected row to the row of this checkbox
+		this._grid.setSelectedRows([row]);
+	}
+
+	// This call is to handle reactive changes in check box UI
+	// This DOES NOT fire UI change Events
+	reactiveCheckboxCheck(row: number, value: boolean) {
+		value ? this._selectedCheckBoxLookup[row] = true : delete this._selectedCheckBoxLookup[row];
+
+		// update row to call formatter
+		this._grid.updateRow(row);
+
+		// ensure that grid reflects the change
+		this._grid.scrollRowIntoView(row);
 	}
 
 	private handleHeaderClick(e: Event, args: Slick.OnHeaderClickEventArgs<T>): void {

src/sql/workbench/api/common/extHostModelView.ts

@@ -1281,6 +1281,14 @@ class TableComponentWrapper extends ComponentWrapper implements azdata.TableComp
 		this.setProperty('focused', v);
 	}
 
+	public get updateCells(): azdata.TableCell[] {
+		return this.properties['updateCells'];
+	}
+
+	public set updateCells(v: azdata.TableCell[]) {
+		this.setProperty('updateCells', v);
+	}
+
 	public get onRowSelected(): vscode.Event<any> {
 		let emitter = this._emitterMap.get(ComponentEventType.onSelectedRowChanged);
 		return emitter && emitter.event;

src/sql/workbench/api/common/sqlExtHostTypes.ts

@@ -203,6 +203,12 @@ export enum ExtensionNodeType {
 	Database = 'Database'
 }
 
+export interface CheckBoxInfo {
+	row: number;
+	columnName: string;
+	checked: boolean;
+}
+
 export interface IComponentShape {
 	type: ModelComponentTypes;
 	id: string;

src/sql/workbench/browser/modelComponents/table.component.ts

@@ -26,6 +26,7 @@ import { Emitter, Event as vsEvent } from 'vs/base/common/event';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { KeyMod, KeyCode } from 'vs/base/common/keyCodes';
 import { slickGridDataItemColumnValueWithNoData, textFormatter } from 'sql/base/browser/ui/table/formatters';
+import { isNullOrUndefined } from 'util';
 
 @Component({
 	selector: 'modelview-table',
@@ -255,10 +256,27 @@ export default class TableComponent extends ComponentBase implements IComponent,
 			this._table.focus();
 		}
 
+		if (this.updateCells !== undefined) {
+			this.updateTableCells(this.updateCells);
+		}
+
 		this.layoutTable();
 		this.validate();
 	}
 
+	private updateTableCells(cellInfos): void {
+		cellInfos.forEach((cellInfo) => {
+			if (isNullOrUndefined(cellInfo.column) || isNullOrUndefined(cellInfo.row) || cellInfo.row < 0 || cellInfo.row > this.data.length) {
+				return;
+			}
+
+			const checkInfo: azdata.CheckBoxCell = cellInfo as azdata.CheckBoxCell;
+			if (checkInfo) {
+				this._checkboxColumns[checkInfo.columnName].reactiveCheckboxCheck(checkInfo.row, checkInfo.checked);
+			}
+		});
+	}
+
 	private createCheckBoxPlugin(col: any, index: number) {
 		let name = col.value;
 		if (!this._checkboxColumns[col.value]) {
@@ -357,4 +375,12 @@ export default class TableComponent extends ComponentBase implements IComponent,
 	public set focused(newValue: boolean) {
 		this.setPropertyFromUI<azdata.RadioButtonProperties, boolean>((properties, value) => { properties.focused = value; }, newValue);
 	}
+
+	public get updateCells(): azdata.TableCell[] {
+		return this.getPropertyOrDefault<azdata.TableComponentProperties, azdata.TableCell[]>((props) => props.updateCells, undefined);
+	}
+
+	public set updateCells(newValue: azdata.TableCell[]) {
+		this.setPropertyFromUI<azdata.TableComponentProperties, azdata.TableCell[]>((properties, value) => { properties.updateCells = value; }, newValue);
+	}
 }