Skip to content

Commit fce64c0

Browse files
feat(compass-aggregations): add confirm banner for edit pipeline and new banners COMPASS-9700 (#7198)
* add confirmation banner * add version incompatible banner * add disabled tooltip * fix incorrect readonly * add tests * add missing default params * move isPipelineSearchQueryable into compass-utils and update dependencies * add mongodb to dependencies in compass-utils * make isSearchPipelineQueryable true for not read only views * fix update-view.spec.ts * add negative test case * remove async * just introduce new function to return whether has search indexes * remove compass-utils dep * remove compass utils changes and cleanup * update lock file * fix accidental remove * fix stage.spec.ts * return false instead of errror * check 8.0+ * fix test * update package-lock
1 parent d719b48 commit fce64c0

File tree

21 files changed

+553
-219
lines changed

21 files changed

+553
-219
lines changed

package-lock.json

Lines changed: 77 additions & 56 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
"@mongodb-js/compass-utils": "^0.9.10",
7373
"@mongodb-js/compass-workspaces": "^0.51.0",
7474
"@mongodb-js/explain-plan-helper": "^1.4.17",
75-
"@mongodb-js/mongodb-constants": "^0.12.2",
75+
"@mongodb-js/mongodb-constants": "^0.14.0",
7676
"@mongodb-js/my-queries-storage": "^0.37.0",
7777
"@mongodb-js/shell-bson-parser": "^1.2.0",
7878
"bson": "^6.10.4",

packages/compass-aggregations/src/modules/search-indexes.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,26 @@ export const createSearchIndex = (): PipelineBuilderThunkAction<void> => {
123123
};
124124
};
125125

126+
/**
127+
* Checks whether a namespace has existing search indexes
128+
*
129+
* @param namespace - collection/view namespace
130+
* @param dataService - dataService instance
131+
* @returns whether namespace has existing search indexes
132+
*/
133+
export const namespaceHasSearchIndexes = async (
134+
namespace: string,
135+
dataService: { getSearchIndexes?: (ns: string) => Promise<SearchIndex[]> }
136+
): Promise<boolean> => {
137+
try {
138+
if (!dataService.getSearchIndexes) {
139+
throw new Error('Cannot get search indexes in this environment');
140+
}
141+
const indexes = await dataService.getSearchIndexes(namespace);
142+
return indexes.length > 0;
143+
} catch {
144+
return false;
145+
}
146+
};
147+
126148
export default reducer;

packages/compass-aggregations/src/modules/update-view.spec.ts

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import {
1010
} from '@mongodb-js/compass-connections/provider';
1111
import { createDefaultConnectionInfo } from '@mongodb-js/testing-library-compass';
1212

13+
// Importing this to stub showConfirmation
14+
import * as updateViewSlice from './update-view';
15+
import * as searchIndexesSlice from './search-indexes';
16+
1317
const TEST_CONNECTION_INFO = { ...createDefaultConnectionInfo(), title: '' };
1418

1519
describe('update-view module', function () {
@@ -48,10 +52,20 @@ describe('update-view module', function () {
4852
let stateMock: any;
4953
let getStateMock: () => any;
5054
let updateCollectionFake = sinon.fake();
55+
let showConfirmationStub: sinon.SinonStub;
56+
let namespaceHasSearchIndexesStub: sinon.SinonStub;
5157

52-
beforeEach(async function () {
58+
beforeEach(function () {
5359
dispatchFake = sinon.fake();
5460
updateCollectionFake = sinon.fake.resolves(undefined);
61+
showConfirmationStub = sinon
62+
.stub(updateViewSlice, 'showConfirmation')
63+
.resolves(true);
64+
65+
namespaceHasSearchIndexesStub = sinon
66+
.stub(searchIndexesSlice, 'namespaceHasSearchIndexes')
67+
.resolves(true);
68+
5569
stateMock = {
5670
pipelineBuilder: { pipelineMode: 'builder-ui' },
5771
focusMode: { isEnabled: false },
@@ -62,20 +76,57 @@ describe('update-view module', function () {
6276
updateCollection: updateCollectionFake,
6377
},
6478
},
79+
serverVersion: '8.1.0',
6580
};
6681
getStateMock = () => stateMock;
82+
});
6783

84+
afterEach(function () {
85+
showConfirmationStub.restore();
86+
namespaceHasSearchIndexesStub.restore();
87+
});
88+
89+
it('first it calls to dismiss any existing error', async function () {
6890
const runUpdateView = updateView();
6991
await runUpdateView(dispatchFake, getStateMock, thunkArg as any);
70-
});
7192

72-
it('first it calls to dismiss any existing error', function () {
7393
expect(dispatchFake.firstCall.args[0]).to.deep.equal({
7494
type: 'aggregations/update-view/DISMISS_VIEW_UPDATE_ERROR',
7595
});
7696
});
7797

78-
it('calls the data service to update the view for the provided ns', function () {
98+
it('does not shows confirmation banner if search indexes are not present', async function () {
99+
namespaceHasSearchIndexesStub.resolves(false);
100+
const runUpdateView = updateView();
101+
await runUpdateView(dispatchFake, getStateMock, thunkArg as any);
102+
103+
expect(showConfirmationStub.calledOnce).to.be.false;
104+
});
105+
106+
it('shows confirmation banner when search indexes are present', async function () {
107+
const runUpdateView = updateView();
108+
await runUpdateView(dispatchFake, getStateMock, thunkArg as any);
109+
110+
expect(showConfirmationStub.calledOnce).to.be.true;
111+
expect(showConfirmationStub.firstCall.args[0]).to.deep.include({
112+
title: `Are you sure you want to update the view?`,
113+
buttonText: 'Update',
114+
});
115+
});
116+
117+
it('does not update view if not confirmed', async function () {
118+
showConfirmationStub.resolves(false);
119+
120+
const runUpdateView = updateView();
121+
await runUpdateView(dispatchFake, getStateMock, thunkArg as any);
122+
123+
expect(updateCollectionFake.calledOnce).to.be.false;
124+
});
125+
126+
it('calls the data service to update the view for the provided ns', async function () {
127+
const runUpdateView = updateView();
128+
await runUpdateView(dispatchFake, getStateMock, thunkArg as any);
129+
79130
expect(updateCollectionFake.firstCall.args[0]).to.equal('aa.bb');
80131
expect(updateCollectionFake.firstCall.args[1]).to.deep.equal({
81132
viewOn: 'bb',

packages/compass-aggregations/src/modules/update-view.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import {
88
import type { PipelineBuilderThunkAction } from '.';
99
import { isAction } from '../utils/is-action';
1010
import type { AnyAction } from 'redux';
11+
import { showConfirmation as showConfirmationModal } from '@mongodb-js/compass-components';
12+
import { namespaceHasSearchIndexes } from './search-indexes';
13+
import { VIEW_PIPELINE_UTILS } from '@mongodb-js/mongodb-constants';
1114

1215
export type UpdateViewState = null | string;
1316

@@ -73,6 +76,8 @@ export const dismissViewError = (): DismissViewUpdateErrorAction => ({
7376
type: DISMISS_VIEW_UPDATE_ERROR,
7477
});
7578

79+
//Exporting this for test only to stub it and set its value
80+
export const showConfirmation = showConfirmationModal;
7681
/**
7782
* Updates a view.
7883
*
@@ -96,6 +101,7 @@ export const updateView = (): PipelineBuilderThunkAction<Promise<void>> => {
96101
const state = getState();
97102
const ds = state.dataService.dataService;
98103
const viewNamespace = state.editViewName;
104+
const serverVersion = state.serverVersion;
99105

100106
if (!viewNamespace) {
101107
return;
@@ -107,6 +113,30 @@ export const updateView = (): PipelineBuilderThunkAction<Promise<void>> => {
107113
getState(),
108114
pipelineBuilder
109115
);
116+
117+
if (
118+
VIEW_PIPELINE_UTILS.isVersionSearchCompatibleForViewsDataExplorer(
119+
serverVersion
120+
) &&
121+
ds &&
122+
(await namespaceHasSearchIndexes(viewNamespace, ds))
123+
) {
124+
const pipelineIsSearchQueryable =
125+
VIEW_PIPELINE_UTILS.isPipelineSearchQueryable(viewPipeline);
126+
const confirmed = await showConfirmation({
127+
title: `Are you sure you want to update the view?`,
128+
description: pipelineIsSearchQueryable
129+
? 'There are search indexes created on this view. Updating the view will result in an index rebuild, which will consume additional resources on your cluster.'
130+
: 'This update will make the view incompatible with search indexes and will cause all search indexes to fail. Only views containing $addFields, $set or $match stages with the $expr operator are compatible with search indexes.',
131+
buttonText: 'Update',
132+
variant: pipelineIsSearchQueryable ? 'primary' : 'danger',
133+
});
134+
135+
if (!confirmed) {
136+
return;
137+
}
138+
}
139+
110140
const options = {
111141
viewOn: toNS(state.namespace).collection,
112142
pipeline: viewPipeline,

packages/compass-aggregations/src/utils/stage.spec.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ describe('utils', function () {
6666
expect(searchMeta.length).to.be.equal(1);
6767
});
6868

69+
it('returns $search stage for a view', function () {
70+
const search = filterStageOperators({
71+
...filter,
72+
sourceName: 'simple.sample',
73+
}).filter((o) => o.name === '$search');
74+
75+
expect(search.length).to.be.equal(1);
76+
});
77+
78+
it('returns $searchMeta stage for a view', function () {
79+
const searchMeta = filterStageOperators({
80+
...filter,
81+
sourceName: 'simple.sample',
82+
}).filter((o) => o.name === '$searchMeta');
83+
84+
expect(searchMeta.length).to.be.equal(1);
85+
});
86+
6987
// $documents only works for db.aggregate, not coll.aggregate
7088
it('does not return $documents stage for a regular collection', function () {
7189
const documents = filterStageOperators({ ...filter }).filter(
@@ -97,17 +115,6 @@ describe('utils', function () {
97115

98116
expect(searchStages.length).to.be.equal(0);
99117
});
100-
101-
it('does not return full-text search stages for views', function () {
102-
const searchStages = filterStageOperators({
103-
...filter,
104-
sourceName: 'simple.sample',
105-
}).filter((o) =>
106-
['$search', '$searchMeta', '$documents'].includes(o.name)
107-
);
108-
109-
expect(searchStages.length).to.be.equal(0);
110-
});
111118
});
112119

113120
context('when on-prem', function () {

packages/compass-collection/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"@mongodb-js/compass-telemetry": "^1.13.0",
5757
"@mongodb-js/compass-workspaces": "^0.51.0",
5858
"@mongodb-js/connection-info": "^0.17.1",
59-
"@mongodb-js/mongodb-constants": "^0.12.2",
59+
"@mongodb-js/mongodb-constants": "^0.14.0",
6060
"compass-preferences-model": "^2.50.0",
6161
"hadron-document": "^8.9.5",
6262
"mongodb": "^6.17.0",

packages/compass-editor/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
"@codemirror/view": "^6.38.0",
7474
"@lezer/highlight": "^1.2.1",
7575
"@mongodb-js/compass-components": "^1.48.0",
76-
"@mongodb-js/mongodb-constants": "^0.12.2",
76+
"@mongodb-js/mongodb-constants": "^0.14.0",
7777
"mongodb-query-parser": "^4.3.0",
7878
"polished": "^4.2.2",
7979
"prettier": "^2.7.1",

packages/compass-indexes/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676
"@mongodb-js/compass-logging": "^1.7.11",
7777
"@mongodb-js/compass-telemetry": "^1.13.0",
7878
"@mongodb-js/compass-workspaces": "^0.51.0",
79-
"@mongodb-js/mongodb-constants": "^0.12.2",
79+
"@mongodb-js/mongodb-constants": "^0.14.0",
8080
"@mongodb-js/shell-bson-parser": "^1.2.0",
8181
"@mongodb-js/connection-info": "^0.17.1",
8282
"bson": "^6.10.4",

packages/compass-indexes/src/components/indexes-toolbar/indexes-toolbar.spec.tsx

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { IndexesToolbar } from './indexes-toolbar';
1313
import type { PreferencesAccess } from 'compass-preferences-model';
1414
import { createSandboxFromDefaultPreferences } from 'compass-preferences-model';
1515
import { PreferencesProvider } from 'compass-preferences-model/provider';
16+
import type { Document } from 'mongodb';
1617

1718
describe('IndexesToolbar Component', function () {
1819
before(cleanup);
@@ -39,6 +40,7 @@ describe('IndexesToolbar Component', function () {
3940
onRefreshIndexes={() => {}}
4041
isSearchIndexesSupported={false}
4142
isRefreshing={false}
43+
collectionStats={{ index_count: 0, index_size: 0, pipeline: [] }}
4244
onIndexViewChanged={() => {}}
4345
onCreateRegularIndexClick={() => {}}
4446
onCreateSearchIndexClick={() => {}}
@@ -153,10 +155,38 @@ describe('IndexesToolbar Component', function () {
153155
expect(screen.getByText('Create Search Index')).to.be.visible;
154156
});
155157

156-
it('should not render the refresh button', function () {
158+
it('should render the refresh button', function () {
157159
expect(screen.queryByText('Refresh')).to.be.visible;
158160
});
159161
});
162+
163+
describe('and pipeline is not queryable', function () {
164+
it('should disable the create search index button', function () {
165+
const pipelineMock: Document[] = [
166+
{ $project: { newField: 'testValue' } },
167+
];
168+
const mockCollectionStats = {
169+
index_count: 0,
170+
index_size: 0,
171+
pipeline: pipelineMock,
172+
};
173+
174+
renderIndexesToolbar({
175+
isReadonlyView: true,
176+
serverVersion: '8.1.0',
177+
indexView: 'search-indexes',
178+
collectionStats: mockCollectionStats,
179+
});
180+
181+
expect(screen.getByText('Create Search Index')).to.be.visible;
182+
expect(
183+
screen
184+
.getByText('Create Search Index')
185+
.closest('button')
186+
?.getAttribute('aria-disabled')
187+
).to.equal('true');
188+
});
189+
});
160190
});
161191

162192
describe('when it is preferences ReadOnly', function () {

0 commit comments

Comments
 (0)