PR Feedback

microsoft · Dec 8, 2019 · 2750e82 · 2750e82
1 parent bb5f71f
commit 2750e82
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 9 deletions.
diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts
@@ -38,6 +38,7 @@ import { UntitledTextEditorModel } from 'vs/workbench/common/editor/untitledText
 
 export const CODE_SELECTOR: string = 'code-component';
 const MARKDOWN_CLASS = 'markdown';
+const DEFAULT_OR_LOCAL_CONTEXT_ID = '-1';
 
 @Component({
 	selector: CODE_SELECTOR,
@@ -146,7 +147,7 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 			let connectionService = this.connectionService;
 			if (!shouldConnect && connectionService && connectionService.isConnected(cellUri)) {
 				connectionService.disconnect(cellUri).catch(e => this.logService.error(e));
-			} else if (shouldConnect && this._model.context && this._model.context.id !== '-1') {
+			} else if (shouldConnect && this._model.context && this._model.context.id !== DEFAULT_OR_LOCAL_CONTEXT_ID) {
 				connectionService.connect(this._model.context, cellUri).catch(e => this.logService.error(e));
 			}
 		}

diff --git a/src/sql/workbench/contrib/notebook/browser/models/notebookModel.ts b/src/sql/workbench/contrib/notebook/browser/models/notebookModel.ts
@@ -681,7 +681,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 
 			if (newConnection) {
 				this._activeConnection = newConnection;
-				this.refreshConnections(newConnection);
+				this.setActiveConnectionIfDifferent(newConnection);
 				this._activeClientSession.updateConnection(newConnection.toIConnectionProfile()).then(
 					result => {
 						//Remove 'Select connection' from 'Attach to' drop-down since its a valid connection
@@ -705,11 +705,11 @@ export class NotebookModel extends Disposable implements INotebookModel {
 		}
 	}
 
-	private refreshConnections(newConnection: ConnectionProfile) {
+	private setActiveConnectionIfDifferent(newConnection: ConnectionProfile) {
 		if (this.isValidConnection(newConnection) &&
 			this._activeConnection.id !== '-1' &&
 			this._activeConnection.id !== newConnection.id) {
-			// Change the defaultConnection to newConnection
+			// Change the active connection to newConnection
 			this._activeConnection = newConnection;
 		}
 	}

diff --git a/src/sql/workbench/contrib/notebook/browser/notebookActions.ts b/src/sql/workbench/contrib/notebook/browser/notebookActions.ts
@@ -374,6 +374,9 @@ export class AttachToDropdown extends SelectBox {
 			modelReady
 				.then(model => {
 					this.updateModel(model);
+					this._register(model.onValidConnectionSelected(validConnection => {
+						this.handleContextsChanged(!validConnection);
+					}));
 				})
 				.catch(err => {
 					// No-op for now

diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookContexts.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookContexts.test.ts
@@ -40,26 +40,26 @@ suite('Notebook Contexts', function (): void {
 
 		// No profile info provided
 		let conns = NotebookContexts.getContextForKernel(undefined, [mssqlProviderName]);
-		assert.deepStrictEqual(conns, defaultContext);
+		assert.deepStrictEqual(conns, defaultContext, 'Contexts not the same when no profile info passed in');
 
 		// Profile, but no provider IDs
 		let testConn = createTestConnProfile();
 		conns = NotebookContexts.getContextForKernel(testConn, []);
-		assert.deepStrictEqual(conns, localContext);
+		assert.deepStrictEqual(conns, localContext, 'Contexts not the same when no provider ids passed in');
 
 		// Normal use case
 		connService.setup(c => c.getActiveConnections()).returns(() => [testConn]);
 		conns = NotebookContexts.getContextForKernel(testConn, [mssqlProviderName]);
-		assert.deepStrictEqual(conns, testConn);
+		assert.deepStrictEqual(conns, testConn, 'Contexts not the same when testing mssql provider connections');
 
 		// Multiple provider IDs including mssql
 		connService.setup(c => c.getActiveConnections()).returns(() => [testConn]);
 		conns = NotebookContexts.getContextForKernel(testConn, [mssqlProviderName, 'fakeProvider']);
-		assert.deepStrictEqual(conns, testConn);
+		assert.deepStrictEqual(conns, testConn, 'Contexts not the same when multiple providers passed in');
 
 		// Connection provider IDs do not match
 		connService.setup(c => c.getActiveConnections()).returns(() => [testConn]);
 		conns = NotebookContexts.getContextForKernel(testConn, ['fakeProvider']);
-		assert.deepStrictEqual(conns, defaultContext);
+		assert.deepStrictEqual(conns, defaultContext, 'Contexts not the same when provider ids do not match');
 	});
 });