From a7d5a4b130bf1933f42e5805c4b36f70473352c5 Mon Sep 17 00:00:00 2001 From: "Xavier Bol (xbo)" Date: Mon, 30 Oct 2023 17:25:59 +0100 Subject: [PATCH] [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) --- .../static/src/hierarchy_model.js | 8 ++ .../static/tests/hierarchy_view_tests.js | 123 +++++++++++++++++- 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/addons/web_hierarchy/static/src/hierarchy_model.js b/addons/web_hierarchy/static/src/hierarchy_model.js index 90bfef71b26..39fca17904f 100644 --- a/addons/web_hierarchy/static/src/hierarchy_model.js +++ b/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, diff --git a/addons/web_hierarchy/static/tests/hierarchy_view_tests.js b/addons/web_hierarchy/static/tests/hierarchy_view_tests.js index 3e4a6ce3c36..2906872f41d 100644 --- a/addons/web_hierarchy/static/tests/hierarchy_view_tests.js +++ b/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("", ""); + 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("", ""); + 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([]); + }); });