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 c63e221e437..d0c7521525e 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 @@ -93,8 +93,8 @@ export function getIsCollectionNameValid( const isDuplicate = namespacesWithoutCurrent.some( (ns) => - ns === `${toNS(namespace).database}.${collectionName}` || - ns === `${toNS(namespace).database}.${collectionName.trim()}` + ns.trim() === + `${toNS(namespace).database}.${collectionName.trim()}`.trim() ); return { @@ -155,6 +155,7 @@ const CollectionDrawerContent: React.FunctionComponent< { + expect(screen.getByLabelText('Name')).to.have.value('countries'); + }); + + userEvent.click(screen.getByLabelText(/delete collection/i)); + + await waitFor(() => { + expect(screen.queryByText('countries')).not.to.exist; + }); + expect(screen.queryByLabelText('Name')).to.not.exist; + + const modifiedCollection = selectCurrentModelFromState( + result.plugin.store.getState() + ).collections.find((coll) => { + return coll.ns === 'flights.countries'; + }); + + expect(modifiedCollection).to.be.undefined; + }); + it('should open and edit a collection name', async function () { const result = renderDrawer(); result.plugin.store.dispatch(selectCollection('flights.countries')); diff --git a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.tsx b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.tsx index b1f3530a80f..2e52603312b 100644 --- a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.tsx +++ b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.tsx @@ -9,6 +9,7 @@ import { import CollectionDrawerContent from './collection-drawer-content'; import RelationshipDrawerContent from './relationship-drawer-content'; import { + deleteCollection, deleteRelationship, selectCurrentModelFromState, } from '../../store/diagram'; @@ -36,11 +37,13 @@ type DiagramEditorSidePanelProps = { type: 'relationship' | 'collection'; label: string; } | null; + onDeleteCollection: (ns: string) => void; onDeleteRelationship: (rId: string) => void; }; function DiagramEditorSidePanel({ selectedItems, + onDeleteCollection, onDeleteRelationship, }: DiagramEditorSidePanelProps) { const { content, label, actions, handleAction } = useMemo(() => { @@ -53,8 +56,18 @@ function DiagramEditorSidePanel({ namespace={selectedItems.id} > ), - actions: [], - handleAction: () => {}, + actions: [ + { + action: 'delete', + label: 'Delete Collection', + icon: 'Trash' as const, + }, + ], + handleAction: (actionName: string) => { + if (actionName === 'delete') { + onDeleteCollection(selectedItems.id); + } + }, }; } @@ -79,7 +92,7 @@ function DiagramEditorSidePanel({ } return { content: null }; - }, [selectedItems, onDeleteRelationship]); + }, [selectedItems, onDeleteCollection, onDeleteRelationship]); if (!content) { return null; @@ -97,6 +110,7 @@ function DiagramEditorSidePanel({ { + return collection.ns === selected.id; + }); + + if (!doesCollectionExist) { + // TODO(COMPASS-9680): When the selected collection doesn't exist then we + // don't show any selection. We can get into this state with undo/redo. + return { + selectedItems: null, + }; + } + return { selectedItems: { ...selected, @@ -137,15 +165,16 @@ export default connect( } if (selected.type === 'relationship') { - const model = selectCurrentModelFromState(state); const relationship = model.relationships.find((relationship) => { return relationship.id === selected.id; }); if (!relationship) { - throw new Error( - 'Can not find corresponding relationship when rendering DiagramEditorSidePanel' - ); + // TODO(COMPASS-9680): When the selected relationship doesn't exist we don't + // show any selection. We can get into this state with undo/redo. + return { + selectedItems: null, + }; } return { @@ -157,6 +186,7 @@ export default connect( } }, { + onDeleteCollection: deleteCollection, onDeleteRelationship: deleteRelationship, } )(DiagramEditorSidePanel); 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 c80170e1a0d..cb2266dff78 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,10 @@ const EditSchemaVariants = z.discriminatedUnion('type', [ ns: z.string(), newPosition: z.tuple([z.number(), z.number()]), }), + z.object({ + type: z.literal('RemoveCollection'), + ns: z.string(), + }), z.object({ type: z.literal('RenameCollection'), fromNS: z.string(), diff --git a/packages/compass-data-modeling/src/store/diagram.ts b/packages/compass-data-modeling/src/store/diagram.ts index 927c088b8e6..c5ac862d958 100644 --- a/packages/compass-data-modeling/src/store/diagram.ts +++ b/packages/compass-data-modeling/src/store/diagram.ts @@ -269,15 +269,6 @@ const updateSelectedItemsFromAppliedEdit = ( } switch (edit.type) { - case 'RemoveRelationship': { - if ( - currentSelection?.type === 'relationship' && - currentSelection.id === edit.relationshipId - ) { - return null; - } - break; - } case 'RenameCollection': { if ( currentSelection?.type === 'collection' && @@ -534,6 +525,12 @@ export function deleteRelationship( }; } +export function deleteCollection( + ns: string +): DataModelingThunkAction { + return applyEdit({ type: 'RemoveCollection', ns }); +} + export function updateCollectionNote( ns: string, note: string @@ -591,6 +588,20 @@ function _applyEdit(edit: Edit, model?: StaticModel): StaticModel { }), }; } + case 'RemoveCollection': { + return { + ...model, + // Remove any relationships involving the collection being removed. + relationships: model.relationships.filter((r) => { + return !( + r.relationship[0].ns === edit.ns || r.relationship[1].ns === edit.ns + ); + }), + collections: model.collections.filter( + (collection) => collection.ns !== edit.ns + ), + }; + } case 'RenameCollection': { return { ...model, diff --git a/packages/compass-e2e-tests/helpers/selectors.ts b/packages/compass-e2e-tests/helpers/selectors.ts index 18369d9b11e..a2846b6cb77 100644 --- a/packages/compass-e2e-tests/helpers/selectors.ts +++ b/packages/compass-e2e-tests/helpers/selectors.ts @@ -1479,7 +1479,9 @@ export const DataModelsListItemActions = (diagramName: string) => `${DataModelsListItem(diagramName)} [aria-label="Show actions"]`; export const DataModelsListItemDeleteButton = `[data-action="delete"]`; export const DataModelAddRelationshipBtn = 'aria/Add relationship'; -export const DataModelNameInput = '//label[text()="Name"]'; +export const DataModelNameInputLabel = '//label[text()="Name"]'; +export const DataModelNameInput = + 'input[data-testid="data-model-collection-drawer-name-input"]'; export const DataModelRelationshipLocalCollectionSelect = '//label[text()="Local collection"]'; export const DataModelRelationshipLocalFieldSelect = @@ -1498,6 +1500,8 @@ export const DataModelCollectionRelationshipItem = (relationshipId: string) => `li[data-relationship-id="${relationshipId}"]`; export const DataModelCollectionRelationshipItemEdit = `[aria-label="Edit relationship"]`; export const DataModelCollectionRelationshipItemDelete = `[aria-label="Delete relationship"]`; +export const DataModelCollectionSidebarItemDelete = `[aria-label="Delete collection"]`; +export const DataModelCollectionSidebarItemDeleteButton = `[data-action="delete"]`; // Side drawer export const SideDrawer = `[data-testid="${getDrawerIds().root}"]`; 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 805a6f4f9e0..2733131a6ff 100644 --- a/packages/compass-e2e-tests/tests/data-modeling-tab.test.ts +++ b/packages/compass-e2e-tests/tests/data-modeling-tab.test.ts @@ -95,7 +95,10 @@ async function selectCollectionOnTheDiagram( // If the drawer is open, close it // Otherwise the drawer or the minimap can cover the collection node const drawer = browser.$(Selectors.SideDrawer); - if (await drawer.isDisplayed()) { + if ( + (await drawer.isDisplayed()) && + (await drawer.$(Selectors.SideDrawerCloseButton).isClickable()) + ) { await browser.clickVisible(Selectors.SideDrawerCloseButton); await drawer.waitForDisplayed({ reverse: true }); } @@ -109,7 +112,7 @@ async function selectCollectionOnTheDiagram( await drawer.waitForDisplayed(); const collectionName = await browser.getInputByLabel( - drawer.$(Selectors.DataModelNameInput) + browser.$(Selectors.SideDrawer).$(Selectors.DataModelNameInputLabel) ); expect(await collectionName.getValue()).to.equal(toNS(ns).collection); } @@ -439,8 +442,6 @@ describe('Data Modeling tab', function () { const text = data.text.toLowerCase(); - console.log(`Recognized PNG export text:`, text); - expect(text).to.include('testCollection-one'.toLowerCase()); expect(text).to.include('testCollection-two'.toLowerCase()); @@ -675,5 +676,62 @@ describe('Data Modeling tab', function () { 'testCollection-two' ); }); + + it('allows collection management via the sidebar', async function () { + const dataModelName = 'Test Edit Collection'; + await setupDiagram(browser, { + diagramName: dataModelName, + connectionName: DEFAULT_CONNECTION_NAME_1, + databaseName: 'test', + }); + + const dataModelEditor = browser.$(Selectors.DataModelEditor); + await dataModelEditor.waitForDisplayed(); + + // Click on the collection to open the drawer. + await selectCollectionOnTheDiagram(browser, 'test.testCollection-one'); + + const drawer = browser.$(Selectors.SideDrawer); + + // Rename the collection (it submits on unfocus). + await browser.setValueVisible( + browser.$(Selectors.DataModelNameInput), + 'testCollection-renamedOne' + ); + await drawer.click(); // Unfocus the input. + + // Verify that the renamed collection is still selected. + await browser.waitUntil(async () => { + const collectionName = await browser.getInputByLabel( + browser.$(Selectors.SideDrawer).$(Selectors.DataModelNameInputLabel) + ); + return ( + (await collectionName.getValue()) === 'testCollection-renamedOne' + ); + }); + + // Select the second collection and verify that the new name is in the diagram. + await selectCollectionOnTheDiagram(browser, 'test.testCollection-two'); + const nodes = await getDiagramNodes(browser, 2); + expect(nodes).to.have.lengthOf(2); + expect(nodes[0].id).to.equal('test.testCollection-renamedOne'); + expect(nodes[1].id).to.equal('test.testCollection-two'); + + // Remove the collection. + await drawer + .$(Selectors.DataModelCollectionSidebarItemDeleteButton) + .click(); + // Ensure the drawer closed. + if (await drawer.$(Selectors.DataModelNameInputLabel).isDisplayed()) { + await drawer + .$(Selectors.DataModelNameInputLabel) + .waitForDisplayed({ reverse: true }); + } + + // Verify that the collection is removed from the list and the diagram. + const nodesPostDelete = await getDiagramNodes(browser, 1); + expect(nodesPostDelete).to.have.lengthOf(1); + expect(nodesPostDelete[0].id).to.equal('test.testCollection-renamedOne'); + }); }); });