From 353ac75f39ee520f9954ed6c29818677b79f7a8d Mon Sep 17 00:00:00 2001 From: mcasimir Date: Fri, 11 Mar 2022 16:46:01 +0100 Subject: [PATCH 1/7] move fav / recents inside the store --- .../connection-list/connection-list.tsx | 27 ++------ .../src/components/connections.tsx | 28 ++++++--- .../src/components/resizeable-sidebar.tsx | 63 ++++++------------- .../src/stores/connections-store.ts | 25 +++++++- 4 files changed, 65 insertions(+), 78 deletions(-) diff --git a/packages/compass-connections/src/components/connection-list/connection-list.tsx b/packages/compass-connections/src/components/connection-list/connection-list.tsx index dbcb610f9c6..6954fff871e 100644 --- a/packages/compass-connections/src/components/connection-list/connection-list.tsx +++ b/packages/compass-connections/src/components/connection-list/connection-list.tsx @@ -106,7 +106,8 @@ export type ConnectionInfoFavorite = ConnectionInfo & function ConnectionList({ activeConnectionId, - connections, + recentConnections, + favoriteConnections, createNewConnection, setActiveConnectionId, onDoubleClick, @@ -115,7 +116,8 @@ function ConnectionList({ removeConnection, }: { activeConnectionId?: string; - connections: ConnectionInfo[]; + recentConnections: ConnectionInfo[]; + favoriteConnections: ConnectionInfo[]; createNewConnection: () => void; setActiveConnectionId: (connectionId: string) => void; onDoubleClick: (connectionInfo: ConnectionInfo) => void; @@ -124,27 +126,6 @@ function ConnectionList({ removeConnection: (connectionInfo: ConnectionInfo) => void; }): React.ReactElement { const [recentHeaderHover, setRecentHover] = useState(false); - const favoriteConnections = connections - .filter( - (connectionInfo): connectionInfo is ConnectionInfoFavorite => - !!connectionInfo.favorite - ) - .sort((a: ConnectionInfoFavorite, b: ConnectionInfoFavorite) => { - return b.favorite.name.toLocaleLowerCase() < - a.favorite.name.toLocaleLowerCase() - ? 1 - : -1; - }); - - const recentConnections = connections - .filter((connectionInfo) => !connectionInfo.favorite) - .sort((a, b) => { - // The `lastUsed` value hasn't always existed, so we assign - // them a date in 2016 for sorting if it isn't there. - const aLastUsed = a.lastUsed ? a.lastUsed.getTime() : 1463658247465; - const bLastUsed = b.lastUsed ? b.lastUsed.getTime() : 1463658247465; - return bLastUsed - aLastUsed; - }); return ( diff --git a/packages/compass-connections/src/components/connections.tsx b/packages/compass-connections/src/components/connections.tsx index 816edab50d3..4570ede1c5e 100644 --- a/packages/compass-connections/src/components/connections.tsx +++ b/packages/compass-connections/src/components/connections.tsx @@ -19,6 +19,7 @@ import FormHelp from './form-help/form-help'; import Connecting from './connecting/connecting'; import { useConnections } from '../stores/connections-store'; import { cloneDeep } from 'lodash'; +import ConnectionList from './connection-list/connection-list'; const { debug } = createLoggerAndTelemetry( 'mongodb-compass:connections:connections' @@ -46,6 +47,9 @@ const formContainerStyles = css({ gap: spacing[4], }); +const initialSidebarWidth = spacing[4] * 10 + spacing[2]; // 248px +const minSidebarWidth = spacing[4] * 7; // 168px + function Connections({ onConnected, connectionStorage = new ConnectionStorage(), @@ -89,15 +93,21 @@ function Connections({ className={connectStyles} > + minWidth={minSidebarWidth} + initialWidth={initialSidebarWidth} + > + +
void; - setActiveConnectionId: (newConnectionId: string) => void; - onConnectionDoubleClicked: (connectionInfo: ConnectionInfo) => void; - removeAllRecentsConnections: () => void; - duplicateConnection: (connectionInfo: ConnectionInfo) => void; - removeConnection: (connectionInfo: ConnectionInfo) => void; -}): React.ReactElement { - const [width, setWidth] = useState(initialSidebarWidth); + initialWidth: number; + minWidth: number; + children: JSX.Element; +}): JSX.Element => { + const [width, setWidth] = useState(initialWidth); + + const getMaxSidebarWidth = useCallback(() => { + return Math.max(minWidth, window.innerWidth - 100); + }, [minWidth]); return (
- + {children} setWidth(newWidth)} direction={ResizeDirection.RIGHT} value={width} - minValue={minSidebarWidth} + minValue={minWidth} maxValue={getMaxSidebarWidth()} title="sidebar" />
); -} +}; export default ResizableSidebar; diff --git a/packages/compass-connections/src/stores/connections-store.ts b/packages/compass-connections/src/stores/connections-store.ts index 90a0bb51963..acec4137f3f 100644 --- a/packages/compass-connections/src/stores/connections-store.ts +++ b/packages/compass-connections/src/stores/connections-store.ts @@ -5,7 +5,7 @@ import type { ConnectionStorage, } from 'mongodb-data-service'; import { getConnectionTitle } from 'mongodb-data-service'; -import { useEffect, useReducer, useRef } from 'react'; +import { useEffect, useMemo, useReducer, useRef } from 'react'; import debugModule from 'debug'; import { cloneDeep } from 'lodash'; import { v4 as uuidv4 } from 'uuid'; @@ -214,6 +214,8 @@ export function useConnections({ appName: string; }): { state: State; + recentConnections: ConnectionInfo[]; + favoriteConnections: ConnectionInfo[]; cancelConnectionAttempt: () => void; connect: (connectionInfo: ConnectionInfo) => Promise; createNewConnection: () => void; @@ -236,6 +238,25 @@ export function useConnections({ const connectedConnectionInfo = useRef(); const connectedDataService = useRef(); + const { recentConnections, favoriteConnections } = useMemo(() => { + const favoriteConnections = state.connections + .filter((connectionInfo) => !!connectionInfo.favorite) + .sort((a, b) => { + return b.favorite!.name?.toLocaleLowerCase() < + a.favorite!.name?.toLocaleLowerCase() + ? 1 + : -1; + }); + + const recentConnections = connections + .filter((connectionInfo) => !connectionInfo.favorite) + .sort((a, b) => { + return (b.lastUsed?.getTime() ?? 0) - (a.lastUsed?.getTime() ?? 0); + }); + + return { recentConnections, favoriteConnections }; + }, [state.connections]); + async function saveConnectionInfo( connectionInfo: ConnectionInfo ): Promise { @@ -323,6 +344,8 @@ export function useConnections({ return { state, + recentConnections, + favoriteConnections, cancelConnectionAttempt() { connectionAttempt?.cancelConnectionAttempt(); From bffc63e2b850ffb223d578d19bad0836412fe5b0 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Fri, 11 Mar 2022 17:58:44 +0100 Subject: [PATCH 2/7] use recents and favorites directly --- .../compass-connections/src/components/connections.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/compass-connections/src/components/connections.tsx b/packages/compass-connections/src/components/connections.tsx index 4570ede1c5e..79109bffd52 100644 --- a/packages/compass-connections/src/components/connections.tsx +++ b/packages/compass-connections/src/components/connections.tsx @@ -74,6 +74,8 @@ function Connections({ removeAllRecentsConnections, removeConnection, saveConnection, + favoriteConnections, + recentConnections, } = useConnections({ onConnected, connectionStorage, connectFn, appName }); const { activeConnectionId, @@ -81,7 +83,6 @@ function Connections({ connectionAttempt, connectionErrorMessage, connectingStatusText, - connections, isConnected, } = state; @@ -98,8 +99,8 @@ function Connections({ > Date: Mon, 14 Mar 2022 14:46:18 +0100 Subject: [PATCH 3/7] feat: limit recent connections to 10 --- .../connection-list/connection-list.spec.tsx | 70 +++---- .../src/stores/connections-store.spec.ts | 175 +++++++++++++++++- .../src/stores/connections-store.ts | 62 ++++--- 3 files changed, 241 insertions(+), 66 deletions(-) diff --git a/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx b/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx index 6b9518b32c8..dc2c5a59ef5 100644 --- a/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx +++ b/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx @@ -3,6 +3,7 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { expect } from 'chai'; import sinon from 'sinon'; import type { ConnectionInfo } from 'mongodb-data-service'; +import _ from 'lodash'; import ConnectionList from './connection-list'; @@ -13,10 +14,11 @@ for (let i = 0; i < 5; i++) { connectionOptions: { connectionString: `mongodb://localhost:2${5000 + i}`, }, - lastUsed: new Date(Date.now() - (Date.now() / 2) * Math.random()), + lastUsed: new Date(1647022100487 - i), }); } -const mockConnections = [ + +const mockFavorites = [ { id: 'mock-connection-atlas', connectionOptions: { @@ -58,7 +60,6 @@ const mockConnections = [ }, lastUsed: new Date(), }, - ...mockRecents, ]; describe('ConnectionList Component', function () { @@ -72,8 +73,9 @@ describe('ConnectionList Component', function () { beforeEach(function () { render( true} @@ -89,39 +91,21 @@ describe('ConnectionList Component', function () { it('renders all of the connections in the lists', function () { const listItems = screen.getAllByRole('listitem'); - expect(listItems.length).to.equal(mockConnections.length); - }); - - it('favorites are alphabetically sorted', function () { - const listItems = screen.getAllByTestId('favorite-connection'); - expect(listItems.length).to.equal(3); + expect(listItems.length).to.equal( + mockFavorites.length + mockRecents.length + ); }); it('renders the favorite connections in a list', function () { const listItems = screen.getAllByTestId('favorite-connection-title'); - expect(listItems[0].textContent).to.equal('Atlas test'); - expect(listItems[1].textContent).to.equal('favorite'); - expect(listItems[2].textContent).to.equal( - 'super long favorite name - super long favorite name - super long favorite name - super long favorite name' - ); + expect(listItems[0].textContent).to.equal(mockFavorites[0].favorite.name); + expect(listItems[1].textContent).to.equal(mockFavorites[1].favorite.name); + expect(listItems[2].textContent).to.equal(mockFavorites[2].favorite.name); }); it('renders the recent connections in a list', function () { const listItems = screen.getAllByTestId('recent-connection'); - expect(listItems.length).to.equal(6); - }); - - it('renders the recent connections in most recent first order', function () { - const listItems = screen.getAllByTestId('recent-connection-description'); - expect( - new Date(listItems[0].textContent).getTime() - ).to.be.greaterThanOrEqual(new Date(listItems[1].textContent).getTime()); - expect( - new Date(listItems[1].textContent).getTime() - ).to.be.greaterThanOrEqual(new Date(listItems[2].textContent).getTime()); - expect( - new Date(listItems[2].textContent).getTime() - ).to.be.greaterThanOrEqual(new Date(listItems[3].textContent).getTime()); + expect(listItems.length).to.equal(mockRecents.length); }); }); @@ -129,8 +113,9 @@ describe('ConnectionList Component', function () { beforeEach(function () { render( true} @@ -160,8 +145,9 @@ describe('ConnectionList Component', function () { beforeEach(function () { render( true} @@ -172,7 +158,7 @@ describe('ConnectionList Component', function () { expect(setActiveConnectionIdSpy.called).to.equal(false); const button = screen - .getByText(mockConnections[1].favorite.name) + .getByText(mockFavorites[1].favorite.name) .closest('button'); fireEvent( button, @@ -195,8 +181,9 @@ describe('ConnectionList Component', function () { beforeEach(function () { render( true} @@ -207,9 +194,7 @@ describe('ConnectionList Component', function () { expect(setActiveConnectionIdSpy.called).to.equal(false); const button = screen - .getByText( - mockConnections[7].connectionOptions.connectionString.substr(10) - ) + .getByText(mockRecents[3].connectionOptions.connectionString.substr(10)) .closest('button'); fireEvent( button, @@ -233,8 +218,9 @@ describe('ConnectionList Component', function () { removeAllRecentsConnectionsSpy = sinon.spy(); render( true} removeAllRecentsConnections={removeAllRecentsConnectionsSpy} diff --git a/packages/compass-connections/src/stores/connections-store.spec.ts b/packages/compass-connections/src/stores/connections-store.spec.ts index 9c513d2c648..06bebdf8b8c 100644 --- a/packages/compass-connections/src/stores/connections-store.spec.ts +++ b/packages/compass-connections/src/stores/connections-store.spec.ts @@ -3,14 +3,30 @@ import { waitFor } from '@testing-library/react'; import type { RenderResult } from '@testing-library/react-hooks'; import { renderHook, act } from '@testing-library/react-hooks'; import sinon from 'sinon'; +import util from 'util'; import { useConnections } from './connections-store'; -import type { ConnectionStorage } from 'mongodb-data-service'; +import type { ConnectionInfo, ConnectionStorage } from 'mongodb-data-service'; const noop = (): any => { /* no-op */ }; +function createMockRecents(num: number): ConnectionInfo[] { + const mockRecents = []; + for (let i = 0; i < num; i++) { + mockRecents.push({ + id: `${i}`, + lastUsed: new Date(1647023468898 - i), + connectionOptions: { + connectionString: 'mongodb://localhost', + }, + }); + } + + return mockRecents; +} + const mockConnections = [ { id: 'turtle', @@ -75,6 +91,163 @@ describe('use-connections hook', function () { expect(loadAllSpyWithData.callCount).to.equal(1); expect(result.current.state.connections.length).to.equal(2); }); + + it('filters and sort favorites connections', async function () { + const connectionOptions = { + connectionString: 'mongodb://turtle', + }; + const loadAllSpyWithData = sinon.fake.resolves([ + { id: '1', favorite: { name: 'bcd' }, connectionOptions }, + { id: '2', lastUsed: new Date(), connectionOptions }, + { id: '3', favorite: { name: 'abc' }, connectionOptions }, + ]); + mockConnectionStorage.loadAll = loadAllSpyWithData; + + const { result } = renderHook(() => + useConnections({ + onConnected: noop, + connectionStorage: mockConnectionStorage, + connectFn: noop, + appName: 'Test App Name', + }) + ); + + // Wait for the async loading of connections to complete. + await waitFor(() => + expect(result.current.favoriteConnections).to.deep.equal([ + { id: '3', favorite: { name: 'abc' }, connectionOptions }, + { id: '1', favorite: { name: 'bcd' }, connectionOptions }, + ]) + ); + }); + + it('filters and sort recents connections', async function () { + const connectionOptions = { + connectionString: 'mongodb://turtle', + }; + const loadAllSpyWithData = sinon.fake.resolves([ + { id: '1', favorite: { name: 'bcd' }, connectionOptions }, + { id: '2', lastUsed: new Date(1647020087550), connectionOptions }, + { id: '3', favorite: { name: 'abc' }, connectionOptions }, + { id: '4', connectionOptions }, // very old recent connection without lastUsed + { id: '5', lastUsed: new Date(1647020087551), connectionOptions }, + ]); + mockConnectionStorage.loadAll = loadAllSpyWithData; + + const { result } = renderHook(() => + useConnections({ + onConnected: noop, + connectionStorage: mockConnectionStorage, + connectFn: noop, + appName: 'Test App Name', + }) + ); + + await waitFor(() => + expect(result.current.state.connections.length).to.equal(5) + ); + + expect(result.current.recentConnections).to.deep.equal([ + { id: '5', lastUsed: new Date(1647020087551), connectionOptions }, + { id: '2', lastUsed: new Date(1647020087550), connectionOptions }, + { id: '4', connectionOptions }, + ]); + }); + }); + + describe('#connect', function () { + const maxRecents = 10; + + it(`calls onConnected`, async function () { + const onConnected = sinon.spy(); + const { result } = renderHook(() => + useConnections({ + onConnected, + connectionStorage: mockConnectionStorage, + connectFn: () => Promise.resolve({} as any), + appName: 'Test App Name', + }) + ); + + await result.current.connect({ + id: 'new', + connectionOptions: { + connectionString: 'mongodb://new-recent', + }, + }); + + await waitFor(() => { + expect(onConnected).to.have.been.called; + }); + }); + + it(`truncates recents to ${maxRecents}`, async function () { + const mockRecents = createMockRecents(maxRecents); + + mockConnectionStorage.loadAll = () => Promise.resolve(mockRecents); + + const { result } = renderHook(() => + useConnections({ + onConnected: noop, + connectionStorage: mockConnectionStorage, + connectFn: () => Promise.resolve({} as any), + appName: 'Test App Name', + }) + ); + + // Wait for the async loading of connections to complete. + await waitFor(() => + expect(result.current.state.connections.length).to.equal( + mockRecents.length + ) + ); + + await result.current.connect({ + id: 'new', + connectionOptions: { + connectionString: 'mongodb://new-recent', + }, + }); + + await waitFor(() => { + // eslint-disable-next-line @typescript-eslint/unbound-method + expect(mockConnectionStorage.delete).to.have.been.calledWith( + mockRecents[mockRecents.length - 1] + ); + }); + }); + + it(`does not remove recent if there are less then ${maxRecents}`, async function () { + const mockRecents = createMockRecents(maxRecents - 1); + + mockConnectionStorage.loadAll = () => Promise.resolve(mockRecents); + + const onConnected = sinon.spy(); + const { result } = renderHook(() => + useConnections({ + onConnected, + connectionStorage: mockConnectionStorage, + connectFn: () => Promise.resolve({} as any), + appName: 'Test App Name', + }) + ); + + await result.current.connect({ + id: 'new', + connectionOptions: { + connectionString: 'mongodb://new-recent', + }, + }); + + // this may cause a false negative, but there is no other reliable way to + // test this case. It would fail eventually if the functionality is broken. + await util.promisify(setTimeout)(100); + + // eslint-disable-next-line @typescript-eslint/unbound-method + expect(mockConnectionStorage.delete).not.to.have.been.calledWith( + mockRecents[mockRecents.length - 1] + ); + }); }); describe('#saveConnection', function () { diff --git a/packages/compass-connections/src/stores/connections-store.ts b/packages/compass-connections/src/stores/connections-store.ts index acec4137f3f..3eed7d7f6a6 100644 --- a/packages/compass-connections/src/stores/connections-store.ts +++ b/packages/compass-connections/src/stores/connections-store.ts @@ -199,6 +199,7 @@ async function loadConnections( } } +const MAX_RECENT_CONNECTIONS_LENGTH = 10; export function useConnections({ onConnected, connectionStorage, @@ -239,19 +240,20 @@ export function useConnections({ const connectedDataService = useRef(); const { recentConnections, favoriteConnections } = useMemo(() => { - const favoriteConnections = state.connections + const favoriteConnections = (state.connections || []) .filter((connectionInfo) => !!connectionInfo.favorite) .sort((a, b) => { - return b.favorite!.name?.toLocaleLowerCase() < - a.favorite!.name?.toLocaleLowerCase() - ? 1 - : -1; + const aName = a.favorite?.name?.toLocaleLowerCase() || ''; + const bName = b.favorite?.name?.toLocaleLowerCase() || ''; + return bName < aName ? 1 : -1; }); - const recentConnections = connections + const recentConnections = (state.connections || []) .filter((connectionInfo) => !connectionInfo.favorite) .sort((a, b) => { - return (b.lastUsed?.getTime() ?? 0) - (a.lastUsed?.getTime() ?? 0); + const aTime = a.lastUsed?.getTime() ?? 0; + const bTime = b.lastUsed?.getTime() ?? 0; + return bTime - aTime; }); return { recentConnections, favoriteConnections }; @@ -287,6 +289,21 @@ export function useConnections({ } } + async function removeConnection(connectionInfo: ConnectionInfo) { + await connectionStorage.delete(connectionInfo); + dispatch({ + type: 'set-connections', + connections: connections.filter((conn) => conn.id !== connectionInfo.id), + }); + if (activeConnectionId === connectionInfo.id) { + const nextActiveConnection = createNewConnectionInfo(); + dispatch({ + type: 'set-active-connection', + connectionInfo: nextActiveConnection, + }); + } + } + async function onConnectSuccess( connectionInfo: ConnectionInfo, dataService: DataService @@ -304,6 +321,20 @@ export function useConnections({ ...cloneDeep(connectionInfoToBeSaved), lastUsed: new Date(), }); + + // remove the oldest recent connection if are adding a new one and + // there are already MAX_RECENT_CONNECTIONS_LENGTH recents. + // NOTE: there are edge cases that may lead to more than MAX_RECENT_CONNECTIONS_LENGTH + // to be saved (ie. concurrent run of Compass), + // however we accept it as long as the list of + // recents won't grow indefinitely. + if ( + !connectionInfoToBeSaved.favorite && + !connectionInfoToBeSaved.lastUsed && + recentConnections.length >= MAX_RECENT_CONNECTIONS_LENGTH + ) { + await removeConnection(recentConnections[recentConnections.length - 1]); + } } catch (err) { debug( `error occurred connection with id ${connectionInfo.id || ''}: ${ @@ -465,22 +496,7 @@ export function useConnections({ }); } }, - async removeConnection(connectionInfo: ConnectionInfo) { - await connectionStorage.delete(connectionInfo); - dispatch({ - type: 'set-connections', - connections: connections.filter( - (conn) => conn.id !== connectionInfo.id - ), - }); - if (activeConnectionId === connectionInfo.id) { - const nextActiveConnection = createNewConnectionInfo(); - dispatch({ - type: 'set-active-connection', - connectionInfo: nextActiveConnection, - }); - } - }, + removeConnection, async duplicateConnection(connectionInfo: ConnectionInfo) { const duplicate: ConnectionInfo = { ...cloneDeep(connectionInfo), From b4164ea8c7e64cd97dc1e97b86ac4d6c4c9e44e6 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Mon, 14 Mar 2022 14:54:22 +0100 Subject: [PATCH 4/7] removed unused dep --- .../src/components/connection-list/connection-list.spec.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx b/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx index dc2c5a59ef5..d4859ec4bb6 100644 --- a/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx +++ b/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx @@ -3,7 +3,6 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { expect } from 'chai'; import sinon from 'sinon'; import type { ConnectionInfo } from 'mongodb-data-service'; -import _ from 'lodash'; import ConnectionList from './connection-list'; From 9bcfe2c5730163d457dd1a50ad8ef15861d48328 Mon Sep 17 00:00:00 2001 From: mcasimir Date: Mon, 14 Mar 2022 18:48:44 +0100 Subject: [PATCH 5/7] fix tests --- .../connection-list/connection-list.spec.tsx | 9 +- .../src/stores/connections-store.spec.ts | 4 +- .../src/stores/connections-store.ts | 211 +++++++++--------- 3 files changed, 113 insertions(+), 111 deletions(-) diff --git a/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx b/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx index d4859ec4bb6..1f327873676 100644 --- a/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx +++ b/packages/compass-connections/src/components/connection-list/connection-list.spec.tsx @@ -1,5 +1,11 @@ import React from 'react'; -import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { + render, + screen, + fireEvent, + waitFor, + cleanup, +} from '@testing-library/react'; import { expect } from 'chai'; import sinon from 'sinon'; import type { ConnectionInfo } from 'mongodb-data-service'; @@ -68,6 +74,7 @@ describe('ConnectionList Component', function () { setActiveConnectionIdSpy = sinon.spy(); createNewConnectionSpy = sinon.spy(); }); + afterEach(cleanup); describe('when rendered', function () { beforeEach(function () { render( diff --git a/packages/compass-connections/src/stores/connections-store.spec.ts b/packages/compass-connections/src/stores/connections-store.spec.ts index 06bebdf8b8c..2ef639b881b 100644 --- a/packages/compass-connections/src/stores/connections-store.spec.ts +++ b/packages/compass-connections/src/stores/connections-store.spec.ts @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import { waitFor } from '@testing-library/react'; +import { waitFor, cleanup } from '@testing-library/react'; import type { RenderResult } from '@testing-library/react-hooks'; import { renderHook, act } from '@testing-library/react-hooks'; import sinon from 'sinon'; @@ -69,6 +69,8 @@ describe('use-connections hook', function () { }; }); + afterEach(cleanup); + describe('#loadConnections', function () { it('loads the connections from the connection storage', async function () { const loadAllSpyWithData = sinon.fake.resolves(mockConnections); diff --git a/packages/compass-connections/src/stores/connections-store.ts b/packages/compass-connections/src/stores/connections-store.ts index 3eed7d7f6a6..f1dcc4fdab3 100644 --- a/packages/compass-connections/src/stores/connections-store.ts +++ b/packages/compass-connections/src/stores/connections-store.ts @@ -5,7 +5,7 @@ import type { ConnectionStorage, } from 'mongodb-data-service'; import { getConnectionTitle } from 'mongodb-data-service'; -import { useEffect, useMemo, useReducer, useRef } from 'react'; +import { useCallback, useEffect, useMemo, useReducer, useRef } from 'react'; import debugModule from 'debug'; import { cloneDeep } from 'lodash'; import { v4 as uuidv4 } from 'uuid'; @@ -236,8 +236,6 @@ export function useConnections({ state; const connectingConnectionAttempt = useRef(); - const connectedConnectionInfo = useRef(); - const connectedDataService = useRef(); const { recentConnections, favoriteConnections } = useMemo(() => { const favoriteConnections = (state.connections || []) @@ -304,58 +302,53 @@ export function useConnections({ } } - async function onConnectSuccess( - connectionInfo: ConnectionInfo, - dataService: DataService - ) { - // After connecting and the UI is updated we notify the rest of Compass. - try { - onConnected(connectionInfo, dataService); + const onConnectSuccess = useCallback( + async (connectionInfo: ConnectionInfo, dataService: DataService) => { + // After connecting and the UI is updated we notify the rest of Compass. + try { + onConnected(connectionInfo, dataService); - // if a connection has been saved already we only want to update the lastUsed - // attribute, otherwise we are going to save the entire connection info. - const connectionInfoToBeSaved = - (await connectionStorage.load(connectionInfo.id)) ?? connectionInfo; + // if a connection has been saved already we only want to update the lastUsed + // attribute, otherwise we are going to save the entire connection info. + const connectionInfoToBeSaved = + (await connectionStorage.load(connectionInfo.id)) ?? connectionInfo; - await saveConnectionInfo({ - ...cloneDeep(connectionInfoToBeSaved), - lastUsed: new Date(), - }); + await saveConnectionInfo({ + ...cloneDeep(connectionInfoToBeSaved), + lastUsed: new Date(), + }); - // remove the oldest recent connection if are adding a new one and - // there are already MAX_RECENT_CONNECTIONS_LENGTH recents. - // NOTE: there are edge cases that may lead to more than MAX_RECENT_CONNECTIONS_LENGTH - // to be saved (ie. concurrent run of Compass), - // however we accept it as long as the list of - // recents won't grow indefinitely. - if ( - !connectionInfoToBeSaved.favorite && - !connectionInfoToBeSaved.lastUsed && - recentConnections.length >= MAX_RECENT_CONNECTIONS_LENGTH - ) { - await removeConnection(recentConnections[recentConnections.length - 1]); + // remove the oldest recent connection if are adding a new one and + // there are already MAX_RECENT_CONNECTIONS_LENGTH recents. + // NOTE: there are edge cases that may lead to more than MAX_RECENT_CONNECTIONS_LENGTH + // to be saved (ie. concurrent run of Compass), + // however we accept it as long as the list of + // recents won't grow indefinitely. + if ( + !connectionInfoToBeSaved.favorite && + !connectionInfoToBeSaved.lastUsed && + recentConnections.length >= MAX_RECENT_CONNECTIONS_LENGTH + ) { + await removeConnection( + recentConnections[recentConnections.length - 1] + ); + } + } catch (err) { + debug( + `error occurred connection with id ${connectionInfo.id || ''}: ${ + (err as Error).message + }` + ); } - } catch (err) { - debug( - `error occurred connection with id ${connectionInfo.id || ''}: ${ - (err as Error).message - }` - ); - } - } - - useEffect(() => { - if ( - isConnected && - connectedConnectionInfo.current && - connectedDataService.current - ) { - void onConnectSuccess( - connectedConnectionInfo.current, - connectedDataService.current - ); - } - }, [isConnected, onConnected]); + }, + [ + onConnected, + connectionStorage, + saveConnectionInfo, + removeConnection, + recentConnections, + ] + ); useEffect(() => { // Load connections after first render. @@ -373,76 +366,76 @@ export function useConnections({ }; }, []); - return { - state, - recentConnections, - favoriteConnections, - cancelConnectionAttempt() { - connectionAttempt?.cancelConnectionAttempt(); + const connect = async (connectionInfo: ConnectionInfo) => { + if (connectionAttempt || isConnected) { + // Ensure we aren't currently connecting. + return; + } - dispatch({ - type: 'cancel-connection-attempt', + const newConnectionAttempt = createConnectionAttempt(connectFn); + connectingConnectionAttempt.current = newConnectionAttempt; + + dispatch({ + type: 'attempt-connect', + connectingStatusText: `Connecting to ${getConnectionTitle( + connectionInfo + )}`, + connectionAttempt: newConnectionAttempt, + }); + + trackConnectionAttemptEvent(connectionInfo); + debug('connecting with connectionInfo', connectionInfo); + + try { + const connectionStringWithAppName = setAppNameParamIfMissing( + connectionInfo.connectionOptions.connectionString, + appName + ); + const newConnectionDataService = await newConnectionAttempt.connect({ + ...cloneDeep(connectionInfo.connectionOptions), + connectionString: connectionStringWithAppName, }); - }, - async connect(connectionInfo: ConnectionInfo) { - if (connectionAttempt || isConnected) { - // Ensure we aren't currently connecting. + connectingConnectionAttempt.current = undefined; + + if (!newConnectionDataService || newConnectionAttempt.isClosed()) { + // The connection attempt was cancelled. return; } - const newConnectionAttempt = createConnectionAttempt(connectFn); - connectingConnectionAttempt.current = newConnectionAttempt; + void onConnectSuccess(connectionInfo, newConnectionDataService); dispatch({ - type: 'attempt-connect', - connectingStatusText: `Connecting to ${getConnectionTitle( - connectionInfo - )}`, - connectionAttempt: newConnectionAttempt, + type: 'connection-attempt-succeeded', }); + trackNewConnectionEvent(connectionInfo, newConnectionDataService); + debug( + 'connection attempt succeeded with connection info', + connectionInfo + ); + } catch (error) { + connectingConnectionAttempt.current = undefined; + trackConnectionFailedEvent(connectionInfo, error as Error); + debug('connect error', error); - trackConnectionAttemptEvent(connectionInfo); - debug('connecting with connectionInfo', connectionInfo); - - try { - const connectionStringWithAppName = setAppNameParamIfMissing( - connectionInfo.connectionOptions.connectionString, - appName - ); - const newConnectionDataService = await newConnectionAttempt.connect({ - ...cloneDeep(connectionInfo.connectionOptions), - connectionString: connectionStringWithAppName, - }); - connectingConnectionAttempt.current = undefined; - - if (!newConnectionDataService || newConnectionAttempt.isClosed()) { - // The connection attempt was cancelled. - return; - } - - // Successfully connected. - connectedConnectionInfo.current = connectionInfo; - connectedDataService.current = newConnectionDataService; + dispatch({ + type: 'connection-attempt-errored', + connectionErrorMessage: (error as Error).message, + }); + } + }; - dispatch({ - type: 'connection-attempt-succeeded', - }); - trackNewConnectionEvent(connectionInfo, newConnectionDataService); - debug( - 'connection attempt succeeded with connection info', - connectionInfo - ); - } catch (error) { - connectingConnectionAttempt.current = undefined; - trackConnectionFailedEvent(connectionInfo, error as Error); - debug('connect error', error); + return { + state, + recentConnections, + favoriteConnections, + cancelConnectionAttempt() { + connectionAttempt?.cancelConnectionAttempt(); - dispatch({ - type: 'connection-attempt-errored', - connectionErrorMessage: (error as Error).message, - }); - } + dispatch({ + type: 'cancel-connection-attempt', + }); }, + connect, createNewConnection() { dispatch({ type: 'new-connection', From ab0a3790636db3d27977b75db1fb08fc160d647a Mon Sep 17 00:00:00 2001 From: mcasimir Date: Tue, 15 Mar 2022 15:23:27 +0100 Subject: [PATCH 6/7] promisify delay manually --- .../compass-connections/src/stores/connections-store.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/compass-connections/src/stores/connections-store.spec.ts b/packages/compass-connections/src/stores/connections-store.spec.ts index 2ef639b881b..22cae643295 100644 --- a/packages/compass-connections/src/stores/connections-store.spec.ts +++ b/packages/compass-connections/src/stores/connections-store.spec.ts @@ -3,7 +3,6 @@ import { waitFor, cleanup } from '@testing-library/react'; import type { RenderResult } from '@testing-library/react-hooks'; import { renderHook, act } from '@testing-library/react-hooks'; import sinon from 'sinon'; -import util from 'util'; import { useConnections } from './connections-store'; import type { ConnectionInfo, ConnectionStorage } from 'mongodb-data-service'; @@ -243,7 +242,7 @@ describe('use-connections hook', function () { // this may cause a false negative, but there is no other reliable way to // test this case. It would fail eventually if the functionality is broken. - await util.promisify(setTimeout)(100); + await new Promise((resolve) => setTimeout(resolve, 100)); // eslint-disable-next-line @typescript-eslint/unbound-method expect(mockConnectionStorage.delete).not.to.have.been.calledWith( From 3d4fffae09e84730e95c8473dfca1389fedd596b Mon Sep 17 00:00:00 2001 From: mcasimir Date: Thu, 17 Mar 2022 12:28:37 +0100 Subject: [PATCH 7/7] apply fixes --- packages/compass-connections/src/components/connections.tsx | 2 +- .../src/components/resizeable-sidebar.tsx | 5 +++-- packages/compass-connections/src/stores/connections-store.ts | 1 - packages/data-service/src/connection-storage.ts | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/compass-connections/src/components/connections.tsx b/packages/compass-connections/src/components/connections.tsx index 79109bffd52..e85968c06e3 100644 --- a/packages/compass-connections/src/components/connections.tsx +++ b/packages/compass-connections/src/components/connections.tsx @@ -48,7 +48,7 @@ const formContainerStyles = css({ }); const initialSidebarWidth = spacing[4] * 10 + spacing[2]; // 248px -const minSidebarWidth = spacing[4] * 7; // 168px +const minSidebarWidth = spacing[4] * 9; // 216px function Connections({ onConnected, diff --git a/packages/compass-connections/src/components/resizeable-sidebar.tsx b/packages/compass-connections/src/components/resizeable-sidebar.tsx index a6abc0d79fb..b4ffe8c6c9f 100644 --- a/packages/compass-connections/src/components/resizeable-sidebar.tsx +++ b/packages/compass-connections/src/components/resizeable-sidebar.tsx @@ -6,7 +6,7 @@ import { css, } from '@mongodb-js/compass-components'; -const listContainerStyles = css({ +const containerStyles = css({ display: 'flex', flexDirection: 'column', margin: 0, @@ -35,10 +35,11 @@ const ResizableSidebar = ({ return (
{children} diff --git a/packages/compass-connections/src/stores/connections-store.ts b/packages/compass-connections/src/stores/connections-store.ts index f1dcc4fdab3..b14adf1b490 100644 --- a/packages/compass-connections/src/stores/connections-store.ts +++ b/packages/compass-connections/src/stores/connections-store.ts @@ -304,7 +304,6 @@ export function useConnections({ const onConnectSuccess = useCallback( async (connectionInfo: ConnectionInfo, dataService: DataService) => { - // After connecting and the UI is updated we notify the rest of Compass. try { onConnected(connectionInfo, dataService); diff --git a/packages/data-service/src/connection-storage.ts b/packages/data-service/src/connection-storage.ts index f1860191cdb..7bec0cb0694 100644 --- a/packages/data-service/src/connection-storage.ts +++ b/packages/data-service/src/connection-storage.ts @@ -80,7 +80,7 @@ export class ConnectionStorage { log.error( mongoLogId(1_001_000_103), 'Connection Storage', - 'Failed to save connection, error while converting to model', + 'Failed to save connection', { message: (err as Error).message } ); @@ -113,7 +113,7 @@ export class ConnectionStorage { log.error( mongoLogId(1_001_000_104), 'Connection Storage', - 'Failed to delete connection, error while converting to model', + 'Failed to delete connection', { message: (err as Error).message } );