diff --git a/package-lock.json b/package-lock.json index e05c4ccb994..d9861d5694f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -45325,6 +45325,7 @@ "mongodb-build-info": "^1.7.2", "mongodb-connection-string-url": "^3.0.1", "mongodb-log-writer": "^2.3.4", + "mongodb-ns": "^2.4.2", "mongodb-runner": "^5.8.0", "node-fetch": "^2.7.0", "nyc": "^15.1.0", @@ -69112,6 +69113,7 @@ "mongodb-build-info": "^1.7.2", "mongodb-connection-string-url": "^3.0.1", "mongodb-log-writer": "^2.3.4", + "mongodb-ns": "^2.4.2", "mongodb-runner": "^5.8.0", "node-fetch": "^2.7.0", "nyc": "^15.1.0", diff --git a/packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx b/packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx index fe5d0522cd2..619c56298cc 100644 --- a/packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx +++ b/packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx @@ -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, @@ -15,6 +16,7 @@ import { import { createNewRelationship, deleteRelationship, + renameCollection, selectCurrentModelFromState, selectRelationship, updateCollectionNote, @@ -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({ @@ -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()}` + ); + + 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(); + 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); }); @@ -92,8 +156,13 @@ const CollectionDrawerContent: React.FunctionComponent< { + setCollectionName(e.target.value); + }} + onBlur={onBlurCollectionName} /> @@ -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 ( @@ -188,5 +258,6 @@ export default connect( onEditRelationshipClick: selectRelationship, onDeleteRelationshipClick: deleteRelationship, onNoteChange: updateCollectionNote, + onRenameCollection: renameCollection, } )(CollectionDrawerContent); diff --git a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx index 4e26d4af196..382e349b5b7 100644 --- a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx +++ b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx @@ -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'; @@ -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( @@ -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( @@ -178,7 +179,7 @@ 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 () { @@ -186,7 +187,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'); }); // Open relationshipt editing form @@ -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 @@ -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; + }); }); diff --git a/packages/compass-data-modeling/src/services/data-model-storage.ts b/packages/compass-data-modeling/src/services/data-model-storage.ts index 82ce6d3ba42..c80170e1a0d 100644 --- a/packages/compass-data-modeling/src/services/data-model-storage.ts +++ b/packages/compass-data-modeling/src/services/data-model-storage.ts @@ -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(), diff --git a/packages/compass-data-modeling/src/store/diagram.ts b/packages/compass-data-modeling/src/store/diagram.ts index 4b94f44ed58..927c088b8e6 100644 --- a/packages/compass-data-modeling/src/store/diagram.ts +++ b/packages/compass-data-modeling/src/store/diagram.ts @@ -180,7 +180,7 @@ export const diagramReducer: Reducer = ( }; } if (isAction(action, DiagramActionTypes.APPLY_EDIT)) { - const newState = { + return { ...state, edits: { prev: [...state.edits.prev, state.edits.current], @@ -189,17 +189,11 @@ export const diagramReducer: Reducer = ( }, editErrors: undefined, updatedAt: new Date().toISOString(), + selectedItems: updateSelectedItemsFromAppliedEdit( + state.selectedItems, + action.edit + ), }; - - if ( - action.edit.type === 'RemoveRelationship' && - state.selectedItems?.type === 'relationship' && - state.selectedItems.id === action.edit.relationshipId - ) { - newState.selectedItems = null; - } - - return newState; } if (isAction(action, DiagramActionTypes.APPLY_EDIT_FAILED)) { return { @@ -261,6 +255,46 @@ export const diagramReducer: Reducer = ( return state; }; +/** + * When an edit impacts the selected item we sometimes need to update + * the selection to reflect that, for instance when renaming a + * collection we update the selection `id` to the new name. + */ +const updateSelectedItemsFromAppliedEdit = ( + currentSelection: SelectedItems | null, + edit: Edit +): SelectedItems | null => { + if (!currentSelection) { + return currentSelection; + } + + switch (edit.type) { + case 'RemoveRelationship': { + if ( + currentSelection?.type === 'relationship' && + currentSelection.id === edit.relationshipId + ) { + return null; + } + break; + } + case 'RenameCollection': { + if ( + currentSelection?.type === 'collection' && + currentSelection.id === edit.fromNS + ) { + return { + type: 'collection', + id: edit.toNS, + }; + } + break; + } + } + + return currentSelection; +}; + export function selectCollection(namespace: string): CollectionSelectedAction { return { type: DiagramActionTypes.COLLECTION_SELECTED, namespace }; } @@ -344,6 +378,27 @@ export function moveCollection( return applyEdit(edit); } +export function renameCollection( + fromNS: string, + toNS: string +): DataModelingThunkAction< + void, + ApplyEditAction | ApplyEditFailedAction | CollectionSelectedAction +> { + return (dispatch) => { + const edit: Omit< + Extract, + 'id' | 'timestamp' + > = { + type: 'RenameCollection', + fromNS, + toNS, + }; + + dispatch(applyEdit(edit)); + }; +} + export function applyEdit( rawEdit: EditAction ): DataModelingThunkAction { @@ -536,6 +591,34 @@ function _applyEdit(edit: Edit, model?: StaticModel): StaticModel { }), }; } + case 'RenameCollection': { + return { + ...model, + // Update relationships to point to the renamed namespace. + relationships: model.relationships.map((relationship) => { + const [local, foreign] = relationship.relationship; + + return { + ...relationship, + relationship: [ + { + ...local, + ns: local.ns === edit.fromNS ? edit.toNS : local.ns, + }, + { + ...foreign, + ns: foreign.ns === edit.fromNS ? edit.toNS : foreign.ns, + }, + ], + }; + }), + collections: model.collections.map((collection) => ({ + ...collection, + // Rename the collection. + ns: collection.ns === edit.fromNS ? edit.toNS : collection.ns, + })), + }; + } case 'UpdateCollectionNote': { return { ...model, diff --git a/packages/compass-e2e-tests/package.json b/packages/compass-e2e-tests/package.json index b97bf7b43d0..a5ef583f18b 100644 --- a/packages/compass-e2e-tests/package.json +++ b/packages/compass-e2e-tests/package.json @@ -62,6 +62,7 @@ "mongodb-build-info": "^1.7.2", "mongodb-connection-string-url": "^3.0.1", "mongodb-log-writer": "^2.3.4", + "mongodb-ns": "^2.4.2", "mongodb-runner": "^5.8.0", "node-fetch": "^2.7.0", "nyc": "^15.1.0", diff --git a/packages/compass-e2e-tests/tests/data-modeling-tab.test.ts b/packages/compass-e2e-tests/tests/data-modeling-tab.test.ts index 161a15503c4..805a6f4f9e0 100644 --- a/packages/compass-e2e-tests/tests/data-modeling-tab.test.ts +++ b/packages/compass-e2e-tests/tests/data-modeling-tab.test.ts @@ -18,6 +18,7 @@ import { } from '../helpers/downloads'; import { readFileSync } from 'fs'; import { recognize } from 'tesseract.js'; +import toNS from 'mongodb-ns'; import path from 'path'; import os from 'os'; import fs from 'fs/promises'; @@ -110,7 +111,7 @@ async function selectCollectionOnTheDiagram( const collectionName = await browser.getInputByLabel( drawer.$(Selectors.DataModelNameInput) ); - expect(await collectionName.getValue()).to.equal(ns); + expect(await collectionName.getValue()).to.equal(toNS(ns).collection); } async function getDiagramNodes(