feat(compass-data-modeling): delete and rename field COMPASS-9659

[paula-stacho]

Description

Screen.Recording.2025-08-27.at.13.13.43.mov

Includes:

• Rename and delete field actions

Does not include (will be follow ups)

• Type change
• E2E tests

Checklist

• New tests and/or benchmarks are included
• Documentation is changed or added
• If this change updates the UI, screenshots/videos are added and a design review is requested
• I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

• Bugfix
• New feature
• Dependency update
• Misc

Open Questions

Dependents

Types of changes

• Backport Needed
• Patch (non-breaking change which fixes an issue)
• Minor (non-breaking change which adds functionality)
• Major (fix or feature that would cause existing functionality to change)

[Copilot]

Pull Request Overview

This PR implements field deletion and renaming functionality for the Compass Data Modeling feature. It adds the ability to remove fields from schemas and rename existing fields while maintaining relationship consistency.

• Adds utility functions for field path comparison and relationship management
• Implements schema update operations for removing and renaming fields
• Updates the UI to enable field renaming and deletion actions

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/compass-data-modeling/src/utils/utils.ts	Adds utility functions for field path comparison and relationship detection
packages/compass-data-modeling/src/utils/utils.spec.tsx	Test coverage for new utility functions
packages/compass-data-modeling/src/utils/schema-traversal.tsx	Implements updateSchema function for field removal and renaming operations
packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx	Comprehensive test coverage for schema update operations
packages/compass-data-modeling/src/store/diagram.ts	Integrates field operations into the store with relationship updates
packages/compass-data-modeling/src/services/data-model-storage.ts	Updates RenameField schema to use field + newName instead of from/to
packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx	Enables field name input and connects rename functionality
packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.tsx	Adds field deletion action and updates action label
packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx	Test coverage for field deletion and renaming operations

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[gribnoysup]

Seems to work fine, I think we might want to disable root level _id field renaming and deleting before merging this, otherwise just a few nits.

[gribnoysup]

Nit: it's kinda peanuts here, so feel free to ignore, but you're running this in a loop over an arbitrary number of items, so the more cheap checks we can do to short circuit the expression, the better. Suggesting mostly as an example to keep in mind when writing code like that

Suggested change

	fieldPath.length === currentFieldPath.length &&
	areFieldPathsEqual(
	fieldPath.slice(0, fieldPath.length - 1),
	currentFieldPath.slice(0, fieldPath.length - 1)
	) &&
	!areFieldPathsEqual(fieldPath, currentFieldPath)
	fieldPath.length === currentFieldPath.length &&
	fieldPath[fieldPath.length - 1] !== currentFieldPath[currentFieldPath.length - 1] &&
	areFieldPathsEqual(
	fieldPath.slice(0, fieldPath.length - 1),
	currentFieldPath.slice(0, currentFieldPath.length - 1)
	)

[paula-stacho]

Thanks! The slice check is indeed better, but I'm not sure if the order needs to change. The 3rd check only eliminates fields with the same name, so unless there is a lot of field name repetition, both checks would be executed most of the time. The other check eliminates non-siblings, so it has a higher chance of reducing the number of checks.. even if it does get more expensive with a long path and high number of close relatives

[gribnoysup]

I'd suggest to make the interface of your component tighther here and only accept the actual data that you need inside the component and pick field paths for the parent of the field you're rendering this for, roughly something like:

Suggested change

	fieldPaths: extractFieldsFromSchema(collectionSchema),
	fieldPaths: Object.keys(getFieldFromSchema({jsonSchema: collectionSchema, fieldPath: ownProps.fieldPath.slice(0, ownProps.fieldPath - 1)}).properties),

You're only using these to validate whether or not the field is duplicated with another field, so precalculating this array of fields here makes the interface of the component clearer.

Just generally, from the perspective of how to think about component interfaces, I would strongly suggest to limit it to the absolute minimum of what the component needs. Start from component itself, define the bare minimum of what you need without connecting it to state first, then proceed to map state to required properties. This usually leads to way cleaner component inteface, usually more composable, and as a side-effect usually means that your connect functions actually have a chance of avoiding unnecessary re-renders (that's almost impossible when non primitive values are passed, so we're not going to get it here, but you'd generally get more simple values if that's how you approach this)

[gribnoysup]

Don't mind the runtime checks, but shouldn't this just be enforced by types? If we're renaming a field, new name should be required, right?

[gribnoysup]

I don't have anything to suggest here immediately, but it bothers me that we added some methods to traverse the schema and are not using them for that purpose adding more methods that all share some similar logic to traverse the schema slightly differently from the traverse method, means we didn't really get the interface we need from the one we added. Should probalby revisit this later

[paula-stacho]

Yeah, I originally planned to have one method, but then I thought that the method would get convoluted if it tried to achieve all the goals (full traverse, target retrieval or full retrieval after updating the target). But if we'll have time when we move this, I'm adding a bunch of unit tests so we can play around with refactors.