Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 5 additions & 1 deletion packages/compass-components/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,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
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

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:

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import React, { useCallback } from 'react';
import {
css,
cx,
focusRing,
palette,
spacing,
transitionDuration,
transparentize,
useDarkMode,
} from '@mongodb-js/compass-components';

import PlusWithSquare from '../icons/plus-with-square';

const objectTypeContainerStyles = css({
display: 'flex',
justifyContent: 'flex-end',
alignItems: 'center',
});

const iconButtonHoverStyles = css({
color: palette.gray.dark1,

'&::before': {
content: '""',
transition: `${transitionDuration.default}ms all ease-in-out`,
position: 'absolute',
top: 0,
bottom: 0,
left: 0,
right: 0,
borderRadius: '100%',
transform: 'scale(0.8)',
},

[`&:active:before,
&:hover:before,
&:focus:before,
&[data-hover='true']:before,
&[data-focus='true']:before`]: {
transform: 'scale(1)',
},

[`&:active,
&:hover,
&[data-hover='true'],
&:focus-visible,
&[data-focus='true']`]: {
color: palette.black,

'&::before': {
backgroundColor: transparentize(0.9, palette.gray.dark2),
},
},
});

const iconButtonHoverDarkModeStyles = css({
color: palette.gray.light1,

[`&:active,
&:hover,
&[data-hover='true'],
&:focus-visible,
&[data-focus='true']`]: {
color: palette.gray.light3,

'&::before': {
backgroundColor: transparentize(0.9, palette.gray.light2),
},
},
});

const addNestedFieldStyles = css(iconButtonHoverStyles, focusRing, {
background: 'none',
border: 'none',
padding: spacing[100],
margin: 0,
marginLeft: spacing[100],
cursor: 'pointer',
color: 'inherit',
display: 'flex',
});

type ObjectFieldTypeProps = {
onClickAddNestedField: (event: React.MouseEvent<HTMLButtonElement>) => void;
['data-testid']: string;
};

const ObjectFieldType: React.FunctionComponent<ObjectFieldTypeProps> = ({
'data-testid': dataTestId,
onClickAddNestedField: _onClickAddNestedField,
}) => {
const darkMode = useDarkMode();

const onClickAddNestedField = useCallback(
(event: React.MouseEvent<HTMLButtonElement>) => {
// Don't click on the field element.
event.stopPropagation();
_onClickAddNestedField(event);
},
[_onClickAddNestedField]
);

return (
<div className={objectTypeContainerStyles}>
{'{}'}
<button
className={cx(
addNestedFieldStyles,
darkMode && iconButtonHoverDarkModeStyles
)}
data-testid={dataTestId}
onClick={onClickAddNestedField}
aria-label="Add new field"
title="Add Field"
>
<PlusWithSquare />
</button>
</div>
);
};

export { ObjectFieldType };
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe('DiagramEditorSidePanel', function () {
expect(nameInput).to.be.visible;
expect(nameInput).to.have.value('alias');

const selectedTypes = getMultiComboboxValues('lg-combobox-datatype');
const selectedTypes = getMultiComboboxValues('field-type-combobox');
expect(selectedTypes).to.have.lengthOf(2);
expect(selectedTypes).to.include('string');
expect(selectedTypes).to.include('int');
Expand All @@ -190,7 +190,7 @@ describe('DiagramEditorSidePanel', function () {
expect(nameInput).to.be.visible;
expect(nameInput).to.have.value('_id');

const selectedTypes = getMultiComboboxValues('lg-combobox-datatype');
const selectedTypes = getMultiComboboxValues('field-type-combobox');
expect(selectedTypes).to.have.lengthOf(1);
expect(selectedTypes).to.include('string');
});
Expand Down Expand Up @@ -286,9 +286,7 @@ describe('DiagramEditorSidePanel', function () {
expect(screen.getByTitle('routes.airline.name')).to.be.visible;

// before - string
const selectedTypesBefore = getMultiComboboxValues(
'lg-combobox-datatype'
);
const selectedTypesBefore = getMultiComboboxValues('field-type-combobox');
expect(selectedTypesBefore).to.have.members(['string']);

// add int and bool and remove string
Expand Down Expand Up @@ -317,9 +315,7 @@ describe('DiagramEditorSidePanel', function () {
expect(screen.getByTitle('routes.airline.name')).to.be.visible;

// before - string
const selectedTypesBefore = getMultiComboboxValues(
'lg-combobox-datatype'
);
const selectedTypesBefore = getMultiComboboxValues('field-type-combobox');
expect(selectedTypesBefore).to.have.members(['string']);

// remove string without adding anything else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({

<DMFormFieldContainer>
<Combobox
data-testid="lg-combobox-datatype"
data-testid="field-type-combobox"
label="Datatype"
aria-label="Datatype"
disabled={isReadOnly}
Expand Down
32 changes: 32 additions & 0 deletions packages/compass-data-modeling/src/store/diagram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,38 @@ export function addNewFieldToCollection(
};
}

export function onAddNestedField(
ns: string,
parentFieldPath: string[]
): DataModelingThunkAction<void, ApplyEditAction | ApplyEditFailedAction> {
return (dispatch, getState) => {
const modelState = selectCurrentModelFromState(getState());

const collection = modelState.collections.find((c) => c.ns === ns);
if (!collection) {
throw new Error('Collection to add field to not found');
}

const edit: Omit<
Extract<Edit, { type: 'AddField' }>,
'id' | 'timestamp'
> = {
type: 'AddField',
ns,
// Use the first unique field name we can use.
field: [
...parentFieldPath,
getNewUnusedFieldName(collection.jsonSchema, parentFieldPath),
],
jsonSchema: {
bsonType: 'string',
},
};

return dispatch(applyEdit(edit));
};
}

export function moveCollection(
ns: string,
newPosition: [number, number]
Expand Down
Loading
Loading