Skip to content

feat(data-modeling): add add field button to object in diagram COMPASS-9742 #7300

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.

Sign up for GitHub

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

Closed
wants to merge 15 commits into from
Closed

feat(data-modeling): add add field button to object in diagram COMPASS-9742 #7300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e8db58d
feat(data-modeling): add add field button to object in diagram COMPAS…
Anemy Sep 9, 2025
d23a8f1
fixup: e2e test drag node for clickability
Anemy Sep 11, 2025
d1f8d0a
fixup: proper distance calculation
Anemy Sep 11, 2025
4059b7a
fixup: remove .only ahhhhhh
Anemy Sep 11, 2025
de2d5fb
fixup: e2e test
Anemy Sep 17, 2025
0622b93
fixup: drag when drawer opens as well
Anemy Sep 17, 2025
a38fe21
fixup: bump diagramming and e2e test fix
Anemy Sep 18, 2025
52cb7b6
merge main
Anemy Sep 19, 2025
60b1e0a
fixup: fix dragging
Anemy Sep 19, 2025
76714f9
fixup: what a drag
Anemy Sep 19, 2025
b17f18a
merge main
Anemy Sep 19, 2025
ef9bbde
Update packages/compass-e2e-tests/tests/data-modeling-tab.test.ts
Anemy Sep 19, 2025
d9734f9
Update packages/compass-e2e-tests/tests/data-modeling-tab.test.ts
Anemy Sep 19, 2025
a4a0e3a
fixup: add wait for animations
Anemy Sep 19, 2025
9ec8c7c
fixup: package-lock, drag click
Anemy Sep 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
222 changes: 181 additions & 41 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion packages/compass-components/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ export { VisuallyHidden } from '@react-aria/visually-hidden';

export { openToast, closeToast, ToastArea } from './hooks/use-toast';

export { breakpoints, spacing } from '@leafygreen-ui/tokens';
export {
breakpoints,
spacing,
transitionDuration,
} from '@leafygreen-ui/tokens';
import IndexIcon from './components/index-icon';

export { default as FormFieldContainer } from './components/form-field-container';
Expand Down
2 changes: 1 addition & 1 deletion packages/compass-data-modeling/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"@mongodb-js/compass-user-data": "^0.10.0",
"@mongodb-js/compass-utils": "^0.9.15",
"@mongodb-js/compass-workspaces": "^0.57.0",
"@mongodb-js/diagramming": "^1.5.1",
"@mongodb-js/diagramming": "^1.6.0",
"bson": "^6.10.4",
"compass-preferences-model": "^2.55.0",
"html-to-image": "1.11.11",
Expand Down
7 changes: 7 additions & 0 deletions packages/compass-data-modeling/src/components/diagram-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { connect } from 'react-redux';
import type { DataModelingState } from '../store/reducer';
import {
addNewFieldToCollection,
onAddNestedField,
moveCollection,
selectCollection,
selectRelationship,
Expand Down Expand Up @@ -113,6 +114,7 @@ const DiagramContent: React.FunctionComponent<{
editErrors?: string[];
newCollection?: string;
onAddNewFieldToCollection: (ns: string) => void;
onAddNestedField: (ns: string, parentFieldPath: string[]) => void;
onMoveCollection: (ns: string, newPosition: [number, number]) => void;
onCollectionSelect: (namespace: string) => void;
onRelationshipSelect: (rId: string) => void;
Expand All @@ -133,6 +135,7 @@ const DiagramContent: React.FunctionComponent<{
isInRelationshipDrawingMode,
newCollection,
onAddNewFieldToCollection,
onAddNestedField,
onMoveCollection,
onCollectionSelect,
onRelationshipSelect,
Expand Down Expand Up @@ -183,6 +186,8 @@ const DiagramContent: React.FunctionComponent<{
: undefined,
onClickAddNewFieldToCollection: () =>
onAddNewFieldToCollection(coll.ns),
onClickAddNestedField: (parentFieldPath: string[]) =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing that the next field name logic is working fine and managed to get into some weird state where if I rename the field and then without unfocusing first I click on the "add new field" button, the new field is created with the name "interact" in the diagram, but the drawer shows the correct one, I can't figure out where the "interact" part is even coming from

Kapture 2025-09-12 at 12 26 07

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh, I was rewatching the gif and it seems to replace the next field in the document that is a sibling of an object this is being added for

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooo nice catch

Copy link
Member Author

@Anemy Anemy Sep 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking this is most likely caused by the same issue as COMPASS-9737 and COMPASS-9738. We're re-rendering while a state change is happening, and as we're supplying entirely new nodes to ReactFlow, which diagramming is using in an uncontrolled way, we have multiple race conditions starting to happen.

I'm working on a fix for those in that ticket, however I'm not sure yet if the fix will catch this issue. A user could initiate a rename field edit that renames a field that their subsequent click (i.e. add nested field) would conflict with, thus generating an edit that would be invalid after the state change they just made. For instance they could rename a field they are try to add a field to, which would create an edit on the old field name, as the visual state hasn't changed yet.

I'm thinking the first and catch all approach here is to for our error handling to disregard invalid edits.
We already have some rough validation for edits:

compass/packages/compass-data-modeling/src/store/diagram.ts

Line 571 in 7f8d06c

const { result: isValid, errors } = validateEdit(edit);

That's fairly general parsing, and we don't have as much in how the static model is impacted by the subsequent requests.
We could update how our error handling works around when an edit is applied. We would take into consideration that the edit's collection name, field path, or field type could be invalid or no longer exist. So before we actually apply the edit, without a change made, which is what it does in most situations currently, we instead would disregard or throw away the edit. I'm thinking for now we wouldn't notify the user, maybe a log though.
https://github.com/mongodb-js/compass/pull/7300/files#diff-e1ebded80983ea1ae9d6db540f7967e84df56cbe83888336b0a809a53a11d36bR10
Right now we just error (in this one case, there are a few more).

For instance right now we throw if the field path doesn't exist in getNewUnusedFieldName. Instead we update it to catch that error and log, and not apply the edit.

onAddNestedField(coll.ns, parentFieldPath),
selected,
isInRelationshipDrawingMode,
});
Expand All @@ -193,6 +198,7 @@ const DiagramContent: React.FunctionComponent<{
model?.relationships,
selectedItems,
isInRelationshipDrawingMode,
onAddNestedField,
]);

// Fit to view on initial mount
Expand Down Expand Up @@ -309,6 +315,7 @@ const ConnectedDiagramContent = connect(
},
{
onAddNewFieldToCollection: addNewFieldToCollection,
onAddNestedField: onAddNestedField,
onMoveCollection: moveCollection,
onCollectionSelect: selectCollection,
onRelationshipSelect: selectRelationship,
Expand Down
Loading
Loading