Skip to content

feat(data-modeling): add rename collection to side panel COMPASS-9658 #7174

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

Merged
merged 9 commits into from
Aug 11, 2025
Merged

feat(data-modeling): add rename collection to side panel COMPASS-9658 #7174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b63e03c
feat(data-modeling): add rename collection to side panel
Anemy Aug 7, 2025
975bb26
fixup: better naming, add test for invalid state
Anemy Aug 7, 2025
c483f82
merge main
Anemy Aug 7, 2025
15ad8fc
merge main
Anemy Aug 8, 2025
0a62d72
fixup: add selection update from edit case
Anemy Aug 8, 2025
19ca3c9
fixup: remove batch
Anemy Aug 8, 2025
e4d0f09
fixup: don't revert collection name on blur when invalid
Anemy Aug 8, 2025
e8c49a9
fixup: remove old comment
Anemy Aug 8, 2025
73976e8
fixup: tests
Anemy Aug 8, 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
2 changes: 2 additions & 0 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.

81 changes: 76 additions & 5 deletions packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import React, { useCallback, useLayoutEffect, useMemo, useState } from 'react';
import { connect } from 'react-redux';
import toNS from 'mongodb-ns';
import type { Relationship } from '../../services/data-model-storage';
import {
Badge,
Expand All @@ -15,6 +16,7 @@ import {
import {
createNewRelationship,
deleteRelationship,
renameCollection,
selectCurrentModelFromState,
selectRelationship,
updateCollectionNote,
Expand All @@ -29,12 +31,14 @@ import { useChangeOnBlur } from './use-change-on-blur';

type CollectionDrawerContentProps = {
namespace: string;
namespaces: string[];
note?: string;
relationships: Relationship[];
onCreateNewRelationshipClick: (namespace: string) => void;
onEditRelationshipClick: (rId: string) => void;
onDeleteRelationshipClick: (rId: string) => void;
note?: string;
onNoteChange: (namespace: string, note: string) => void;
onRenameCollection: (fromNS: string, toNS: string) => void;
};

const titleBtnStyles = css({
Expand Down Expand Up @@ -70,17 +74,77 @@ const relationshipContentStyles = css({
marginTop: spacing[400],
});

export function getIsCollectionNameValid(
collectionName: string,
namespaces: string[],
namespace: string
): {
isValid: boolean;
errorMessage?: string;
} {
if (collectionName.trim().length === 0) {
return {
isValid: false,
errorMessage: 'Collection name cannot be empty.',
};
}

const namespacesWithoutCurrent = namespaces.filter((ns) => ns !== namespace);

const isDuplicate = namespacesWithoutCurrent.some(
(ns) =>
ns === `${toNS(namespace).database}.${collectionName}` ||
ns === `${toNS(namespace).database}.${collectionName.trim()}`
Copy link
Preview

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The duplicate check logic is redundant - checking both trimmed and untrimmed versions when the second condition will always include the first. Consider simplifying to only check the trimmed version.

Suggested change
ns === `${toNS(namespace).database}.${collectionName}` ||

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean, obviously Copilot's suggestion behaves differently from the current logic, but I think you could actually condense this into a single condition if you want to, e.g.

      ns.trim() === `${toNS(namespace).database}.${collectionName.trim()}`.trim()

);

return {
isValid: !isDuplicate,
errorMessage: isDuplicate ? 'Collection name must be unique.' : undefined,
};
}

const CollectionDrawerContent: React.FunctionComponent<
CollectionDrawerContentProps
> = ({
namespace,
namespaces,
note = '',
relationships,
onCreateNewRelationshipClick,
onEditRelationshipClick,
onDeleteRelationshipClick,
note = '',
onNoteChange,
onRenameCollection,
}) => {
const [collectionName, setCollectionName] = useState(
() => toNS(namespace).collection
);

const {
isValid: isCollectionNameValid,
errorMessage: collectionNameEditErrorMessage,
} = useMemo(
() => getIsCollectionNameValid(collectionName, namespaces, namespace),
[collectionName, namespaces, namespace]
);

useLayoutEffect(() => {
setCollectionName(toNS(namespace).collection);
}, [namespace]);

const onBlurCollectionName = useCallback(() => {
const trimmedName = collectionName.trim();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just FYI, added a shared hook for that in another PR when adding notes #7171 (comment)

Copy link
Member Author

@Anemy Anemy Aug 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good mention, I was thinking about using this, however we have some additional logic around the invalid states. It felt like it would be passing around too many things or keep two collection name state around. Maybe there is something we could do for the validation to have it nicely wrapped in. Not going to include in this pr. This'll come up again in field renaming as well so we can take another look there.

if (trimmedName === toNS(namespace).collection) {
return;
}

if (!isCollectionNameValid) {
return;
}

onRenameCollection(namespace, `${toNS(namespace).database}.${trimmedName}`);
}, [collectionName, namespace, onRenameCollection, isCollectionNameValid]);

const noteInputProps = useChangeOnBlur(note, (newNote) => {
onNoteChange(namespace, newNote);
});
Expand All @@ -92,8 +156,13 @@ const CollectionDrawerContent: React.FunctionComponent<
<TextInput
label="Name"
sizeVariant="small"
value={namespace}
disabled={true}
value={collectionName}
state={isCollectionNameValid ? undefined : 'error'}
errorMessage={collectionNameEditErrorMessage}
onChange={(e) => {
setCollectionName(e.target.value);
}}
onBlur={onBlurCollectionName}
/>
</DMFormFieldContainer>
</DMDrawerSection>
Expand Down Expand Up @@ -175,6 +244,7 @@ export default connect(
model.collections.find((collection) => {
return collection.ns === ownProps.namespace;
})?.note ?? '',
namespaces: model.collections.map((c) => c.ns),
relationships: model.relationships.filter((r) => {
const [local, foreign] = r.relationship;
return (
Expand All @@ -188,5 +258,6 @@ export default connect(
onEditRelationshipClick: selectRelationship,
onDeleteRelationshipClick: deleteRelationship,
onNoteChange: updateCollectionNote,
onRenameCollection: renameCollection,
}
)(CollectionDrawerContent);
107 changes: 100 additions & 7 deletions packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import dataModel from '../../../test/fixtures/data-model-with-relationships.json';
import type {
MongoDBDataModelDescription,
DataModelCollection,
Relationship,
} from '../../services/data-model-storage';
import { DrawerAnchor } from '@mongodb-js/compass-components';
Expand Down Expand Up @@ -73,12 +74,12 @@ describe('DiagramEditorSidePanel', function () {
result.plugin.store.dispatch(selectCollection('flights.airlines'));

await waitFor(() => {
expect(screen.getByTitle('flights.airlines')).to.exist;
expect(screen.getByTitle('flights.airlines')).to.be.visible;
});

const nameInput = screen.getByLabelText('Name');
expect(nameInput).to.be.visible;
expect(nameInput).to.have.value('flights.airlines');
expect(nameInput).to.have.value('airlines');

userEvent.click(screen.getByRole('textbox', { name: 'Notes' }));
userEvent.type(
Expand Down Expand Up @@ -149,14 +150,14 @@ describe('DiagramEditorSidePanel', function () {
result.plugin.store.dispatch(selectCollection('flights.airlines'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.value('flights.airlines');
expect(screen.getByLabelText('Name')).to.have.value('airlines');
});

result.plugin.store.dispatch(
selectCollection('flights.airports_coordinates_for_schema')
);
expect(screen.getByLabelText('Name')).to.have.value(
'flights.airports_coordinates_for_schema'
'airports_coordinates_for_schema'
);

result.plugin.store.dispatch(
Expand All @@ -178,15 +179,15 @@ describe('DiagramEditorSidePanel', function () {
).to.be.visible;

result.plugin.store.dispatch(selectCollection('flights.planes'));
expect(screen.getByLabelText('Name')).to.have.value('flights.planes');
expect(screen.getByLabelText('Name')).to.have.value('planes');
});

it('should open and edit relationship starting from collection', async function () {
const result = renderDrawer();
result.plugin.store.dispatch(selectCollection('flights.countries'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.value('flights.countries');
expect(screen.getByLabelText('Name')).to.have.value('countries');
});

// Open relationshipt editing form
Expand Down Expand Up @@ -249,7 +250,7 @@ describe('DiagramEditorSidePanel', function () {
result.plugin.store.dispatch(selectCollection('flights.countries'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.value('flights.countries');
expect(screen.getByLabelText('Name')).to.have.value('countries');
});

// Find the relationhip item
Expand All @@ -270,4 +271,96 @@ describe('DiagramEditorSidePanel', function () {
.exist;
});
});

it('should open and edit a collection name', async function () {
const result = renderDrawer();
result.plugin.store.dispatch(selectCollection('flights.countries'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.value('countries');
});

// Update the name.
userEvent.clear(screen.getByLabelText('Name'));
userEvent.type(screen.getByLabelText('Name'), 'pineapple');

// Blur/unfocus the input.
userEvent.click(document.body);

// Check the name in the model.
const modifiedCollection = selectCurrentModelFromState(
result.plugin.store.getState()
).collections.find((c: DataModelCollection) => {
return c.ns === 'flights.pineapple';
});

expect(modifiedCollection).to.exist;
});

it('should prevent editing to an empty collection name', async function () {
const result = renderDrawer();
result.plugin.store.dispatch(selectCollection('flights.countries'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.value('countries');
expect(screen.getByLabelText('Name')).to.have.attribute(
'aria-invalid',
'false'
);
});

userEvent.clear(screen.getByLabelText('Name'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.attribute(
'aria-invalid',
'true'
);
});

// Blur/unfocus the input.
userEvent.click(document.body);

const notModifiedCollection = selectCurrentModelFromState(
result.plugin.store.getState()
).collections.find((c: DataModelCollection) => {
return c.ns === 'flights.countries';
});

expect(notModifiedCollection).to.exist;
});

it('should prevent editing to a duplicate collection name', async function () {
const result = renderDrawer();
result.plugin.store.dispatch(selectCollection('flights.countries'));

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.value('countries');
expect(screen.getByLabelText('Name')).to.have.attribute(
'aria-invalid',
'false'
);
});

userEvent.clear(screen.getByLabelText('Name'));
userEvent.type(screen.getByLabelText('Name'), 'airlines');

await waitFor(() => {
expect(screen.getByLabelText('Name')).to.have.attribute(
'aria-invalid',
'true'
);
});

// Blur/unfocus the input.
userEvent.click(document.body);

const notModifiedCollection = selectCurrentModelFromState(
result.plugin.store.getState()
).collections.find((c: DataModelCollection) => {
return c.ns === 'flights.countries';
});

expect(notModifiedCollection).to.exist;
});
});
5 changes: 5 additions & 0 deletions packages/compass-data-modeling/src/services/data-model-storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ const EditSchemaVariants = z.discriminatedUnion('type', [
ns: z.string(),
newPosition: z.tuple([z.number(), z.number()]),
}),
z.object({
type: z.literal('RenameCollection'),
fromNS: z.string(),
toNS: z.string(),
}),
z.object({
type: z.literal('UpdateCollectionNote'),
ns: z.string(),
Expand Down
Loading
Loading