Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ async function loadSavedConnectionAndConnect(connectionInfo: ConnectionInfo) {
)
);

const connectButton = screen.getByRole('button', { name: 'Connect' });
const connectButton = screen.getByRole('button', { name: 'Save & Connect' });
userEvent.click(connectButton);

// Wait for the connecting... modal to hide.
Expand Down Expand Up @@ -267,7 +267,9 @@ describe.skip('Connections Component', function () {
)
);

const connectButton = screen.getByRole('button', { name: 'Connect' });
const connectButton = screen.getByRole('button', {
name: 'Save & Connect',
});
userEvent.click(connectButton);

// Wait for the connecting... modal to be shown.
Expand All @@ -288,7 +290,9 @@ describe.skip('Connections Component', function () {
});

it('should enable the connect button', function () {
const connectButton = screen.getByText('Connect');
const connectButton = screen.getByRole('button', {
name: 'Save & Connect',
});
expect(connectButton).to.not.match('disabled');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@ type ConnectionAutoconnectCheckAction = {
type ConnectionAttemptStartAction = {
type: ActionTypes.ConnectionAttemptStart;
connectionInfo: ConnectionInfo;
options: {
forceSave: boolean;
};
};

type ConnectionAttemptSuccessAction = {
Expand Down Expand Up @@ -823,10 +826,13 @@ const reducer: Reducer<State, Action> = (state = INITIAL_STATE, action) => {
state.connections,
action.connectionInfo.id,
{
...(isNewConnection(state, action.connectionInfo.id)
...(isNewConnection(state, action.connectionInfo.id) ||
action.options.forceSave
? {
// For new connections, update the state with new info right
// away (we will also save it to the storage at the end)
// For new connections or when we're forcing a
// save (the Save & Connect button), update the state with new
// info right away (we will also save it to the storage at the
// end)
info: action.connectionInfo,
}
: {
Expand Down Expand Up @@ -1464,6 +1470,35 @@ export const connect = (
| ConnectionAttemptSuccessAction
| ConnectionAttemptCancelledAction
| OidcNotifyDeviceAuthAction
> => {
return connectWithOptions(connectionInfo, { forceSave: false });
};

export const saveAndConnect = (
connectionInfo: ConnectionInfo
): ConnectionsThunkAction<
Promise<void>,
| ConnectionAttemptStartAction
| ConnectionAttemptErrorAction
| ConnectionAttemptSuccessAction
| ConnectionAttemptCancelledAction
| OidcNotifyDeviceAuthAction
> => {
return connectWithOptions(connectionInfo, { forceSave: true });
};

const connectWithOptions = (
connectionInfo: ConnectionInfo,
options: {
forceSave: boolean;
}
): ConnectionsThunkAction<
Promise<void>,
| ConnectionAttemptStartAction
| ConnectionAttemptErrorAction
| ConnectionAttemptSuccessAction
| ConnectionAttemptCancelledAction
| OidcNotifyDeviceAuthAction
> => {
return async (
dispatch,
Expand Down Expand Up @@ -1516,6 +1551,7 @@ export const connect = (
dispatch({
type: ActionTypes.ConnectionAttemptStart,
connectionInfo,
options: { forceSave: options.forceSave },
});

track(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,37 @@ describe('useConnections', function () {
}
});

describe('#saveAndConnect', function () {
it('saves the connection options before connecting', async function () {
const { result, track, connectionStorage } = renderHookWithConnections(
useConnections,
{
connections: mockConnections,
preferences: defaultPreferences,
}
);

const updatedConnection = {
...mockConnections[0],
savedConnectionType: 'recent' as const,
};

await result.current.saveAndConnect(updatedConnection);

await waitFor(() => {
expect(track.getCall(1).firstArg).to.eq('New Connection');
});

expect(
await connectionStorage.load({ id: updatedConnection.id })
).to.have.property('savedConnectionType', 'recent');

expect(
screen.getByText(`Connected to ${mockConnections[0].favorite.name}`)
).to.exist;
});
});

describe('#disconnect', function () {
it('disconnect even if connection is in progress cleaning up progress toasts', async function () {
const { result, track } = renderHookWithConnections(useConnections, {
Expand Down
12 changes: 8 additions & 4 deletions packages/compass-connections/src/stores/connections-store.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ type State = {
connectionErrors: Record<string, Error | null>;
editingConnectionInfo: ConnectionInfo;
isEditingConnectionInfoModalOpen: boolean;
isEditingNewConnection: boolean;
oidcDeviceAuthState: Record<string, { url: string; code: string }>;
};

Expand All @@ -15,6 +16,7 @@ export function useConnections(): {
state: State;

connect: (connectionInfo: ConnectionInfo) => Promise<void>;
saveAndConnect: (connectionInfo: ConnectionInfo) => Promise<void>;
disconnect: (connectionId: string) => void;

createNewConnection: () => void;
Expand All @@ -32,16 +34,16 @@ export function useConnections(): {
showNonGenuineMongoDBWarningModal: (connectionId: string) => void;
} {
const connectionsState = useConnectionsState();
const editingConnection =
connectionsState.connections.byId[connectionsState.editingConnectionInfoId];
const state = {
connectionErrors: Object.fromEntries(
Object.entries(connectionsState.connections.byId).map(([k, v]) => {
return [k, v.error ?? null];
})
),
editingConnectionInfo:
connectionsState.connections.byId[
connectionsState.editingConnectionInfoId
].info,
editingConnectionInfo: editingConnection.info,
isEditingNewConnection: !!editingConnection.isBeingCreated,
isEditingConnectionInfoModalOpen:
connectionsState.isEditingConnectionInfoModalOpen,
oidcDeviceAuthState: Object.fromEntries(
Expand All @@ -52,6 +54,7 @@ export function useConnections(): {
};
const {
connect,
saveAndConnect,
disconnect,
createNewConnection,
editConnection,
Expand All @@ -66,6 +69,7 @@ export function useConnections(): {
return {
state,
connect,
saveAndConnect,
disconnect,
createNewConnection,
editConnection,
Expand Down
4 changes: 4 additions & 0 deletions packages/compass-connections/src/stores/store-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
import {
cancelEditConnection,
connect as connectionsConnect,
saveAndConnect,
connectionsEventEmitter,
createNewConnection,
disconnect,
Expand Down Expand Up @@ -87,6 +88,9 @@ function getConnectionsActions(dispatch: ConnectionsStore['dispatch']) {
connect: (connectionInfo: ConnectionInfo) => {
return dispatch(connectionsConnect(connectionInfo));
},
saveAndConnect: (connectionInfo: ConnectionInfo) => {
return dispatch(saveAndConnect(connectionInfo));
},
disconnect: (connectionId: ConnectionId) => {
return dispatch(disconnect(connectionId));
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export function MultipleConnectionSidebar({
const connectionsWithStatus = useConnectionsWithStatus();
const {
connect,
saveAndConnect,
disconnect,
createNewConnection,
editConnection,
Expand All @@ -120,6 +121,7 @@ export function MultipleConnectionSidebar({
showNonGenuineMongoDBWarningModal,
state: {
editingConnectionInfo,
isEditingNewConnection,
isEditingConnectionInfoModalOpen,
connectionErrors,
},
Expand Down Expand Up @@ -184,6 +186,10 @@ export function MultipleConnectionSidebar({
[globalAppRegistry]
);

const disableEditingConnectedConnection = !!findActiveConnection(
editingConnectionInfo.id
);

return (
<ResizableSidebar data-testid="navigation-sidebar" useNewTheme={true}>
<aside className={sidebarStyles}>
Expand Down Expand Up @@ -229,7 +235,7 @@ export function MultipleConnectionSidebar({
{editingConnectionInfo && (
<ConnectionFormModal
disableEditingConnectedConnection={
!!findActiveConnection(editingConnectionInfo.id)
disableEditingConnectedConnection
}
onDisconnectClicked={() => disconnect(editingConnectionInfo.id)}
isOpen={isEditingConnectionInfoModalOpen}
Expand All @@ -240,20 +246,31 @@ export function MultipleConnectionSidebar({
}
}}
initialConnectionInfo={editingConnectionInfo}
onSaveAndConnectClicked={(connectionInfo) => {
void connect(connectionInfo);
}}
onSaveClicked={(connectionInfo) => {
return saveEditedConnection(connectionInfo);
}}
onCancel={() => {
cancelEditConnection(editingConnectionInfo.id);
}}
connectionErrorMessage={
connectionErrors[editingConnectionInfo.id]?.message
}
openSettingsModal={openSettingsModal}
{...formPreferences}
onCancel={() => {
cancelEditConnection(editingConnectionInfo.id);
}}
onSaveClicked={(connectionInfo) => {
return saveEditedConnection(connectionInfo);
}}
onConnectClicked={
isEditingNewConnection || disableEditingConnectedConnection
? undefined
: (connectionInfo) => {
void connect(connectionInfo);
}
}
onSaveAndConnectClicked={
disableEditingConnectedConnection
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed all these to not include the button callback prop if it shouldn't show up. That makes the logic easier and less error prone because you don't have to check both the boolean AND the button to know which buttons to include inside the form and to know which one to call.

disableEditingConnectedConnection is then only used when a button isn't directly involved like when showing the banner.

? undefined
: (connectionInfo) => {
void saveAndConnect(connectionInfo);
}
}
/>
)}
<MappedCsfleModal
Expand Down
4 changes: 2 additions & 2 deletions packages/compass-web/src/entrypoint.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ describe('CompassWeb', function () {
screen.getAllByRole('button', { name: 'Add new connection' })[0]
);
await waitFor(() => {
screen.getByRole('button', { name: 'Connect' });
screen.getByRole('button', { name: 'Save & Connect' });
});
userEvent.click(screen.getByRole('button', { name: 'Connect' }));
userEvent.click(screen.getByRole('button', { name: 'Save & Connect' }));
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { render, screen, userEvent } from '@mongodb-js/testing-library-compass';
import { expect } from 'chai';
import sinon from 'sinon';

import { ConnectionFormModalActions } from './connection-form-actions';
import { ConnectionFormModalActions } from './connection-form-modal-actions';

describe('<ConnectionFormModalActions />', function () {
it('should show warnings', function () {
Expand All @@ -30,7 +30,7 @@ describe('<ConnectionFormModalActions />', function () {
expect(screen.getByText('Error!')).to.be.visible;
});

describe('Connect Button', function () {
describe('Save&Connect Button', function () {
it('should call onSaveAndConnect function', function () {
const onSaveAndConnectSpy = sinon.spy();
render(
Expand All @@ -41,7 +41,9 @@ describe('<ConnectionFormModalActions />', function () {
onSaveAndConnect={onSaveAndConnectSpy}
></ConnectionFormModalActions>
);
const connectButton = screen.getByRole('button', { name: 'Connect' });
const connectButton = screen.getByRole('button', {
name: 'Save & Connect',
});
userEvent.click(connectButton);

expect(onSaveAndConnectSpy).to.have.been.calledOnce;
Expand All @@ -54,7 +56,8 @@ describe('<ConnectionFormModalActions />', function () {
warnings={[]}
></ConnectionFormModalActions>
);
expect(screen.queryByRole('button', { name: 'Connect' })).to.not.exist;
expect(screen.queryByRole('button', { name: 'Save & Connect' })).to.not
.exist;
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export type ConnectionFormModalActionsProps = {
onCancel?(): void;
onSave?(): void;
onSaveAndConnect?(): void;
onConnect?(): void;
};

export function ConnectionFormModalActions({
Expand All @@ -53,6 +54,7 @@ export function ConnectionFormModalActions({
onCancel,
onSave,
onSaveAndConnect,
onConnect,
}: ConnectionFormModalActionsProps): React.ReactElement {
const saveAndConnectLabel = useConnectionFormSetting('saveAndConnectLabel');
return (
Expand Down Expand Up @@ -102,9 +104,21 @@ export function ConnectionFormModalActions({
</div>
)}

{onConnect && (
<Button
data-testid={'connect-button'}
variant={ButtonVariant.PrimaryOutline}
onClick={onConnect}
>
Connect
</Button>
)}

{onSaveAndConnect && (
<Button
data-testid="connect-button"
data-testid={
onConnect ? 'save-and-connect-button' : 'connect-button'
}
variant={ButtonVariant.Primary}
onClick={onSaveAndConnect}
>
Expand Down
Loading
Loading