chore(compass-assistant): hide connection error in data explorer COMPASS-9920 #7694
Conversation
This seems like the simplest way to hide it... That said I understand we don't want this kind of DE vs Compass logic and am open to alternatives.
gagik
requested review from
Copilot and
tculig
and removed request for
Copilot
gagik
changed the title
gagik
changed the title
There was a problem hiding this comment.
Pull request overview
This PR hides the connection error entry point for the MongoDB Assistant in Data Explorer while keeping it available in Compass. The change prevents the "Debug" button from appearing on connection errors in Data Explorer.
Changes:
- Conditionally exclude
interpretConnectionErrorwhen running in Data Explorer - Add optional chaining and guards to handle the undefined
interpretConnectionErrorfunction - Skip E2E tests for connection error entry points when running in web/Data Explorer mode
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/compass-assistant/src/compass-assistant-provider.tsx | Makes interpretConnectionError undefined for Data Explorer by checking appNameForPrompt |
| packages/compass-connections/src/stores/connections-store-redux.ts | Adds optional chaining and existence checks for interpretConnectionError before use |
| packages/compass-assistant/src/compass-assistant-provider.spec.tsx | Adds test coverage for Data Explorer not having interpretConnectionError |
| packages/compass-e2e-tests/tests/assistant.test.ts | Skips connection error entry point tests in web/Data Explorer environment |
| // interpretConnectionError is not available in Data Explorer | ||
| interpretConnectionError: | ||
| appNameForPrompt === APP_NAMES_FOR_PROMPT.DataExplorer | ||
| ? undefined | ||
| : createEntryPointHandler( |
There was a problem hiding this comment.
The comment states 'Data Explorer' but the code checks against APP_NAMES_FOR_PROMPT.DataExplorer. Consider updating the comment to reference the constant for consistency and to make it clear which exact value is being checked.
| connectionInfo | ||
| ? () => { | ||
| compassAssistant.interpretConnectionError({ | ||
| compassAssistant.interpretConnectionError?.({ |
There was a problem hiding this comment.
This optional chaining is redundant since the condition on lines 1287-1289 already checks for interpretConnectionError existence. The function call is guaranteed to be defined at this point, so the ?. operator is unnecessary.
| compassAssistant.interpretConnectionError?.({ | |
| compassAssistant.interpretConnectionError({ |
|
We can probably close this now that we have #7757 ? |
2 participants
This seems like the simplest way to hide it... That said I understand we don't want this kind of DE vs Compass logic and am open to alternatives.