feat(compass-data-modeling): delete and rename field COMPASS-9659 (#7237)

Author: paula-stacho

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

@@ -36,6 +36,15 @@ const waitForDrawerToOpen = async () => {
   });
 };
 
+const updateInputWithBlur = (label: string, text: string) => {
+  const input = screen.getByLabelText(label);
+  userEvent.clear(input);
+  if (text.length) userEvent.type(input, text);
+
+  // Blur/unfocus the input.
+  userEvent.click(document.body);
+};
+
 async function comboboxSelectItem(
   label: string,
   value: string,
@@ -136,8 +145,8 @@ describe('DiagramEditorSidePanel', function () {
     ).to.be.visible;
   });
 
-  describe('Field context drawer', function () {
-    it('should render a field context drawer when field is clicked', async function () {
+  describe('When a field is selected', function () {
+    it('should render a field context drawer', async function () {
       const result = renderDrawer();
       result.plugin.store.dispatch(selectField('flights.airlines', ['alias']));
 
@@ -154,7 +163,7 @@ describe('DiagramEditorSidePanel', function () {
       expect(selectedTypes).to.include('int');
     });
 
-    it('should render a nested field context drawer when field is clicked', async function () {
+    it('should render a nested field context drawer', async function () {
       const result = renderDrawer();
       result.plugin.store.dispatch(
         selectField('flights.routes', ['airline', 'id'])
@@ -171,6 +180,87 @@ describe('DiagramEditorSidePanel', function () {
       expect(selectedTypes).to.have.lengthOf(1);
       expect(selectedTypes).to.include('string');
     });
+
+    it('should delete a field', async function () {
+      const result = renderDrawer();
+      result.plugin.store.dispatch(
+        selectField('flights.routes', ['airline', 'id'])
+      );
+
+      await waitForDrawerToOpen();
+      expect(screen.getByTitle('routes.airline.id')).to.be.visible;
+
+      userEvent.click(screen.getByLabelText(/delete field/i));
+
+      await waitFor(() => {
+        expect(screen.queryByText('routes.airline.id')).not.to.exist;
+      });
+      expect(screen.queryByLabelText('Name')).to.not.exist;
+
+      const modifiedCollection = selectCurrentModelFromState(
+        result.plugin.store.getState()
+      ).collections.find((coll) => {
+        return coll.ns === 'flights.routes';
+      });
+
+      expect(
+        modifiedCollection?.jsonSchema.properties?.airline.properties
+      ).to.not.have.property('id'); // deleted field
+      expect(
+        modifiedCollection?.jsonSchema.properties?.airline.properties
+      ).to.have.property('name'); // sibling field remains
+    });
+
+    it('should rename a field and keep it selected', async function () {
+      const result = renderDrawer();
+      result.plugin.store.dispatch(
+        selectField('flights.routes', ['airline', 'name'])
+      );
+
+      await waitForDrawerToOpen();
+      expect(screen.getByTitle('routes.airline.name')).to.be.visible;
+
+      updateInputWithBlur('Field name', 'new_name');
+
+      await waitFor(() => {
+        expect(screen.queryByText('routes.airline.name')).not.to.exist;
+        expect(screen.queryByText('routes.airline.new_name')).to.exist;
+      });
+    });
+
+    it('should not rename a field to an empty string', async function () {
+      const result = renderDrawer();
+      result.plugin.store.dispatch(
+        selectField('flights.routes', ['airline', 'name'])
+      );
+
+      await waitForDrawerToOpen();
+      expect(screen.getByTitle('routes.airline.name')).to.be.visible;
+
+      updateInputWithBlur('Field name', '');
+
+      await waitFor(() => {
+        expect(screen.queryByText('Field name cannot be empty.')).to.exist;
+        expect(screen.queryByText('routes.airline.name')).to.exist;
+      });
+    });
+
+    it('should not rename a field to a duplicate', async function () {
+      const result = renderDrawer();
+      result.plugin.store.dispatch(
+        selectField('flights.routes', ['airline', 'name'])
+      );
+
+      await waitForDrawerToOpen();
+      expect(screen.getByTitle('routes.airline.name')).to.be.visible;
+
+      updateInputWithBlur('Field name', 'id');
+
+      await waitFor(() => {
+        expect(screen.queryByText('Field already exists.')).to.exist;
+        expect(screen.queryByText('routes.airline.name')).to.exist;
+      });
+    });
   });
 
   it('should change the content of the drawer when selecting different items', async function () {
@@ -456,11 +546,7 @@ describe('DiagramEditorSidePanel', function () {
       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);
+      updateInputWithBlur('Name', 'pineapple');
 
       // Check the name in the model.
       const modifiedCollection = selectCurrentModelFromState(
@@ -486,11 +572,7 @@ describe('DiagramEditorSidePanel', function () {
       expect(activeElement).to.equal(nameInput);
 
       // Update the name.
-      userEvent.clear(nameInput);
-      userEvent.type(nameInput, 'pineapple');
-
-      // Blur/unfocus the input - now the collection should be names
-      userEvent.click(document.body);
+      updateInputWithBlur('Name', 'pineapple');
 
       // Check the name in the model.
       const newCollection = selectCurrentModelFromState(
@@ -516,7 +598,7 @@ describe('DiagramEditorSidePanel', function () {
         'false'
       );
 
-      userEvent.clear(screen.getByLabelText('Name'));
+      updateInputWithBlur('Name', '');
 
       await waitFor(() => {
         expect(screen.getByLabelText('Name')).to.have.attribute(
@@ -525,9 +607,6 @@ describe('DiagramEditorSidePanel', function () {
         );
       });
 
-      // Blur/unfocus the input.
-      userEvent.click(document.body);
-
       const notModifiedCollection = selectCurrentModelFromState(
         result.plugin.store.getState()
       ).collections.find((c: DataModelCollection) => {
@@ -549,8 +628,7 @@ describe('DiagramEditorSidePanel', function () {
         'false'
       );
 
-      userEvent.clear(screen.getByLabelText('Name'));
-      userEvent.type(screen.getByLabelText('Name'), 'airlines');
+      updateInputWithBlur('Name', 'airlines');
 
       await waitFor(() => {
         expect(screen.getByLabelText('Name')).to.have.attribute(
@@ -559,9 +637,6 @@ describe('DiagramEditorSidePanel', function () {
         );
       });
 
-      // Blur/unfocus the input.
-      userEvent.click(document.body);
-
       const notModifiedCollection = selectCurrentModelFromState(
         result.plugin.store.getState()
       ).collections.find((c: DataModelCollection) => {

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

@@ -12,13 +12,15 @@ import RelationshipDrawerContent from './relationship-drawer-content';
 import {
   deleteCollection,
   deleteRelationship,
+  removeField,
   selectCurrentModelFromState,
   type SelectedItems,
 } from '../../store/diagram';
 import { getDefaultRelationshipName } from '../../utils';
 import FieldDrawerContent from './field-drawer-content';
 import type { FieldPath } from '../../services/data-model-storage';
 import { getFieldFromSchema } from '../../utils/schema-traversal';
+import { isIdField } from '../../utils/utils';
 
 export const DATA_MODELING_DRAWER_ID = 'data-modeling-drawer';
 
@@ -109,7 +111,15 @@ function DiagramEditorSidePanel({
           ></FieldDrawerContent>
         ),
         actions: [
-          { action: 'delete', label: 'Delete', icon: 'Trash' as const },
+          ...(!isIdField(selectedItems.fieldPath)
+            ? [
+                {
+                  action: 'delete',
+                  label: 'Delete Field',
+                  icon: 'Trash' as const,
+                },
+              ]
+            : []),
         ],
         handleAction: (actionName: string) => {
           if (actionName === 'delete') {
@@ -241,6 +251,6 @@ export default connect(
   {
     onDeleteCollection: deleteCollection,
     onDeleteRelationship: deleteRelationship,
-    onDeleteField: () => {}, // TODO(COMPASS-9659) part 2 - implement onDeleteField,
+    onDeleteField: removeField,
   }
 )(DiagramEditorSidePanel);

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

@@ -13,7 +13,10 @@ import { BSONType } from 'mongodb';
 import {
   createNewRelationship,
   deleteRelationship,
-  selectCurrentModelFromState,
+  getCurrentDiagramFromState,
+  renameField,
+  selectCurrentModel,
+  selectFieldsForCurrentModel,
   selectRelationship,
 } from '../../store/diagram';
 import type { DataModelingState } from '../../store/reducer';
@@ -24,7 +27,11 @@ import {
 import { useChangeOnBlur } from './use-change-on-blur';
 import { RelationshipsSection } from './relationships-section';
 import { getFieldFromSchema } from '../../utils/schema-traversal';
-import { areFieldPathsEqual } from '../../utils/utils';
+import {
+  areFieldPathsEqual,
+  isIdField,
+  isRelationshipOfAField,
+} from '../../utils/utils';
 
 type FieldDrawerContentProps = {
   namespace: string;
@@ -44,7 +51,7 @@ type FieldDrawerContentProps = {
   onRenameField: (
     namespace: string,
     fromFieldPath: FieldPath,
-    toFieldPath: FieldPath
+    newName: string
   ) => void;
   onChangeFieldType: (
     namespace: string,
@@ -72,11 +79,23 @@ export function getIsFieldNameValid(
     };
   }
 
-  const fieldsNamesWithoutCurrent = existingFields
-    .filter((fieldPath) => !areFieldPathsEqual(fieldPath, currentFieldPath))
+  const siblingFields = existingFields
+    .filter(
+      (fieldPath) =>
+        // same level
+        fieldPath.length === currentFieldPath.length &&
+        // same path to that level
+        areFieldPathsEqual(
+          fieldPath.slice(0, fieldPath.length - 1),
+          currentFieldPath.slice(0, fieldPath.length - 1)
+        ) &&
+        // not the same field
+        fieldPath[fieldPath.length - 1] !==
+          currentFieldPath[currentFieldPath.length - 1]
+    )
     .map((fieldPath) => fieldPath[fieldPath.length - 1]);
 
-  const isDuplicate = fieldsNamesWithoutCurrent.some(
+  const isDuplicate = siblingFields.some(
     (fieldName) => fieldName === trimmedName
   );
 
@@ -108,10 +127,7 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
       if (!isFieldNameValid) {
         return;
       }
-      onRenameField(namespace, fieldPath, [
-        ...fieldPath.slice(0, fieldPath.length - 1),
-        trimmedName,
-      ]);
+      onRenameField(namespace, fieldPath, trimmedName);
     }
   );
 
@@ -131,7 +147,7 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
         <DMFormFieldContainer>
           <TextInput
             label="Field name"
-            disabled={true} // TODO: enable when field renaming is implemented
+            disabled={isIdField(fieldPath)}
             data-testid="data-model-collection-drawer-name-input"
             sizeVariant="small"
             value={fieldName}
@@ -181,35 +197,37 @@ export default connect(
     state: DataModelingState,
     ownProps: { namespace: string; fieldPath: FieldPath }
   ) => {
-    const model = selectCurrentModelFromState(state);
+    const diagram = getCurrentDiagramFromState(state);
+    const model = selectCurrentModel(diagram.edits);
+    const collectionSchema = model.collections.find(
+      (collection) => collection.ns === ownProps.namespace
+    )?.jsonSchema;
+    if (!collectionSchema) {
+      throw new Error('Collection not found');
+    }
     return {
       types:
         getFieldFromSchema({
-          jsonSchema:
-            model.collections.find(
-              (collection) => collection.ns === ownProps.namespace
-            )?.jsonSchema ?? {},
+          jsonSchema: collectionSchema,
           fieldPath: ownProps.fieldPath,
         })?.fieldTypes ?? [],
-      fieldPaths: [], // TODO(COMPASS-9659): get existing field paths
-      relationships: model.relationships.filter((r) => {
-        const [local, foreign] = r.relationship;
-        return (
-          (local.ns === ownProps.namespace &&
-            local.fields &&
-            areFieldPathsEqual(local.fields, ownProps.fieldPath)) ||
-          (foreign.ns === ownProps.namespace &&
-            foreign.fields &&
-            areFieldPathsEqual(foreign.fields, ownProps.fieldPath))
-        );
-      }),
+      fieldPaths: selectFieldsForCurrentModel(diagram.edits)[
+        ownProps.namespace
+      ],
+      relationships: model.relationships.filter(({ relationship }) =>
+        isRelationshipOfAField(
+          relationship,
+          ownProps.namespace,
+          ownProps.fieldPath
+        )
+      ),
     };
   },
   {
     onCreateNewRelationshipClick: createNewRelationship,
     onEditRelationshipClick: selectRelationship,
     onDeleteRelationshipClick: deleteRelationship,
-    onRenameField: () => {}, // TODO(COMPASS-9659): renameField,
+    onRenameField: renameField,
     onChangeFieldType: () => {}, // TODO(COMPASS-9659): updateFieldSchema,
   }
 )(FieldDrawerContent);

packages/compass-data-modeling/src/services/data-model-storage.ts

@@ -98,8 +98,8 @@ const EditSchemaVariants = z.discriminatedUnion('type', [
   z.object({
     type: z.literal('RenameField'),
     ns: z.string(),
-    from: FieldPathSchema,
-    to: FieldPathSchema,
+    field: FieldPathSchema,
+    newName: z.string(),
   }),
   z.object({
     type: z.literal('RemoveField'),