add more tests

Author: paula-stacho

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

@@ -204,10 +204,10 @@ describe('DiagramEditorSidePanel', function () {
       });
 
       expect(
-        modifiedCollection.jsonSchema.properties.airline.properties
+        modifiedCollection?.jsonSchema.properties?.airline.properties
       ).to.not.have.property('id'); // deleted field
       expect(
-        modifiedCollection.jsonSchema.properties.airline.properties
+        modifiedCollection?.jsonSchema.properties?.airline.properties
       ).to.have.property('name'); // sibling field remains
     });
 

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: z.string(),
+    field: FieldPathSchema,
+    newName: z.string(),
   }),
   z.object({
     type: z.literal('RemoveField'),

packages/compass-data-modeling/src/store/diagram.ts

@@ -32,7 +32,7 @@ import toNS from 'mongodb-ns';
 import { traverseSchema, updateSchema } from '../utils/schema-traversal';
 import {
   isRelationshipInvolvingField,
-  isSameFieldOrChild,
+  isSameFieldOrAncestor,
 } from '../utils/utils';
 
 function isNonEmptyArray<T>(arr: T[]): arr is [T, ...T[]] {
@@ -391,7 +391,10 @@ const updateSelectedItemsFromAppliedEdit = (
       return {
         type: 'field',
         namespace: edit.ns,
-        fieldPath: [...edit.from.slice(0, edit.from.length - 1), edit.to],
+        fieldPath: [
+          ...edit.field.slice(0, edit.field.length - 1),
+          edit.newName,
+        ],
       };
     }
   }
@@ -681,7 +684,7 @@ export function renameField(
   field: FieldPath,
   newName: string
 ): DataModelingThunkAction<boolean, ApplyEditAction | ApplyEditFailedAction> {
-  return applyEdit({ type: 'RenameField', ns, from: field, to: newName });
+  return applyEdit({ type: 'RenameField', ns, field, newName });
 }
 
 function getPositionForNewCollection(
@@ -765,16 +768,16 @@ function renameFieldInRelationshipSide(
   if (
     side.ns !== edit.ns ||
     !side.fields ||
-    !isSameFieldOrChild(side.fields, edit.from)
+    !isSameFieldOrAncestor(edit.field, side.fields)
   ) {
     return side;
   }
   return {
     ...side,
     fields: [
-      ...side.fields.slice(0, edit.from.length - 1),
-      edit.to,
-      ...side.fields.slice(edit.from.length),
+      ...side.fields.slice(0, edit.field.length - 1),
+      edit.newName,
+      ...side.fields.slice(edit.field.length),
     ],
   };
 }
@@ -927,7 +930,7 @@ function _applyEdit(edit: Edit, model?: StaticModel): StaticModel {
         // Update any relationships involving the field being renamed.
         relationships: model.relationships.map((r) => {
           if (
-            !isRelationshipInvolvingField(r.relationship, edit.ns, edit.from)
+            !isRelationshipInvolvingField(r.relationship, edit.ns, edit.field)
           ) {
             return r;
           }
@@ -945,9 +948,9 @@ function _applyEdit(edit: Edit, model?: StaticModel): StaticModel {
             ...collection,
             jsonSchema: updateSchema({
               jsonSchema: collection.jsonSchema,
-              fieldPath: edit.from,
+              fieldPath: edit.field,
               update: 'renameField',
-              newFieldName: edit.to,
+              newFieldName: edit.newName,
             }),
           };
         }),

packages/compass-data-modeling/src/utils/utils.ts

@@ -7,18 +7,20 @@ export function areFieldPathsEqual(
   return JSON.stringify(fieldA) === JSON.stringify(fieldB);
 }
 
-export function isSameFieldOrChild(
-  fieldA: FieldPath,
-  fieldB: FieldPath
+export function isSameFieldOrAncestor(
+  ancestor: FieldPath,
+  child: FieldPath
 ): boolean {
-  if (fieldA.length === fieldB.length)
-    return areFieldPathsEqual(fieldA, fieldB);
-  if (fieldA.length < fieldB.length) return false;
-  // fieldA is shorter than fieldB, check if fieldA is a parent of fieldB
-  const pathA = JSON.stringify(fieldA);
-  const pathB = JSON.stringify(fieldB);
-  // ignore the closing bracket - last character in pathB
-  return pathA.slice(0, pathB.length - 1) === pathB.slice(0, pathB.length - 1);
+  if (ancestor.length === child.length)
+    return areFieldPathsEqual(ancestor, child);
+  if (ancestor.length > child.length) return false;
+  const pathAncestor = JSON.stringify(ancestor);
+  const pathChild = JSON.stringify(child);
+  // ignore the last character - closing bracket
+  return (
+    pathAncestor.slice(0, pathAncestor.length - 1) ===
+    pathChild.slice(0, pathAncestor.length - 1)
+  );
 }
 
 export function isRelationshipOfAField(
@@ -29,10 +31,10 @@ export function isRelationshipOfAField(
   const [local, foreign] = relationship;
   return (
     (local.ns === namespace &&
-      local.fields !== null &&
+      !!local.fields &&
       areFieldPathsEqual(local.fields, fieldPath)) ||
     (foreign.ns === namespace &&
-      foreign.fields !== null &&
+      !!foreign.fields &&
       areFieldPathsEqual(foreign.fields, fieldPath))
   );
 }
@@ -45,10 +47,10 @@ export function isRelationshipInvolvingField(
   const [local, foreign] = relationship;
   return (
     (local.ns === namespace &&
-      local.fields !== null &&
-      isSameFieldOrChild(local.fields, fieldPath)) ||
+      !!local.fields &&
+      isSameFieldOrAncestor(fieldPath, local.fields)) ||
     (foreign.ns === namespace &&
-      foreign.fields !== null &&
-      isSameFieldOrChild(foreign.fields, fieldPath))
+      !!foreign.fields &&
+      isSameFieldOrAncestor(fieldPath, foreign.fields))
   );
 }