feat(data-modeling): add remove collection to diagram drawer COMPASS-9658 (#7186)

Author: Anemy

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 {
@@ -151,6 +151,7 @@ const CollectionDrawerContent: React.FunctionComponent<
         <DMFormFieldContainer>
           <TextInput
             label="Name"
+            data-testid="data-model-collection-drawer-name-input"
             sizeVariant="small"
             value={collectionName}
             {...nameInputProps}

packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx

@@ -272,6 +272,30 @@ describe('DiagramEditorSidePanel', function () {
     });
   });
 
+  it('should delete a collection', async function () {
+    const result = renderDrawer();
+    result.plugin.store.dispatch(selectCollection('flights.countries'));
+
+    await waitFor(() => {
+      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'));

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}
           ></CollectionDrawerContent>
         ),
-        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({
           <ItemActionControls
             actions={actions}
             iconSize="small"
+            data-testid="data-modeling-drawer-actions"
             onAction={handleAction}
             className={drawerTitleActionGroupStyles}
             // Because the close button here is out of our control, we have do
@@ -127,7 +141,21 @@ export default connect(
       };
     }
 
+    const model = selectCurrentModelFromState(state);
+
     if (selected.type === 'collection') {
+      const doesCollectionExist = model.collections.find((collection) => {
+        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);

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(),

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<boolean, ApplyEditAction | ApplyEditFailedAction> {
+  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,

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}"]`;

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');
+    });
   });
 });