-
Notifications
You must be signed in to change notification settings - Fork 239
fix: proper schema traversal for addField COMPASS-9945 #7463
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
This PR refactors the field addition logic to use the same schema traversal mechanism as other schema update methods, ensuring consistent handling of nested and complex schema structures (arrays, mixed types, tuples).
Key changes:
- Replaces the recursive
addFieldToJSONSchemafunction with integration into the existingupdateSchematraversal system - Updates
getNewUnusedFieldNameto usegetFieldFromSchemaandgetDirectChildrenfor proper schema traversal - Adds comprehensive test coverage for field addition across various schema structures (nested objects, arrays, mixed types, tuples)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-data-modeling/src/utils/schema.ts | Removes addFieldToJSONSchema function and refactors getNewUnusedFieldName to use new traversal utilities |
| packages/compass-data-modeling/src/utils/schema.spec.ts | Removes tests for deleted function and adds tests for getNewUnusedFieldName with various schema structures |
| packages/compass-data-modeling/src/utils/schema-traversal.tsx | Adds addField operation support to updateSchema, implements helper functions for adding fields to different schema types, and adds getDirectChildren utility |
| packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx | Adds comprehensive test coverage for addField operation across flat, nested, array, tuple, and mixed type schemas |
| packages/compass-data-modeling/src/store/apply-edit.ts | Updates field addition to use updateSchema instead of deleted addFieldToJSONSchema function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Looks good!
Co-authored-by: Anna Henningsen <[email protected]>
Description
Integrating the field addition with the other schema update methods, so that it uses the same traversal mechanism.
Additionally fixing the name-fetching mechanism so that it aligns with what we consider a child.
Note:
The main goal at this point is to have a predictable and robust behaviour of adding a field:
+button on a field in diagram, a single new child will appearMongoDBSchemaMongoDBJSONSchemaIt's not always clear how exactly the resulting schema will look like (consider a field that has multiple object variants ... if the user clicks
+button we don't know where they wanna add it 🤷 ).With this in mind, we apply these heuristics:
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes