From 71bae9905df6ba9d316ed4249c1029d5b474c0b4 Mon Sep 17 00:00:00 2001 From: Anemy Date: Tue, 3 May 2022 13:35:37 -0400 Subject: [PATCH 1/2] avoid ace editor snippet completion, complete on our end, remove un-needed fromStageOperators flag --- .../pipeline-builder-workspace.jsx | 1 - .../pipeline-builder-workspace.spec.jsx | 2 -- .../components/stage-editor/stage-editor.jsx | 16 ++++++--------- .../stage-editor/stage-editor.spec.jsx | 1 - .../src/components/stage/stage.jsx | 3 --- .../src/modules/pipeline.ts | 20 ++++++++----------- .../src/typings.d.ts | 1 - .../test/fixtures.ts | 1 - 8 files changed, 14 insertions(+), 31 deletions(-) diff --git a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx index cd9b786cee6..21646c1f039 100644 --- a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx +++ b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx @@ -115,7 +115,6 @@ class PipelineWorkspace extends PureComponent { isExpanded={stage.isExpanded} isCommenting={this.props.isCommenting} isAutoPreviewing={this.props.isAutoPreviewing} - fromStageOperators={stage.fromStageOperators || false} previewDocuments={stage.previewDocuments} runStage={this.props.runStage} openLink={this.props.openLink} diff --git a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx index 99b7477bf1c..2f4bfbe3ffc 100644 --- a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx +++ b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx @@ -19,7 +19,6 @@ const PIPELINE_1 = [ 'error': null, 'projections': [], 'snippet': '/**\n * query: The query in MQL.\n */\n{\n ${1:query}\n}', - 'fromStageOperators': false, 'executor': { '$match': { 'x': 1 @@ -40,7 +39,6 @@ const PIPELINE_1 = [ 'error': null, 'projections': [], 'snippet': '/**\n * Provide the number of documents to limit.\n */\n${1:number}', - 'fromStageOperators': false, 'executor': { '$limit': 3 } diff --git a/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx b/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx index 2cca9fa5259..87687061b9a 100644 --- a/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx +++ b/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx @@ -6,8 +6,6 @@ import { StageAutoCompleter } from 'mongodb-ace-autocompleter'; import styles from './stage-editor.module.less'; -const INDEX_STATS = '$indexStats'; - /** * Edit a single stage in the aggregation pipeline. */ @@ -27,7 +25,6 @@ class StageEditor extends Component { stageChanged: PropTypes.func.isRequired, isAutoPreviewing: PropTypes.bool.isRequired, isValid: PropTypes.bool.isRequired, - fromStageOperators: PropTypes.bool.isRequired, setIsModified: PropTypes.func.isRequired, projections: PropTypes.array.isRequired, projectionsChanged: PropTypes.func.isRequired, @@ -87,9 +84,12 @@ class StageEditor extends Component { ); this.completer.version = this.props.serverVersion; if (this.props.stageOperator !== prevProps.stageOperator && this.editor) { - this.editor.setValue(''); - this.editor.insertSnippet(this.props.snippet || ''); this.editor.focus(); + + // When the underlying stage operator changes, re-run the preview. + if (this.props.isAutoPreviewing) { + this.debounceRun(); + } } } @@ -110,11 +110,7 @@ class StageEditor extends Component { this.props.projectionsChanged(); this.props.setIsModified(true); - if ( - (this.props.fromStageOperators === false || - this.props.stageOperator === INDEX_STATS) && - this.props.isAutoPreviewing - ) { + if (this.props.isAutoPreviewing) { this.debounceRun(); } }; diff --git a/packages/compass-aggregations/src/components/stage-editor/stage-editor.spec.jsx b/packages/compass-aggregations/src/components/stage-editor/stage-editor.spec.jsx index 926891bc58c..7d1dd3d7f0a 100644 --- a/packages/compass-aggregations/src/components/stage-editor/stage-editor.spec.jsx +++ b/packages/compass-aggregations/src/components/stage-editor/stage-editor.spec.jsx @@ -25,7 +25,6 @@ describe('StageEditor [Component]', function() { isValid={isValid} index={0} isAutoPreviewing - fromStageOperators={false} fields={[]} serverVersion="3.6.0" runStage={runStageSpy} diff --git a/packages/compass-aggregations/src/components/stage/stage.jsx b/packages/compass-aggregations/src/components/stage/stage.jsx index aee21e9c3ca..6acec18f7a8 100644 --- a/packages/compass-aggregations/src/components/stage/stage.jsx +++ b/packages/compass-aggregations/src/components/stage/stage.jsx @@ -60,7 +60,6 @@ class Stage extends Component { isComplete: PropTypes.bool.isRequired, // Can be undefined on the initial render isMissingAtlasOnlyStageSupport: PropTypes.bool, - fromStageOperators: PropTypes.bool.isRequired, previewDocuments: PropTypes.array.isRequired, index: PropTypes.number.isRequired, isCommenting: PropTypes.bool.isRequired, @@ -98,7 +97,6 @@ class Stage extends Component { nextProps.isLoading !== this.props.isLoading || nextProps.isComplete !== this.props.isComplete || nextProps.isMissingAtlasOnlyStageSupport !== this.props.isMissingAtlasOnlyStageSupport || - nextProps.fromStageOperators !== this.props.fromStageOperators || nextProps.index !== this.props.index || nextProps.isCommenting !== this.props.isCommenting || nextProps.isAutoPreviewing !== this.props.isAutoPreviewing || @@ -154,7 +152,6 @@ class Stage extends Component { error={this.props.error} syntaxError={this.props.syntaxError} isValid={this.props.isValid} - fromStageOperators={this.props.fromStageOperators} runStage={this.props.runStage} index={this.props.index} serverVersion={this.props.serverVersion} diff --git a/packages/compass-aggregations/src/modules/pipeline.ts b/packages/compass-aggregations/src/modules/pipeline.ts index bfd6e79cb64..e312d08130e 100644 --- a/packages/compass-aggregations/src/modules/pipeline.ts +++ b/packages/compass-aggregations/src/modules/pipeline.ts @@ -45,7 +45,6 @@ export type Pipeline = { syntaxError: string | null; error: string | null; projections: Projection[]; - fromStageOperators?: boolean; snippet?: string; isMissingAtlasOnlyStageSupport?: boolean; executor?: Record; @@ -244,7 +243,6 @@ const changeStage = (state: State, action: AnyAction): State => { const newState = copyState(state); newState[action.index].stage = action.stage; newState[action.index].isComplete = false; - newState[action.index].fromStageOperators = false; return newState; }; @@ -329,7 +327,6 @@ const selectStageOperator = (state: State, action: AnyAction): State => { newState[action.index].snippet = value; newState[action.index].isExpanded = true; newState[action.index].isComplete = false; - newState[action.index].fromStageOperators = true; newState[action.index].previewDocuments = []; if ( [SEARCH, SEARCH_META, DOCUMENTS].includes(newState[action.index].stageOperator) && @@ -348,14 +345,6 @@ const selectStageOperator = (state: State, action: AnyAction): State => { return state; }; - -const getStageDefaultValue = (stageOperator: string, isCommenting: boolean, env: string): string => { - const operatorDetails = getStageOperator(stageOperator, env); - const snippet = (operatorDetails || {}).snippet || DEFAULT_SNIPPET; - const comment = (operatorDetails || {}).comment || ''; - return isCommenting ? `${comment}${snippet}` : snippet; -}; - export const replaceAceTokens = (str: string): string => { const regex = /\${[0-9]+:?([a-z0-9.()]+)?}/ig; return str.replace(regex, function (_match, replaceWith) { @@ -363,6 +352,13 @@ export const replaceAceTokens = (str: string): string => { }); } +const getStageDefaultValue = (stageOperator: string, isCommenting: boolean, env: string): string => { + const operatorDetails = getStageOperator(stageOperator, env); + const snippet = (operatorDetails || {}).snippet || DEFAULT_SNIPPET; + const comment = (operatorDetails || {}).comment || ''; + return replaceAceTokens(isCommenting ? `${comment}${snippet}` : snippet); +}; + const hasUserChangedStage = (stage: Pipeline, env: string): boolean => { if (!stage.stageOperator || !stage.stage) { return false; @@ -370,7 +366,7 @@ const hasUserChangedStage = (stage: Pipeline, env: string): boolean => { const value = decomment(stage.stage); // The default value contains ace specific tokens (${1:name}). const defaultValue = getStageDefaultValue(stage.stageOperator, false, env); - return value !== replaceAceTokens(defaultValue); + return value !== defaultValue; }; /** diff --git a/packages/compass-saved-aggregations-queries/src/typings.d.ts b/packages/compass-saved-aggregations-queries/src/typings.d.ts index 634e544ae52..7a6da6b1f57 100644 --- a/packages/compass-saved-aggregations-queries/src/typings.d.ts +++ b/packages/compass-saved-aggregations-queries/src/typings.d.ts @@ -44,7 +44,6 @@ interface Pipeline { syntaxError: unknown; error: unknown; projections?: unknown[]; - fromStageOperators?: boolean; executor?: unknown; isMissingAtlasOnlyStageSupport?: boolean; snippet?: string; diff --git a/packages/compass-saved-aggregations-queries/test/fixtures.ts b/packages/compass-saved-aggregations-queries/test/fixtures.ts index 41d0d423316..b0ceb3d90c3 100644 --- a/packages/compass-saved-aggregations-queries/test/fixtures.ts +++ b/packages/compass-saved-aggregations-queries/test/fixtures.ts @@ -139,7 +139,6 @@ export const pipelines: Item[] = [ projections: [], snippet: '/**\n * Provide the number of documents to limit.\n */\n${1:number}', - fromStageOperators: false, executor: { $limit: 3, }, From a8fdb7edda1c30d7b85405840345ec9545a03ce6 Mon Sep 17 00:00:00 2001 From: Anemy Date: Tue, 3 May 2022 13:57:09 -0400 Subject: [PATCH 2/2] improve naming on token replacing function, remove snippet on model --- .../pipeline-builder-workspace.jsx | 1 - .../pipeline-builder-workspace.spec.jsx | 2 -- .../components/stage-editor/stage-editor.jsx | 4 +--- .../src/components/stage/stage.jsx | 3 --- .../src/modules/pipeline.spec.js | 22 ++++--------------- .../src/modules/pipeline.ts | 6 ++--- .../src/modules/stage.spec.js | 7 +----- .../src/modules/update-view.spec.js | 1 - .../src/typings.d.ts | 1 - .../test/fixtures.ts | 2 -- 10 files changed, 8 insertions(+), 41 deletions(-) diff --git a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx index 21646c1f039..89c0d1db954 100644 --- a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx +++ b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.jsx @@ -104,7 +104,6 @@ class PipelineWorkspace extends PureComponent { sourceName={this.props.sourceName} stage={stage.stage} stageOperator={stage.stageOperator} - snippet={stage.snippet} error={stage.error} syntaxError={stage.syntaxError} isValid={stage.isValid} diff --git a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx index 2f4bfbe3ffc..837bb5cf3d6 100644 --- a/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx +++ b/packages/compass-aggregations/src/components/pipeline-builder-workspace/pipeline-builder-workspace.spec.jsx @@ -18,7 +18,6 @@ const PIPELINE_1 = [ 'syntaxError': null, 'error': null, 'projections': [], - 'snippet': '/**\n * query: The query in MQL.\n */\n{\n ${1:query}\n}', 'executor': { '$match': { 'x': 1 @@ -38,7 +37,6 @@ const PIPELINE_1 = [ 'syntaxError': null, 'error': null, 'projections': [], - 'snippet': '/**\n * Provide the number of documents to limit.\n */\n${1:number}', 'executor': { '$limit': 3 } diff --git a/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx b/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx index 87687061b9a..ce9ae2c3105 100644 --- a/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx +++ b/packages/compass-aggregations/src/components/stage-editor/stage-editor.jsx @@ -15,7 +15,6 @@ class StageEditor extends Component { static propTypes = { stage: PropTypes.string, stageOperator: PropTypes.string, - snippet: PropTypes.string, error: PropTypes.string, syntaxError: PropTypes.string, runStage: PropTypes.func.isRequired, @@ -73,8 +72,6 @@ class StageEditor extends Component { } /** - * On update if the stage operator is changed insert the snippet and focus on the editor. - * * @param {Object} prevProps - The previous properties. */ componentDidUpdate(prevProps) { @@ -84,6 +81,7 @@ class StageEditor extends Component { ); this.completer.version = this.props.serverVersion; if (this.props.stageOperator !== prevProps.stageOperator && this.editor) { + // Focus the editor when the stage operator has changed. this.editor.focus(); // When the underlying stage operator changes, re-run the preview. diff --git a/packages/compass-aggregations/src/components/stage/stage.jsx b/packages/compass-aggregations/src/components/stage/stage.jsx index 6acec18f7a8..6b2295ef4f5 100644 --- a/packages/compass-aggregations/src/components/stage/stage.jsx +++ b/packages/compass-aggregations/src/components/stage/stage.jsx @@ -50,7 +50,6 @@ class Stage extends Component { sourceName: PropTypes.string, stage: PropTypes.string.isRequired, stageOperator: PropTypes.string, - snippet: PropTypes.string, error: PropTypes.string, syntaxError: PropTypes.string, isValid: PropTypes.bool.isRequired, @@ -88,7 +87,6 @@ class Stage extends Component { shouldComponentUpdate(nextProps) { const should = ( nextProps.stageOperator !== this.props.stageOperator || - nextProps.snippet !== this.props.snippet || nextProps.error !== this.props.error || nextProps.syntaxError !== this.props.syntaxError || nextProps.isValid !== this.props.isValid || @@ -148,7 +146,6 @@ class Stage extends Component { x.name !== '$geoNear').forEach( ({ name, comment, snippet, env: envs }) => { @@ -151,11 +146,7 @@ describe('pipeline module', function () { expect( newState[0].stage, `${name} stage on ${env} env.` - ).to.equal(`${comment}${snippet}`); - expect( - newState[0].snippet, - `${name} snippet on ${env} env.` - ).to.equal(`${comment}${snippet}`); + ).to.equal(replaceOperatorSnippetTokens(`${comment}${snippet}`)); }); } ); @@ -165,7 +156,6 @@ describe('pipeline module', function () { const stage = { stageOperator: limit.name, stage: '20', - snippet: `${limit.comment}${limit.snippet}`, }; STAGE_OPERATORS.filter((x) => x.name !== '$limit').forEach( ({ name, env: envs }) => { @@ -182,10 +172,6 @@ describe('pipeline module', function () { newState[0].stage, `${name} stage on ${env} env.` ).to.equal('20'); - expect( - newState[0].snippet, - `${name} snippet on ${env} env.` - ).to.equal('20'); }); } ); diff --git a/packages/compass-aggregations/src/modules/pipeline.ts b/packages/compass-aggregations/src/modules/pipeline.ts index e312d08130e..c2f5bc3fe00 100644 --- a/packages/compass-aggregations/src/modules/pipeline.ts +++ b/packages/compass-aggregations/src/modules/pipeline.ts @@ -45,7 +45,6 @@ export type Pipeline = { syntaxError: string | null; error: string | null; projections: Projection[]; - snippet?: string; isMissingAtlasOnlyStageSupport?: boolean; executor?: Record; } @@ -324,7 +323,6 @@ const selectStageOperator = (state: State, action: AnyAction): State => { } newState[action.index].stageOperator = operatorName; newState[action.index].stage = value; - newState[action.index].snippet = value; newState[action.index].isExpanded = true; newState[action.index].isComplete = false; newState[action.index].previewDocuments = []; @@ -345,7 +343,7 @@ const selectStageOperator = (state: State, action: AnyAction): State => { return state; }; -export const replaceAceTokens = (str: string): string => { +export const replaceOperatorSnippetTokens = (str: string): string => { const regex = /\${[0-9]+:?([a-z0-9.()]+)?}/ig; return str.replace(regex, function (_match, replaceWith) { return replaceWith ?? ''; @@ -356,7 +354,7 @@ const getStageDefaultValue = (stageOperator: string, isCommenting: boolean, env: const operatorDetails = getStageOperator(stageOperator, env); const snippet = (operatorDetails || {}).snippet || DEFAULT_SNIPPET; const comment = (operatorDetails || {}).comment || ''; - return replaceAceTokens(isCommenting ? `${comment}${snippet}` : snippet); + return replaceOperatorSnippetTokens(isCommenting ? `${comment}${snippet}` : snippet); }; const hasUserChangedStage = (stage: Pipeline, env: string): boolean => { diff --git a/packages/compass-aggregations/src/modules/stage.spec.js b/packages/compass-aggregations/src/modules/stage.spec.js index a7f847f8641..b2dae9f158d 100644 --- a/packages/compass-aggregations/src/modules/stage.spec.js +++ b/packages/compass-aggregations/src/modules/stage.spec.js @@ -181,7 +181,6 @@ describe('Stage module', function() { isEnabled: true, isExpanded: true, isValid: true, - snippet: '', stageOperator: '$addFields', stage: `{ totalHomework: { $sum: "$homework" } , @@ -216,7 +215,6 @@ describe('Stage module', function() { isEnabled: true, isExpanded: true, isValid: true, - snippet: '', stageOperator: '$project', stage: '{_id: 0, avg_price: {$avg: "$price"}}' }; @@ -248,7 +246,6 @@ describe('Stage module', function() { isEnabled: true, isExpanded: true, isValid: true, - snippet: '', stageOperator: '$bucket', stage: `{ groupBy: "$price", @@ -302,7 +299,6 @@ describe('Stage module', function() { isEnabled: true, isExpanded: true, isValid: true, - snippet: '', stageOperator: '$count', stage: '"fieldname"' }; @@ -322,7 +318,6 @@ describe('Stage module', function() { isEnabled: true, isExpanded: true, isValid: true, - snippet: '', stageOperator: '$addFields', stage: `{ isFound: @@ -406,7 +401,7 @@ describe('Stage module', function() { context('when the stage has BSON types', function() { const stage = { - id: 0, isEnabled: true, isExpanded: true, isValid: true, snippet: '', + id: 0, isEnabled: true, isExpanded: true, isValid: true, stageOperator: '$match', stage: '{\n' + ' code: Code(\'some code\'),\n' + diff --git a/packages/compass-aggregations/src/modules/update-view.spec.js b/packages/compass-aggregations/src/modules/update-view.spec.js index a9b1c329a17..6f3855c883a 100644 --- a/packages/compass-aggregations/src/modules/update-view.spec.js +++ b/packages/compass-aggregations/src/modules/update-view.spec.js @@ -24,7 +24,6 @@ describe('large-limit module', function() { isEnabled: true, isExpanded: true, isValid: true, - snippet: '', stageOperator: '$project', stage: '{_id: 0, avg_price: {$avg: "$price"}}' }], diff --git a/packages/compass-saved-aggregations-queries/src/typings.d.ts b/packages/compass-saved-aggregations-queries/src/typings.d.ts index 7a6da6b1f57..79b74fdb22d 100644 --- a/packages/compass-saved-aggregations-queries/src/typings.d.ts +++ b/packages/compass-saved-aggregations-queries/src/typings.d.ts @@ -46,7 +46,6 @@ interface Pipeline { projections?: unknown[]; executor?: unknown; isMissingAtlasOnlyStageSupport?: boolean; - snippet?: string; } type QueryUpdateAttributes = { diff --git a/packages/compass-saved-aggregations-queries/test/fixtures.ts b/packages/compass-saved-aggregations-queries/test/fixtures.ts index b0ceb3d90c3..cd153ff6381 100644 --- a/packages/compass-saved-aggregations-queries/test/fixtures.ts +++ b/packages/compass-saved-aggregations-queries/test/fixtures.ts @@ -137,8 +137,6 @@ export const pipelines: Item[] = [ syntaxError: null, error: null, projections: [], - snippet: - '/**\n * Provide the number of documents to limit.\n */\n${1:number}', executor: { $limit: 3, },