diff --git a/src/sql/workbench/contrib/notebook/browser/notebookActions.ts b/src/sql/workbench/contrib/notebook/browser/notebookActions.ts index e219387e4d..716337706d 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookActions.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookActions.ts @@ -45,6 +45,8 @@ const msgLocalHost = localize('localhost', "localhost"); export const noKernel: string = localize('noKernel', "No Kernel"); const baseIconClass = 'codicon'; const maskedIconClass = 'masked-icon'; +export const noParameterCell: string = localize('noParametersCell', "This notebook cannot run with parameters until a parameter cell is added. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization)."); +export const noParametersInCell: string = localize('noParametersInCell', "This notebook cannot run with parameters until there are parameters added to the parameter cell. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization)."); // Action to add a cell to notebook based on cell type(code/markdown). export class AddCellAction extends Action { @@ -308,14 +310,29 @@ export class RunParametersAction extends TooltipFromLabelAction { const editor = this._notebookService.findNotebookEditor(context); // Set defaultParameters to the parameter values in parameter cell let defaultParameters = new Map(); - editor.cells.forEach(cell => { + for (let cell of editor?.cells) { if (cell.isParameter) { + // Check if parameter cell is empty + const cellSource = typeof cell.source === 'string' ? [cell.source] : cell.source; + // Check to see if every line in the cell is empty or contains whitespace + const emptyParameterCell = cellSource.every(s => /^\s*$/.test(s)); + if (emptyParameterCell) { + // If there is no parameters in the cell indicate to user to add them + this.notificationService.notify({ + severity: Severity.Info, + message: noParametersInCell, + }); + return; + } for (let parameter of cell.source) { - let param = parameter.split('=', 2); - defaultParameters.set(param[0].trim(), param[1].trim()); + // Only add parameters that contain the proper parameters format (ex. x = 1) shown in the Parameterization Doc. + if (parameter.includes('=')) { + let param = parameter.split('=', 2); + defaultParameters.set(param[0].trim(), param[1].trim()); + } } } - }); + } // Store new parameters values the user inputs let inputParameters = new Map(); @@ -325,7 +342,7 @@ export class RunParametersAction extends TooltipFromLabelAction { // If there is no parameter cell indicate to user to create one this.notificationService.notify({ severity: Severity.Info, - message: localize('noParametersCell', "This notebook cannot run with parameters until a parameter cell is added. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization)."), + message: noParameterCell, }); return; } else { diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts index 0b6adb8671..5ea8f76dbd 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookActions.test.ts @@ -7,7 +7,7 @@ import * as assert from 'assert'; import * as azdata from 'azdata'; import * as sinon from 'sinon'; import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService'; -import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, KernelsDropdown, msgChanging, NewNotebookAction, noKernelName, RunAllCellsAction, RunParametersAction, TrustedAction } from 'sql/workbench/contrib/notebook/browser/notebookActions'; +import { AddCellAction, ClearAllOutputsAction, CollapseCellsAction, KernelsDropdown, msgChanging, NewNotebookAction, noKernelName, noParameterCell, noParametersInCell, RunAllCellsAction, RunParametersAction, TrustedAction } from 'sql/workbench/contrib/notebook/browser/notebookActions'; import { ClientSessionStub, ContextViewProviderStub, NotebookComponentStub, NotebookModelStub, NotebookServiceStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon'; import { ICellModel, INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; @@ -20,13 +20,12 @@ import { TestCommandService } from 'vs/editor/test/browser/editorTestServices'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { IConfigurationChangeEvent, IConfigurationOverrides, IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; -import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; +import { INotificationService } from 'vs/platform/notification/common/notification'; import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices'; import { URI } from 'vs/base/common/uri'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; import { MockQuickInputService } from 'sql/workbench/contrib/notebook/test/common/quickInputServiceMock'; -import { localize } from 'vs/nls'; class TestClientSession extends ClientSessionStub { private _errorState: boolean = false; @@ -302,7 +301,7 @@ suite('Notebook Actions', function (): void { assert.call(mockNotebookService.object.openNotebook(testUri, testShowOptions), 'Should Open Parameterized Notebook'); }); - test('Run with Parameters Action with no parameters in notebook', async function (): Promise { + test('Run with Parameters Action with no parameter cell in notebook', async function (): Promise { const testContents: azdata.nb.INotebookContents = { cells: [{ cell_type: CellTypes.Code, @@ -320,11 +319,7 @@ suite('Notebook Actions', function (): void { nbformat: 4, nbformat_minor: 5 }; - let expectedMsg: string = localize('noParametersCell', "This notebook cannot run with parameters until a parameter cell is added. [Learn more](https://docs.microsoft.com/sql/azure-data-studio/notebooks/notebooks-parameterization)."); - let notification = { - severity: Severity.Info, - message: expectedMsg, - }; + let expectedMsg: string = noParameterCell; let actualMsg: string; let mockNotification = TypeMoq.Mock.ofType(TestNotificationService); @@ -342,8 +337,137 @@ suite('Notebook Actions', function (): void { // Run Parameters Action await action.run(testUri); - assert.ok(actualMsg !== undefined, 'Should have received a notification'); - assert.call(mockNotification.object.notify(notification), 'Should notify user there is no parameter cell'); + assert.strictEqual(actualMsg, expectedMsg); + }); + + test('Run with Parameters Action with empty string parameter cell in notebook', async function (): Promise { + const testContents: azdata.nb.INotebookContents = { + cells: [{ + cell_type: CellTypes.Code, + source: [' '], + metadata: { language: 'python' }, + execution_count: 1 + }], + metadata: { + kernelspec: { + name: 'python', + language: 'python', + display_name: 'Python 3' + } + }, + nbformat: 4, + nbformat_minor: 5 + }; + let expectedMsg: string = noParametersInCell; + + let actualMsg: string; + let mockNotification = TypeMoq.Mock.ofType(TestNotificationService); + mockNotification.setup(n => n.notify(TypeMoq.It.isAny())).returns(notification => { + actualMsg = notification.message; + return undefined; + }); + let quickInputService = new MockQuickInputService; + let mockNotebookModel = new NotebookModelStub(undefined, undefined, testContents); + + let action = new RunParametersAction('TestId', true, testUri, quickInputService, mockNotebookService.object, mockNotification.object); + const testCells = [{ + isParameter: true, + source: [' '] + }]; + mockNotebookEditor.setup(x => x.model).returns(() => mockNotebookModel); + mockNotebookEditor.setup(x => x.cells).returns(() => testCells); + + // Run Parameters Action + await action.run(testUri); + + assert.strictEqual(actualMsg, expectedMsg); + }); + + test('Run with Parameters Action with empty array string parameter cell in notebook', async function (): Promise { + const testContents: azdata.nb.INotebookContents = { + cells: [{ + cell_type: CellTypes.Code, + source: [' ', ' '], + metadata: { language: 'python' }, + execution_count: 1 + }], + metadata: { + kernelspec: { + name: 'python', + language: 'python', + display_name: 'Python 3' + } + }, + nbformat: 4, + nbformat_minor: 5 + }; + let expectedMsg: string = noParametersInCell; + + let actualMsg: string; + let mockNotification = TypeMoq.Mock.ofType(TestNotificationService); + mockNotification.setup(n => n.notify(TypeMoq.It.isAny())).returns(notification => { + actualMsg = notification.message; + return undefined; + }); + let quickInputService = new MockQuickInputService; + let mockNotebookModel = new NotebookModelStub(undefined, undefined, testContents); + + let action = new RunParametersAction('TestId', true, testUri, quickInputService, mockNotebookService.object, mockNotification.object); + + const testCells = [{ + isParameter: true, + source: [' ', ' '] + }]; + mockNotebookEditor.setup(x => x.model).returns(() => mockNotebookModel); + mockNotebookEditor.setup(x => x.cells).returns(() => testCells); + + // Run Parameters Action + await action.run(testUri); + + assert.strictEqual(actualMsg, expectedMsg); + }); + + test('Run with Parameters Action with empty parameter cell in notebook', async function (): Promise { + const testContents: azdata.nb.INotebookContents = { + cells: [{ + cell_type: CellTypes.Code, + source: [], + metadata: { language: 'python' }, + execution_count: 1 + }], + metadata: { + kernelspec: { + name: 'python', + language: 'python', + display_name: 'Python 3' + } + }, + nbformat: 4, + nbformat_minor: 5 + }; + let expectedMsg: string = noParametersInCell; + + let actualMsg: string; + let mockNotification = TypeMoq.Mock.ofType(TestNotificationService); + mockNotification.setup(n => n.notify(TypeMoq.It.isAny())).returns(notification => { + actualMsg = notification.message; + return undefined; + }); + let quickInputService = new MockQuickInputService; + let mockNotebookModel = new NotebookModelStub(undefined, undefined, testContents); + + let action = new RunParametersAction('TestId', true, testUri, quickInputService, mockNotebookService.object, mockNotification.object); + + const testCells = [{ + isParameter: true, + source: [] + }]; + mockNotebookEditor.setup(x => x.model).returns(() => mockNotebookModel); + mockNotebookEditor.setup(x => x.cells).returns(() => testCells); + + // Run Parameters Action + await action.run(testUri); + assert.strictEqual(actualMsg, expectedMsg); });