fixup: better naming, add test for invalid state

Author: Anemy

packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx

@@ -116,7 +116,10 @@ const CollectionDrawerContent: React.FunctionComponent<
     () => toNS(namespace).collection
   );
 
-  const isCollectionNameValid = useMemo(
+  const {
+    isValid: isCollectionNameValid,
+    errorMessage: collectionNameEditErrorMessage,
+  } = useMemo(
     () => getIsCollectionNameValid(collectionName, namespaces, namespace),
     [collectionName, namespaces, namespace]
   );
@@ -131,20 +134,15 @@ const CollectionDrawerContent: React.FunctionComponent<
       return;
     }
 
-    if (!isCollectionNameValid.isValid) {
+    if (!isCollectionNameValid) {
       // Reset to previous valid name.
       // TODO: Maybe we don't reset.
       setCollectionName(toNS(namespace).collection);
       return;
     }
 
     onRenameCollection(namespace, `${toNS(namespace).database}.${trimmedName}`);
-  }, [
-    collectionName,
-    namespace,
-    onRenameCollection,
-    isCollectionNameValid.isValid,
-  ]);
+  }, [collectionName, namespace, onRenameCollection, isCollectionNameValid]);
 
   return (
     <>
@@ -154,8 +152,8 @@ const CollectionDrawerContent: React.FunctionComponent<
             label="Name"
             sizeVariant="small"
             value={collectionName}
-            state={isCollectionNameValid.isValid ? undefined : 'error'}
-            errorMessage={isCollectionNameValid.errorMessage}
+            state={isCollectionNameValid ? undefined : 'error'}
+            errorMessage={collectionNameEditErrorMessage}
             onChange={(e) => {
               setCollectionName(e.target.value);
             }}

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

@@ -261,4 +261,71 @@ describe('DiagramEditorSidePanel', function () {
 
     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 = selectCurrentModel(
+      getCurrentDiagramFromState(result.plugin.store.getState()).edits
+    ).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 = selectCurrentModel(
+      getCurrentDiagramFromState(result.plugin.store.getState()).edits
+    ).collections.find((c: DataModelCollection) => {
+      return c.ns === 'flights.countries';
+    });
+
+    expect(notModifiedCollection).to.exist;
+  });
 });