From 8497f5e861fa6e0a86425e00067a0be9ed16f504 Mon Sep 17 00:00:00 2001 From: Rhys Howell Date: Fri, 27 Sep 2024 16:13:25 -0400 Subject: [PATCH] fix(chat): avoid passing file path when its not used in playground run --- package.json | 10 +- src/commands/index.ts | 4 +- src/editors/playgroundController.ts | 101 ++++++++++++------ src/mdbExtensionController.ts | 14 +-- src/participant/participant.ts | 8 +- .../editors/playgroundController.test.ts | 6 +- .../suite/participant/participant.test.ts | 8 +- .../suite/telemetry/telemetryService.test.ts | 4 +- 8 files changed, 98 insertions(+), 57 deletions(-) diff --git a/package.json b/package.json index d8c108c3d..52002702a 100644 --- a/package.json +++ b/package.json @@ -191,11 +191,11 @@ "title": "MongoDB: Change Active Connection with Participant" }, { - "command": "mdb.runParticipantQuery", + "command": "mdb.runParticipantCode", "title": "Run Content Generated by Participant" }, { - "command": "mdb.openParticipantQueryInPlayground", + "command": "mdb.openParticipantCodeInPlayground", "title": "Open Generated by Participant Content In Playground" }, { @@ -765,15 +765,15 @@ "when": "false" }, { - "command": "mdb.runParticipantQuery", + "command": "mdb.runParticipantCode", "when": "false" }, { - "command": "mdb.openParticipantQueryInPlayground", + "command": "mdb.openParticipantCodeInPlayground", "when": "false" }, { - "command": "mdb.runParticipantQuery", + "command": "mdb.runParticipantCode", "when": "false" }, { diff --git a/src/commands/index.ts b/src/commands/index.ts index 767b99ffb..c36c477d2 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -77,8 +77,8 @@ enum EXTENSION_COMMANDS { MDB_DROP_STREAM_PROCESSOR = 'mdb.dropStreamProcessor', // Chat participant. - OPEN_PARTICIPANT_QUERY_IN_PLAYGROUND = 'mdb.openParticipantQueryInPlayground', - RUN_PARTICIPANT_QUERY = 'mdb.runParticipantQuery', + OPEN_PARTICIPANT_CODE_IN_PLAYGROUND = 'mdb.openParticipantCodeInPlayground', + RUN_PARTICIPANT_CODE = 'mdb.runParticipantCode', CONNECT_WITH_PARTICIPANT = 'mdb.connectWithParticipant', SELECT_DATABASE_WITH_PARTICIPANT = 'mdb.selectDatabaseWithParticipant', SELECT_COLLECTION_WITH_PARTICIPANT = 'mdb.selectCollectionWithParticipant', diff --git a/src/editors/playgroundController.ts b/src/editors/playgroundController.ts index 505459191..118a7a376 100644 --- a/src/editors/playgroundController.ts +++ b/src/editors/playgroundController.ts @@ -59,10 +59,14 @@ interface ToCompile { let dummySandbox; +function getActiveEditorFilePath(): string | undefined { + return vscode.window.activeTextEditor?.document.uri.fsPath; +} + // TODO: this function was copied from the compass-export-to-language module // https://github.com/mongodb-js/compass/blob/7c4bc0789a7b66c01bb7ba63955b3b11ed40c094/packages/compass-export-to-language/src/modules/count-aggregation-stages-in-string.js // and should be updated as well when the better solution for the problem will be found. -const countAggregationStagesInString = (str: string) => { +const countAggregationStagesInString = (str: string): number => { if (!dummySandbox) { dummySandbox = vm.createContext(Object.create(null), { codeGeneration: { strings: false, wasm: false }, @@ -112,6 +116,9 @@ const exportModeMapping: Record< [ExportToLanguageMode.OTHER]: undefined, }; +const connectBeforeRunningMessage = + 'Please connect to a database before running a playground.'; + /** * This controller manages playground. */ @@ -160,7 +167,7 @@ export default class PlaygroundController { this._playgroundSelectedCodeActionProvider = playgroundSelectedCodeActionProvider; - this._activeConnectionChangedHandler = () => { + this._activeConnectionChangedHandler = (): void => { void this._activeConnectionChanged(); }; this._connectionController.addEventListener( @@ -170,7 +177,7 @@ export default class PlaygroundController { const onDidChangeActiveTextEditor = ( editor: vscode.TextEditor | undefined - ) => { + ): void => { if (editor?.document.uri.scheme === PLAYGROUND_RESULT_SCHEME) { this._playgroundResultViewColumn = editor.viewColumn; this._playgroundResultTextDocument = editor?.document; @@ -373,7 +380,7 @@ export default class PlaygroundController { return this._createPlaygroundFileWithContent(content); } - createPlaygroundFromParticipantQuery({ + createPlaygroundFromParticipantCode({ text, }: { text: string; @@ -438,13 +445,17 @@ export default class PlaygroundController { return this._createPlaygroundFileWithContent(content); } - async _evaluate(codeToEvaluate: string): Promise { + async _evaluate({ + codeToEvaluate, + filePath, + }: { + codeToEvaluate: string; + filePath?: string; + }): Promise { const connectionId = this._connectionController.getActiveConnectionId(); if (!connectionId) { - throw new Error( - 'Please connect to a database before running a playground.' - ); + throw new Error(connectBeforeRunningMessage); } this._statusView.showMessage('Getting results...'); @@ -455,7 +466,7 @@ export default class PlaygroundController { result = await this._languageServerController.evaluate({ codeToEvaluate, connectionId, - filePath: vscode.window.activeTextEditor?.document.uri.fsPath, + filePath, }); } catch (error) { const msg = @@ -482,13 +493,15 @@ export default class PlaygroundController { return this._activeTextEditor?.document.getText(selection) || ''; } - async _evaluateWithCancelModal( - codeToEvaluate: string - ): Promise { + async _evaluateWithCancelModal({ + codeToEvaluate, + filePath, + }: { + codeToEvaluate: string; + filePath?: string; + }): Promise { if (!this._connectionController.isCurrentlyConnected()) { - throw new Error( - 'Please connect to a database before running a playground.' - ); + throw new Error(connectBeforeRunningMessage); } try { @@ -507,9 +520,10 @@ export default class PlaygroundController { }); // Run all playground scripts. - const result: ShellEvaluateResult = await this._evaluate( - codeToEvaluate - ); + const result: ShellEvaluateResult = await this._evaluate({ + codeToEvaluate, + filePath, + }); return result; } @@ -572,11 +586,18 @@ export default class PlaygroundController { } } - async evaluateParticipantQuery(text: string): Promise { + async evaluateParticipantCode(codeToEvaluate: string): Promise { const shouldConfirmRunCopilotCode = vscode.workspace .getConfiguration('mdb') .get('confirmRunCopilotCode'); + if (!this._connectionController.isCurrentlyConnected()) { + // TODO(VSCODE-618): Prompt user to connect when clicked. + void vscode.window.showErrorMessage(connectBeforeRunningMessage); + + return false; + } + if (shouldConfirmRunCopilotCode === true) { const name = this._connectionController.getActiveConnectionName(); const confirmRunCopilotCode = await vscode.window.showInformationMessage( @@ -591,7 +612,9 @@ export default class PlaygroundController { } const evaluateResponse: ShellEvaluateResult = - await this._evaluateWithCancelModal(text); + await this._evaluateWithCancelModal({ + codeToEvaluate, + }); if (!evaluateResponse || !evaluateResponse.result) { return false; @@ -602,15 +625,19 @@ export default class PlaygroundController { return true; } - async _evaluatePlayground(text: string): Promise { + async _evaluatePlayground({ + codeToEvaluate, + filePath, + }: { + codeToEvaluate: string; + filePath?: string; + }): Promise { const shouldConfirmRunAll = vscode.workspace .getConfiguration('mdb') .get('confirmRunAll'); if (!this._connectionController.isCurrentlyConnected()) { - void vscode.window.showErrorMessage( - 'Please connect to a database before running a playground.' - ); + void vscode.window.showErrorMessage(connectBeforeRunningMessage); return false; } @@ -629,7 +656,10 @@ export default class PlaygroundController { } const evaluateResponse: ShellEvaluateResult = - await this._evaluateWithCancelModal(text); + await this._evaluateWithCancelModal({ + codeToEvaluate, + filePath, + }); if (!evaluateResponse || !evaluateResponse.result) { return false; @@ -652,7 +682,10 @@ export default class PlaygroundController { this._isPartialRun = true; - return this._evaluatePlayground(this._selectedText || ''); + return this._evaluatePlayground({ + codeToEvaluate: this._selectedText || '', + filePath: getActiveEditorFilePath(), + }); } runAllPlaygroundBlocks(): Promise { @@ -669,7 +702,10 @@ export default class PlaygroundController { this._isPartialRun = false; - return this._evaluatePlayground(this._getAllText()); + return this._evaluatePlayground({ + codeToEvaluate: this._getAllText(), + filePath: getActiveEditorFilePath(), + }); } runAllOrSelectedPlaygroundBlocks(): Promise { @@ -693,14 +729,17 @@ export default class PlaygroundController { codeToEvaluate = this._selectedText; } - return this._evaluatePlayground(codeToEvaluate); + return this._evaluatePlayground({ + codeToEvaluate, + filePath: getActiveEditorFilePath(), + }); } async fixThisInvalidInteractiveSyntax({ documentUri, range, fix, - }: ThisDiagnosticFix) { + }: ThisDiagnosticFix): Promise { const edit = new vscode.WorkspaceEdit(); edit.replace(documentUri, range, fix); await vscode.workspace.applyEdit(edit); @@ -710,7 +749,7 @@ export default class PlaygroundController { async fixAllInvalidInteractiveSyntax({ documentUri, diagnostics, - }: AllDiagnosticFixes) { + }: AllDiagnosticFixes): Promise { const edit = new vscode.WorkspaceEdit(); for (const { range, fix } of diagnostics) { @@ -884,7 +923,7 @@ export default class PlaygroundController { language, num_stages: selectedText ? countAggregationStagesInString(selectedText) - : null, + : undefined, with_import_statements: importStatements, with_builders: builders, with_driver_syntax: driverSyntax, diff --git a/src/mdbExtensionController.ts b/src/mdbExtensionController.ts index 7103183ee..e481a57ea 100644 --- a/src/mdbExtensionController.ts +++ b/src/mdbExtensionController.ts @@ -44,7 +44,7 @@ import { ConnectionStorage } from './storage/connectionStorage'; import type StreamProcessorTreeItem from './explorer/streamProcessorTreeItem'; import type { ParticipantCommand, - RunParticipantQueryCommandArgs, + RunParticipantCodeCommandArgs, } from './participant/participant'; import ParticipantController from './participant/participant'; import type { OpenSchemaCommandArgs } from './participant/prompts/schema'; @@ -296,17 +296,17 @@ export default class MDBExtensionController implements vscode.Disposable { // ------ CHAT PARTICIPANT ------ // this.registerParticipantCommand( - EXTENSION_COMMANDS.OPEN_PARTICIPANT_QUERY_IN_PLAYGROUND, - ({ runnableContent }: RunParticipantQueryCommandArgs) => { - return this._playgroundController.createPlaygroundFromParticipantQuery({ + EXTENSION_COMMANDS.OPEN_PARTICIPANT_CODE_IN_PLAYGROUND, + ({ runnableContent }: RunParticipantCodeCommandArgs) => { + return this._playgroundController.createPlaygroundFromParticipantCode({ text: runnableContent, }); } ); this.registerParticipantCommand( - EXTENSION_COMMANDS.RUN_PARTICIPANT_QUERY, - ({ runnableContent }: RunParticipantQueryCommandArgs) => { - return this._playgroundController.evaluateParticipantQuery( + EXTENSION_COMMANDS.RUN_PARTICIPANT_CODE, + ({ runnableContent }: RunParticipantCodeCommandArgs) => { + return this._playgroundController.evaluateParticipantCode( runnableContent ); } diff --git a/src/participant/participant.ts b/src/participant/participant.ts index 8d57f0726..bb55d4949 100644 --- a/src/participant/participant.ts +++ b/src/participant/participant.ts @@ -53,7 +53,7 @@ interface NamespaceQuickPicks { data: string; } -export type RunParticipantQueryCommandArgs = { +export type RunParticipantCodeCommandArgs = { runnableContent: string; }; @@ -210,16 +210,16 @@ export default class ParticipantController { return; } - const commandArgs: RunParticipantQueryCommandArgs = { + const commandArgs: RunParticipantCodeCommandArgs = { runnableContent, }; stream.button({ - command: EXTENSION_COMMANDS.RUN_PARTICIPANT_QUERY, + command: EXTENSION_COMMANDS.RUN_PARTICIPANT_CODE, title: vscode.l10n.t('▶️ Run'), arguments: [commandArgs], }); stream.button({ - command: EXTENSION_COMMANDS.OPEN_PARTICIPANT_QUERY_IN_PLAYGROUND, + command: EXTENSION_COMMANDS.OPEN_PARTICIPANT_CODE_IN_PLAYGROUND, title: vscode.l10n.t('Open in playground'), arguments: [commandArgs], }); diff --git a/src/test/suite/editors/playgroundController.test.ts b/src/test/suite/editors/playgroundController.test.ts index 7d5458161..125ba4a7e 100644 --- a/src/test/suite/editors/playgroundController.test.ts +++ b/src/test/suite/editors/playgroundController.test.ts @@ -324,9 +324,9 @@ suite('Playground Controller Test Suite', function () { sandbox.fake.rejects(false) ); - const result = await testPlaygroundController._evaluateWithCancelModal( - '' - ); + const result = await testPlaygroundController._evaluateWithCancelModal({ + codeToEvaluate: '', + }); expect(result).to.deep.equal({ result: undefined }); }); diff --git a/src/test/suite/participant/participant.test.ts b/src/test/suite/participant/participant.test.ts index 2610ccddf..be8c173a1 100644 --- a/src/test/suite/participant/participant.test.ts +++ b/src/test/suite/participant/participant.test.ts @@ -489,7 +489,7 @@ suite('Participant Controller Test Suite', function () { expect(res?.metadata.intent).to.equal('generic'); expect(chatStreamStub?.button.getCall(0).args[0]).to.deep.equal({ - command: 'mdb.runParticipantQuery', + command: 'mdb.runParticipantCode', title: '▶️ Run', arguments: [ { @@ -517,7 +517,7 @@ suite('Participant Controller Test Suite', function () { }; await invokeChatHandler(chatRequestMock); expect(chatStreamStub?.button.getCall(0).args[0]).to.deep.equal({ - command: 'mdb.runParticipantQuery', + command: 'mdb.runParticipantCode', title: '▶️ Run', arguments: [ { @@ -945,7 +945,7 @@ suite('Participant Controller Test Suite', function () { expect(chatStreamStub?.button.callCount).to.equal(2); expect(chatStreamStub?.button.getCall(0).args[0]).to.deep.equal({ - command: 'mdb.runParticipantQuery', + command: 'mdb.runParticipantCode', title: '▶️ Run', arguments: [ { @@ -955,7 +955,7 @@ suite('Participant Controller Test Suite', function () { ], }); expect(chatStreamStub?.button.getCall(1).args[0]).to.deep.equal({ - command: 'mdb.openParticipantQueryInPlayground', + command: 'mdb.openParticipantCodeInPlayground', title: 'Open in playground', arguments: [ { diff --git a/src/test/suite/telemetry/telemetryService.test.ts b/src/test/suite/telemetry/telemetryService.test.ts index 0055f0e50..f6e78e374 100644 --- a/src/test/suite/telemetry/telemetryService.test.ts +++ b/src/test/suite/telemetry/telemetryService.test.ts @@ -235,7 +235,9 @@ suite('Telemetry Controller Test Suite', () => { test('track playground code executed event', async () => { const testPlaygroundController = mdbTestExtension.testExtensionController._playgroundController; - await testPlaygroundController._evaluate('show dbs'); + await testPlaygroundController._evaluate({ + codeToEvaluate: 'show dbs', + }); sandbox.assert.calledWith( fakeSegmentAnalyticsTrack, sinon.match({