From 8723bfbff0e5c0c3c797994c1281d1f55df6dadd Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 26 Feb 2025 16:57:51 +0100 Subject: [PATCH 01/15] wip --- packages/compass-components/src/index.ts | 1 + .../src/components/document-list.tsx | 10 +- .../src/components/insert-document-dialog.tsx | 497 +++++++++--------- .../compass-crud/src/stores/crud-store.ts | 31 +- 4 files changed, 285 insertions(+), 254 deletions(-) diff --git a/packages/compass-components/src/index.ts b/packages/compass-components/src/index.ts index a79645f7f9d..d8c05eb97be 100644 --- a/packages/compass-components/src/index.ts +++ b/packages/compass-components/src/index.ts @@ -92,6 +92,7 @@ export { ModalBody } from './components/modals/modal-body'; export { ModalHeader } from './components/modals/modal-header'; export { FormModal } from './components/modals/form-modal'; export { InfoModal } from './components/modals/info-modal'; +export { ErrorDetailsModal } from './components/modals/error-details-modal'; export type { FileInputBackend, diff --git a/packages/compass-crud/src/components/document-list.tsx b/packages/compass-crud/src/components/document-list.tsx index bc5fe807684..c5311c5f2c5 100644 --- a/packages/compass-crud/src/components/document-list.tsx +++ b/packages/compass-crud/src/components/document-list.tsx @@ -9,6 +9,7 @@ import { WorkspaceContainer, spacing, withDarkMode, + ErrorDetailsModal, } from '@mongodb-js/compass-components'; import type { InsertDocumentDialogProps } from './insert-document-dialog'; import InsertDocumentDialog from './insert-document-dialog'; @@ -84,7 +85,7 @@ export type DocumentListProps = { | 'doc' | 'csfleState' | 'isOpen' - | 'message' + | 'error' | 'mode' | 'jsonDoc' | 'isCommentNeeded' @@ -583,6 +584,13 @@ const DocumentList: React.FunctionComponent = (props) => { updateComment={updateComment} {...insert} /> + false} + details={insert.error?.info} + closeAction="back" + // TODO + /> void; insertMany: () => void; isOpen: boolean; - message: string; + error: WriteError; mode: 'modifying' | 'error'; version: string; updateJsonDoc: (value: string | null) => void; @@ -61,119 +68,181 @@ export type InsertDocumentDialogProps = InsertCSFLEWarningBannerProps & { updateComment: (isCommentNeeded: boolean) => void; logger?: Logger; track?: TrackFunction; + showErrorDetails?: () => void; }; -type InsertDocumentDialogState = { - insertInProgress: boolean; +const DocumentOrJsonView: React.FC<{ + jsonView: InsertDocumentDialogProps['jsonView']; + doc: InsertDocumentDialogProps['doc']; + hasManyDocuments: () => boolean; + updateJsonDoc: InsertDocumentDialogProps['updateJsonDoc']; + jsonDoc: InsertDocumentDialogProps['jsonDoc']; + isCommentNeeded: InsertDocumentDialogProps['isCommentNeeded']; + updateComment: InsertDocumentDialogProps['updateComment']; +}> = ({ + jsonView, + doc, + hasManyDocuments, + updateJsonDoc, + jsonDoc, + isCommentNeeded, + updateComment, +}) => { + if (!jsonView) { + if (hasManyDocuments()) { + return ( + + This view is not supported for multiple documents. To specify data + types and use other functionality of this view, please insert + documents one at a time. + + ); + } + + if (!doc) { + return null; + } + + return ; + } + + return ( + + ); }; /** * Component for the insert document dialog. */ -class InsertDocumentDialog extends React.PureComponent< - InsertDocumentDialogProps, - InsertDocumentDialogState -> { - invalidElements: Document['uuid'][]; +const InsertDocumentDialog: React.FC = ({ + isOpen, + jsonView, + jsonDoc, + doc, + isCommentNeeded, + error, + ns, + csfleState, + track, + insertMany, + insertDocument, + toggleInsertDocument, + toggleInsertDocumentView, + updateJsonDoc, + updateComment, + closeInsertDocumentDialog, + showErrorDetails, +}) => { + const [invalidElements, setInvalidElements] = useState( + [] + ); + const [insertInProgress, setInsertInProgress] = useState(false); - /** - * The component constructor. - * - * @param {Object} props - The properties. - */ - constructor(props: InsertDocumentDialogProps) { - super(props); - this.state = { insertInProgress: false }; - this.invalidElements = []; - } + const hasManyDocuments = useCallback(() => { + let parsed: unknown; + try { + parsed = JSON.parse(jsonDoc); + } catch { + return false; + } + return Array.isArray(parsed); + }, [jsonDoc]); /** - * Handle subscriptions to the document. + * Does the document have errors with the bson types? Checks for + * invalidElements in hadron doc if in HadronDocument view, or parsing error + * in JsonView of the modal * - * @param {Object} prevProps - The previous properties. + * Checks for invalidElements in hadron doc if in HadronDocument view, or + * parsing error in JsonView of the modal + * + * @returns {Boolean} If the document has errors. */ - componentDidUpdate( - prevProps: InsertDocumentDialogProps, - state: InsertDocumentDialogState - ) { - if (prevProps.isOpen !== this.props.isOpen && this.props.isOpen) { - this.props.track && - this.props.track( - 'Screen', - { name: 'insert_document_modal' }, - undefined + const hasErrors = useCallback(() => { + if (jsonView) { + try { + JSON.parse(jsonDoc); + return false; + } catch { + return true; + } + } + return invalidElements.length > 0; + }, [invalidElements, jsonDoc, jsonView]); + + const handleInvalid = useCallback( + (el: Element) => { + if (!invalidElements.includes(el.uuid)) { + setInvalidElements((elements) => [...elements, el.uuid]); + } + }, + [invalidElements] + ); + + const handleValid = useCallback( + (el: Element) => { + if (hasErrors()) { + setInvalidElements((invalidElements) => + without(invalidElements, el.uuid) ); + } + }, + [hasErrors, setInvalidElements] + ); + + useEffect(() => { + if (isOpen && track) { + track('Screen', { name: 'insert_document_modal' }, undefined); } + }, [isOpen, track]); - if (this.props.isOpen && !this.hasManyDocuments()) { - if (prevProps.jsonView && !this.props.jsonView) { + useEffect(() => { + if (isOpen && !hasManyDocuments()) { + if (!jsonView) { // When switching to Hadron Document View. // Reset the invalid elements list, which contains the // uuids of each element that has BSON type cast errors. - this.invalidElements = []; + setInvalidElements([]); // Subscribe to the validation errors for BSON types on the document. - this.props.doc.on(Element.Events.Invalid, this.handleInvalid); - this.props.doc.on(Element.Events.Valid, this.handleValid); - } else if (!prevProps.jsonView && this.props.jsonView) { + doc.on(Element.Events.Invalid, handleInvalid); + doc.on(Element.Events.Valid, handleValid); + } else { // When switching to JSON View. // Remove the listeners to the BSON type validation errors in order to clean up properly. - this.props.doc.removeListener( - Element.Events.Invalid, - this.handleInvalid - ); - this.props.doc.removeListener(Element.Events.Valid, this.handleValid); + doc.removeListener(Element.Events.Invalid, handleInvalid); + doc.removeListener(Element.Events.Valid, handleValid); } } + }, [isOpen, jsonView, doc, handleValid, handleInvalid, hasManyDocuments]); - if (state.insertInProgress) { - this.setState({ insertInProgress: false }); + useEffect(() => { + if (insertInProgress) { + setInsertInProgress(false); } - } + }, [insertInProgress]); - componentWillUnount() { - if (!this.hasManyDocuments()) { + useEffect(() => { + if (!hasManyDocuments() && doc) { // When closing the modal. // Remove the listeners to the BSON type validation errors in order to clean up properly. - this.props.doc.removeListener(Element.Events.Invalid, this.handleInvalid); - this.props.doc.removeListener(Element.Events.Valid, this.handleValid); + doc.removeListener(Element.Events.Invalid, handleInvalid); + doc.removeListener(Element.Events.Valid, handleValid); } - } + }); - /** - * Handles an element in the document becoming valid from invalid. - * - * @param {Element} el - Element - */ - handleValid = (el: Element) => { - if (this.hasErrors()) { - pull(this.invalidElements, el.uuid); - this.forceUpdate(); - } - }; - - /** - * Handles a valid element in the document becoming invalid. - * - * @param {Element} el - Element - */ - handleInvalid = (el: Element) => { - if (!this.invalidElements.includes(el.uuid)) { - this.invalidElements.push(el.uuid); - this.forceUpdate(); - } - }; - - /** - * Handle the insert. - */ - handleInsert() { - this.setState({ insertInProgress: true }); - if (this.hasManyDocuments()) { - this.props.insertMany(); + const handleInsert = useCallback(() => { + setInsertInProgress(true); + if (hasManyDocuments()) { + insertMany(); } else { - this.props.insertDocument(); + insertDocument(); } - } + }, [setInsertInProgress, insertMany, insertDocument, hasManyDocuments]); /** * Switches between JSON and Hadron Document views. @@ -183,168 +252,108 @@ class InsertDocumentDialog extends React.PureComponent< * * @param {String} view - which view we are looking at: JSON or LIST. */ - switchInsertDocumentView(view: string) { - if (!this.hasManyDocuments()) { - this.props.toggleInsertDocument(view as 'JSON' | 'List'); - } else { - this.props.toggleInsertDocumentView(view as 'JSON' | 'List'); - } - } - - /** - * Does the document have errors with the bson types? Checks for - * invalidElements in hadron doc if in HadronDocument view, or parsing error - * in JsonView of the modal - * - * Checks for invalidElements in hadron doc if in HadronDocument view, or - * parsing error in JsonView of the modal - * - * @returns {Boolean} If the document has errors. - */ - hasErrors() { - if (this.props.jsonView) { - try { - JSON.parse(this.props.jsonDoc); - return false; - } catch { - return true; + const switchInsertDocumentView = useCallback( + (view: string) => { + if (!hasManyDocuments()) { + toggleInsertDocument(view as 'JSON' | 'List'); + } else { + toggleInsertDocumentView(view as 'JSON' | 'List'); } - } - return this.invalidElements.length > 0; - } + }, + [hasManyDocuments, toggleInsertDocument, toggleInsertDocumentView] + ); - /** - * Check if the json pasted is multiple documents (array). - * - * @returns {bool} If many documents are currently being inserted. - */ - hasManyDocuments() { - let jsonDoc: unknown; - try { - jsonDoc = JSON.parse(this.props.jsonDoc); - } catch { - return false; - } - return Array.isArray(jsonDoc); - } - - /** - * Render the document or json editor. - * - * @returns {React.Component} The component. - */ - renderDocumentOrJsonView() { - if (!this.props.jsonView) { - if (this.hasManyDocuments()) { - return ( - - This view is not supported for multiple documents. To specify data - types and use other functionality of this view, please insert - documents one at a time. - - ); - } - - if (!this.props.doc) { - return; - } - - return ; - } + const currentView = jsonView ? 'JSON' : 'List'; + const variant = insertInProgress ? 'info' : 'danger'; - return ( - - ); + if (hasErrors()) { + error = { message: INSERT_INVALID_MESSAGE }; } - /** - * Render the modal dialog. - * - * @returns {React.Component} The react component. - */ - render() { - const currentView = this.props.jsonView ? 'JSON' : 'List'; - const variant = this.state.insertInProgress ? 'info' : 'danger'; - - let message = this.props.message; - - if (this.hasErrors()) { - message = INSERT_INVALID_MESSAGE; - } - - if (this.state.insertInProgress) { - message = 'Inserting Document'; - } - - return ( - -
- - } - onClick={(evt) => { - // We override the `onClick` functionality to prevent form submission. - // The value changing occurs in the `onChange` in the `SegmentedControl`. - evt.preventDefault(); - }} - > - { - // We override the `onClick` functionality to prevent form submission. - // The value changing occurs in the `onChange` in the `SegmentedControl`. - evt.preventDefault(); - }} - glyph={} - > - -
-
- {this.renderDocumentOrJsonView()} -
- {message && ( - - {message} - - )} - - - ); + if (insertInProgress) { + error = { message: 'Inserting Document' }; } -} + + return ( + +
+ + } + onClick={(evt) => { + // We override the `onClick` functionality to prevent form submission. + // The value changing occurs in the `onChange` in the `SegmentedControl`. + evt.preventDefault(); + }} + > + { + // We override the `onClick` functionality to prevent form submission. + // The value changing occurs in the `onChange` in the `SegmentedControl`. + evt.preventDefault(); + }} + glyph={} + > + +
+
+ +
+ {error && ( + + {error?.message} + {error?.info && ( + + )} + + )} + + + ); +}; export default withLogger(InsertDocumentDialog, 'COMPASS-CRUD-UI'); diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts index 13afb48418a..bda8a90ee98 100644 --- a/packages/compass-crud/src/stores/crud-store.ts +++ b/packages/compass-crud/src/stores/crud-store.ts @@ -2,6 +2,7 @@ import type { Listenable, Store } from 'reflux'; import Reflux from 'reflux'; import toNS from 'mongodb-ns'; import { findIndex, isEmpty, isEqual } from 'lodash'; +import type { MongoServerError } from 'mongodb'; import semver from 'semver'; import StateMixin from '@mongodb-js/reflux-state-mixin'; import type { Element } from 'hadron-document'; @@ -267,10 +268,15 @@ export type InsertCSFLEState = { encryptedFields?: string[]; }; +export type WriteError = { + message: string; + info?: Record; +}; + type InsertState = { doc: null | Document; jsonDoc: null | string; - message: string; + error?: WriteError; csfleState: InsertCSFLEState; mode: 'modifying' | 'error'; jsonView: boolean; @@ -463,7 +469,6 @@ class CrudStoreImpl return { doc: null, jsonDoc: null, - message: '', csfleState: { state: 'none' }, mode: MODIFYING, jsonView: false, @@ -533,6 +538,13 @@ class CrudStoreImpl void navigator.clipboard.writeText(documentEJSON); } + getWriteError(error: Error): WriteError { + return { + message: error.message, + info: (error as MongoServerError).errInfo, + }; + } + updateMaxDocumentsPerPage(docsPerPage: number) { const previousDocsPerPage = this.state.docsPerPage; localStorage.setItem(MAX_DOCS_PER_PAGE_STORAGE_KEY, String(docsPerPage)); @@ -1005,7 +1017,7 @@ class CrudStoreImpl doc: hadronDoc, jsonDoc: jsonDoc, jsonView: true, - message: '', + error: undefined, csfleState, mode: MODIFYING, isOpen: true, @@ -1268,7 +1280,7 @@ class CrudStoreImpl doc: this.state.insert.doc, jsonView: true, jsonDoc: jsonDoc ?? null, - message: '', + error: undefined, csfleState: this.state.insert.csfleState, mode: MODIFYING, isOpen: true, @@ -1289,7 +1301,7 @@ class CrudStoreImpl doc: hadronDoc, jsonView: false, jsonDoc: this.state.insert.jsonDoc, - message: '', + error: undefined, csfleState: this.state.insert.csfleState, mode: MODIFYING, isOpen: true, @@ -1311,7 +1323,7 @@ class CrudStoreImpl doc: new Document({}), jsonDoc: this.state.insert.jsonDoc, jsonView: jsonView, - message: '', + error: undefined, csfleState: this.state.insert.csfleState, mode: MODIFYING, isOpen: true, @@ -1332,7 +1344,7 @@ class CrudStoreImpl doc: new Document({}), jsonDoc: value, jsonView: true, - message: '', + error: undefined, csfleState: this.state.insert.csfleState, mode: MODIFYING, isOpen: true, @@ -1381,7 +1393,7 @@ class CrudStoreImpl doc: new Document({}), jsonDoc: this.state.insert.jsonDoc, jsonView: true, - message: (error as Error).message, + error: this.getWriteError(error as Error), csfleState: this.state.insert.csfleState, mode: ERROR, isOpen: true, @@ -1438,12 +1450,13 @@ class CrudStoreImpl this.state.insert = this.getInitialInsertState(); } catch (error) { + console.log('Error', error); this.setState({ insert: { doc: this.state.insert.doc, jsonDoc: this.state.insert.jsonDoc, jsonView: this.state.insert.jsonView, - message: (error as Error).message, + error: this.getWriteError(error as Error), csfleState: this.state.insert.csfleState, mode: ERROR, isOpen: true, From 8175e10cfc136eb5b751952d8d377dfb79219959 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 26 Feb 2025 17:27:53 +0100 Subject: [PATCH 02/15] feat: show error details on insert --- .../src/components/document-list.tsx | 34 +++++++++++++++---- .../src/components/insert-document-dialog.tsx | 7 ++-- .../compass-crud/src/stores/crud-store.ts | 2 +- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/packages/compass-crud/src/components/document-list.tsx b/packages/compass-crud/src/components/document-list.tsx index c5311c5f2c5..fba5a8ee661 100644 --- a/packages/compass-crud/src/components/document-list.tsx +++ b/packages/compass-crud/src/components/document-list.tsx @@ -1,4 +1,10 @@ -import React, { useCallback, useLayoutEffect, useMemo, useRef } from 'react'; +import React, { + useCallback, + useLayoutEffect, + useMemo, + useRef, + useState, +} from 'react'; import { ObjectId } from 'bson'; import { Button, @@ -321,6 +327,14 @@ const DocumentList: React.FunctionComponent = (props) => { updateMaxDocumentsPerPage, } = props; + const [errorDetailsOpen, setErrorDetailsOpen] = useState< + | undefined + | { + details: Record; + closeAction?: 'back' | 'close'; + } + >(undefined); + const onOpenInsert = useCallback( (key: 'insert-document' | 'import-file') => { if (key === 'insert-document') { @@ -582,14 +596,22 @@ const DocumentList: React.FunctionComponent = (props) => { version={version} ns={ns} updateComment={updateComment} + showErrorDetails={() => + setErrorDetailsOpen({ + details: insert.error.info || {}, + closeAction: 'back', + }) + } {...insert} /> false} - details={insert.error?.info} - closeAction="back" - // TODO + open={!!errorDetailsOpen} + onClose={() => { + console.log('is closing'); + setErrorDetailsOpen(undefined); + }} + details={errorDetailsOpen?.details} + closeAction={errorDetailsOpen?.closeAction || 'close'} /> void; logger?: Logger; track?: TrackFunction; - showErrorDetails?: () => void; + showErrorDetails: () => void; }; const DocumentOrJsonView: React.FC<{ @@ -346,7 +345,7 @@ const InsertDocumentDialog: React.FC = ({ className={errorDetailsBtnStyles} onClick={showErrorDetails} > - ƒ VIEW ERROR DETAILS + VIEW ERROR DETAILS )} diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts index bda8a90ee98..a938432b4b5 100644 --- a/packages/compass-crud/src/stores/crud-store.ts +++ b/packages/compass-crud/src/stores/crud-store.ts @@ -1450,7 +1450,7 @@ class CrudStoreImpl this.state.insert = this.getInitialInsertState(); } catch (error) { - console.log('Error', error); + console.log('Error', error, '?', Object.keys(error as Error)); this.setState({ insert: { doc: this.state.insert.doc, From ec35c4435802d425278763c7acc779bf38a0ac8e Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 26 Feb 2025 20:51:01 +0100 Subject: [PATCH 03/15] update tests --- .../src/stores/crud-store.spec.ts | 60 ++++++++++++++++--- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/packages/compass-crud/src/stores/crud-store.spec.ts b/packages/compass-crud/src/stores/crud-store.spec.ts index 50eba4ff896..c89b4946427 100644 --- a/packages/compass-crud/src/stores/crud-store.spec.ts +++ b/packages/compass-crud/src/stores/crud-store.spec.ts @@ -296,7 +296,6 @@ describe('store', function () { isOpen: false, jsonDoc: null, jsonView: false, - message: '', csfleState: { state: 'none' }, mode: 'modifying', }, @@ -1125,7 +1124,7 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(null); expect(state.insert.isOpen).to.equal(false); expect(state.insert.jsonView).to.equal(false); - expect(state.insert.message).to.equal(''); + expect(state.insert.error).to.equal(undefined); }); store.state.insert.doc = doc; @@ -1153,7 +1152,7 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(null); expect(state.insert.isOpen).to.equal(false); expect(state.insert.jsonView).to.equal(false); - expect(state.insert.message).to.equal(''); + expect(state.insert.error).to.equal(undefined); }); void store.insertDocument(); @@ -1181,7 +1180,8 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(jsonDoc); expect(state.insert.isOpen).to.equal(true); expect(state.insert.jsonView).to.equal(true); - expect(state.insert.message).to.not.equal(''); + expect(state.insert.error).to.exist; + expect(state.insert.error.message).to.not.be.empty; expect(state.insert.mode).to.equal('error'); }); @@ -1216,7 +1216,8 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(jsonDoc); expect(state.insert.isOpen).to.equal(true); expect(state.insert.jsonView).to.equal(true); - expect(state.insert.message).to.not.equal(''); + expect(state.insert.error).to.exist; + expect(state.insert.error.message).to.not.be.empty; }); void store.insertDocument(); @@ -1246,7 +1247,8 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(jsonDoc); expect(state.insert.isOpen).to.equal(true); expect(state.insert.jsonView).to.equal(false); - expect(state.insert.message).to.not.equal(''); + expect(state.insert.error).to.exist; + expect(state.insert.error.message).to.not.be.empty; }); store.state.insert.doc = doc; @@ -1255,6 +1257,48 @@ describe('store', function () { await listener; }); }); + + context('when it is a validation error', function () { + const hadronDoc = new HadronDocument({}); + // this should be invalid according to the validation rules + const jsonDoc = '{ "status": "testing" }'; + + beforeEach(async function () { + store.state.insert.jsonView = true; + store.state.insert.doc = hadronDoc; + store.state.insert.jsonDoc = jsonDoc; + await dataService.updateCollection('compass-crud.test', { + validator: { $jsonSchema: { required: ['abc'] } }, + validationAction: 'error', + validationLevel: 'strict', + }); + }); + + afterEach(async function () { + await dataService.deleteMany('compass-crud.test', {}); + await dataService.updateCollection('compass-crud.test', { + validationLevel: 'off', + }); + }); + + it('does not insert the document', async function () { + const listener = waitForState(store, (state) => { + expect(state.docs.length).to.equal(0); + expect(state.count).to.equal(0); + expect(state.insert.doc).to.deep.equal(hadronDoc); + expect(state.insert.jsonDoc).to.equal(jsonDoc); + expect(state.insert.isOpen).to.equal(true); + expect(state.insert.jsonView).to.equal(true); + expect(state.insert.error).to.exist; + expect(state.insert.error.message).to.not.be.empty; + expect(state.insert.error.info).not.to.be.empty; + }); + + void store.insertDocument(); + + await listener; + }); + }); }); }); @@ -1287,7 +1331,7 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(null); expect(state.insert.isOpen).to.equal(false); expect(state.insert.jsonView).to.equal(false); - expect(state.insert.message).to.equal(''); + expect(state.insert.error).to.equal(undefined); expect(state.status).to.equal('fetching'); expect(state.abortController).to.not.be.null; @@ -1347,7 +1391,7 @@ describe('store', function () { expect(state.insert.jsonDoc).to.equal(null); expect(state.insert.isOpen).to.equal(false); expect(state.insert.jsonView).to.equal(false); - expect(state.insert.message).to.equal(''); + expect(state.insert.error).to.equal(undefined); }); store.state.insert.jsonDoc = docs; From 7e1fffbfad74b3115722e31c290b7fa0a96f460e Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 27 Feb 2025 11:15:37 +0100 Subject: [PATCH 04/15] include file --- .../components/modals/error-details-modal.tsx | 58 +++++++++++++++++++ .../compass-crud/src/stores/crud-store.ts | 1 - 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 packages/compass-components/src/components/modals/error-details-modal.tsx diff --git a/packages/compass-components/src/components/modals/error-details-modal.tsx b/packages/compass-components/src/components/modals/error-details-modal.tsx new file mode 100644 index 00000000000..6a12390ead0 --- /dev/null +++ b/packages/compass-components/src/components/modals/error-details-modal.tsx @@ -0,0 +1,58 @@ +import React, { useMemo } from 'react'; + +import { css, cx } from '@leafygreen-ui/emotion'; + +import { Modal } from './modal'; +import { Button, Code, ModalFooter } from '../leafygreen'; +import { ModalBody } from './modal-body'; +import { ModalHeader } from './modal-header'; +import { ModalFooterButton } from './modal-footer-button'; + +const backButtonStyles = css({ + float: 'left', +}); + +type ModalProps = React.ComponentProps; +type ErrorDetailsModalProps = Omit & { + title?: string; + subtitle?: string; + details?: Record; + closeAction: 'close' | 'back'; + onClose: () => void; +}; + +function ErrorDetailsModal({ + title = 'Error details', + subtitle, + details, + closeAction, + onClose, + ...modalProps +}: ErrorDetailsModalProps) { + const prettyDetails = useMemo(() => { + return JSON.stringify(details, undefined, 2); + }, [details]); + return ( + + + + {prettyDetails} + + + + + + ); +} + +export { ErrorDetailsModal }; diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts index a938432b4b5..4ec244ec0f1 100644 --- a/packages/compass-crud/src/stores/crud-store.ts +++ b/packages/compass-crud/src/stores/crud-store.ts @@ -1450,7 +1450,6 @@ class CrudStoreImpl this.state.insert = this.getInitialInsertState(); } catch (error) { - console.log('Error', error, '?', Object.keys(error as Error)); this.setState({ insert: { doc: this.state.insert.doc, From 5b75d6b31dc2bcf1d226f10753efc6a0a9461aae Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 27 Feb 2025 12:19:20 +0100 Subject: [PATCH 05/15] add e2e test --- .../components/modals/error-details-modal.tsx | 6 ++- .../src/components/insert-document-dialog.tsx | 1 + .../commands/try-to-insert-document.ts | 34 +++++++++++++ .../compass-e2e-tests/helpers/selectors.ts | 7 +++ .../tests/collection-documents-tab.test.ts | 49 ++++++++++++++++++- .../tests/connection.test.ts | 21 ++------ 6 files changed, 96 insertions(+), 22 deletions(-) create mode 100644 packages/compass-e2e-tests/helpers/commands/try-to-insert-document.ts diff --git a/packages/compass-components/src/components/modals/error-details-modal.tsx b/packages/compass-components/src/components/modals/error-details-modal.tsx index 6a12390ead0..45c3a288225 100644 --- a/packages/compass-components/src/components/modals/error-details-modal.tsx +++ b/packages/compass-components/src/components/modals/error-details-modal.tsx @@ -36,11 +36,13 @@ function ErrorDetailsModal({ - {prettyDetails} + + {prettyDetails} + diff --git a/packages/compass-e2e-tests/helpers/commands/try-to-insert-document.ts b/packages/compass-e2e-tests/helpers/commands/try-to-insert-document.ts new file mode 100644 index 00000000000..87334159004 --- /dev/null +++ b/packages/compass-e2e-tests/helpers/commands/try-to-insert-document.ts @@ -0,0 +1,34 @@ +import type { CompassBrowser } from '../compass-browser'; +import * as Selectors from '../selectors'; +import { expect } from 'chai'; + +export async function tryToInsertDocument( + browser: CompassBrowser, + document?: string +) { + // browse to the "Insert to Collection" modal + await browser.clickVisible(Selectors.AddDataButton); + const insertDocumentOption = browser.$(Selectors.InsertDocumentOption); + await insertDocumentOption.waitForDisplayed(); + await browser.clickVisible(Selectors.InsertDocumentOption); + + // wait for the modal to appear + const insertDialog = browser.$(Selectors.InsertDialog); + await insertDialog.waitForDisplayed(); + + if (document) { + // set the text in the editor + await browser.setCodemirrorEditorValue( + Selectors.InsertJSONEditor, + document + ); + } + + // confirm + const insertConfirm = browser.$(Selectors.InsertConfirm); + // this selector is very brittle, so just make sure it works + expect(await insertConfirm.isDisplayed()).to.be.true; + expect(await insertConfirm.getText()).to.equal('Insert'); + await insertConfirm.waitForEnabled(); + await browser.clickVisible(Selectors.InsertConfirm); +} diff --git a/packages/compass-e2e-tests/helpers/selectors.ts b/packages/compass-e2e-tests/helpers/selectors.ts index cba86e97ae9..6ac2a1cd740 100644 --- a/packages/compass-e2e-tests/helpers/selectors.ts +++ b/packages/compass-e2e-tests/helpers/selectors.ts @@ -615,12 +615,19 @@ export const JSONEditDocumentButton = `${JSONDocumentCard} [data-testid="editor- export const ShowMoreFieldsButton = '[data-testid="show-more-fields-button"]'; export const OpenBulkUpdateButton = '[data-testid="crud-update"]'; export const OpenBulkDeleteButton = '[data-testid="crud-bulk-delete"]'; +export const ErrorDetailsJson = '[data-testid="error-details-json"]'; +export const ErrorDetailsBackButton = + '[data-testid="error-details-back-button"]'; +export const ErrorDetailsCloseButton = + '[data-testid="error-details-close-button"]'; // Insert Document modal export const InsertDialog = '[data-testid="insert-document-modal"]'; export const InsertDialogErrorMessage = '[data-testid="insert-document-banner"][data-variant="danger"]'; +export const InsertDialogErrorDetailsBtn = + 'button[data-testid="insert-document-error-details-button"]'; export const InsertJSONEditor = '[data-testid="insert-document-json-editor"]'; export const InsertConfirm = '[data-testid="insert-document-modal"] [data-testid="submit-button"]'; diff --git a/packages/compass-e2e-tests/tests/collection-documents-tab.test.ts b/packages/compass-e2e-tests/tests/collection-documents-tab.test.ts index 0757e4f6633..adc926a1d51 100644 --- a/packages/compass-e2e-tests/tests/collection-documents-tab.test.ts +++ b/packages/compass-e2e-tests/tests/collection-documents-tab.test.ts @@ -17,8 +17,9 @@ import { createNestedDocumentsCollection, createNumbersCollection, } from '../helpers/insert-data'; -import { context } from '../helpers/test-runner-context'; +import { context as testRunnerContext } from '../helpers/test-runner-context'; import type { ChainablePromiseElement } from 'webdriverio'; +import { tryToInsertDocument } from '../helpers/commands/try-to-insert-document'; const { expect } = chai; @@ -553,7 +554,7 @@ FindIterable result = collection.find(filter);`); }); it('can copy a document from the contextual toolbar', async function () { - if (context.disableClipboardUsage) { + if (testRunnerContext.disableClipboardUsage) { this.skip(); } @@ -685,4 +686,48 @@ FindIterable result = collection.find(filter);`); expect(numExpandedHadronElementsPostSwitch).to.equal(14); }); }); + + context('with existing validation rule', function () { + const REQUIRE_PHONE_VALIDATOR = + '{ $jsonSchema: { bsonType: "object", required: [ "phone" ] } }'; + beforeEach(async function () { + await browser.navigateToCollectionTab( + DEFAULT_CONNECTION_NAME_1, + 'test', + 'numbers', + 'Validation' + ); + await browser.clickVisible(Selectors.AddRuleButton); + const element = browser.$(Selectors.ValidationEditor); + await element.waitForDisplayed(); + await browser.setValidation(REQUIRE_PHONE_VALIDATOR); + }); + + it('Shows error info when inserting', async function () { + await browser.navigateToCollectionTab( + DEFAULT_CONNECTION_NAME_1, + 'test', + 'numbers', + 'Documents' + ); + await tryToInsertDocument(browser, '{}'); + + const errorElement = browser.$(Selectors.InsertDialogErrorMessage); + await errorElement.waitForDisplayed(); + expect(await errorElement.getText()).to.include( + 'Document failed validation' + ); + // enter details + const errorDetailsBtn = browser.$(Selectors.InsertDialogErrorDetailsBtn); + await errorElement.waitForDisplayed(); + await errorDetailsBtn.click(); + + const errorDetailsJson = browser.$(Selectors.ErrorDetailsJson); + await errorDetailsJson.waitForDisplayed(); + + // exit details + await browser.clickVisible(Selectors.ErrorDetailsBackButton); + await errorElement.waitForDisplayed(); + }); + }); }); diff --git a/packages/compass-e2e-tests/tests/connection.test.ts b/packages/compass-e2e-tests/tests/connection.test.ts index bf8627129cb..542ab573466 100644 --- a/packages/compass-e2e-tests/tests/connection.test.ts +++ b/packages/compass-e2e-tests/tests/connection.test.ts @@ -24,6 +24,7 @@ import { DEFAULT_CONNECTION_NAMES, isTestingWeb, } from '../helpers/test-runner-context'; +import { tryToInsertDocument } from '../helpers/commands/try-to-insert-document'; async function disconnect(browser: CompassBrowser) { try { @@ -162,25 +163,8 @@ async function assertCannotInsertData( 'Documents' ); - // browse to the "Insert to Collection" modal - await browser.clickVisible(Selectors.AddDataButton); - const insertDocumentOption = browser.$(Selectors.InsertDocumentOption); - await insertDocumentOption.waitForDisplayed(); - await browser.clickVisible(Selectors.InsertDocumentOption); - - // wait for the modal to appear - const insertDialog = browser.$(Selectors.InsertDialog); - await insertDialog.waitForDisplayed(); - // go with the default text which should just be a random new id and therefore valid - - // confirm - const insertConfirm = browser.$(Selectors.InsertConfirm); - // this selector is very brittle, so just make sure it works - expect(await insertConfirm.isDisplayed()).to.be.true; - expect(await insertConfirm.getText()).to.equal('Insert'); - await insertConfirm.waitForEnabled(); - await browser.clickVisible(Selectors.InsertConfirm); + await tryToInsertDocument(browser); // make sure that there's an error and that the insert button is disabled const errorElement = browser.$(Selectors.InsertDialogErrorMessage); @@ -190,6 +174,7 @@ async function assertCannotInsertData( ); // cancel and wait for the modal to go away + const insertDialog = browser.$(Selectors.InsertDialog); await browser.clickVisible(Selectors.InsertCancel); await insertDialog.waitForDisplayed({ reverse: true }); } From 35d8b403871fdddb2af47263e9ad948ef561c373 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Fri, 28 Feb 2025 15:40:30 +0100 Subject: [PATCH 06/15] fix focus --- .../components/modals/error-details-modal.tsx | 23 ++++++++++--------- .../src/components/document-list.tsx | 5 +--- .../src/components/export-code-view.tsx | 1 + .../src/components/export-modal.tsx | 7 +++++- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/compass-components/src/components/modals/error-details-modal.tsx b/packages/compass-components/src/components/modals/error-details-modal.tsx index 45c3a288225..fd4c9968b56 100644 --- a/packages/compass-components/src/components/modals/error-details-modal.tsx +++ b/packages/compass-components/src/components/modals/error-details-modal.tsx @@ -6,10 +6,9 @@ import { Modal } from './modal'; import { Button, Code, ModalFooter } from '../leafygreen'; import { ModalBody } from './modal-body'; import { ModalHeader } from './modal-header'; -import { ModalFooterButton } from './modal-footer-button'; -const backButtonStyles = css({ - float: 'left', +const leftDirectionFooter = css({ + justifyContent: 'left', }); type ModalProps = React.ComponentProps; @@ -33,22 +32,24 @@ function ErrorDetailsModal({ return JSON.stringify(details, undefined, 2); }, [details]); return ( - + - + {prettyDetails} - + diff --git a/packages/compass-crud/src/components/document-list.tsx b/packages/compass-crud/src/components/document-list.tsx index fba5a8ee661..df5c60d4b6b 100644 --- a/packages/compass-crud/src/components/document-list.tsx +++ b/packages/compass-crud/src/components/document-list.tsx @@ -606,10 +606,7 @@ const DocumentList: React.FunctionComponent = (props) => { /> { - console.log('is closing'); - setErrorDetailsOpen(undefined); - }} + onClose={() => setErrorDetailsOpen(undefined)} details={errorDetailsOpen?.details} closeAction={errorDetailsOpen?.closeAction || 'close'} /> diff --git a/packages/compass-import-export/src/components/export-code-view.tsx b/packages/compass-import-export/src/components/export-code-view.tsx index 4260ed16c34..111f7f82306 100644 --- a/packages/compass-import-export/src/components/export-code-view.tsx +++ b/packages/compass-import-export/src/components/export-code-view.tsx @@ -74,6 +74,7 @@ function ExportCodeView({ diff --git a/packages/compass-import-export/src/components/export-modal.tsx b/packages/compass-import-export/src/components/export-modal.tsx index 82e101ed450..1278bc2593d 100644 --- a/packages/compass-import-export/src/components/export-modal.tsx +++ b/packages/compass-import-export/src/components/export-modal.tsx @@ -246,7 +246,12 @@ function ExportModal({ }, [isOpen, resetExportFormState]); return ( - + Date: Fri, 28 Feb 2025 17:17:00 +0100 Subject: [PATCH 07/15] move it to the store --- packages/compass-crud/src/actions/index.ts | 2 ++ .../src/components/document-list.tsx | 35 ++++++++----------- .../compass-crud/src/stores/crud-store.ts | 24 +++++++++++++ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/packages/compass-crud/src/actions/index.ts b/packages/compass-crud/src/actions/index.ts index 5600c4c103c..4eb718b3053 100644 --- a/packages/compass-crud/src/actions/index.ts +++ b/packages/compass-crud/src/actions/index.ts @@ -19,6 +19,8 @@ const configureActions = () => { 'toggleInsertDocumentView', 'toggleInsertDocument', 'openInsertDocumentDialog', + 'openErrorDetailsDialog', + 'closeErrorDetailsDialog', 'openBulkUpdateModal', 'updateBulkUpdatePreview', 'runBulkUpdate', diff --git a/packages/compass-crud/src/components/document-list.tsx b/packages/compass-crud/src/components/document-list.tsx index df5c60d4b6b..387ba136ebc 100644 --- a/packages/compass-crud/src/components/document-list.tsx +++ b/packages/compass-crud/src/components/document-list.tsx @@ -1,10 +1,4 @@ -import React, { - useCallback, - useLayoutEffect, - useMemo, - useRef, - useState, -} from 'react'; +import React, { useCallback, useLayoutEffect, useMemo, useRef } from 'react'; import { ObjectId } from 'bson'; import { Button, @@ -36,10 +30,11 @@ import { DOCUMENTS_STATUS_FETCHING, DOCUMENTS_STATUS_FETCHED_INITIAL, } from '../constants/documents-statuses'; -import { - type CrudStore, - type BSONObject, - type DocumentView, +import type { + CrudStore, + BSONObject, + DocumentView, + ErrorDetailsDialogOptions, } from '../stores/crud-store'; import { getToolbarSignal } from '../utils/toolbar-signal'; import BulkDeleteModal from './bulk-delete-modal'; @@ -77,6 +72,8 @@ const loaderContainerStyles = css({ export type DocumentListProps = { store: CrudStore; openInsertDocumentDialog?: (doc: BSONObject, cloned: boolean) => void; + openErrorDetailsDialog: (options: ErrorDetailsDialogOptions) => void; + closeErrorDetailsDialog: () => void; openBulkUpdateModal: () => void; updateBulkUpdatePreview: (updateText: string) => void; runBulkUpdate: () => void; @@ -84,6 +81,7 @@ export type DocumentListProps = { openImportFileDialog?: (origin: 'empty-state' | 'crud-toolbar') => void; docs: Document[]; view: DocumentView; + errorDetailsOpen: ErrorDetailsDialogOptions | null; insert: Partial & Required< Pick< @@ -302,7 +300,10 @@ const DocumentList: React.FunctionComponent = (props) => { resultId, isCollectionScan, isSearchIndexesSupported, + errorDetailsOpen, openInsertDocumentDialog, + openErrorDetailsDialog, + closeErrorDetailsDialog, openImportFileDialog, openBulkUpdateModal, docs, @@ -327,14 +328,6 @@ const DocumentList: React.FunctionComponent = (props) => { updateMaxDocumentsPerPage, } = props; - const [errorDetailsOpen, setErrorDetailsOpen] = useState< - | undefined - | { - details: Record; - closeAction?: 'back' | 'close'; - } - >(undefined); - const onOpenInsert = useCallback( (key: 'insert-document' | 'import-file') => { if (key === 'insert-document') { @@ -597,7 +590,7 @@ const DocumentList: React.FunctionComponent = (props) => { ns={ns} updateComment={updateComment} showErrorDetails={() => - setErrorDetailsOpen({ + openErrorDetailsDialog({ details: insert.error.info || {}, closeAction: 'back', }) @@ -606,7 +599,7 @@ const DocumentList: React.FunctionComponent = (props) => { /> setErrorDetailsOpen(undefined)} + onClose={closeErrorDetailsDialog} details={errorDetailsOpen?.details} closeAction={errorDetailsOpen?.closeAction || 'close'} /> diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts index 4ec244ec0f1..01c964f032e 100644 --- a/packages/compass-crud/src/stores/crud-store.ts +++ b/packages/compass-crud/src/stores/crud-store.ts @@ -81,6 +81,11 @@ export type EmittedAppRegistryEvents = | 'document-deleted' | 'document-inserted'; +export type ErrorDetailsDialogOptions = { + details: Record; + closeAction?: 'back' | 'close'; +}; + export type CrudActions = { drillDown( doc: Document, @@ -94,6 +99,8 @@ export type CrudActions = { removeDocument(doc: Document): void; replaceDocument(doc: Document): void; openInsertDocumentDialog(doc: BSONObject, cloned: boolean): void; + openErrorDetailsDialog(options: ErrorDetailsDialogOptions): void; + closeErrorDetailsDialog(): void; copyToClipboard(doc: Document): void; //XXX openBulkDeleteDialog(): void; runBulkUpdate(): void; @@ -341,6 +348,10 @@ type CrudState = { bulkDelete: BulkDeleteState; docsPerPage: number; collectionStats: CollectionStats | null; + errorDetailsOpen: { + details: Record; + closeAction?: 'back' | 'close'; + } | null; }; type CrudStoreActionsOptions = { @@ -447,6 +458,7 @@ class CrudStoreImpl this.instance.topologyDescription.type !== 'Single', docsPerPage: this.getInitialDocsPerPage(), collectionStats: extractCollectionStats(this.collection), + errorDetailsOpen: null, }; } @@ -954,6 +966,18 @@ class CrudStoreImpl }); } + openErrorDetailsDialog(errorDetailsOpen: ErrorDetailsDialogOptions) { + this.setState({ + errorDetailsOpen, + }); + } + + closeErrorDetailsDialog() { + this.setState({ + errorDetailsOpen: null, + }); + } + /** * Open the insert document dialog. * From fb9f42ad9d50352d9e08105f0714e3f1d741d56f Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 3 Mar 2025 15:58:58 +0100 Subject: [PATCH 08/15] update tests and props --- .../compass-crud/src/components/document-json-view.tsx | 1 + .../src/components/document-list-view.spec.tsx | 1 + .../compass-crud/src/components/document-list-view.tsx | 3 +++ packages/compass-crud/src/components/document.tsx | 1 + .../compass-crud/src/components/editable-document.spec.tsx | 1 + packages/compass-crud/src/components/editable-document.tsx | 7 +++++++ packages/compass-crud/src/components/json-editor.tsx | 1 + packages/compass-crud/src/stores/crud-store.spec.ts | 6 +++++- 8 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/compass-crud/src/components/document-json-view.tsx b/packages/compass-crud/src/components/document-json-view.tsx index 700857879de..0cabaaf236d 100644 --- a/packages/compass-crud/src/components/document-json-view.tsx +++ b/packages/compass-crud/src/components/document-json-view.tsx @@ -33,6 +33,7 @@ export type DocumentJsonViewProps = { | 'removeDocument' | 'replaceDocument' | 'updateDocument' + | 'openErrorDetailsDialog' | 'openInsertDocumentDialog' >; diff --git a/packages/compass-crud/src/components/document-list-view.spec.tsx b/packages/compass-crud/src/components/document-list-view.spec.tsx index 536f48e66c2..bfb28904fb0 100644 --- a/packages/compass-crud/src/components/document-list-view.spec.tsx +++ b/packages/compass-crud/src/components/document-list-view.spec.tsx @@ -21,6 +21,7 @@ describe('', function () { replaceDocument={sinon.spy()} updateDocument={sinon.spy()} openInsertDocumentDialog={sinon.spy()} + openErrorDetailsDialog={sinon.spy()} /> ); diff --git a/packages/compass-crud/src/components/document-list-view.tsx b/packages/compass-crud/src/components/document-list-view.tsx index 938ecc6fd1b..5dd0e009a5f 100644 --- a/packages/compass-crud/src/components/document-list-view.tsx +++ b/packages/compass-crud/src/components/document-list-view.tsx @@ -35,6 +35,7 @@ export type DocumentListViewProps = { | 'replaceDocument' | 'updateDocument' | 'openInsertDocumentDialog' + | 'openErrorDetailsDialog' >; /** @@ -66,6 +67,7 @@ class DocumentListView extends React.Component { replaceDocument={this.props.replaceDocument} updateDocument={this.props.updateDocument} openInsertDocumentDialog={this.props.openInsertDocumentDialog} + openErrorDetailsDialog={this.props.openErrorDetailsDialog} /> @@ -97,6 +99,7 @@ class DocumentListView extends React.Component { replaceDocument: PropTypes.func, updateDocument: PropTypes.func, openInsertDocumentDialog: PropTypes.func, + openErrorDetailsDialog: PropTypes.func, copyToClipboard: PropTypes.func, className: PropTypes.string, }; diff --git a/packages/compass-crud/src/components/document.tsx b/packages/compass-crud/src/components/document.tsx index 675367a5f6f..c63d8093122 100644 --- a/packages/compass-crud/src/components/document.tsx +++ b/packages/compass-crud/src/components/document.tsx @@ -57,6 +57,7 @@ Document.propTypes = { replaceDocument: PropTypes.func, updateDocument: PropTypes.func, openInsertDocumentDialog: PropTypes.func, + openErrorDetailsDialog: PropTypes.func, copyToClipboard: PropTypes.func, isExpanded: PropTypes.bool, }; diff --git a/packages/compass-crud/src/components/editable-document.spec.tsx b/packages/compass-crud/src/components/editable-document.spec.tsx index 19667f66d2e..061fe523880 100644 --- a/packages/compass-crud/src/components/editable-document.spec.tsx +++ b/packages/compass-crud/src/components/editable-document.spec.tsx @@ -22,6 +22,7 @@ describe('', function () { updateDocument={sinon.spy(action)} copyToClipboard={sinon.spy(action)} openInsertDocumentDialog={sinon.spy(action)} + openErrorDetailsDialog={sinon.spy(action)} /> ); }); diff --git a/packages/compass-crud/src/components/editable-document.tsx b/packages/compass-crud/src/components/editable-document.tsx index 9040e2378ff..141f45f1991 100644 --- a/packages/compass-crud/src/components/editable-document.tsx +++ b/packages/compass-crud/src/components/editable-document.tsx @@ -19,6 +19,7 @@ export type EditableDocumentProps = { replaceDocument?: CrudActions['replaceDocument']; updateDocument?: CrudActions['updateDocument']; openInsertDocumentDialog?: CrudActions['openInsertDocumentDialog']; + openErrorDetailsDialog?: CrudActions['openErrorDetailsDialog']; copyToClipboard?: CrudActions['copyToClipboard']; showInsights?: boolean; }; @@ -278,6 +279,12 @@ class EditableDocument extends React.Component< onCancel={() => { this.handleCancel(); }} + // onOpenErrorDetails{() => { + // this.props.openErrorDetailsDialog?.({ + // details: '// TODO erro details', + // closeAction: 'close', + // }) + // }} /> ); } diff --git a/packages/compass-crud/src/components/json-editor.tsx b/packages/compass-crud/src/components/json-editor.tsx index 3c86cff5c68..b47c4fd6863 100644 --- a/packages/compass-crud/src/components/json-editor.tsx +++ b/packages/compass-crud/src/components/json-editor.tsx @@ -58,6 +58,7 @@ export type JSONEditorProps = { updateDocument?: CrudActions['updateDocument']; copyToClipboard?: CrudActions['copyToClipboard']; openInsertDocumentDialog?: CrudActions['openInsertDocumentDialog']; + openErrorDetailsDialog?: CrudActions['openErrorDetailsDialog']; }; const JSONEditor: React.FunctionComponent = ({ diff --git a/packages/compass-crud/src/stores/crud-store.spec.ts b/packages/compass-crud/src/stores/crud-store.spec.ts index c89b4946427..1c39ab6feed 100644 --- a/packages/compass-crud/src/stores/crud-store.spec.ts +++ b/packages/compass-crud/src/stores/crud-store.spec.ts @@ -290,6 +290,7 @@ describe('store', function () { docsPerPage: 25, end: 0, error: null, + errorDetailsOpen: null, insert: { doc: null, isCommentNeeded: true, @@ -1447,7 +1448,10 @@ describe('store', function () { expect(state.insert.jsonDoc).to.deep.equal(docs); expect(state.insert.isOpen).to.equal(true); expect(state.insert.jsonView).to.equal(true); - expect(state.insert.message).to.equal('Document failed validation'); + expect(state.insert.error).to.not.be.null; + expect(state.insert.error?.message).to.equal( + 'Document failed validation' + ); }); store.state.insert.jsonDoc = docs; From c2d5a5993f73d28bcf893e22040b2ff919d8cb93 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 3 Mar 2025 16:21:55 +0100 Subject: [PATCH 09/15] add dependency --- package-lock.json | 2 ++ packages/compass-crud/package.json | 1 + 2 files changed, 3 insertions(+) diff --git a/package-lock.json b/package-lock.json index 96c1473540d..1b645ec90f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44225,6 +44225,7 @@ "hadron-type-checker": "^7.4.2", "jsondiffpatch": "^0.5.0", "lodash": "^4.17.21", + "mongodb": "^6.12.0", "mongodb-data-service": "^22.25.2", "mongodb-ns": "^2.4.2", "mongodb-query-parser": "^4.3.0", @@ -56771,6 +56772,7 @@ "jsondiffpatch": "^0.5.0", "lodash": "^4.17.21", "mocha": "^10.2.0", + "mongodb": "^6.12.0", "mongodb-data-service": "^22.25.2", "mongodb-instance-model": "^12.26.2", "mongodb-ns": "^2.4.2", diff --git a/packages/compass-crud/package.json b/packages/compass-crud/package.json index 4dfdd6ab15d..aa40cfeb4cf 100644 --- a/packages/compass-crud/package.json +++ b/packages/compass-crud/package.json @@ -94,6 +94,7 @@ "hadron-type-checker": "^7.4.2", "jsondiffpatch": "^0.5.0", "lodash": "^4.17.21", + "mongodb": "^6.12.0", "mongodb-data-service": "^22.25.2", "mongodb-ns": "^2.4.2", "mongodb-query-parser": "^4.3.0", From 9ee97032aab605d0a3d7970d4b8510565896fcd0 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 3 Mar 2025 19:40:17 +0100 Subject: [PATCH 10/15] cleanup --- .../compass-crud/src/components/document-list-view.tsx | 2 -- packages/compass-crud/src/components/document.tsx | 1 - .../compass-crud/src/components/editable-document.spec.tsx | 1 - packages/compass-crud/src/components/editable-document.tsx | 7 ------- 4 files changed, 11 deletions(-) diff --git a/packages/compass-crud/src/components/document-list-view.tsx b/packages/compass-crud/src/components/document-list-view.tsx index 5dd0e009a5f..825fe7538aa 100644 --- a/packages/compass-crud/src/components/document-list-view.tsx +++ b/packages/compass-crud/src/components/document-list-view.tsx @@ -35,7 +35,6 @@ export type DocumentListViewProps = { | 'replaceDocument' | 'updateDocument' | 'openInsertDocumentDialog' - | 'openErrorDetailsDialog' >; /** @@ -67,7 +66,6 @@ class DocumentListView extends React.Component { replaceDocument={this.props.replaceDocument} updateDocument={this.props.updateDocument} openInsertDocumentDialog={this.props.openInsertDocumentDialog} - openErrorDetailsDialog={this.props.openErrorDetailsDialog} /> diff --git a/packages/compass-crud/src/components/document.tsx b/packages/compass-crud/src/components/document.tsx index c63d8093122..675367a5f6f 100644 --- a/packages/compass-crud/src/components/document.tsx +++ b/packages/compass-crud/src/components/document.tsx @@ -57,7 +57,6 @@ Document.propTypes = { replaceDocument: PropTypes.func, updateDocument: PropTypes.func, openInsertDocumentDialog: PropTypes.func, - openErrorDetailsDialog: PropTypes.func, copyToClipboard: PropTypes.func, isExpanded: PropTypes.bool, }; diff --git a/packages/compass-crud/src/components/editable-document.spec.tsx b/packages/compass-crud/src/components/editable-document.spec.tsx index 061fe523880..19667f66d2e 100644 --- a/packages/compass-crud/src/components/editable-document.spec.tsx +++ b/packages/compass-crud/src/components/editable-document.spec.tsx @@ -22,7 +22,6 @@ describe('', function () { updateDocument={sinon.spy(action)} copyToClipboard={sinon.spy(action)} openInsertDocumentDialog={sinon.spy(action)} - openErrorDetailsDialog={sinon.spy(action)} /> ); }); diff --git a/packages/compass-crud/src/components/editable-document.tsx b/packages/compass-crud/src/components/editable-document.tsx index 141f45f1991..9040e2378ff 100644 --- a/packages/compass-crud/src/components/editable-document.tsx +++ b/packages/compass-crud/src/components/editable-document.tsx @@ -19,7 +19,6 @@ export type EditableDocumentProps = { replaceDocument?: CrudActions['replaceDocument']; updateDocument?: CrudActions['updateDocument']; openInsertDocumentDialog?: CrudActions['openInsertDocumentDialog']; - openErrorDetailsDialog?: CrudActions['openErrorDetailsDialog']; copyToClipboard?: CrudActions['copyToClipboard']; showInsights?: boolean; }; @@ -279,12 +278,6 @@ class EditableDocument extends React.Component< onCancel={() => { this.handleCancel(); }} - // onOpenErrorDetails{() => { - // this.props.openErrorDetailsDialog?.({ - // details: '// TODO erro details', - // closeAction: 'close', - // }) - // }} /> ); } From 8bd3af39bb201dc7d90e7d2db3bcdff1f0320304 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 5 Mar 2025 09:50:55 +0100 Subject: [PATCH 11/15] id as const --- .../compass-import-export/src/components/export-code-view.tsx | 4 +++- .../compass-import-export/src/components/export-modal.tsx | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/compass-import-export/src/components/export-code-view.tsx b/packages/compass-import-export/src/components/export-code-view.tsx index 111f7f82306..0d91d5d9ea2 100644 --- a/packages/compass-import-export/src/components/export-code-view.tsx +++ b/packages/compass-import-export/src/components/export-code-view.tsx @@ -11,6 +11,8 @@ import { queryAsShellJSString, } from '../utils/get-shell-js-string'; +export const codeElementId = 'export-collection-code-preview-wrapper'; + const containerStyles = css({ marginBottom: spacing[3], }); @@ -74,7 +76,7 @@ function ExportCodeView({ diff --git a/packages/compass-import-export/src/components/export-modal.tsx b/packages/compass-import-export/src/components/export-modal.tsx index 1278bc2593d..ca45128eed5 100644 --- a/packages/compass-import-export/src/components/export-modal.tsx +++ b/packages/compass-import-export/src/components/export-modal.tsx @@ -30,7 +30,7 @@ import type { ExportStatus, FieldsToExportOption } from '../modules/export'; import type { RootExportState } from '../stores/export-store'; import { SelectFileType } from './select-file-type'; import { ExportSelectFields } from './export-select-fields'; -import { ExportCodeView } from './export-code-view'; +import { codeElementId, ExportCodeView } from './export-code-view'; import type { ExportAggregation, ExportQuery } from '../export/export-types'; import { queryHasProjection } from '../utils/query-has-projection'; import { FieldsToExportOptions } from './export-field-options'; @@ -250,7 +250,7 @@ function ExportModal({ open={isOpen} setOpen={closeExport} data-testid="export-modal" - initialFocus="#export-collection-code-preview-wrapper" + initialFocus={`#${codeElementId}`} > Date: Wed, 5 Mar 2025 10:05:10 +0100 Subject: [PATCH 12/15] fix view has changed hook --- .../compass-crud/src/components/insert-document-dialog.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/compass-crud/src/components/insert-document-dialog.tsx b/packages/compass-crud/src/components/insert-document-dialog.tsx index 30d55ba0493..bfdbf839cf9 100644 --- a/packages/compass-crud/src/components/insert-document-dialog.tsx +++ b/packages/compass-crud/src/components/insert-document-dialog.tsx @@ -1,5 +1,5 @@ import { without } from 'lodash'; -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import type Document from 'hadron-document'; import { Element } from 'hadron-document'; import { @@ -200,8 +200,11 @@ const InsertDocumentDialog: React.FC = ({ } }, [isOpen, track]); + const prevJsonView = useRef(jsonView); useEffect(() => { - if (isOpen && !hasManyDocuments()) { + const viewHasChanged = prevJsonView.current !== jsonView; + prevJsonView.current = jsonView; + if (isOpen && !hasManyDocuments() && viewHasChanged) { if (!jsonView) { // When switching to Hadron Document View. // Reset the invalid elements list, which contains the From 4df230b7a3930a3708067a952a9dc8bdf60b44d6 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 5 Mar 2025 11:15:47 +0100 Subject: [PATCH 13/15] prevent flickering --- .../components/modals/error-details-modal.tsx | 23 ++++++++--- .../src/components/document-list.tsx | 11 ++--- .../src/stores/crud-store.spec.ts | 40 ++++++++++++++++++- .../compass-crud/src/stores/crud-store.ts | 38 ++++++++++++------ 4 files changed, 88 insertions(+), 24 deletions(-) diff --git a/packages/compass-components/src/components/modals/error-details-modal.tsx b/packages/compass-components/src/components/modals/error-details-modal.tsx index fd4c9968b56..390e836bc12 100644 --- a/packages/compass-components/src/components/modals/error-details-modal.tsx +++ b/packages/compass-components/src/components/modals/error-details-modal.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react'; +import React, { useEffect, useMemo, useRef, useState } from 'react'; import { css, cx } from '@leafygreen-ui/emotion'; @@ -26,13 +26,24 @@ function ErrorDetailsModal({ details, closeAction, onClose, + open, ...modalProps }: ErrorDetailsModalProps) { - const prettyDetails = useMemo(() => { - return JSON.stringify(details, undefined, 2); - }, [details]); + const [stringDetails, setStringDetails] = useState(''); + + useEffect(() => { + if (open) { + setStringDetails(JSON.stringify(details, undefined, 2)); + } + }, [details, open]); + return ( - + - {prettyDetails} + {stringDetails} void; docs: Document[]; view: DocumentView; - errorDetailsOpen: ErrorDetailsDialogOptions | null; + errorDetails: ErrorDetailsDialogState; insert: Partial & Required< Pick< @@ -300,7 +301,7 @@ const DocumentList: React.FunctionComponent = (props) => { resultId, isCollectionScan, isSearchIndexesSupported, - errorDetailsOpen, + errorDetails, openInsertDocumentDialog, openErrorDetailsDialog, closeErrorDetailsDialog, @@ -598,10 +599,10 @@ const DocumentList: React.FunctionComponent = (props) => { {...insert} /> plugin.deactivate(); + }); + + it('manages the errorDetails state', async function () { + const openListener = waitForState(store, (state) => { + expect(state.errorDetails).to.deep.equal({ + isOpen: true, + ...options, + }); + }); + + void store.openErrorDetailsDialog(options); + + await openListener; + + const closeListener = waitForState(store, (state) => { + expect(state.errorDetails).to.deep.equal({ + isOpen: false, + }); + }); + + void store.closeErrorDetailsDialog(); + + await closeListener; + }); + }); + describe('#openInsertDocumentDialog', function () { const doc = { _id: 1, name: 'test' }; let store: CrudStore; diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts index 01c964f032e..35b96d0f421 100644 --- a/packages/compass-crud/src/stores/crud-store.ts +++ b/packages/compass-crud/src/stores/crud-store.ts @@ -81,10 +81,22 @@ export type EmittedAppRegistryEvents = | 'document-deleted' | 'document-inserted'; -export type ErrorDetailsDialogOptions = { - details: Record; - closeAction?: 'back' | 'close'; -}; +export type ErrorDetailsDialogState = + | { + isOpen: false; + details?: never; + closeAction?: never; + } + | { + isOpen: true; + details: Record; + closeAction?: 'back' | 'close'; + }; + +export type ErrorDetailsDialogOptions = Omit< + Extract, + 'isOpen' +>; export type CrudActions = { drillDown( @@ -348,10 +360,7 @@ type CrudState = { bulkDelete: BulkDeleteState; docsPerPage: number; collectionStats: CollectionStats | null; - errorDetailsOpen: { - details: Record; - closeAction?: 'back' | 'close'; - } | null; + errorDetails: ErrorDetailsDialogState; }; type CrudStoreActionsOptions = { @@ -458,7 +467,7 @@ class CrudStoreImpl this.instance.topologyDescription.type !== 'Single', docsPerPage: this.getInitialDocsPerPage(), collectionStats: extractCollectionStats(this.collection), - errorDetailsOpen: null, + errorDetails: { isOpen: false }, }; } @@ -966,15 +975,20 @@ class CrudStoreImpl }); } - openErrorDetailsDialog(errorDetailsOpen: ErrorDetailsDialogOptions) { + openErrorDetailsDialog(options: ErrorDetailsDialogOptions) { this.setState({ - errorDetailsOpen, + errorDetails: { + isOpen: true, + ...options, + }, }); } closeErrorDetailsDialog() { this.setState({ - errorDetailsOpen: null, + errorDetails: { + isOpen: false, + }, }); } From 30eaf1254074597c8f2e8e4973ab7b11d7efed62 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 6 Mar 2025 12:58:06 +0100 Subject: [PATCH 14/15] move details persistence to store --- .../src/components/modals/error-details-modal.tsx | 15 ++++++--------- .../compass-crud/src/stores/crud-store.spec.ts | 4 +--- packages/compass-crud/src/stores/crud-store.ts | 5 +++-- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/compass-components/src/components/modals/error-details-modal.tsx b/packages/compass-components/src/components/modals/error-details-modal.tsx index 390e836bc12..c21e5fe05e5 100644 --- a/packages/compass-components/src/components/modals/error-details-modal.tsx +++ b/packages/compass-components/src/components/modals/error-details-modal.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo, useRef, useState } from 'react'; +import React, { useMemo } from 'react'; import { css, cx } from '@leafygreen-ui/emotion'; @@ -29,13 +29,10 @@ function ErrorDetailsModal({ open, ...modalProps }: ErrorDetailsModalProps) { - const [stringDetails, setStringDetails] = useState(''); - - useEffect(() => { - if (open) { - setStringDetails(JSON.stringify(details, undefined, 2)); - } - }, [details, open]); + const prettyDetails = useMemo( + () => JSON.stringify(details, undefined, 2), + [details] + ); return ( - {stringDetails} + {prettyDetails} { - expect(state.errorDetails).to.deep.equal({ - isOpen: false, - }); + expect(state.errorDetails.isOpen).to.be.false; }); void store.closeErrorDetailsDialog(); diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts index 35b96d0f421..56db6a43287 100644 --- a/packages/compass-crud/src/stores/crud-store.ts +++ b/packages/compass-crud/src/stores/crud-store.ts @@ -84,8 +84,8 @@ export type EmittedAppRegistryEvents = export type ErrorDetailsDialogState = | { isOpen: false; - details?: never; - closeAction?: never; + details?: Record; + closeAction?: 'back' | 'close'; } | { isOpen: true; @@ -987,6 +987,7 @@ class CrudStoreImpl closeErrorDetailsDialog() { this.setState({ errorDetails: { + ...this.state.errorDetails, isOpen: false, }, }); From 492f9ae0e92747b319825056f5788a341ad9454a Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Thu, 6 Mar 2025 13:19:11 +0100 Subject: [PATCH 15/15] cleanup test --- packages/compass-crud/src/stores/crud-store.spec.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/compass-crud/src/stores/crud-store.spec.ts b/packages/compass-crud/src/stores/crud-store.spec.ts index 4ae04f8ea40..045b3cf9141 100644 --- a/packages/compass-crud/src/stores/crud-store.spec.ts +++ b/packages/compass-crud/src/stores/crud-store.spec.ts @@ -1265,22 +1265,14 @@ describe('store', function () { // this should be invalid according to the validation rules const jsonDoc = '{ "status": "testing" }'; - beforeEach(async function () { + beforeEach(function () { store.state.insert.jsonView = true; store.state.insert.doc = hadronDoc; store.state.insert.jsonDoc = jsonDoc; - await dataService.updateCollection('compass-crud.test', { - validator: { $jsonSchema: { required: ['abc'] } }, - validationAction: 'error', - validationLevel: 'strict', - }); }); afterEach(async function () { await dataService.deleteMany('compass-crud.test', {}); - await dataService.updateCollection('compass-crud.test', { - validationLevel: 'off', - }); }); it('does not insert the document', async function () {