fix baserow#4147 - iterator node can be placed after itself (baserow#4151)

* fix baserow#4147 - iterator node can be placed after itself

* Make the formula error log less verbose

Author: jrmi

backend/src/baserow/contrib/automation/nodes/models.py

@@ -121,6 +121,18 @@ def get_previous_service_outputs(self):
 
         return {node.service_id: str(out) for [node, _, out] in previous_positions}
 
+    def get_parent_nodes(self):
+        """
+        Returns the ancestors of this node which are the container nodes that contain
+        the current node instance.
+        """
+
+        return [
+            position[0]
+            for position in self.workflow.get_graph().get_previous_positions(self)
+            if position[1] == "child"
+        ]
+
     def get_next_nodes(
         self, output_uid: str | None = None
     ) -> Iterable["AutomationNode"]:

backend/src/baserow/contrib/automation/nodes/node_types.py

@@ -103,7 +103,7 @@ def before_move(
         Check the container node is not moved inside it self.
         """
 
-        if node in reference_node.get_previous_nodes():
+        if node in reference_node.get_parent_nodes():
             raise AutomationNodeNotMovable(
                 "A container node cannot be moved inside itself"
             )

backend/src/baserow/contrib/automation/workflows/graph_handler.py

@@ -232,9 +232,7 @@ def _get_all_next_nodes(self, node: AutomationNode):
 
         node_info = self.get_info(node)
 
-        return [
-            x for sublist in node_info.get("next", {}).values() for x in sublist
-        ] + node_info.get("children", [])
+        return [x for sublist in node_info.get("next", {}).values() for x in sublist]
 
     def get_next_nodes(
         self, node: AutomationNode, output: str | None = None

backend/tests/baserow/contrib/automation/nodes/test_node_service.py

@@ -635,6 +635,85 @@ def test_move_node_outside_of_container(data_fixture: Fixtures):
     assert move_result.previous_output == ""
 
 
+@pytest.mark.django_db
+def test_move_container_after_itself(data_fixture: Fixtures):
+    user = data_fixture.create_user()
+    workflow = data_fixture.create_automation_workflow(user)
+    action1 = data_fixture.create_automation_node(workflow=workflow, label="action1")
+
+    iterator = data_fixture.create_core_iterator_action_node(workflow=workflow)
+    action2 = data_fixture.create_automation_node(workflow=workflow, label="action2")
+    action3 = data_fixture.create_automation_node(workflow=workflow, label="action3")
+    action4 = data_fixture.create_automation_node(workflow=workflow, label="action4")
+
+    # move `action3` to be the first child of iterator
+    move_result = AutomationNodeService().move_node(
+        user, iterator.id, reference_node_id=action4.id, position="south", output=""
+    )
+
+    workflow.assert_reference(
+        {
+            "0": "rows_created",
+            "rows_created": {"next": {"": ["action1"]}},
+            "action1": {"next": {"": ["action2"]}},
+            "action2": {"next": {"": ["action3"]}},
+            "action3": {"next": {"": ["action4"]}},
+            "action4": {"next": {"": ["iterator"]}},
+            "iterator": {},
+        }
+    )
+
+
+@pytest.mark.django_db
+def test_move_container_inside_itself_should_fail(data_fixture: Fixtures):
+    user = data_fixture.create_user()
+    workflow = data_fixture.create_automation_workflow(user)
+    action1 = data_fixture.create_automation_node(workflow=workflow, label="action1")
+
+    iterator = data_fixture.create_core_iterator_action_node(workflow=workflow)
+    iterator2 = data_fixture.create_core_iterator_action_node(
+        workflow=workflow, reference_node=iterator, position="child"
+    )
+    action2 = data_fixture.create_automation_node(
+        workflow=workflow, label="action2", reference_node=iterator2, position="child"
+    )
+    action3 = data_fixture.create_automation_node(
+        workflow=workflow, label="action3", reference_node=action2, position="south"
+    )
+    action4 = data_fixture.create_automation_node(
+        workflow=workflow, label="action4", reference_node=iterator2, position="south"
+    )
+
+    with pytest.raises(AutomationNodeNotMovable) as exc:
+        AutomationNodeService().move_node(
+            user, iterator.id, reference_node_id=action3.id, position="south", output=""
+        )
+
+    assert exc.value.args[0] == "A container node cannot be moved inside itself"
+
+    with pytest.raises(AutomationNodeNotMovable) as exc:
+        AutomationNodeService().move_node(
+            user,
+            iterator.id,
+            reference_node_id=iterator2.id,
+            position="south",
+            output="",
+        )
+
+    assert exc.value.args[0] == "A container node cannot be moved inside itself"
+
+    with pytest.raises(AutomationNodeNotMovable) as exc:
+        AutomationNodeService().move_node(
+            user,
+            iterator.id,
+            reference_node_id=iterator2.id,
+            position="child",
+            output="",
+        )
+
+    assert exc.value.args[0] == "A container node cannot be moved inside itself"
+
+
 @pytest.mark.django_db
 def test_move_node_invalid_reference_node(data_fixture: Fixtures):
     user = data_fixture.create_user()

backend/tests/baserow/contrib/automation/workflows/test_graph_handler.py

@@ -511,7 +511,7 @@ def test_graph_handler_insert_first_node(
             {
                 "0": 1,
                 "1": {"next": {"": [2]}},
-                "2": {"next": {"": [4, 7]}},
+                "2": {"next": {"": [4]}},  # yes we lose the child for now
                 "4": {"next": {"": [5], "randomUid": [9]}},
                 "7": {"next": {"": [8]}},
                 "8": {"children": []},

web-frontend/modules/builder/elementTypes.js

@@ -804,7 +804,9 @@ export class ElementType extends Registerable {
       page,
       element,
       allowSameElement = true,
-      recordIndexPath,
+      // The default value is used when we try to resolve the name in element menu for
+      // instance.
+      recordIndexPath = [0],
     } = applicationContext
 
     const collectionAncestry = this.getCollectionAncestry({

web-frontend/modules/core/formula/index.js

@@ -28,7 +28,7 @@ export const resolveFormula = (
     const tree = parseBaserowFormula(formulaCtx.formula)
     return new JavascriptExecutor(functions, RuntimeFormulaContext).visit(tree)
   } catch (err) {
-    console.debug('Err in formula resolution', err)
+    console.debug(`FORMULA DEBUG: ${err}`)
     return ''
   }
 }