From 8b184659bc17fc240c2ca7b3e9240c8732ec576c Mon Sep 17 00:00:00 2001 From: Le Roux Bodenstein Date: Wed, 9 Oct 2024 12:30:59 +0100 Subject: [PATCH] allow editing connected connections, but only the name, colour and favorite. --- .../src/item-actions.ts | 6 +- .../multiple-connections/sidebar.tsx | 4 + .../csfle-tab/csfle-tab.spec.tsx | 4 +- .../connection-form-actions.spec.tsx | 10 ++ .../components/connection-form-actions.tsx | 24 ++-- .../src/components/connection-form.spec.tsx | 65 +++++++++ .../src/components/connection-form.tsx | 135 ++++++++++++++---- .../connection-string-input.spec.tsx | 1 + .../components/connection-string-input.tsx | 8 +- .../src/components/form-help/form-help.tsx | 19 +-- 10 files changed, 219 insertions(+), 57 deletions(-) diff --git a/packages/compass-connections-navigation/src/item-actions.ts b/packages/compass-connections-navigation/src/item-actions.ts index 4a78bc2b4c5..7930ed44e8c 100644 --- a/packages/compass-connections-navigation/src/item-actions.ts +++ b/packages/compass-connections-navigation/src/item-actions.ts @@ -7,14 +7,12 @@ export type NavigationItemActions = (ItemAction | ItemSeparator)[]; export const notConnectedConnectionItemActions = ({ connectionInfo, - hideEditConnect = false, }: { connectionInfo: ConnectionInfo; - hideEditConnect?: boolean; }): NavigationItemActions => { const isAtlas = !!connectionInfo.atlasMetadata; const actions: (ItemAction | ItemSeparator | null)[] = [ - hideEditConnect || isAtlas + isAtlas ? null : { action: 'edit-connection', @@ -85,8 +83,6 @@ export const connectedConnectionItemActions = ({ const isAtlas = !!connectionInfo.atlasMetadata; const connectionManagementActions = notConnectedConnectionItemActions({ connectionInfo, - // for connected connections we don't show connect action - hideEditConnect: true, }); const actions: (ItemAction | ItemSeparator | null)[] = [ hasWriteActionsDisabled diff --git a/packages/compass-sidebar/src/components/multiple-connections/sidebar.tsx b/packages/compass-sidebar/src/components/multiple-connections/sidebar.tsx index 3189961eda0..d89835c4a7a 100644 --- a/packages/compass-sidebar/src/components/multiple-connections/sidebar.tsx +++ b/packages/compass-sidebar/src/components/multiple-connections/sidebar.tsx @@ -228,6 +228,10 @@ export function MultipleConnectionSidebar({ /> {editingConnectionInfo && ( disconnect(editingConnectionInfo.id)} isOpen={isEditingConnectionInfoModalOpen} setOpen={(newOpen) => { // This is how leafygreen propagates `X` button click diff --git a/packages/connection-form/src/components/advanced-options-tabs/csfle-tab/csfle-tab.spec.tsx b/packages/connection-form/src/components/advanced-options-tabs/csfle-tab/csfle-tab.spec.tsx index e2ad37fdce4..3752a8ffbf1 100644 --- a/packages/connection-form/src/components/advanced-options-tabs/csfle-tab/csfle-tab.spec.tsx +++ b/packages/connection-form/src/components/advanced-options-tabs/csfle-tab/csfle-tab.spec.tsx @@ -381,7 +381,9 @@ describe('In-Use Encryption', function () { const selector = within(card).getByTestId('csfle-kms-card-name'); userEvent.clear(selector); - userEvent.type(selector, value); + if (value !== '') { + userEvent.type(selector, value); + } userEvent.keyboard('{enter}'); } diff --git a/packages/connection-form/src/components/connection-form-actions.spec.tsx b/packages/connection-form/src/components/connection-form-actions.spec.tsx index 03434261762..23483d33e9a 100644 --- a/packages/connection-form/src/components/connection-form-actions.spec.tsx +++ b/packages/connection-form/src/components/connection-form-actions.spec.tsx @@ -46,6 +46,16 @@ describe('', function () { expect(onSaveAndConnectSpy).to.have.been.calledOnce; }); + + it('should hide "connect" button if there is no callback', function () { + render( + + ); + expect(screen.queryByRole('button', { name: 'Connect' })).to.not.exist; + }); }); describe('Save Button', function () { diff --git a/packages/connection-form/src/components/connection-form-actions.tsx b/packages/connection-form/src/components/connection-form-actions.tsx index 86de081c126..17c4263e86a 100644 --- a/packages/connection-form/src/components/connection-form-actions.tsx +++ b/packages/connection-form/src/components/connection-form-actions.tsx @@ -44,7 +44,7 @@ export type ConnectionFormModalActionsProps = { onCancel?(): void; onSave?(): void; - onSaveAndConnect(): void; + onSaveAndConnect?(): void; }; export function ConnectionFormModalActions({ @@ -89,7 +89,11 @@ export function ConnectionFormModalActions({
)} - + {onSaveAndConnect && ( + + )} ); diff --git a/packages/connection-form/src/components/connection-form.spec.tsx b/packages/connection-form/src/components/connection-form.spec.tsx index 46485aac68a..5dc306317fd 100644 --- a/packages/connection-form/src/components/connection-form.spec.tsx +++ b/packages/connection-form/src/components/connection-form.spec.tsx @@ -77,6 +77,71 @@ describe('ConnectionForm Component', function () { sandbox.restore(); }); + context('when disableEditingConnectedConnection==true', function () { + it('renders a banner, disables the connection string and removes advanced connection options + connect button', function () { + const onDisconnectClicked = Sinon.spy(); + const onSaveClicked = Sinon.spy(); + const onSaveAndConnectClicked = Sinon.spy(); + + renderForm({ + disableEditingConnectedConnection: true, + onDisconnectClicked, + onSaveClicked, + onSaveAndConnectClicked, + }); + + expect( + screen.getByTestId('disabled-connected-connection-banner') + ).to.exist; + expect(screen.getByRole('button', { name: 'Disconnect' })).to.exist; + expect(() => + screen.getByTestId('toggle-edit-connection-string') + ).to.throw; + expect(() => + screen.getByTestId('advanced-connection-options') + ).to.throw; + expect(() => screen.getByRole('button', { name: 'Connect' })).to.throw; + + // pressing enter calls onSubmit which saves + fireEvent.submit(screen.getByRole('form')); + expect(onSaveClicked.callCount).to.equal(1); + expect(onSaveAndConnectClicked.callCount).to.equal(0); + + fireEvent.click(screen.getByRole('button', { name: 'Disconnect' })); + expect(onDisconnectClicked.callCount).to.equal(1); + }); + }); + + context('when disableEditingConnectedConnection==false', function () { + it('leaves the connection string, advanced connection options and connect button intact, does not render a banner', function () { + const onDisconnectClicked = Sinon.spy(); + const onSaveClicked = Sinon.spy(); + const onSaveAndConnectClicked = Sinon.spy(); + + renderForm({ + disableEditingConnectedConnection: false, + onDisconnectClicked, + onSaveClicked, + onSaveAndConnectClicked, + }); + + expect(() => + screen.getByTestId('disabled-connected-connection-banner') + ).to.throw; + expect(() => + screen.getByRole('button', { name: 'Disconnect' }) + ).to.throw; + expect(screen.getByTestId('toggle-edit-connection-string')).to.exist; + expect(screen.getByTestId('advanced-connection-options')).to.exist; + expect(screen.getByRole('button', { name: 'Connect' })).to.exist; + + // pressing enter calls onSubmit which saves and connects (the default) + fireEvent.submit(screen.getByRole('form')); + expect(onSaveClicked.callCount).to.equal(0); + expect(onSaveAndConnectClicked.callCount).to.equal(1); + }); + }); + context( 'when preferences.protectConnectionStringsForNewConnections === true', function () { diff --git a/packages/connection-form/src/components/connection-form.tsx b/packages/connection-form/src/components/connection-form.tsx index ff5b278edc4..c70c0313d05 100644 --- a/packages/connection-form/src/components/connection-form.tsx +++ b/packages/connection-form/src/components/connection-form.tsx @@ -19,6 +19,8 @@ import { cx, palette, useDarkMode, + Button, + Icon, } from '@mongodb-js/compass-components'; import { cloneDeep } from 'lodash'; import ConnectionStringInput from './connection-string-input'; @@ -39,7 +41,7 @@ import { useConnectionColor } from '../hooks/use-connection-color'; import FormHelp from './form-help/form-help'; const descriptionStyles = css({ - marginTop: spacing[2], + marginTop: spacing[200], }); const formStyles = css({ @@ -127,8 +129,18 @@ const headingWithHiddenButtonStyles = css({ }, }); +const disabledConnectedConnectionBannerStyles = css({ + marginTop: spacing[400], + paddingRight: 0, +}); +const disabledConnectedConnectionContentStyles = css({ + display: 'flex', + gap: spacing[400], + alignItems: 'center', +}); + const connectionStringErrorStyles = css({ - marginBottom: spacing[3], + marginBottom: spacing[400], }); const colorPreviewStyles = css({ @@ -185,8 +197,8 @@ const personalizationSectionLayoutStyles = css({ 'name-input color-input' 'favorite-marker favorite-marker' `, - gap: spacing[4], - marginBottom: spacing[4], + gap: spacing[600], + marginBottom: spacing[600], }); const personalizationNameInputStyles = css({ @@ -303,10 +315,12 @@ type ConnectionFormPropsWithoutSettings = { darkMode?: boolean; initialConnectionInfo: ConnectionInfo; connectionErrorMessage?: string | null; + disableEditingConnectedConnection?: boolean; onCancel?: () => void; onConnectClicked?: (connectionInfo: ConnectionInfo) => void; onSaveAndConnectClicked?: (connectionInfo: ConnectionInfo) => void; onSaveClicked?: (connectionInfo: ConnectionInfo) => Promise; + onDisconnectClicked?: () => void; onAdvancedOptionsToggle?: (newState: boolean) => void; openSettingsModal?: (tab?: string) => void; }; @@ -317,8 +331,10 @@ export type ConnectionFormProps = ConnectionFormPropsWithoutSettings & function ConnectionForm({ initialConnectionInfo, connectionErrorMessage, + disableEditingConnectedConnection = false, onSaveAndConnectClicked, onSaveClicked, + onDisconnectClicked, onCancel, onAdvancedOptionsToggle, openSettingsModal, @@ -388,29 +404,6 @@ function ConnectionForm({ }), [initialConnectionInfo, connectionOptions, personalizationOptions] ); - const onSubmitForm = useCallback(() => { - const updatedConnectionOptions = cloneDeep(connectionOptions); - // TODO: this method throws on malformed connection strings instead of - // returning errors - const formErrors = validateConnectionOptionsErrors( - updatedConnectionOptions - ); - if (formErrors.length) { - setErrors(formErrors); - return; - } - onSaveAndConnectClicked?.({ - ...initialConnectionInfo, - ...getConnectionInfoToSave(), - connectionOptions: updatedConnectionOptions, - }); - }, [ - initialConnectionInfo, - onSaveAndConnectClicked, - setErrors, - connectionOptions, - getConnectionInfoToSave, - ]); const callOnSaveConnectionClickedAndStoreErrors = useCallback( async (connectionInfo: ConnectionInfo): Promise => { @@ -429,6 +422,39 @@ function ConnectionForm({ }, [onSaveClicked, setErrors] ); + const onSubmitForm = useCallback(() => { + const updatedConnectionOptions = cloneDeep(connectionOptions); + // TODO: this method throws on malformed connection strings instead of + // returning errors + const formErrors = validateConnectionOptionsErrors( + updatedConnectionOptions + ); + if (formErrors.length) { + setErrors(formErrors); + return; + } + if (disableEditingConnectedConnection) { + // there isn't a connect button in this case, so the default action should + // be to save, not to save and connect + void callOnSaveConnectionClickedAndStoreErrors?.( + getConnectionInfoToSave() + ); + } else { + onSaveAndConnectClicked?.({ + ...initialConnectionInfo, + ...getConnectionInfoToSave(), + connectionOptions: updatedConnectionOptions, + }); + } + }, [ + connectionOptions, + disableEditingConnectedConnection, + setErrors, + callOnSaveConnectionClickedAndStoreErrors, + getConnectionInfoToSave, + onSaveAndConnectClicked, + initialConnectionInfo, + ]); const showPersonalisationForm = useConnectionFormSetting( 'showPersonalisationForm' @@ -436,9 +462,18 @@ function ConnectionForm({ const showHelpCardsInForm = useConnectionFormSetting('showHelpCardsInForm'); + /* + To test onSubmit on a form (ie. what happens when you press {enter} and not + what happens when you click a submit button) you have to use submit() which only + works with role="form", but eslint doesn't like that because role="form" + should be redundant. + https://stackoverflow.com/questions/59362804/pressing-enter-to-submit-form-in-react-testing-library-does-not-work + */ + /* eslint-disable jsx-a11y/no-redundant-roles */ return (
{ // Prevent default html page refresh. @@ -451,7 +486,8 @@ function ConnectionForm({

- {initialConnectionInfo.favorite?.name ?? 'New Connection'} + {(initialConnectionInfo.favorite?.name ?? 'New Connection') || + 'Edit Connection'}

Manage your connection settings @@ -461,9 +497,36 @@ function ConnectionForm({
+ {disableEditingConnectedConnection && onDisconnectClicked && ( + +
+
+ While connected, you may only personalize your + connection's name, color or favorite status. To fully + configure it, you must first disconnect. Beware that + disconnecting might cause work in progress to be lost. +
+
+ +
+
+ + )} )} - {!protectConnectionStrings && ( + {!( + protectConnectionStrings || disableEditingConnectedConnection + ) && (
); + /* eslint-enable jsx-a11y/no-redundant-roles */ } const ConnectionFormWithSettings: React.FunctionComponent< diff --git a/packages/connection-form/src/components/connection-string-input.spec.tsx b/packages/connection-form/src/components/connection-string-input.spec.tsx index 2b04dc5102c..16caad9be36 100644 --- a/packages/connection-form/src/components/connection-string-input.spec.tsx +++ b/packages/connection-form/src/components/connection-string-input.spec.tsx @@ -22,6 +22,7 @@ const renderConnectionStringInput = ( {}} diff --git a/packages/connection-form/src/components/connection-string-input.tsx b/packages/connection-form/src/components/connection-string-input.tsx index 867ce58d777..ba1653c984e 100644 --- a/packages/connection-form/src/components/connection-string-input.tsx +++ b/packages/connection-form/src/components/connection-string-input.tsx @@ -79,6 +79,7 @@ function ConnectionStringInput({ updateConnectionFormField, onSubmit, protectConnectionStrings, + disableEditingConnectedConnection, }: { connectionString: string; enableEditingConnectionString: boolean; @@ -86,6 +87,7 @@ function ConnectionStringInput({ updateConnectionFormField: UpdateConnectionFormField; onSubmit: () => void; protectConnectionStrings: boolean; + disableEditingConnectedConnection: boolean; }): React.ReactElement { const textAreaEl = useRef(null); const [editingConnectionString, setEditingConnectionString] = @@ -173,7 +175,7 @@ function ConnectionStringInput({ href="https://docs.mongodb.com/manual/reference/connection-string/" />
- {!protectConnectionStrings && ( + {!(protectConnectionStrings || disableEditingConnectedConnection) && (