-
Couldn't load subscription status.
- Fork 240
feat(compass-collection): Add connection string to script screen CLOUDP-347558 #7470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a user-friendly Atlas connection string to metadata and surfaces it in the mock data generator script screen.
- Introduces atlasMetadata.userConnectionString and a fallback constant.
- Updates script generation to interpolate the connection string.
- Adds tests for both provided and fallback connection string display.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/connection-info/src/connection-info.ts | Adds userConnectionString field with doc comment. |
| packages/compass-generative-ai/src/atlas-ai-service.spec.ts | Updates mock ConnectionInfo to include new field. |
| packages/compass-collection/src/stores/collection-tab.spec.ts | Adds userConnectionString to mock atlas connection info. |
| packages/compass-collection/src/components/mock-data-generator-modal/script-screen.tsx | Makes mongosh command dynamic based on connection string with fallback. |
| packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx | Adds tests for userConnectionString and fallback behavior. |
| packages/compass-collection/src/components/mock-data-generator-modal/constants.ts | Defines DEFAULT_CONNECTION_STRING_FALLBACK constant. |
| packages/compass-collection/src/components/collection-header/collection-header.spec.tsx | Adds userConnectionString to mock metadata. |
| packages/compass-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx | Adds userConnectionString to mock metadata. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * User-friendly connection string for the Atlas cluster, which uses the SRV address if available. | ||
| * Otherwise, will default to <your-cluster> as a placeholder. |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc comment says the fallback defaults to , but the actual fallback used elsewhere is the full SRV form mongodb+srv://.mongodb.net/. Update the comment to reflect the exact expected format and clarify whether userConnectionString should always be a full connection string (including protocol and optional database) or just a host placeholder.
| * User-friendly connection string for the Atlas cluster, which uses the SRV address if available. | |
| * Otherwise, will default to <your-cluster> as a placeholder. | |
| * User-friendly connection string for the Atlas cluster, typically in the SRV format: | |
| * mongodb+srv://<your-cluster>.mongodb.net/<your-database> | |
| * If the SRV address is not available, the fallback is the full SRV form with placeholders: | |
| * mongodb+srv://<your-cluster>.mongodb.net/<your-database> | |
| * This property should always be a full connection string, including protocol and optional database. |
| const RUN_SCRIPT_COMMAND = ` | ||
| mongosh "mongodb+srv://<your-cluster>.mongodb.net/<your-database>" \\ | ||
| const RUN_SCRIPT_COMMAND = (connectionString: string) => ` | ||
| mongosh "${connectionString}" \\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this is definitely not going to already contain username/password or even placeholders for username/password?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, only the SRV address connection string here, without the username/password.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, but just in case you want some extra certainty for-almost-free, you can use
| mongosh "${connectionString}" \\ | |
| mongosh "${redactConnectionString(connectionString)}" \\ |
with
import { redactConnectionString } from 'mongodb-connection-string-url';That's a dependency we already freely use within Compass so it's basically an extra zero-cost safeguard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, good idea. Let me add this in.
| }; | ||
|
|
||
| export const DEFAULT_CONNECTION_STRING_FALLBACK = | ||
| 'mongodb+srv://<your-cluster>.mongodb.net/<your-database>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just want to confirm that this fallback with the <your-database> at the end is proper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically <your-cluster> alone as well isn't the best representation here of the connection string, since it'll be the SRV address, so something like Cluster0.AHFBW.mongodb.net for example. I took the existing placeholder and just refactored that here.
If we want something more accurate, we could just remove the placeholder and instead could just do '<srv-connection-string>' and have the user grab the connection string on their own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me!
| connectionInfo: atlasConnectionInfo, | ||
| fakerSchemaGeneration: { | ||
| status: 'completed', | ||
| fakerSchema: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge conflict fix:
| fakerSchema: { | |
| originalLlmResponse: { | |
| name: { | |
| fakerMethod: 'person.firstName', | |
| fakerArgs: [], | |
| probability: 1.0, | |
| mongoType: 'String', | |
| }, | |
| }, | |
| editedFakerSchema: { |
| connectionInfo: atlasConnectionInfoWithoutAtlasMetadata, | ||
| fakerSchemaGeneration: { | ||
| status: 'completed', | ||
| fakerSchema: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
| fakerSchema: { | |
| originalLlmResponse: { | |
| name: { | |
| fakerMethod: 'person.firstName', | |
| fakerArgs: [], | |
| probability: 1.0, | |
| mongoType: 'String', | |
| }, | |
| }, | |
| editedFakerSchema: { |
Description
Checklist
Motivation and Context
Types of changes