diff --git a/packages/compass-e2e-tests/helpers/commands/set-validation.ts b/packages/compass-e2e-tests/helpers/commands/set-validation.ts index 8483d48f3a6..ef9e22744ee 100644 --- a/packages/compass-e2e-tests/helpers/commands/set-validation.ts +++ b/packages/compass-e2e-tests/helpers/commands/set-validation.ts @@ -15,6 +15,9 @@ export async function setValidation( await browser.clickVisible(Selectors.UpdateValidationButton); + // Confirm in the confirmation modal. + await browser.clickVisible(Selectors.confirmationModalConfirmButton()); + // both buttons should become hidden if it succeeds await validationActionMessageElement.waitForDisplayed({ reverse: true, diff --git a/packages/compass-e2e-tests/helpers/selectors.ts b/packages/compass-e2e-tests/helpers/selectors.ts index 4b3d5d4431b..a12bf9fd959 100644 --- a/packages/compass-e2e-tests/helpers/selectors.ts +++ b/packages/compass-e2e-tests/helpers/selectors.ts @@ -1144,6 +1144,8 @@ export const UnhideIndexButton = '[data-testid="index-actions-unhide-action"]'; // Validation tab export const AddRuleButton = '[data-testid="add-rule-button"]'; +export const EnableEditValidationButton = + '[data-testid="enable-edit-validation-button"]'; export const ValidationEditor = '[data-testid="validation-editor"]'; export const ValidationActionMessage = '[data-testid="validation-action-message"]'; diff --git a/packages/compass-e2e-tests/tests/collection-validation-tab.test.ts b/packages/compass-e2e-tests/tests/collection-validation-tab.test.ts index b5e35f8d67e..46bdf51550f 100644 --- a/packages/compass-e2e-tests/tests/collection-validation-tab.test.ts +++ b/packages/compass-e2e-tests/tests/collection-validation-tab.test.ts @@ -96,7 +96,14 @@ describe('Collection validation tab', function () { ); }); - // Reset the validation again to make everything valid for future tests + const enableUpdateValidationButtonElement = browser.$( + Selectors.EnableEditValidationButton + ); + // Enable the editing mode and wait for it to be enabled. + await browser.clickVisible(enableUpdateValidationButtonElement); + await enableUpdateValidationButtonElement.waitForDisplayed({ + reverse: true, + }); // the automatic indentation and brackets makes multi-line values very fiddly here await browser.setValidation(PASSING_VALIDATOR); diff --git a/packages/compass-editor/src/editor.tsx b/packages/compass-editor/src/editor.tsx index 9f893996f1d..a69aa2070a2 100644 --- a/packages/compass-editor/src/editor.tsx +++ b/packages/compass-editor/src/editor.tsx @@ -95,6 +95,14 @@ const disabledContainerDarkModeStyles = css({ boxShadow: `0 0 0 1px ${palette.gray.dark2}`, }); +const readOnlyStyle = css({ + // We hide the blinking cursor in read only mode + // as it can appear to users like the editor is editable. + '& .cm-cursor': { + display: 'none !important', + }, +}); + const hiddenScrollStyle = css({ '& .cm-scroller': { overflow: '-moz-scrollbars-none', @@ -1204,6 +1212,7 @@ const BaseEditor = React.forwardRef(function BaseEditor( className={cx( disabled && disabledContainerStyles, disabled && darkMode && disabledContainerDarkModeStyles, + readOnly && readOnlyStyle, className )} style={{ diff --git a/packages/compass-schema-validation/src/components/validation-editor.spec.tsx b/packages/compass-schema-validation/src/components/validation-editor.spec.tsx index 4f921df08bb..5145e12ebbc 100644 --- a/packages/compass-schema-validation/src/components/validation-editor.spec.tsx +++ b/packages/compass-schema-validation/src/components/validation-editor.spec.tsx @@ -1,88 +1,94 @@ import React from 'react'; -import { render, screen } from '@mongodb-js/testing-library-compass'; +import { render, screen, userEvent } from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import sinon from 'sinon'; import { ValidationEditor } from './validation-editor'; +function renderValidationEditor( + props: Partial> +) { + const validation = { + validator: '', + validationAction: 'warn', + validationLevel: 'moderate', + isChanged: false, + syntaxError: null, + error: null, + } as const; + + return render( + {}} + validationActionChanged={() => {}} + validationLevelChanged={() => {}} + cancelValidation={() => {}} + saveValidation={() => {}} + clearSampleDocuments={() => {}} + serverVersion="8.0.5" + onClickEnableEditRules={() => {}} + validation={validation} + isEditable + isEditingEnabled + {...props} + /> + ); +} + describe('ValidationEditor [Component]', function () { - context('when it is an editable mode', function () { - const setValidatorChangedSpy = sinon.spy(); - const setValidationActionChangedSpy = sinon.spy(); - const setValidationLevelChangedSpy = sinon.spy(); - const setCancelValidationSpy = sinon.spy(); - const saveValidationSpy = sinon.spy(); - const clearSampleDocumentsSpy = sinon.spy(); - const serverVersion = '3.6.0'; - const validation = { - validator: '', - validationAction: 'warn', - validationLevel: 'moderate', - isChanged: false, - syntaxError: null, - error: null, - } as const; - const isEditable = true; + context( + 'when it is an editable mode but editing is not yet enabled', + function () { + let onClickEnableEditRulesSpy: sinon.SinonSpy; + beforeEach(function () { + onClickEnableEditRulesSpy = sinon.spy(); + renderValidationEditor({ + onClickEnableEditRules: onClickEnableEditRulesSpy, + isEditingEnabled: false, + isEditable: true, + }); + }); + it('does not allow to edit the editor', function () { + expect(screen.getByTestId('validation-editor')).to.exist; + expect(screen.getByRole('textbox').ariaReadOnly).to.eq('true'); + expect(screen.getByTestId('enable-edit-validation-button')).to.be + .visible; + expect(onClickEnableEditRulesSpy).to.not.have.been.called; + userEvent.click(screen.getByTestId('enable-edit-validation-button')); + expect(onClickEnableEditRulesSpy).to.have.been.calledOnce; + }); + } + ); + + context('when it is an editable mode and editing is enabled', function () { beforeEach(function () { - render( - - ); + renderValidationEditor({ + isEditable: true, + isEditingEnabled: true, + }); }); it('allows to edit the editor', function () { - expect(screen.getByTestId('validation-editor')).to.exist; expect(screen.getByRole('textbox').ariaReadOnly).to.eq(null); + expect( + screen.queryByTestId('enable-edit-validation-button') + ).to.not.exist; }); }); context('when it is a not editable mode', function () { - const setValidatorChangedSpy = sinon.spy(); - const setValidationActionChangedSpy = sinon.spy(); - const setValidationLevelChangedSpy = sinon.spy(); - const setCancelValidationSpy = sinon.spy(); - const saveValidationSpy = sinon.spy(); - const clearSampleDocumentsSpy = sinon.spy(); - const serverVersion = '3.6.0'; - const validation = { - validator: '', - validationAction: 'warn', - validationLevel: 'moderate', - isChanged: false, - syntaxError: null, - error: null, - } as const; - const isEditable = false; - beforeEach(function () { - render( - - ); + renderValidationEditor({ + isEditable: false, + }); }); it('sets editor into readonly mode', function () { expect(screen.getByRole('textbox').ariaReadOnly).to.eq('true'); + expect( + screen.queryByTestId('enable-edit-validation-button') + ).to.not.exist; }); }); }); diff --git a/packages/compass-schema-validation/src/components/validation-editor.tsx b/packages/compass-schema-validation/src/components/validation-editor.tsx index 754211d51ff..fe67c5792be 100644 --- a/packages/compass-schema-validation/src/components/validation-editor.tsx +++ b/packages/compass-schema-validation/src/components/validation-editor.tsx @@ -2,7 +2,6 @@ import React, { useCallback, useEffect, useMemo, useRef } from 'react'; import { debounce } from 'lodash'; import { connect } from 'react-redux'; import { useTelemetry } from '@mongodb-js/compass-telemetry/provider'; -import type { BannerVariant } from '@mongodb-js/compass-components'; import { css, cx, @@ -10,7 +9,9 @@ import { Body, spacing, Banner, + Icon, palette, + useConfirmationModal, useDarkMode, KeylineCard, } from '@mongodb-js/compass-components'; @@ -36,11 +37,11 @@ import { validationActionChanged, validationLevelChanged, } from '../modules/validation'; -import { namespaceChanged } from '../modules/namespace'; import { clearSampleDocuments } from '../modules/sample-documents'; +import { enableEditRules } from '../modules/edit-mode'; const validationEditorStyles = css({ - padding: spacing[3], + padding: spacing[400], }); const validationOptionsStyles = css({ @@ -50,11 +51,12 @@ const validationOptionsStyles = css({ const actionsStyles = css({ display: 'flex', alignItems: 'center', - marginTop: spacing[3], + justifyContent: 'flex-end', + marginTop: spacing[400], }); const editorStyles = css({ - marginTop: spacing[3], + marginTop: spacing[400], }); const editorStylesLight = css({ @@ -66,11 +68,19 @@ const editorStylesDark = css({ }); const modifiedMessageStyles = css({ + color: palette.yellow.dark2, + display: 'flex', + alignItems: 'center', + gap: spacing[200], flex: 1, }); +const modifiedMessageDarkStyles = css({ + color: palette.yellow.light2, +}); + const buttonStyles = css({ - marginLeft: spacing[2], + marginLeft: spacing[200], }); type ValidationCodeEditorProps = Pick< @@ -101,6 +111,7 @@ const ValidationCodeEditor = ({ text={text} onChangeText={onChangeText} readOnly={readOnly} + formattable={!readOnly} completer={completer} /> ); @@ -109,6 +120,7 @@ const ValidationCodeEditor = ({ type ValidationEditorProps = { namespace: string; clearSampleDocuments: () => void; + onClickEnableEditRules: () => void; validatorChanged: (text: string) => void; validationActionChanged: (action: ValidationServerAction) => void; validationLevelChanged: (level: ValidationLevel) => void; @@ -125,6 +137,7 @@ type ValidationEditorProps = { | 'error' >; isEditable: boolean; + isEditingEnabled: boolean; }; /** @@ -135,6 +148,7 @@ export const ValidationEditor: React.FunctionComponent< > = ({ namespace, clearSampleDocuments, + onClickEnableEditRules, validatorChanged, validationActionChanged, validationLevelChanged, @@ -143,9 +157,11 @@ export const ValidationEditor: React.FunctionComponent< serverVersion, validation, isEditable, + isEditingEnabled, }) => { const track = useTelemetry(); const connectionInfoRef = useConnectionInfoRef(); + const { showConfirmation } = useConfirmationModal(); const clearSampleDocumentsRef = useRef(clearSampleDocuments); clearSampleDocumentsRef.current = clearSampleDocuments; @@ -193,29 +209,23 @@ export const ValidationEditor: React.FunctionComponent< [debounceValidatorChanged] ); - const onValidatorSave = useCallback(() => { - saveValidationRef.current(validationRef.current); - }, []); - const darkMode = useDarkMode(); const { validationAction, validationLevel, error, syntaxError, isChanged } = validation; - const hasErrors = !!(error || syntaxError); - - let message = ''; - let variant: BannerVariant = 'info'; - - if (syntaxError) { - message = syntaxError.message; - variant = 'danger'; - } else if (error) { - message = error.message; - variant = 'warning'; - } + const onClickApplyValidation = useCallback(async () => { + const confirmed = await showConfirmation({ + title: 'Are you sure you want to apply these validation rules?', + description: + 'These rules will be enforced on updates & inserts of your document. Please make sure you have reviewed the rules before applying them.', + }); + if (!confirmed) { + return; + } - const hasChangedAndEditable = isChanged && isEditable; + saveValidationRef.current(validationRef.current); + }, [showConfirmation]); return ( { onValidatorChange(text); }} - readOnly={!isEditable} + readOnly={!isEditable || !isEditingEnabled} serverVersion={serverVersion} /> - {variant && message && {message}} - {hasChangedAndEditable && ( + {syntaxError && {syntaxError.message}} + {!syntaxError && error && ( + {error.message} + )} + {isEditable && (
- - Validation modified - - - + {isEditingEnabled ? ( + <> + {isChanged && ( + + Rules are modified & not + applied. Please review before applying. + + )} + + {isChanged && ( + + )} + + ) : ( + + )}
)}
@@ -286,6 +323,7 @@ export const ValidationEditor: React.FunctionComponent< const mapStateToProps = (state: RootState) => ({ serverVersion: state.serverVersion, + isEditingEnabled: state.editMode.isEditingEnabledByUser, validation: state.validation, namespace: state.namespace.ns, }); @@ -298,7 +336,7 @@ export default connect(mapStateToProps, { validatorChanged, cancelValidation, saveValidation, - namespaceChanged, + onClickEnableEditRules: enableEditRules, validationActionChanged, validationLevelChanged, })(ValidationEditor); diff --git a/packages/compass-schema-validation/src/modules/edit-mode.spec.ts b/packages/compass-schema-validation/src/modules/edit-mode.spec.ts index 4c9a2a4f230..0a7ead3e3b9 100644 --- a/packages/compass-schema-validation/src/modules/edit-mode.spec.ts +++ b/packages/compass-schema-validation/src/modules/edit-mode.spec.ts @@ -8,6 +8,7 @@ describe('edit-mode module', function () { const editMode = { collectionReadOnly: true, collectionTimeSeries: false, + isEditingEnabledByUser: false, writeStateStoreReadOnly: false, oldServerReadOnly: false, }; @@ -27,6 +28,7 @@ describe('edit-mode module', function () { expect(reducer(undefined, { type: 'test' } as any)).to.deep.equal({ collectionReadOnly: false, collectionTimeSeries: false, + isEditingEnabledByUser: false, writeStateStoreReadOnly: false, oldServerReadOnly: false, }); @@ -39,6 +41,7 @@ describe('edit-mode module', function () { const editMode = { collectionReadOnly: false, collectionTimeSeries: false, + isEditingEnabledByUser: false, writeStateStoreReadOnly: false, oldServerReadOnly: true, }; diff --git a/packages/compass-schema-validation/src/modules/edit-mode.ts b/packages/compass-schema-validation/src/modules/edit-mode.ts index fe9d20343f8..4856d2a819f 100644 --- a/packages/compass-schema-validation/src/modules/edit-mode.ts +++ b/packages/compass-schema-validation/src/modules/edit-mode.ts @@ -4,19 +4,36 @@ import type { RootAction } from '.'; * The edit mode changed action. */ export const EDIT_MODE_CHANGED = - 'validation/namespace/EDIT_MODE_CHANGED' as const; -interface EditModeChangedAction { + 'validation/edit-mode/EDIT_MODE_CHANGED' as const; +export const ENABLE_EDIT_RULES = + 'validation/edit-mode/ENABLE_EDIT_RULES' as const; +export const DISABLE_EDIT_RULES = + 'validation/edit-mode/DISABLE_EDIT_RULES' as const; + +type EditModeChangedAction = { type: typeof EDIT_MODE_CHANGED; editMode: Partial; -} +}; + +type EnableEditRulesAction = { + type: typeof ENABLE_EDIT_RULES; +}; + +type DisableModeChangedAction = { + type: typeof DISABLE_EDIT_RULES; +}; -export type EditModeAction = EditModeChangedAction; +export type EditModeAction = + | EditModeChangedAction + | EnableEditRulesAction + | DisableModeChangedAction; export interface EditModeState { collectionReadOnly: boolean; collectionTimeSeries: boolean; writeStateStoreReadOnly: boolean; oldServerReadOnly: boolean; + isEditingEnabledByUser: boolean; } /** @@ -27,6 +44,7 @@ export const INITIAL_STATE: EditModeState = { collectionTimeSeries: false, writeStateStoreReadOnly: false, oldServerReadOnly: false, + isEditingEnabledByUser: false, }; /** @@ -45,6 +63,14 @@ export default function reducer( return { ...state, ...action.editMode }; } + if (action.type === ENABLE_EDIT_RULES) { + return { ...state, isEditingEnabledByUser: true }; + } + + if (action.type === DISABLE_EDIT_RULES) { + return { ...state, isEditingEnabledByUser: false }; + } + return state; } @@ -61,3 +87,11 @@ export const editModeChanged = ( type: EDIT_MODE_CHANGED, editMode, }); + +export const enableEditRules = (): EnableEditRulesAction => ({ + type: ENABLE_EDIT_RULES, +}); + +export const disableEditRules = (): DisableModeChangedAction => ({ + type: DISABLE_EDIT_RULES, +}); diff --git a/packages/compass-schema-validation/src/modules/validation.ts b/packages/compass-schema-validation/src/modules/validation.ts index 7752200207f..c2059e38ba7 100644 --- a/packages/compass-schema-validation/src/modules/validation.ts +++ b/packages/compass-schema-validation/src/modules/validation.ts @@ -7,6 +7,7 @@ import { zeroStateChanged } from './zero-state'; import { isLoadedChanged } from './is-loaded'; import { isEqual, pick } from 'lodash'; import type { ThunkDispatch } from 'redux-thunk'; +import { disableEditRules } from './edit-mode'; export type ValidationServerAction = 'error' | 'warn'; export type ValidationLevel = 'off' | 'moderate' | 'strict'; @@ -486,6 +487,7 @@ export const saveValidation = ( } ); dispatch(fetchValidation(namespace)); + dispatch(disableEditRules()); } catch (error) { dispatch(validationSaveFailed(error as Error)); } @@ -505,6 +507,7 @@ export const cancelValidation = () => { const state = getState(); const prevValidation = state.validation.prevValidation; + dispatch(disableEditRules()); dispatch( validationCanceled({ isChanged: false, diff --git a/packages/compass-schema-validation/src/modules/zero-state.ts b/packages/compass-schema-validation/src/modules/zero-state.ts index e3ed225a6d9..11d8cd83d72 100644 --- a/packages/compass-schema-validation/src/modules/zero-state.ts +++ b/packages/compass-schema-validation/src/modules/zero-state.ts @@ -1,4 +1,5 @@ import type { RootAction, SchemaValidationThunkAction } from '.'; +import { enableEditRules } from './edit-mode'; /** * Zero state changed action. @@ -52,6 +53,7 @@ export const changeZeroState = ( if (isZeroState === false) { track('Schema Validation Added', {}, connectionInfoRef.current); } + dispatch(enableEditRules()); return dispatch(zeroStateChanged(isZeroState)); }; }; diff --git a/packages/compass-schema-validation/src/stores/store.spec.ts b/packages/compass-schema-validation/src/stores/store.spec.ts index 598b1f1ba48..ceb32488bef 100644 --- a/packages/compass-schema-validation/src/stores/store.spec.ts +++ b/packages/compass-schema-validation/src/stores/store.spec.ts @@ -101,6 +101,7 @@ describe('Schema Validation Store', function () { expect(store.getState().editMode).to.deep.equal({ collectionReadOnly: false, collectionTimeSeries: false, + isEditingEnabledByUser: false, oldServerReadOnly: false, writeStateStoreReadOnly: true, }); @@ -115,6 +116,7 @@ describe('Schema Validation Store', function () { expect(store.getState().editMode).to.deep.equal({ collectionReadOnly: false, collectionTimeSeries: false, + isEditingEnabledByUser: false, oldServerReadOnly: false, writeStateStoreReadOnly: false, }); diff --git a/packages/compass-schema-validation/src/stores/store.ts b/packages/compass-schema-validation/src/stores/store.ts index e7e0be3f79d..138e8655726 100644 --- a/packages/compass-schema-validation/src/stores/store.ts +++ b/packages/compass-schema-validation/src/stores/store.ts @@ -87,6 +87,7 @@ export function onActivated( collectionReadOnly: options.isReadonly ? true : false, writeStateStoreReadOnly: !instance.isWritable, oldServerReadOnly: semver.gte(MIN_VERSION, instance.build.version), + isEditingEnabledByUser: false, }, }, { diff --git a/packages/compass-schema/src/components/export-schema-modal.tsx b/packages/compass-schema/src/components/export-schema-modal.tsx index d35d192ec80..394d8f8fb11 100644 --- a/packages/compass-schema/src/components/export-schema-modal.tsx +++ b/packages/compass-schema/src/components/export-schema-modal.tsx @@ -237,7 +237,7 @@ const ExportSchemaModal: React.FunctionComponent<{ onClick={onSchemaDownload} data-testid="schema-export-download-button" > - Export + Export…