From 1479cc2f5202f22ce7cf15940ed5a26f97931d87 Mon Sep 17 00:00:00 2001 From: Le Roux Bodenstein Date: Thu, 28 Aug 2025 15:26:36 +0100 Subject: [PATCH 1/3] move the debug for me button to the connection error toast --- .../src/hooks/use-toast.tsx | 1 + .../connection-status-notifications.tsx | 175 ++++++++++-------- .../stores/connections-store-redux.spec.tsx | 5 +- .../src/stores/connections-store-redux.ts | 33 ++-- .../helpers/commands/connect.ts | 2 +- .../compass-e2e-tests/helpers/selectors.ts | 2 + .../tests/connection.test.ts | 50 +++-- packages/compass-web/src/entrypoint.spec.tsx | 2 +- 8 files changed, 151 insertions(+), 119 deletions(-) diff --git a/packages/compass-components/src/hooks/use-toast.tsx b/packages/compass-components/src/hooks/use-toast.tsx index 9463ae4a674..c650e59b224 100644 --- a/packages/compass-components/src/hooks/use-toast.tsx +++ b/packages/compass-components/src/hooks/use-toast.tsx @@ -17,6 +17,7 @@ export type ToastProperties = Pick< | 'timeout' | 'dismissible' | 'onClose' + | 'className' >; const defaultToastProperties: Partial = { diff --git a/packages/compass-connections/src/components/connection-status-notifications.tsx b/packages/compass-connections/src/components/connection-status-notifications.tsx index 8679c4ec581..51669ce8a06 100644 --- a/packages/compass-connections/src/components/connection-status-notifications.tsx +++ b/packages/compass-connections/src/components/connection-status-notifications.tsx @@ -8,6 +8,8 @@ import { spacing, openToast, closeToast, + Icon, + Button, } from '@mongodb-js/compass-components'; import type { ConnectionInfo } from '@mongodb-js/connection-info'; import { getConnectionTitle } from '@mongodb-js/connection-info'; @@ -38,74 +40,101 @@ export function getConnectingStatusText(connectionInfo: ConnectionInfo) { type ConnectionErrorToastBodyProps = { info?: ConnectionInfo | null; + error: Error; showReviewButton: boolean; + showDebugButton: boolean; onReview: () => void; + onDebug: () => void; }; +const connectionErrorToastStyles = css({ + // the gap on the right after the buttons takes up a lot of space from the + // description, so we remove it and add a little bit of margin elsewhere + gap: 0, + '[data-testid="lg-toast-content"] > div, [data-testid="lg-toast-content"] > div > p + p': + { + // don't cut off the glow of the button + overflow: 'visible', + }, +}); + const connectionErrorToastBodyStyles = css({ display: 'grid', gridAutoFlow: 'column', gap: spacing[200], }); -const connectionErrorToastActionMessageStyles = css({}); +const connectionErrorActionsStyles = css({ + display: 'flex', + flexDirection: 'column', + textAlign: 'right', + // replacing the gap with a margin so the button glow does not get cut off + marginRight: spacing[100], + gap: spacing[100], + justifyContent: 'center', +}); -const connectionErrorTextStyles = css({ - overflow: 'hidden', - textOverflow: 'ellipsis', +const connectionErrorStyles = css({ + display: 'flex', + flexDirection: 'column', +}); + +const connectionErrorTitleStyles = css({ + fontWeight: 'bold', +}); + +const debugActionStyles = css({ + display: 'flex', + alignItems: 'center', + gap: spacing[100], + justifyContent: 'right', + textWrap: 'nowrap', }); function ConnectionErrorToastBody({ info, + error, showReviewButton, + showDebugButton, onReview, + onDebug, }: ConnectionErrorToastBodyProps): React.ReactElement { return ( - - There was a problem connecting{' '} - {info ? `to ${getConnectionTitle(info)}` : ''} - - {info && showReviewButton && ( - + - REVIEW - - )} - - ); -} - -type ConnectionDebugToastBodyProps = { - onDebug: () => void; -}; - -function ConnectionDebugToastBody({ - onDebug, -}: ConnectionDebugToastBodyProps): React.ReactElement { - return ( - - - Diagnose the issue and explore solutions with the assistant + {info ? getConnectionTitle(info) : 'Connection failed'} + + {error.message} + + + {info && showReviewButton && ( + + + + )} + {info && showDebugButton && ( + + + + Debug for me + + + )} - - DEBUG FOR ME - ); } @@ -150,51 +179,50 @@ const openConnectionSucceededToast = (connectionInfo: ConnectionInfo) => { }); }; -const openConnectionFailedToast = ( +const openConnectionFailedToast = ({ + connectionInfo, + error, + showReviewButton, + showDebugButton, + onReviewClick, + onDebugClick, +}: { // Connection info might be missing if we failed connecting before we // could even resolve connection info. Currently the only case where this // can happen is autoconnect flow - connectionInfo: ConnectionInfo | null | undefined, - error: Error, - showReviewButton: boolean, - onReviewClick: () => void -) => { + connectionInfo: ConnectionInfo | null | undefined; + error: Error; + showReviewButton: boolean; + showDebugButton: boolean; + onReviewClick: () => void; + onDebugClick: () => void; +}) => { const failedToastId = connectionInfo?.id ?? 'failed'; - // TODO(COMPASS-9746): close the existing connection toast and make a new one - // for the failure so that the debug toast will appear below the failure one openToast(`connection-status--${failedToastId}`, { - title: error.message, + // we place the title inside the description to get the layout we need + title: '', description: ( { - closeToast(`connection-status--${failedToastId}`); + if (!showDebugButton) { + // don't close the toast if there are two actions so that the user + // can still use the other one + closeToast(`connection-status--${failedToastId}`); + } onReviewClick(); }} - /> - ), - variant: 'warning', - }); -}; - -const openDebugConnectionErrorToast = ( - connectionInfo: ConnectionInfo, - error: Error, - onDebugClick: () => void -) => { - openToast(`debug-connection-error--${connectionInfo.id}`, { - title: 'Need help debugging your connection error?', - description: ( - { - closeToast(`debug-connection-error--${connectionInfo.id}`); onDebugClick(); }} /> ), - variant: 'note', + variant: 'warning', + className: connectionErrorToastStyles, }); }; @@ -262,7 +290,6 @@ export function getNotificationTriggers() { openConnectionStartedToast, openConnectionSucceededToast, openConnectionFailedToast, - openDebugConnectionErrorToast, openMaximumConnectionsReachedToast, closeConnectionStatusToast: (connectionId: string) => { return closeToast(`connection-status--${connectionId}`); diff --git a/packages/compass-connections/src/stores/connections-store-redux.spec.tsx b/packages/compass-connections/src/stores/connections-store-redux.spec.tsx index 062ff51fb84..a7466be1262 100644 --- a/packages/compass-connections/src/stores/connections-store-redux.spec.tsx +++ b/packages/compass-connections/src/stores/connections-store-redux.spec.tsx @@ -150,7 +150,7 @@ describe('CompassConnections store', function () { } }); - it('should show debug toast in addition to the connection error toast if connection fails and the assistant is enabled', async function () { + it('should show debug action in addition to review if connection fails and the assistant is enabled', async function () { const { connectionsStore } = renderCompassConnections({ preferences: { enableAIAssistant: true, @@ -169,8 +169,7 @@ describe('CompassConnections store', function () { }); await waitFor(() => { - expect(screen.getByText('Need help debugging your connection error?')) - .to.exist; + expect(screen.getByText('Debug for me')).to.exist; }); }); diff --git a/packages/compass-connections/src/stores/connections-store-redux.ts b/packages/compass-connections/src/stores/connections-store-redux.ts index d1130df80a5..874bb49e4e8 100644 --- a/packages/compass-connections/src/stores/connections-store-redux.ts +++ b/packages/compass-connections/src/stores/connections-store-redux.ts @@ -1270,24 +1270,29 @@ const connectionAttemptError = ( _getState, { track, getExtraConnectionData, compassAssistant } ) => { - const { openConnectionFailedToast, openDebugConnectionErrorToast } = - getNotificationTriggers(); + const { openConnectionFailedToast } = getNotificationTriggers(); const isAssistanceEnabled = compassAssistant.getIsAssistantEnabled(); - if (isAssistanceEnabled && connectionInfo) { - openDebugConnectionErrorToast(connectionInfo, err, () => { - compassAssistant.interpretConnectionError({ - connectionInfo, - error: err, - }); - }); - } const showReviewButton = !!connectionInfo && !connectionInfo.atlasMetadata; - openConnectionFailedToast(connectionInfo, err, showReviewButton, () => { - if (connectionInfo) { - dispatch(editConnection(connectionInfo.id)); - } + openConnectionFailedToast({ + connectionInfo, + error: err, + showReviewButton, + showDebugButton: isAssistanceEnabled, + onReviewClick() { + if (connectionInfo) { + dispatch(editConnection(connectionInfo.id)); + } + }, + onDebugClick() { + if (connectionInfo) { + compassAssistant.interpretConnectionError({ + connectionInfo, + error: err, + }); + } + }, }); track( diff --git a/packages/compass-e2e-tests/helpers/commands/connect.ts b/packages/compass-e2e-tests/helpers/commands/connect.ts index ce5bc3f3050..7216a941e5b 100644 --- a/packages/compass-e2e-tests/helpers/commands/connect.ts +++ b/packages/compass-e2e-tests/helpers/commands/connect.ts @@ -170,7 +170,7 @@ export async function waitForConnectionResult( await browser .$(Selectors.ConnectionToastErrorText) .waitForDisplayed(waitOptions); - return browser.$(Selectors.LGToastTitle).getText(); + return browser.$(Selectors.ConnectionToastErrorText).getText(); } else { const exhaustiveCheck: never = connectionStatus; throw new Error(`Unhandled connectionStatus case: ${exhaustiveCheck}`); diff --git a/packages/compass-e2e-tests/helpers/selectors.ts b/packages/compass-e2e-tests/helpers/selectors.ts index 2b646478906..3ec1e087645 100644 --- a/packages/compass-e2e-tests/helpers/selectors.ts +++ b/packages/compass-e2e-tests/helpers/selectors.ts @@ -237,6 +237,8 @@ export const ConnectionModalSaveButton = '[data-testid="save-button"]'; export const connectionToastById = (connectionId: string) => { return `[data-testid="toast-connection-status--${connectionId}"]`; }; +export const ConnectionToastTitleText = + '[data-testid="connection-error-title"]'; export const ConnectionToastErrorText = '[data-testid="connection-error-text"]'; export const ConnectionToastErrorReviewButton = '[data-testid="connection-error-review"]'; diff --git a/packages/compass-e2e-tests/tests/connection.test.ts b/packages/compass-e2e-tests/tests/connection.test.ts index 97687c3b625..a86539edce5 100644 --- a/packages/compass-e2e-tests/tests/connection.test.ts +++ b/packages/compass-e2e-tests/tests/connection.test.ts @@ -273,9 +273,7 @@ describe('Connection string', function () { before(async function () { compass = await init(this.test?.fullTitle()); browser = compass.browser; - // TODO(COMPASS-9746) the debug toast obscures the connection toast which - // breaks the connect custom commands and who knows what else - //await browser.setFeature('enableAIAssistant', true) + await browser.setFeature('enableAIAssistant', true); }); beforeEach(async function () { @@ -312,15 +310,15 @@ describe('Connection string', function () { }); // check the error - const toastTitle = await browser.$(Selectors.LGToastTitle).getText(); - expect(toastTitle).to.equal('Authentication failed.'); + const connectionError = await browser + .$(Selectors.ConnectionToastErrorText) + .getText(); + expect(connectionError).to.equal('Authentication failed.'); const errorMessage = await browser .$(Selectors.ConnectionToastErrorText) .getText(); - expect(errorMessage).to.equal( - 'There was a problem connecting to 127.0.0.1:27091' - ); + expect(errorMessage).to.equal('Authentication failed.'); // click the review button in the toast await browser.clickVisible(Selectors.ConnectionToastErrorReviewButton); @@ -336,22 +334,22 @@ describe('Connection string', function () { .$(Selectors.ConnectionModal) .waitForDisplayed({ reverse: true }); - // TODO(COMPASS-9746) the toasts should be swapped around before this can work - /* await browser.clickVisible(Selectors.ConnectionToastErrorDebugButton); - const messagesElement = browser.$(Selectors.AssistantChatMessages) + // TODO(COMPASS-9759) we might have to opt-in via the modal once that's a thing + const messagesElement = browser.$(Selectors.AssistantChatMessages); await messagesElement.waitForDisplayed(); - // TODO(COMPASS-9744) check the response from the chatbot too - + // TODO(COMPASS-9748) check the response from the chatbot too + await browser.waitUntil(async () => { - return (await messagesElement.getText()).includes('Given the error message below,'); + return (await messagesElement.getText()).includes( + 'Given the error message below,' + ); }); // clear the chat so that a broken message doesn't break every future message await browser.clickVisible(Selectors.AssistantClearChatButton); await browser.clickVisible(Selectors.ConfirmClearChatModalConfirmButton); await browser.clickVisible(Selectors.SideDrawerCloseButton); - */ }); it('can connect to an Atlas replicaset without srv', async function () { @@ -1001,7 +999,7 @@ describe('Connection form', function () { const connections: { state: ConnectFormState; connectionId?: string; - connectionError: string; + toastErrorTitle: string; toastErrorText: string; }[] = [ { @@ -1011,16 +1009,16 @@ describe('Connection form', function () { defaultPassword: 'b', connectionName: connection1Name, }, - connectionError: 'Authentication failed.', - toastErrorText: `There was a problem connecting to ${connection1Name}`, + toastErrorTitle: connection1Name, + toastErrorText: `Authentication failed.`, }, { state: { hosts: ['127.0.0.1:16666'], connectionName: connection2Name, }, - connectionError: 'connect ECONNREFUSED 127.0.0.1:16666', - toastErrorText: `There was a problem connecting to ${connection2Name}`, + toastErrorTitle: connection2Name, + toastErrorText: `connect ECONNREFUSED 127.0.0.1:16666`, }, ]; @@ -1058,13 +1056,13 @@ describe('Connection form', function () { ); await browser.$(toastSelector).waitForDisplayed(); - // check the toast title - const toastTitle = await browser - .$(`${toastSelector} ${Selectors.LGToastTitle}`) + // check the title + const errorTitle = await browser + .$(`${toastSelector} ${Selectors.ConnectionToastTitleText}`) .getText(); - expect(toastTitle).to.equal(expected.connectionError); + expect(errorTitle).to.equal(expected.toastErrorTitle); - // check the toast body text + // check the text const errorMessage = await browser .$(`${toastSelector} ${Selectors.ConnectionToastErrorText}`) .getText(); @@ -1083,7 +1081,7 @@ describe('Connection form', function () { const errorText = await browser .$(Selectors.ConnectionFormErrorMessage) .getText(); - expect(errorText).to.equal(expected.connectionError); + expect(errorText).to.equal(expected.toastErrorText); const state = await browser.getConnectFormState(); expect(state.hosts).to.deep.equal(expected.state.hosts); diff --git a/packages/compass-web/src/entrypoint.spec.tsx b/packages/compass-web/src/entrypoint.spec.tsx index e574f38003f..efe43b17d21 100644 --- a/packages/compass-web/src/entrypoint.spec.tsx +++ b/packages/compass-web/src/entrypoint.spec.tsx @@ -115,7 +115,7 @@ describe('CompassWeb', function () { }) as any); await waitFor(() => { - screen.getByText('There was a problem connecting to localhost:27017'); + screen.getByText('Failed to connect'); }); expect(onTrackSpy).to.have.been.calledWith('Connection Failed'); From 26610398cf2b65fbe9616b9565def0b7b691d777 Mon Sep 17 00:00:00 2001 From: Le Roux Bodenstein Date: Thu, 28 Aug 2025 16:30:41 +0100 Subject: [PATCH 2/3] no assistant in web for now --- .../tests/connection.test.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/compass-e2e-tests/tests/connection.test.ts b/packages/compass-e2e-tests/tests/connection.test.ts index a86539edce5..33d31ba076b 100644 --- a/packages/compass-e2e-tests/tests/connection.test.ts +++ b/packages/compass-e2e-tests/tests/connection.test.ts @@ -334,22 +334,24 @@ describe('Connection string', function () { .$(Selectors.ConnectionModal) .waitForDisplayed({ reverse: true }); - await browser.clickVisible(Selectors.ConnectionToastErrorDebugButton); - // TODO(COMPASS-9759) we might have to opt-in via the modal once that's a thing - const messagesElement = browser.$(Selectors.AssistantChatMessages); - await messagesElement.waitForDisplayed(); - // TODO(COMPASS-9748) check the response from the chatbot too - - await browser.waitUntil(async () => { - return (await messagesElement.getText()).includes( - 'Given the error message below,' - ); - }); + if (!TEST_COMPASS_WEB) { + await browser.clickVisible(Selectors.ConnectionToastErrorDebugButton); + // TODO(COMPASS-9759) we might have to opt-in via the modal once that's a thing + const messagesElement = browser.$(Selectors.AssistantChatMessages); + await messagesElement.waitForDisplayed(); + // TODO(COMPASS-9748) check the response from the chatbot too + + await browser.waitUntil(async () => { + return (await messagesElement.getText()).includes( + 'Given the error message below,' + ); + }); - // clear the chat so that a broken message doesn't break every future message - await browser.clickVisible(Selectors.AssistantClearChatButton); - await browser.clickVisible(Selectors.ConfirmClearChatModalConfirmButton); - await browser.clickVisible(Selectors.SideDrawerCloseButton); + // clear the chat so that a broken message doesn't break every future message + await browser.clickVisible(Selectors.AssistantClearChatButton); + await browser.clickVisible(Selectors.ConfirmClearChatModalConfirmButton); + await browser.clickVisible(Selectors.SideDrawerCloseButton); + } }); it('can connect to an Atlas replicaset without srv', async function () { From 3b6d4f82cd56effdcf4772abf5ba23c1f472d8cf Mon Sep 17 00:00:00 2001 From: Le Roux Bodenstein Date: Thu, 28 Aug 2025 17:02:03 +0100 Subject: [PATCH 3/3] add ticket --- packages/compass-e2e-tests/tests/connection.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/compass-e2e-tests/tests/connection.test.ts b/packages/compass-e2e-tests/tests/connection.test.ts index 33d31ba076b..a7567c22b7a 100644 --- a/packages/compass-e2e-tests/tests/connection.test.ts +++ b/packages/compass-e2e-tests/tests/connection.test.ts @@ -334,6 +334,7 @@ describe('Connection string', function () { .$(Selectors.ConnectionModal) .waitForDisplayed({ reverse: true }); + // TODO(COMPASS-9768) this should work on compass web if (!TEST_COMPASS_WEB) { await browser.clickVisible(Selectors.ConnectionToastErrorDebugButton); // TODO(COMPASS-9759) we might have to opt-in via the modal once that's a thing