Skip to content

Commit 315d413

Browse files
mabaasitmabaasit
authored
fix(compass-crud): use ejson to validate input COMPASS-10101 (#7587)
* use ejson to valide data * show error * fix ts * handle on insert document modal as well * add tests * convert to useMemo * clean up, no set state in effect * fix test
1 parent 37ecf9b commit 315d413

File tree

7 files changed

+236
-43
lines changed

7 files changed

+236
-43
lines changed

‎packages/compass-components/src/components/document-list/document-edit-actions-footer.spec.tsx‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import React from 'react';
2+
import { expect } from 'chai';
3+
import { render, screen, userEvent } from '@mongodb-js/testing-library-compass';
4+
import HadronDocument from 'hadron-document';
5+
import DocumentEditActionsFooter from './document-edit-actions-footer';
6+
import Sinon from 'sinon';
7+
8+
const DEFAULT_PROPS = {
9+
doc: new HadronDocument({}),
10+
editing: true,
11+
deleting: false,
12+
onDelete: () => {},
13+
onUpdate: () => {},
14+
validationError: null,
15+
} as const;
16+
17+
describe('DocumentEditActionsFooter', function () {
18+
it('should render the error message', function () {
19+
const { rerender } = render(
20+
<DocumentEditActionsFooter
21+
{...DEFAULT_PROPS}
22+
validationError={new Error('Invalid JSON')}
23+
/>
24+
);
25+
expect(screen.getByText('Invalid JSON')).to.exist;
26+
rerender(
27+
<DocumentEditActionsFooter {...DEFAULT_PROPS} validationError={null} />
28+
);
29+
expect(screen.queryByText('Invalid JSON')).to.not.exist;
30+
});
31+
it('should render cancel button', function () {
32+
render(<DocumentEditActionsFooter {...DEFAULT_PROPS} />);
33+
expect(screen.getByRole('button', { name: /cancel/i })).to.exist;
34+
});
35+
it('should render update button as disabled when there is a validation error', function () {
36+
render(
37+
<DocumentEditActionsFooter
38+
{...DEFAULT_PROPS}
39+
validationError={new Error('Invalid JSON')}
40+
/>
41+
);
42+
expect(screen.getByRole('button', { name: /update/i })).to.have.attribute(
43+
'aria-disabled',
44+
'true'
45+
);
46+
});
47+
it('should call onCancel when cancel button is clicked', function () {
48+
const onCancelSpy = Sinon.spy();
49+
render(
50+
<DocumentEditActionsFooter {...DEFAULT_PROPS} onCancel={onCancelSpy} />
51+
);
52+
expect(onCancelSpy).to.not.have.been.called;
53+
userEvent.click(screen.getByRole('button', { name: /cancel/i }));
54+
expect(onCancelSpy).to.have.been.calledOnce;
55+
});
56+
it('should call onUpdate when update button is clicked and there is no validation error and is modified', function () {
57+
const onUpdateSpy = Sinon.spy();
58+
render(
59+
<DocumentEditActionsFooter
60+
{...DEFAULT_PROPS}
61+
onUpdate={onUpdateSpy}
62+
modified={true}
63+
validationError={null}
64+
/>
65+
);
66+
expect(onUpdateSpy).to.not.have.been.called;
67+
userEvent.click(screen.getByRole('button', { name: /update/i }));
68+
expect(onUpdateSpy).to.have.been.calledOnce;
69+
});
70+
});

‎packages/compass-components/src/components/document-list/document-edit-actions-footer.tsx‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ const StatusMessages: Record<Status, string> = {
7878
function useHadronDocumentStatus(
7979
doc: HadronDocument,
8080
editing: boolean,
81-
deleting: boolean
81+
deleting: boolean,
82+
initialError: Error | null = null
8283
) {
8384
const [status, setStatus] = useState<Status>(() => {
8485
return editing
@@ -221,7 +222,19 @@ function useHadronDocumentStatus(
221222
return status;
222223
}, [status, editing, deleting]);
223224

224-
return { status: derivedStatus, updateStatus, error };
225+
const derivedError = useMemo(() => {
226+
if (error) {
227+
return error;
228+
}
229+
if (initialError) {
230+
return {
231+
message: initialError.message,
232+
};
233+
}
234+
return null;
235+
}, [error, initialError]);
236+
237+
return { status: derivedStatus, updateStatus, error: derivedError };
225238
}
226239

227240
const container = css({
@@ -294,7 +307,7 @@ const EditActionsFooter: React.FunctionComponent<{
294307
editing: boolean;
295308
deleting: boolean;
296309
modified?: boolean;
297-
containsErrors?: boolean;
310+
validationError?: Error | null;
298311
alwaysForceUpdate?: boolean;
299312
onUpdate(force: boolean): void;
300313
onDelete(): void;
@@ -304,7 +317,7 @@ const EditActionsFooter: React.FunctionComponent<{
304317
editing,
305318
deleting,
306319
modified = false,
307-
containsErrors = false,
320+
validationError: initialError = null,
308321
alwaysForceUpdate = false,
309322
onUpdate,
310323
onDelete,
@@ -314,14 +327,13 @@ const EditActionsFooter: React.FunctionComponent<{
314327
status: _status,
315328
updateStatus,
316329
error,
317-
} = useHadronDocumentStatus(doc, editing, deleting);
318-
330+
} = useHadronDocumentStatus(doc, editing, deleting, initialError);
319331
const darkMode = useDarkMode();
320332

321333
// Allow props to override event based status of the document (helpful for
322334
// JSON editor where changing the document text doesn't really generate any
323335
// changes of the HadronDocument)
324-
const status = containsErrors
336+
const status = initialError
325337
? 'ContainsErrors'
326338
: modified
327339
? 'Modified'

‎packages/compass-crud/src/components/insert-document-dialog.spec.tsx‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import React, { type ComponentProps } from 'react';
2+
import { expect } from 'chai';
3+
import { render, screen } from '@mongodb-js/testing-library-compass';
4+
import InsertDocumentDialog from './insert-document-dialog';
5+
import HadronDocument from 'hadron-document';
6+
import { setCodemirrorEditorValue } from '@mongodb-js/compass-editor';
7+
8+
const noop = () => {};
9+
const defaultProps = {
10+
closeInsertDocumentDialog: noop,
11+
insertDocument: noop,
12+
insertMany: noop,
13+
toggleInsertDocument: noop,
14+
toggleInsertDocumentView: noop,
15+
isCommentNeeded: false,
16+
csfleState: { state: 'none' },
17+
isOpen: true,
18+
ns: 'airbnb.listings',
19+
updateComment: noop,
20+
error: null,
21+
} as unknown as ComponentProps<typeof InsertDocumentDialog>;
22+
23+
describe('InsertDocumentDialog', function () {
24+
it('show error message for invalid EJSON', async function () {
25+
let jsonDoc = '{}';
26+
const doc = new HadronDocument({});
27+
doc.editing = true;
28+
function updateJsonDoc(value: string | null) {
29+
doc.setModifiedEJSONString(value);
30+
jsonDoc = value ?? '{}';
31+
}
32+
const { rerender } = render(
33+
<InsertDocumentDialog
34+
{...defaultProps}
35+
doc={doc}
36+
jsonDoc={jsonDoc}
37+
updateJsonDoc={updateJsonDoc}
38+
jsonView
39+
/>
40+
);
41+
await setCodemirrorEditorValue(
42+
screen.getByTestId('insert-document-json-editor'),
43+
'{ "invalid_long": { "$numberLong": "1234567234324812317654321" } } '
44+
);
45+
rerender(
46+
<InsertDocumentDialog
47+
{...defaultProps}
48+
doc={doc}
49+
jsonDoc={jsonDoc}
50+
updateJsonDoc={updateJsonDoc}
51+
jsonView
52+
/>
53+
);
54+
const errorMessage = await screen.findByText(
55+
/numberLong string is too long/i
56+
);
57+
expect(errorMessage).to.exist;
58+
});
59+
});

‎packages/compass-crud/src/components/insert-document-dialog.tsx‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
spacing,
2020
showErrorDetails,
2121
} from '@mongodb-js/compass-components';
22+
import HadronDocument from 'hadron-document';
2223

2324
import type { InsertCSFLEWarningBannerProps } from './insert-csfle-warning-banner';
2425
import InsertCSFLEWarningBanner from './insert-csfle-warning-banner';
@@ -165,19 +166,18 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
165166
* Checks for invalidElements in hadron doc if in HadronDocument view, or
166167
* parsing error in JsonView of the modal
167168
*
168-
* @returns {Boolean} If the document has errors.
169169
*/
170-
const hasErrors = useCallback(() => {
170+
const documentErrors = useMemo(() => {
171171
if (jsonView) {
172172
try {
173-
JSON.parse(jsonDoc);
173+
HadronDocument.FromEJSON(jsonDoc);
174174
return false;
175-
} catch {
176-
return true;
175+
} catch (e) {
176+
return (e as Error).message;
177177
}
178178
}
179-
return invalidElements.length > 0;
180-
}, [invalidElements, jsonDoc, jsonView]);
179+
return invalidElements.length > 0 ? INSERT_INVALID_MESSAGE : false;
180+
}, [jsonDoc, jsonView, invalidElements]);
181181

182182
const handleInvalid = useCallback(
183183
(el: Element) => {
@@ -190,15 +190,15 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
190190

191191
const handleValid = useCallback(
192192
(el: Element) => {
193-
if (hasErrors()) {
193+
if (documentErrors) {
194194
setInvalidElements((invalidElements) =>
195195
without(invalidElements, el.uuid)
196196
);
197197
} else {
198198
setInvalidElements([]);
199199
}
200200
},
201-
[hasErrors, setInvalidElements]
201+
[documentErrors, setInvalidElements]
202202
);
203203

204204
useEffect(() => {
@@ -281,17 +281,26 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
281281
);
282282

283283
const currentView = jsonView ? 'JSON' : 'List';
284-
const variant = insertInProgress ? 'info' : 'danger';
285284

286-
const error = useMemo(() => {
287-
if (hasErrors()) {
288-
return { message: INSERT_INVALID_MESSAGE };
285+
const banner = useMemo(() => {
286+
if (documentErrors) {
287+
return {
288+
message: documentErrors,
289+
variant: 'danger' as const,
290+
};
289291
}
290292
if (insertInProgress) {
291-
return { message: 'Inserting Document' };
293+
return { message: 'Inserting Document', variant: 'info' as const };
292294
}
293-
return _error;
294-
}, [_error, hasErrors, insertInProgress]);
295+
if (_error) {
296+
return {
297+
message: _error.message,
298+
variant: 'danger' as const,
299+
info: _error.info,
300+
};
301+
}
302+
return null;
303+
}, [_error, documentErrors, insertInProgress]);
295304

296305
return (
297306
<FormModal
@@ -302,7 +311,7 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
302311
onSubmit={handleInsert.bind(this)}
303312
onCancel={closeInsertDocumentDialog}
304313
submitButtonText="Insert"
305-
submitDisabled={hasErrors()}
314+
submitDisabled={Boolean(documentErrors)}
306315
data-testid="insert-document-modal"
307316
minBodyHeight={spacing[1600] * 2} // make sure there is enough space for the menu
308317
>
@@ -315,7 +324,7 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
315324
onChange={switchInsertDocumentView.bind(this)}
316325
>
317326
<SegmentedControlOption
318-
disabled={hasErrors()}
327+
disabled={Boolean(documentErrors)}
319328
data-testid="insert-document-dialog-view-json"
320329
aria-label="E-JSON View"
321330
value="JSON"
@@ -327,7 +336,7 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
327336
}}
328337
></SegmentedControlOption>
329338
<SegmentedControlOption
330-
disabled={hasErrors()}
339+
disabled={Boolean(documentErrors)}
331340
data-testid="insert-document-dialog-view-list"
332341
aria-label="Document list"
333342
value="List"
@@ -351,21 +360,21 @@ const InsertDocumentDialog: React.FC<InsertDocumentDialogProps> = ({
351360
updateComment={updateComment}
352361
/>
353362
</div>
354-
{error && (
363+
{banner && (
355364
<Banner
356365
data-testid="insert-document-banner"
357-
data-variant={variant}
358-
variant={variant}
366+
data-variant={banner.variant}
367+
variant={banner.variant}
359368
className={bannerStyles}
360369
>
361-
{error?.message}
362-
{error?.info && (
370+
{banner.message}
371+
{banner.info && (
363372
<Button
364373
size="xsmall"
365374
className={errorDetailsBtnStyles}
366375
onClick={() =>
367376
showErrorDetails({
368-
details: error.info!,
377+
details: banner.info!,
369378
closeAction: 'back',
370379
})
371380
}

‎packages/compass-crud/src/components/json-editor.spec.tsx‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import React, { type ComponentProps } from 'react';
2+
import { expect } from 'chai';
3+
import { render, screen } from '@mongodb-js/testing-library-compass';
4+
import JSONEditor from './json-editor';
5+
import HadronDocument from 'hadron-document';
6+
import { setCodemirrorEditorValue } from '@mongodb-js/compass-editor';
7+
8+
function renderJSONEditor(
9+
props: Partial<ComponentProps<typeof JSONEditor>> = {}
10+
) {
11+
const doc = new HadronDocument({});
12+
doc.editing = true;
13+
return render(
14+
<JSONEditor doc={doc} editable namespace="airbnb.listings" {...props} />
15+
);
16+
}
17+
18+
describe('JSONEditor', function () {
19+
context('shows error messages', function () {
20+
it('shows error message for invalid JSON', async function () {
21+
renderJSONEditor();
22+
await setCodemirrorEditorValue(
23+
screen.getByTestId('json-editor'),
24+
'{ "name": } '
25+
);
26+
const errorMessage = await screen.findByText(/unexpected token/i);
27+
expect(errorMessage).to.exist;
28+
});
29+
it('show error message for valid EJSON', async function () {
30+
renderJSONEditor();
31+
await setCodemirrorEditorValue(
32+
screen.getByTestId('json-editor'),
33+
'{ "invalid_long": { "$numberLong": "1234567234324812317654321" } } '
34+
);
35+
const errorMessage = await screen.findByText(
36+
/numberLong string is too long/i
37+
);
38+
expect(errorMessage).to.exist;
39+
});
40+
});
41+
});

0 commit comments

Comments
 (0)