[FIX] web_hierarchy: detect cyclic before altering the parent field

Before this commit, the user could drag the first employee displayed
in the gantt and drop him on one of his subordinate. By doing that,
the manager of that employee will be his subordinate and a cyclic
will occur and no record will appear when the org chart will be
reloaded because no employee will have no manager set.

This commit fixes the issue by improving the cyclic detection when
the user uses the drag and drop feature. That is, the action will be
blocking when the system detects the user tries to drag and drop a manager
to one of his subordinates.

closes odoo/odoo#140951

Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>

addons/web_hierarchy/static/src/hierarchy_model.js

@@ -824,6 +824,14 @@ export class HierarchyModel extends Model {
                         }
                     );
                     return;
+                } else if (node.allSubsidiaryResIds.includes(parentNode.resId)) {
+                    this.notification.add(
+                        _t("Cannot change the parent because it will cause a cyclic."),
+                        {
+                            type: "danger",
+                        }
+                    );
+                    return;
                 }
                 domain = Domain.or([
                     domain,

addons/web_hierarchy/static/tests/hierarchy_view_tests.js

@@ -1,6 +1,14 @@
 /** @odoo-module **/
 
-import { click, drag, dragAndDrop, getFixture, getNodesTextContent } from "@web/../tests/helpers/utils";
+import { browser } from "@web/core/browser/browser";
+import {
+    click,
+    drag,
+    dragAndDrop,
+    getFixture,
+    getNodesTextContent,
+    patchWithCleanup
+} from "@web/../tests/helpers/utils";
 import { makeView, setupViewRegistries } from "@web/../tests/views/helpers";
 
 let serverData, target;
@@ -685,4 +693,117 @@ QUnit.module("Views", (hooks) => {
         );
         assert.containsOnce(target, ".o_hierarchy_node_container button[name=hierarchy_search_parent_node]");
     });
+
+    QUnit.test("cannot set the record dragged as parent", async function (assert) {
+        serverData.views["hr.employee,false,hierarchy"] = serverData.views["hr.employee,false,hierarchy"].replace("<hierarchy>", "<hierarchy draggable='1'>");
+        await makeView({
+            type: "hierarchy",
+            resModel: "hr.employee",
+            serverData,
+            mockRPC(route, { method, model }) {
+                if (method === "write" && model === "hr.employee") {
+                    assert.step("setManager");
+                }
+            },
+        });
+
+        patchWithCleanup(browser, {
+            setTimeout: () => 1,
+        });
+        assert.containsN(target, ".o_hierarchy_row", 2);
+        assert.containsN(target, ".o_hierarchy_node", 3);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert"]
+        );
+        const rows = target.querySelectorAll(".o_hierarchy_row");
+        await dragAndDrop(
+            target.querySelector(".o_hierarchy_node"), // select first node (Albert)
+            rows[1]
+        );
+        assert.containsN(target, ".o_hierarchy_row", 2);
+        assert.containsN(target, ".o_hierarchy_node", 3);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert"]
+        );
+        assert.containsOnce(target, ".o_notification");
+        assert.containsOnce(target, ".o_notification.border-danger");
+
+        assert.verifySteps([]);
+    });
+
+    QUnit.test("cannot create cyclic", async function (assert) {
+        serverData.views["hr.employee,false,hierarchy"] = serverData.views["hr.employee,false,hierarchy"].replace("<hierarchy>", "<hierarchy draggable='1'>");
+        await makeView({
+            type: "hierarchy",
+            resModel: "hr.employee",
+            serverData,
+            mockRPC(route, { method, model }) {
+                if (method === "write" && model === "hr.employee") {
+                    assert.step("setManager");
+                }
+            },
+        });
+
+        patchWithCleanup(browser, {
+            setTimeout: () => 1,
+        });
+        assert.containsN(target, ".o_hierarchy_row", 2);
+        assert.containsN(target, ".o_hierarchy_node", 3);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert"]
+        );
+        let nodes = target.querySelectorAll(".o_hierarchy_node");
+        await dragAndDrop(
+            nodes[0], // albert node
+            nodes[1] // georges node
+        );
+        assert.containsN(target, ".o_hierarchy_row", 2);
+        assert.containsN(target, ".o_hierarchy_node", 3);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert"]
+        );
+        assert.containsOnce(target, ".o_notification");
+        assert.containsOnce(target, ".o_notification.border-danger");
+
+        await click(target, ".o_hierarchy_node_button.btn-primary");
+        assert.containsN(target, ".o_hierarchy_row", 3);
+        assert.containsN(target, ".o_hierarchy_node", 4);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert", "LouisJosephine"]
+        );
+        nodes = target.querySelectorAll(".o_hierarchy_node");
+        await dragAndDrop(
+            nodes[0],
+            nodes[3]
+        );
+        assert.containsN(target, ".o_hierarchy_row", 3);
+        assert.containsN(target, ".o_hierarchy_node", 4);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert", "LouisJosephine"]
+        );
+        assert.containsN(target, ".o_notification", 2);
+        assert.containsN(target, ".o_notification.border-danger", 2);
+
+        const rows = target.querySelectorAll(".o_hierarchy_row");
+        await dragAndDrop(
+            nodes[0],
+            rows[2]
+        );
+        assert.containsN(target, ".o_hierarchy_row", 3);
+        assert.containsN(target, ".o_hierarchy_node", 4);
+        assert.deepEqual(
+            getNodesTextContent(target.querySelectorAll(".o_hierarchy_node_content")),
+            ["Albert", "GeorgesAlbert", "JosephineAlbert", "LouisJosephine"]
+        );
+        assert.containsN(target, ".o_notification", 3);
+        assert.containsN(target, ".o_notification.border-danger", 3);
+
+        assert.verifySteps([]);
+    });
 });