Skip to content

Commit 2133549

Browse files
feat(compass-aggregations): add unindexed aggregation insight COMPASS-6833 (#4551)
* chore(compass-aggregations): clean-up pipeline header code * feat(compass-aggregations): add unindexed aggregation insight * chore: fix test description Co-authored-by: Himanshu Singh <[email protected]> --------- Co-authored-by: Himanshu Singh <[email protected]>
1 parent dc17952 commit 2133549

File tree

24 files changed

+309
-103
lines changed

24 files changed

+309
-103
lines changed

package-lock.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/compass-aggregations/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"@mongodb-js/compass-editor": "^0.9.0",
4444
"@mongodb-js/compass-logging": "^1.1.6",
4545
"@mongodb-js/compass-utils": "^0.3.1",
46+
"@mongodb-js/explain-plan-helper": "^1.0.7",
4647
"@mongodb-js/mongodb-constants": "^0.6.0",
4748
"@mongodb-js/mongodb-redux-common": "^2.0.8",
4849
"bson": "^5.2.0",
@@ -102,6 +103,7 @@
102103
"@mongodb-js/compass-editor": "^0.9.0",
103104
"@mongodb-js/compass-logging": "^1.1.6",
104105
"@mongodb-js/compass-utils": "^0.3.1",
106+
"@mongodb-js/explain-plan-helper": "^1.0.7",
105107
"@mongodb-js/mongodb-constants": "^0.6.0",
106108
"@mongodb-js/mongodb-redux-common": "^2.0.8",
107109
"bson": "^5.2.0",

packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/index.spec.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,17 @@ import { PipelineHeader } from '.';
1111

1212
describe('PipelineHeader', function () {
1313
let container: HTMLElement;
14-
let onShowSavedPipelinesSpy: SinonSpy;
1514
let onToggleOptionsSpy: SinonSpy;
1615
beforeEach(function () {
17-
onShowSavedPipelinesSpy = spy();
1816
onToggleOptionsSpy = spy();
1917
render(
2018
<Provider store={configureStore()}>
2119
<PipelineHeader
2220
isOpenPipelineVisible
23-
isSavedPipelineVisible={false}
2421
isOptionsVisible
2522
showRunButton
2623
showExportButton
2724
showExplainButton
28-
onToggleSavedPipelines={onShowSavedPipelinesSpy}
2925
onToggleOptions={onToggleOptionsSpy}
3026
/>
3127
</Provider>
@@ -37,14 +33,14 @@ describe('PipelineHeader', function () {
3733
expect(within(container).getByText('Pipeline')).to.exist;
3834
});
3935

40-
it('open saved pipelines button', function () {
36+
it('open saved pipelines button', async function () {
4137
const button = within(container).getByTestId(
4238
'pipeline-toolbar-open-pipelines-button'
4339
);
4440
expect(button).to.exist;
4541

4642
userEvent.click(button);
4743

48-
expect(onShowSavedPipelinesSpy.calledOnce).to.be.true;
44+
expect(await screen.findByTestId('saved-pipelines')).to.exist;
4945
});
5046
});

packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/index.tsx

Lines changed: 51 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useMemo } from 'react';
1+
import React, { useState } from 'react';
22
import {
33
Body,
44
Icon,
@@ -12,7 +12,6 @@ import { connect } from 'react-redux';
1212

1313
import PipelineStages from './pipeline-stages';
1414
import PipelineActions from './pipeline-actions';
15-
import { setShowSavedPipelines } from '../../../modules/saved-pipeline';
1615
import SavedPipelines from '../../saved-pipelines/saved-pipelines';
1716
import type { RootState } from '../../../modules';
1817

@@ -66,65 +65,67 @@ type PipelineHeaderProps = {
6665
showRunButton: boolean;
6766
showExportButton: boolean;
6867
showExplainButton: boolean;
69-
onToggleSavedPipelines: (show: boolean) => void;
7068
onToggleOptions: () => void;
7169
isOpenPipelineVisible: boolean;
72-
isSavedPipelineVisible: boolean;
70+
};
71+
72+
const containedElements = [
73+
'[data-id="open-pipeline-confirmation-modal"]',
74+
'[data-id="delete-pipeline-confirmation-modal"]',
75+
];
76+
77+
const SavedPipelinesButton: React.FunctionComponent = () => {
78+
const [isVisible, setIsVisible] = useState(false);
79+
return (
80+
<InteractivePopover
81+
className={savedAggregationsPopoverStyles}
82+
// To prevent popover from closing when confirmation modal is shown
83+
containedElements={containedElements}
84+
trigger={({ onClick, ref, children }) => {
85+
return (
86+
<button
87+
data-testid="pipeline-toolbar-open-pipelines-button"
88+
onClick={onClick}
89+
className={openSavedPipelinesStyles}
90+
aria-label="Open saved pipelines"
91+
aria-haspopup="true"
92+
aria-expanded={isVisible ? true : undefined}
93+
title="Saved Pipelines"
94+
type="button"
95+
ref={ref}
96+
>
97+
<Icon glyph="Folder" />
98+
<Icon glyph="CaretDown" />
99+
{children}
100+
</button>
101+
);
102+
}}
103+
open={isVisible}
104+
setOpen={setIsVisible}
105+
>
106+
<SavedPipelines />
107+
</InteractivePopover>
108+
);
73109
};
74110

75111
export const PipelineHeader: React.FunctionComponent<PipelineHeaderProps> = ({
76112
showRunButton,
77113
showExportButton,
78114
showExplainButton,
79115
onToggleOptions,
80-
onToggleSavedPipelines,
81116
isOptionsVisible,
82117
isOpenPipelineVisible,
83-
isSavedPipelineVisible,
84118
}) => {
85-
const containedElements = useMemo(() => {
86-
return [
87-
'[data-id="open-pipeline-confirmation-modal"]',
88-
'[data-id="delete-pipeline-confirmation-modal"]',
89-
];
90-
}, []);
91-
92119
return (
93120
<div className={containerStyles} data-testid="pipeline-header">
94-
{/* TODO: PipelineHeader component should only be concerned with layout, move the popover to a more appropriate place */}
95121
{isOpenPipelineVisible && (
96-
<InteractivePopover
97-
className={savedAggregationsPopoverStyles}
98-
// To prevent popover from closing when confirmation modal is shown
99-
containedElements={containedElements}
100-
trigger={({ onClick, ref, children }) => (
101-
<div
102-
data-testid="saved-pipelines-popover"
103-
className={pipelineTextAndOpenStyles}
104-
>
105-
<Body weight="medium">Pipeline</Body>
106-
<button
107-
data-testid="pipeline-toolbar-open-pipelines-button"
108-
onClick={onClick}
109-
className={openSavedPipelinesStyles}
110-
aria-label="Open saved pipelines"
111-
aria-haspopup="true"
112-
aria-expanded={isSavedPipelineVisible ? true : undefined}
113-
title="Saved Pipelines"
114-
type="button"
115-
ref={ref}
116-
>
117-
<Icon glyph="Folder" />
118-
<Icon glyph="CaretDown" />
119-
</button>
120-
{children}
121-
</div>
122-
)}
123-
open={isSavedPipelineVisible}
124-
setOpen={onToggleSavedPipelines}
122+
<div
123+
data-testid="saved-pipelines-popover"
124+
className={pipelineTextAndOpenStyles}
125125
>
126-
<SavedPipelines />
127-
</InteractivePopover>
126+
<Body weight="medium">Pipeline</Body>
127+
<SavedPipelinesButton></SavedPipelinesButton>
128+
</div>
128129
)}
129130
<div className={pipelineStagesStyles}>
130131
<PipelineStages />
@@ -142,14 +143,8 @@ export const PipelineHeader: React.FunctionComponent<PipelineHeaderProps> = ({
142143
);
143144
};
144145

145-
export default connect(
146-
(state: RootState) => {
147-
return {
148-
isOpenPipelineVisible: !state.editViewName && !state.isAtlasDeployed,
149-
isSavedPipelineVisible: state.savedPipeline.isListVisible,
150-
};
151-
},
152-
{
153-
onToggleSavedPipelines: setShowSavedPipelines,
154-
}
155-
)(PipelineHeader);
146+
export default connect((state: RootState) => {
147+
return {
148+
isOpenPipelineVisible: !state.editViewName && !state.isAtlasDeployed,
149+
};
150+
})(PipelineHeader);

packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.spec.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('PipelineActions', function () {
3636
isExplainButtonDisabled={false}
3737
onExplainAggregation={onExplainAggregationSpy}
3838
onUpdateView={() => {}}
39+
onCollectionScanInsightActionButtonClick={() => {}}
3940
/>
4041
);
4142
});
@@ -101,6 +102,7 @@ describe('PipelineActions', function () {
101102
onUpdateView={() => {}}
102103
onExplainAggregation={() => {}}
103104
isAtlasDeployed={false}
105+
onCollectionScanInsightActionButtonClick={() => {}}
104106
/>
105107
);
106108
});
@@ -137,6 +139,7 @@ describe('PipelineActions', function () {
137139
onUpdateView={() => {}}
138140
onExplainAggregation={() => {}}
139141
isAtlasDeployed={true}
142+
onCollectionScanInsightActionButtonClick={() => {}}
140143
/>
141144
);
142145
});
@@ -170,6 +173,7 @@ describe('PipelineActions', function () {
170173
onExportAggregationResults={onExportAggregationResultsSpy}
171174
onExplainAggregation={onExplainAggregationSpy}
172175
onUpdateView={() => {}}
176+
onCollectionScanInsightActionButtonClick={() => {}}
173177
/>
174178
);
175179
});

packages/compass-aggregations/src/components/pipeline-toolbar/pipeline-header/pipeline-actions.tsx

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { connect } from 'react-redux';
33
import {
44
Button,
55
MoreOptionsToggle,
6+
SignalPopover,
67
css,
78
spacing,
89
} from '@mongodb-js/compass-components';
@@ -18,6 +19,7 @@ import {
1819
getPipelineStageOperatorsFromBuilderState,
1920
} from '../../../modules/pipeline-builder/builder-helpers';
2021
import { isOutputStage } from '../../../utils/stage';
22+
import { openCreateIndexModal } from '../../../modules/insights';
2123

2224
const containerStyles = css({
2325
display: 'flex',
@@ -46,6 +48,9 @@ type PipelineActionsProps = {
4648
onToggleOptions: () => void;
4749

4850
isAtlasDeployed?: boolean;
51+
52+
showCollectionScanInsight?: boolean;
53+
onCollectionScanInsightActionButtonClick: () => void;
4954
};
5055

5156
export const PipelineActions: React.FunctionComponent<PipelineActionsProps> = ({
@@ -64,9 +69,34 @@ export const PipelineActions: React.FunctionComponent<PipelineActionsProps> = ({
6469
onExportAggregationResults,
6570
onExplainAggregation,
6671
isAtlasDeployed,
72+
showCollectionScanInsight,
73+
onCollectionScanInsightActionButtonClick,
6774
}) => {
6875
return (
6976
<div className={containerStyles}>
77+
{showCollectionScanInsight && (
78+
<div>
79+
<SignalPopover
80+
signals={{
81+
id: 'aggregation-executed-without-index',
82+
title: 'Aggregation executed without index',
83+
description: (
84+
<>
85+
This aggregation ran without an index. If you plan on using
86+
this query <strong>heavily</strong> in your application, you
87+
should create an index that covers this aggregation.
88+
</>
89+
),
90+
learnMoreLink:
91+
'https://www.mongodb.com/docs/v6.0/core/data-model-operations/#indexes',
92+
primaryActionButtonLabel: 'Create index',
93+
primaryActionButtonIcon: 'Plus',
94+
onPrimaryActionButtonClick:
95+
onCollectionScanInsightActionButtonClick,
96+
}}
97+
></SignalPopover>
98+
</div>
99+
)}
70100
{showUpdateViewButton && (
71101
<Button
72102
aria-label="Update view"
@@ -141,6 +171,7 @@ const mapState = (state: RootState) => {
141171
showUpdateViewButton: Boolean(state.editViewName),
142172
isUpdateViewButtonDisabled: !state.isModified || hasSyntaxErrors,
143173
isAtlasDeployed: state.isAtlasDeployed,
174+
showCollectionScanInsight: state.insights.isCollectionScan,
144175
};
145176
};
146177

@@ -149,6 +180,7 @@ const mapDispatch = {
149180
onRunAggregation: runAggregation,
150181
onExportAggregationResults: exportAggregationResults,
151182
onExplainAggregation: explainAggregation,
183+
onCollectionScanInsightActionButtonClick: openCreateIndexModal,
152184
};
153185

154186
export default connect(mapState, mapDispatch)(React.memo(PipelineActions));

packages/compass-aggregations/src/components/saved-pipelines/saved-pipeline-card.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ const controls = css({
4040

4141
const button = css({
4242
flex: 'none',
43+
// Because leafygreen buttons have transition: all by default and this causes
44+
// a lag when trying to hide the buttons, because the browser will transition
45+
// properties like display or visibility
46+
transitionProperty: 'background-color, box-shadow, border-color',
4347
});
4448

4549
export const SavePipelineCard: React.FunctionComponent<

packages/compass-aggregations/src/components/saved-pipelines/saved-pipelines.spec.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const renderSavedPipelines = (
2626
namespace="test.test123"
2727
onDeletePipeline={() => {}}
2828
onOpenPipeline={() => {}}
29+
onMount={() => {}}
2930
{...props}
3031
/>
3132
);

packages/compass-aggregations/src/components/saved-pipelines/saved-pipelines.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react';
1+
import React, { useEffect, useRef } from 'react';
22
import {
33
css,
44
spacing,
@@ -11,6 +11,7 @@ import { SavePipelineCard } from './saved-pipeline-card';
1111
import {
1212
confirmOpenPipeline,
1313
confirmDeletePipeline,
14+
getSavedPipelines,
1415
} from '../../modules/saved-pipeline';
1516
import type { RootState } from '../../modules';
1617
import type { StoredPipeline } from '../../utils/pipeline-storage';
@@ -60,15 +61,21 @@ type SavedPipelinesProps = {
6061
savedPipelines: StoredPipeline[];
6162
onOpenPipeline: (pipelineData: StoredPipeline) => void;
6263
onDeletePipeline: (pipelineId: string) => void;
64+
onMount: () => void;
6365
};
6466

6567
export const SavedPipelines = ({
6668
namespace,
6769
savedPipelines,
6870
onOpenPipeline,
6971
onDeletePipeline,
72+
onMount,
7073
}: SavedPipelinesProps) => {
7174
const darkMode = useDarkMode();
75+
const onMountRef = useRef(onMount);
76+
useEffect(() => {
77+
onMountRef.current();
78+
}, []);
7279
return (
7380
<div className={savedPipelinesStyles} data-testid="saved-pipelines">
7481
<div className={toolbarContentStyles}>
@@ -120,6 +127,7 @@ const mapState = (state: RootState) => ({
120127
const mapDispatch = {
121128
onOpenPipeline: confirmOpenPipeline,
122129
onDeletePipeline: confirmDeletePipeline,
130+
onMount: getSavedPipelines,
123131
};
124132

125133
export default connect(mapState, mapDispatch)(SavedPipelines);

0 commit comments

Comments
 (0)