Notebook Parameterization - Papermill Compatibility (#13034)

* Parameterization papermill fix

* Utilize isParameter instead

* Address PR comments, and fix tests

* Address comment

src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts

@@ -15,7 +15,6 @@ import { NotebookEditor } from 'sql/workbench/contrib/notebook/browser/notebookE
 import { NBTestQueryManagementService } from 'sql/workbench/contrib/notebook/test/nbTestQueryManagementService';
 import * as stubs from 'sql/workbench/contrib/notebook/test/stubs';
 import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon';
-import { CellModel } from 'sql/workbench/services/notebook/browser/models/cell';
 import { ICellModel, NotebookContentChange } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
 import { INotebookEditor, INotebookParams, INotebookService, NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService';
 import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl';
@@ -61,13 +60,12 @@ import { TestNotificationService } from 'vs/platform/notification/test/common/te
 import { INotificationService } from 'vs/platform/notification/common/notification';
 
 class NotebookModelStub extends stubs.NotebookModelStub {
-	private _cells: Array<ICellModel> = [new CellModel(undefined, undefined)];
 	public contentChangedEmitter = new Emitter<NotebookContentChange>();
 	private _kernelChangedEmitter = new Emitter<nb.IKernelChangedArgs>();
 	private _onActiveCellChanged = new Emitter<ICellModel>();
 
-	get cells(): ReadonlyArray<ICellModel> {
-		return this._cells;
+	get cells(): ICellModel[] {
+		return this.cells;
 	}
 	public get contentChanged(): Event<NotebookContentChange> {
 		return this.contentChangedEmitter.event;

src/sql/workbench/contrib/notebook/test/stubs.ts

@@ -19,7 +19,7 @@ import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryText
 import { IContextViewProvider, IDelegate } from 'vs/base/browser/ui/contextview/contextview';
 
 export class NotebookModelStub implements INotebookModel {
-	constructor(private _languageInfo?: nb.ILanguageInfo) {
+	constructor(private _languageInfo?: nb.ILanguageInfo, private _cells?: ICellModel[]) {
 	}
 	trustedMode: boolean;
 	language: string;
@@ -31,8 +31,8 @@ export class NotebookModelStub implements INotebookModel {
 	onCellChange(cell: ICellModel, change: NotebookChangeType): void {
 		// Default: do nothing
 	}
-	get cells(): ReadonlyArray<ICellModel> {
-		throw new Error('method not implemented.');
+	get cells(): ICellModel[] | undefined {
+		return this._cells;
 	}
 	get activeCell(): ICellModel {
 		throw new Error('method not implemented.');

src/sql/workbench/services/notebook/browser/models/cell.ts

@@ -53,10 +53,10 @@ export class CellModel extends Disposable implements ICellModel {
 	private _cellUri: URI;
 	private _connectionManagementService: IConnectionManagementService;
 	private _stdInHandler: nb.MessageHandler<nb.IStdinMessage>;
+	private _metadata: { language?: string; tags?: string[]; cellGuid?: string; };
 	private _onCellLoaded = new Emitter<string>();
 	private _loaded: boolean;
 	private _stdInVisible: boolean;
-	private _metadata: { language?: string; tags?: string[]; cellGuid?: string; };
 	private _isCollapsed: boolean;
 	private _onCollapseStateChanged = new Emitter<boolean>();
 	private _modelContentChangedEvent: IModelContentChangedEvent;
@@ -355,6 +355,12 @@ export class CellModel extends Disposable implements ICellModel {
 		if (this.cellType !== CellTypes.Code) {
 			return;
 		}
+
+		/**
+		 * The value will not be updated if there is already a parameter cell in the Notebook.
+		**/
+		value = this.notebookModel?.cells?.find(cell => cell.isParameter) ? false : value;
+
 		let stateChanged = this._isParameter !== value;
 		this._isParameter = value;
 
@@ -801,11 +807,10 @@ export class CellModel extends Disposable implements ICellModel {
 		this.executionCount = cell.execution_count;
 		this._source = this.getMultilineSource(cell.source);
 		this._metadata = cell.metadata || {};
-
-		if (this._metadata.tags && this._metadata.tags.some(x => x === HideInputTag || x === ParametersTag || x === InjectedParametersTag) && this._cellType === CellTypes.Code) {
-			this._isCollapsed = true;
-			this._isParameter = true;
-			this._isInjectedParameter = true;
+		if (this._metadata.tags && this._cellType === CellTypes.Code) {
+			this._isCollapsed = this._metadata.tags.some(x => x === HideInputTag);
+			this._isParameter = this._metadata.tags.some(x => x === ParametersTag);
+			this._isInjectedParameter = this._metadata.tags.some(x => x === InjectedParametersTag);
 		} else {
 			this._isCollapsed = false;
 			this._isParameter = false;

src/sql/workbench/services/notebook/browser/models/notebookModel.ts

@@ -359,6 +359,16 @@ export class NotebookModel extends Disposable implements INotebookModel {
 				if (contents.cells && contents.cells.length > 0) {
 					this._cells = contents.cells.map(c => {
 						let cellModel = factory.createCell(c, { notebook: this, isTrusted: isTrusted });
+						/*
+						In a parameterized notebook there will be an injected parameter cell.
+						Papermill originally inserts the injected parameter with the comment "# Parameters"
+						which would make it confusing to the user between the difference between this cell and the tagged parameters cell.
+						So to make it clear we edit the injected parameters comment to indicate it is the Injected-Parameters cell.
+						*/
+						if (cellModel.isInjectedParameter) {
+							cellModel.source = cellModel.source.slice(1);
+							cellModel.source = '# Injected-Parameters\n' + cellModel.source;
+						}
 						this.trackMarkdownTelemetry(<nb.ICellContents>c, cellModel);
 						return cellModel;
 					});