hotfix: isDroppable rules

packages/core/src/editor/NodeHelpers.ts

@@ -168,6 +168,14 @@ export function NodeHelpers(state: EditorState, id: NodeId) {
           return true;
         }
 
+        const targetDeepNodes = nodeHelpers(targetNode.id).descendants(true);
+
+        invariant(
+          !targetDeepNodes.includes(newParentNode.id) &&
+            newParentNode.id !== targetNode.id,
+          ERROR_MOVE_TO_DESCENDANT
+        );
+
         const currentParentNode =
           targetNode.data.parent && state.nodes[targetNode.data.parent];
 
@@ -179,21 +187,18 @@ export function NodeHelpers(state: EditorState, id: NodeId) {
           ERROR_DUPLICATE_NODEID
         );
 
-        const targetDeepNodes = nodeHelpers(targetNode.id).descendants();
-
-        invariant(
-          !targetDeepNodes.includes(newParentNode.id) &&
-            newParentNode.id !== targetNode.id,
-          ERROR_MOVE_TO_DESCENDANT
-        );
-        invariant(
-          currentParentNode.rules.canMoveOut(
-            targetNode,
-            currentParentNode,
-            nodeHelpers
-          ),
-          ERROR_MOVE_OUTGOING_PARENT
-        );
+        // If the Node we're checking for is not the same as the currentParentNode
+        // Check if the currentParentNode allows the targetNode to be dragged out
+        if (node !== currentParentNode) {
+          invariant(
+            currentParentNode.rules.canMoveOut(
+              targetNode,
+              currentParentNode,
+              nodeHelpers
+            ),
+            ERROR_MOVE_OUTGOING_PARENT
+          );
+        }
 
         return true;
       } catch (err) {

packages/core/src/editor/tests/NodeHelpers.test.ts

@@ -72,31 +72,37 @@ describe('NodeHelpers', () => {
 
   describe('descendants', () => {
     it('should return immediate child and linked node ids', () => {
-      expect(helper('canvas-node-reject-dnd').descendants()).toStrictEqual(
-        helper('canvas-node-reject-dnd').get().data.nodes
+      expect(
+        helper('canvas-node-reject-outgoing-dnd').descendants()
+      ).toStrictEqual(
+        helper('canvas-node-reject-outgoing-dnd').get().data.nodes
       );
     });
     describe('when "includeOnly" is unset', () => {
       it('should return all child and linked nodes', () => {
         expect(
-          helper('canvas-node-reject-dnd').descendants(true)
+          helper('canvas-node-reject-outgoing-dnd').descendants(true)
         ).toStrictEqual([
-          ...documentWithVariousNodes.nodes['canvas-node-reject-dnd'].data
-            .nodes,
+          ...documentWithVariousNodes.nodes['canvas-node-reject-outgoing-dnd']
+            .data.nodes,
           ...Object.values(
             documentWithVariousNodes.nodes['parent-of-linked-node'].data
               .linkedNodes || {}
           ),
+          ...documentWithVariousNodes.nodes['linked-node'].data.nodes,
         ]);
       });
     });
     describe('when "includeOnly" is set to childNodes', () => {
       it('should return all child nodes only', () => {
         expect(
-          helper('canvas-node-reject-dnd').descendants(true, 'childNodes')
+          helper('canvas-node-reject-outgoing-dnd').descendants(
+            true,
+            'childNodes'
+          )
         ).toStrictEqual([
-          ...documentWithVariousNodes.nodes['canvas-node-reject-dnd'].data
-            .nodes,
+          ...documentWithVariousNodes.nodes['canvas-node-reject-outgoing-dnd']
+            .data.nodes,
         ]);
       });
     });
@@ -154,17 +160,27 @@ describe('NodeHelpers', () => {
     });
     it("should return false if node's rule rejects incoming target", () => {
       expect(
-        helper('canvas-node-reject-dnd').isDroppable(secondaryButton.id)
+        helper('canvas-node-reject-incoming-dnd').isDroppable(
+          secondaryButton.id
+        )
       ).toEqual(false);
     });
     it("should return false if node's rule rejects outgoing target", () => {
+      // Should not return false if the target is moving within the same parent
+      expect(
+        helper('canvas-node-reject-outgoing-dnd').isDroppable(
+          'fixed-child-node'
+        )
+      ).toEqual(true);
+
+      // should return false if the target moved to a different parent
       expect(helper('canvas-node').isDroppable('fixed-child-node')).toEqual(
         false
       );
     });
     it('should return false if target is a descendant', () => {
       expect(
-        helper('parent-of-linked-node').isDroppable('canvas-node-reject-dnd')
+        helper('linked-node-child-canvas').isDroppable('parent-of-linked-node')
       ).toEqual(false);
     });
   });

packages/core/src/tests/fixtures.ts

@@ -121,13 +121,18 @@ export const documentWithCardState = {
   },
 };
 
+// TODO: Find a better way to create test child nodes
 export const documentWithVariousNodes = {
   ...documentWithCardState,
   nodes: {
     ...documentWithCardState.nodes,
     'canvas-node': createTestNode('canvas-node', {
       isCanvas: true,
-      nodes: ['node-reject-dnd', 'canvas-node-reject-dnd'],
+      nodes: [
+        'node-reject-dnd',
+        'canvas-node-incoming-dnd',
+        'canvas-node-reject-outgoing-dnd',
+      ],
     }),
     'node-reject-dnd': createTestNode(
       'node-reject-dnd',
@@ -141,16 +146,28 @@ export const documentWithVariousNodes = {
         },
       }
     ),
-    'canvas-node-reject-dnd': createTestNode(
-      'canvas-node-reject-dnd',
+    'canvas-node-reject-incoming-dnd': createTestNode(
+      'canvas-node-reject-incoming-dnd',
       {
-        nodes: ['fixed-child-node', 'parent-of-linked-node'],
+        nodes: [],
         parent: 'canvas-node',
         isCanvas: true,
       },
       {
         rules: {
           canMoveIn: () => false,
+        },
+      }
+    ),
+    'canvas-node-reject-outgoing-dnd': createTestNode(
+      'canvas-node-reject-outgoing-dnd',
+      {
+        nodes: ['fixed-child-node', 'parent-of-linked-node'],
+        parent: 'canvas-node',
+        isCanvas: true,
+      },
+      {
+        rules: {
           canMoveOut: () => false,
         },
       }
@@ -159,18 +176,23 @@ export const documentWithVariousNodes = {
       parent: 'node-reject-dnd',
     }),
     'fixed-child-node': createTestNode('fixed-child-node', {
-      parent: 'canvas-node-reject-dnd',
+      parent: 'canvas-node-reject-outgoing-dnd',
     }),
     'parent-of-linked-node': createTestNode('parent-of-linked-node', {
       isCanvas: true,
-      parent: 'canvas-node-reject-dnd',
+      parent: 'canvas-node-reject-outgoing-dnd',
       linkedNodes: {
         test: 'linked-node',
       },
     }),
     'linked-node': createTestNode('linked-node', {
       isCanvas: true,
       parent: 'parent-of-linked-node',
+      nodes: ['linked-node-child-canvas'],
+    }),
+    'linked-node-child-canvas': createTestNode('linked-node-child-canvas', {
+      isCanvas: true,
+      parent: 'linked-node',
     }),
   },
 };

packages/core/src/utils/createTestNode.ts

@@ -1,5 +1,8 @@
-export const createTestNode = (id, data, config = {}) => {
+import { Node } from '../interfaces';
+
+export const createTestNode = (id, data, config: Partial<Node> = {}) => {
   return {
+    ...config,
     id,
     data: {
       props: {},
@@ -9,12 +12,17 @@ export const createTestNode = (id, data, config = {}) => {
       ...data,
     },
     related: {},
-    events: { selected: false, dragged: false, hovered: false },
+    events: {
+      selected: false,
+      dragged: false,
+      hovered: false,
+      ...config.events,
+    },
     rules: {
       canMoveIn: () => true,
       canMoveOut: () => true,
       canDrag: () => true,
+      ...config.rules,
     },
-    ...config,
   };
 };