-
Notifications
You must be signed in to change notification settings - Fork 234
feat(compass-collection): CLOUDP-348131: Schema Editor Dropdowns: Add mongo type to faker method mapping #7425
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
base: main
Are you sure you want to change the base?
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
This PR implements a mongo type to faker method mapping system for schema editor dropdowns, enabling users to select appropriate faker methods based on MongoDB field types.
- Adds a comprehensive mapping of MongoDB types to corresponding Faker.js methods
- Implements dynamic dropdown population based on selected MongoDB type
- Includes dark mode compatibility fixes for schema display
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/compass-generative-ai/src/index.ts | Exports MongoDBFieldTypeValues enum for external use |
packages/compass-generative-ai/src/atlas-ai-service.ts | Makes MongoDBFieldTypeValues enum public by removing internal comment |
packages/compass-collection/src/components/mock-data-generator-modal/raw-schema-confirmation-screen.tsx | Adds dark mode support for document container styling |
packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor-screen.tsx | Implements automatic faker method selection when MongoDB type changes |
packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx | Replaces hardcoded dropdown options with dynamic mapping-based options |
packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.spec.tsx | Adds comprehensive test coverage for the new dropdown functionality |
packages/compass-collection/src/components/mock-data-generator-modal/constants.ts | Defines the mapping of MongoDB types to available Faker methods |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
import FakerMappingSelector from './faker-mapping-selector'; | ||
import type { FakerSchema, MockDataGeneratorState } from './types'; | ||
import type { MongoDBFieldType } from '@mongodb-js/compass-generative-ai'; | ||
import { getDefaultFakerMethod } from './script-generation-utils'; |
Copilot
AI
Oct 7, 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 import of getDefaultFakerMethod
from ./script-generation-utils
is added but this function is not defined or implemented in the visible changes. This will cause a compilation error.
import { getDefaultFakerMethod } from './script-generation-utils'; |
Copilot uses AI. Check for mistakes.
/** | ||
* Map of MongoDB types to available Faker methods. | ||
* Not all Faker methods are included here. | ||
* More can be found in the Faker.js API: https://fakerjs.dev/api/ |
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.
This is linking to faker v10, while we're currently on v9— maybe we should just take this opportunity to bump to v10. Think we could just update in package.json
: "@faker-js/faker": "^10.0.0"
, I think should be straightforward, only change I see on first pass would be internet.userName
-> internet.username
in this file
* Not all Faker methods are included here. | ||
* More can be found in the Faker.js API: https://fakerjs.dev/api/ | ||
*/ | ||
export const MONGO_TYPE_TO_FAKER_METHODS: Record< |
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.
Gave Augment link to https://v9.fakerjs.dev/api/ and had it come up with some more additions. (If you upgrade to v10 can do the same with the new API link):
String: [
// Current methods are good, but could add:
'internet.password', // For password fields
'internet.displayName', // For display names/usernames
'internet.emoji', // For emoji/reaction fields
'system.fileName', // For file name fields
'system.filePath', // For file path fields
'system.mimeType', // For content type fields
'book.title', // For title fields
'music.songName', // For song/media titles
'food.dish', // For food-related strings
'animal.type', // For animal/pet fields
'vehicle.model', // For vehicle/product models
'hacker.phrase', // For tech-related descriptions
'science.chemicalElement', // For scientific data
]
Number: [
// Current methods are good, but could add:
'number.binary', // For binary numbers
'number.octal', // For octal numbers
'number.hex', // For hexadecimal numbers
'commerce.price', // For price fields (returns number)
'date.weekday', // For day numbers
'internet.port', // For port numbers
]
...currentMapping, | ||
mongoType: newJsonType, | ||
fakerMethod: defaultFakerMethod, | ||
fakerArgs: [], // Reset args when changing type |
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.
Can we store these args so if the user re-selects the LLM-suggested method, they're re-displayed? I think we can store it in the state of faker-schema-editor-screen.tsx
with a useEffect
dependent on fakerSchema
, and then in onFakerFunctionSelect
restore original args if user reselects original LLM method
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.
Quick example code mock-up in this comment
const currentMapping = fakerSchemaFormValues[activeField]; | ||
if (currentMapping) { | ||
// When MongoDB type changes, update the faker method to a default for that type | ||
const defaultFakerMethod = getDefaultFakerMethod(newJsonType); |
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.
Similarly to my comment about faker args, can we persist the LLM-suggested faker method so that if the user changes the Mongo Type and then reselects the original type, we default to the LLM-suggested faker method? Like with the args, we can store in state.
Quick AI mock-up just as an example, please review ofc:
// Add this state near the other useState declarations (around line 61)
const [originalLlmData, setOriginalLlmData] = useState<Record<string, {
mongoType: MongoDBFieldType;
fakerMethod: string;
fakerArgs: any[];
}>>({});
// Add this useEffect after the existing state declarations
useEffect(() => {
const originalData = Object.fromEntries(
Object.entries(fakerSchema).map(([field, mapping]) => [
field,
{
mongoType: mapping.mongoType,
fakerMethod: mapping.fakerMethod,
fakerArgs: mapping.fakerArgs
}
])
);
setOriginalLlmData(originalData);
}, [fakerSchema]);
// Update the onJsonTypeSelect function (around line 73)
const onJsonTypeSelect = (newJsonType: MongoDBFieldType) => {
const currentMapping = fakerSchemaFormValues[activeField];
const originalData = originalLlmData[activeField];
if (currentMapping) {
// If switching back to original type, restore original LLM data
const shouldRestoreOriginal = originalData &&
newJsonType === originalData.mongoType &&
originalData.fakerMethod !== UNRECOGNIZED_FAKER_METHOD;
const fakerMethod = shouldRestoreOriginal
? originalData.fakerMethod
: getDefaultFakerMethod(newJsonType);
const fakerArgs = shouldRestoreOriginal
? originalData.fakerArgs
: [];
setFakerSchemaFormValues({
...fakerSchemaFormValues,
[activeField]: {
...currentMapping,
mongoType: newJsonType,
fakerMethod,
fakerArgs,
},
});
resetIsSchemaConfirmed();
}
};
// Update the onFakerFunctionSelect function (around line 91)
const onFakerFunctionSelect = (newFakerFunction: string) => {
const currentMapping = fakerSchemaFormValues[activeField];
const originalData = originalLlmData[activeField];
if (currentMapping) {
// Restore original args if user reselects original LLM method
const fakerArgs = (originalData?.fakerMethod === newFakerFunction)
? originalData.fakerArgs
: currentMapping.fakerArgs;
setFakerSchemaFormValues({
...fakerSchemaFormValues,
[activeField]: {
...currentMapping,
fakerMethod: newFakerFunction,
fakerArgs,
},
});
resetIsSchemaConfirmed();
}
};
}: Props) => { | ||
const fakerMethodOptions = useMemo(() => { | ||
const methods = | ||
MONGO_TYPE_TO_FAKER_METHODS[activeJsonType as MongoDBFieldType] || []; |
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.
Let's just type activeJsonType
as MongoDBFieldType
so we don't have to cast
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.
Yeah good idea. I see that we already type onJsonTypeSelect
with MongoDBFieldType
} from '@mongodb-js/testing-library-compass'; | ||
import sinon from 'sinon'; | ||
import FakerMappingSelector from './faker-mapping-selector'; | ||
import { UNRECOGNIZED_FAKER_METHOD } from '../../modules/collection-tab'; |
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.
[Drive-by suggestion]: This made me realize, due to parallel implementation there's a slight mismatch with script-generation-utils.ts
, we should change that file to use UNRECOGNIZED_FAKER_METHOD
instead of "unrecognized"
, if you could make that tweak it'd be much appreciated, I think it's just in this one spot (and then update tests accordingly)
css({ | ||
backgroundColor: isDarkMode ? palette.gray.dark3 : palette.gray.light3, | ||
border: `1px solid ${ | ||
isDarkMode ? palette.gray.dark2 : palette.gray.light2 |
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.
Awesome 😎
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.
Great fix. However, with this change, css()
is called on every render and it is kinda expensive wrt to performance. Within Compass, we usually have two different styles, light
and dark
and then in component we merge the two based on darkmode. Something like this:
isDarkMode ? palette.gray.dark2 : palette.gray.light2 | |
const documentContainerStyles = css({ | |
border: `1px solid ${palette.gray.light2}`, | |
... | |
}); | |
const documentContainerDarkStyles = css({ | |
border: `1px solid ${palette.gray.dark2}`, | |
}); | |
... | |
return ( | |
<div className={cx(documentContainerStyles, isDarkmode && documentContainerDarkStyles)} /> | |
); |
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.
Good catch. This works, but maybe we can just memoize the CSS object? That way, we don't need to have both a light and dark mode documentContainerStyles.
Object.entries(MONGO_TYPE_TO_FAKER_METHODS).forEach( | ||
([mongoType, methods]) => { |
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.
[nit]: this reads better
Object.entries(MONGO_TYPE_TO_FAKER_METHODS).forEach( | |
([mongoType, methods]) => { | |
for (const [mongoType, methods] of Object.entries(MONGO_TYPE_TO_FAKER_METHODS)) { | |
it(`should display faker methods for ${mongoType}`, () => {``` |
Description
CLOUDP-348131
Checklist
Motivation and Context
Types of changes
Screen.Recording.2025-10-06.at.11.26.02.AM.mov