From 9accece7d3633fec7c12ac8fe664e67c364eb15b Mon Sep 17 00:00:00 2001 From: Adrien Schoffeniels Date: Tue, 28 Jun 2022 09:02:54 +0000 Subject: [PATCH] [FIX] knowledge: fix member inheritance in share panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description of the issue: ========================= Suppose we have 3 articles with the following structure: root L child L grand child When granting the "write" permission to a user in "root", this user will inherit this permission in "child" and in "grand child". When we then restrict this user with the "no access" permission in child, this permission is inherited by grand-child (correct). Then, when a member is invited in the root article, the right is not herited by "child" nor "grand child" (correct). However, this permission is shown as being inherited from "root" in the permission panel in "grand child" (incorrect: "grand child" does not have this permission, and is in fact inherited from "child" as expected). Solution: ========= Add the boolean "article_is_desynchronized" check in the inner join condition of the recursive SQL request used to fetch the permissions of an article and its parents. This boolean check allows to prune the recursion when an article is desynchronized, so that the permissions of the parents of this desynchronized article are not fetched, since they are not inherited in the desynchronized articles nor in its children. Some tests have been added to ensure the correct behaviour of the share panel. Especially, the scenario described in the description of the issue is tested. Task-2866950 closes odoo/odoo#94834 X-original-commit: 252fce15f20a13ab3e71f7aafc247b5ac88a354a Signed-off-by: Warnon Aurélien (awa) --- addons/knowledge/models/knowledge_article.py | 11 +++--- .../test_knowledge_article_permissions.py | 35 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/addons/knowledge/models/knowledge_article.py b/addons/knowledge/models/knowledge_article.py index 0935bd35d08..fcb0e6c22a4 100644 --- a/addons/knowledge/models/knowledge_article.py +++ b/addons/knowledge/models/knowledge_article.py @@ -1342,23 +1342,26 @@ class Article(models.Model): sql = f''' WITH article_permission as ( WITH RECURSIVE article_perms as ( - SELECT a.id, a.parent_id, m.id as member_id, m.partner_id, - m.permission + SELECT a.id, a.parent_id, a.is_desynchronized, m.id as member_id, + m.partner_id, m.permission FROM knowledge_article a LEFT JOIN knowledge_article_member m ON a.id = m.article_id ), article_rec as ( SELECT perms1.id, perms1.id as article_id, perms1.parent_id, perms1.member_id, perms1.partner_id, perms1.permission, - perms1.id as origin_id, 0 as level + perms1.id as origin_id, 0 as level, + perms1.is_desynchronized FROM article_perms as perms1 UNION SELECT perms2.id, perms_rec.article_id, perms2.parent_id, perms2.member_id, perms2.partner_id, perms2.permission, - perms2.id as origin_id, perms_rec.level + 1 + perms2.id as origin_id, perms_rec.level + 1, + perms2.is_desynchronized FROM article_perms as perms2 INNER JOIN article_rec perms_rec ON perms_rec.parent_id=perms2.id + AND perms_rec.is_desynchronized is not true ) SELECT article_id, origin_id, member_id, partner_id, permission, min(level) as min_level diff --git a/addons/knowledge/tests/test_knowledge_article_permissions.py b/addons/knowledge/tests/test_knowledge_article_permissions.py index 7d55120b359..7f974feee50 100644 --- a/addons/knowledge/tests/test_knowledge_article_permissions.py +++ b/addons/knowledge/tests/test_knowledge_article_permissions.py @@ -269,12 +269,23 @@ class TestKnowledgeArticlePermissionsTools(KnowledgeArticlePermissionsCase): {self.partner_employee_manager: 'write'}) # remove partner employee manager that has rights based on inheritance + writable_children = self.article_write_contents_children.with_env(self.env) + for child in writable_children: + self.assertIn( + self.partner_employee_manager.id, + child._get_article_member_permissions()[child.id], + 'Share Panel: if an article inherits a permission, its children should inherit that permission too') manager_member = writable_root.article_member_ids.filtered(lambda m: m.partner_id == self.partner_employee_manager) writable._remove_member(manager_member) self.assertTrue(writable.is_desynchronized, 'Permission: when removing a member having inherited rights it has be be desynchronized') self.assertMembers(writable, 'write', {self.partner_portal: 'read'}) + for child in writable_children: + self.assertNotIn( + self.partner_employee_manager.id, + child._get_article_member_permissions()[child.id], + 'Share Panel: when removing a member having inherited rights, the member should be removed from the children that inherited that right') # resync writable.restore_article_access() @@ -326,12 +337,36 @@ class TestKnowledgeArticlePermissionsTools(KnowledgeArticlePermissionsCase): {self.partner_portal: 'none'}) # downgrade a permission, should desynchronize from parent + writable_children = self.article_write_contents_children.with_env(self.env) + for child in writable_children: + self.assertEqual( + child._get_article_member_permissions()[child.id][self.partner_employee_manager.id]['permission'], + 'write', + 'Share Panel: if an article inherits a permission, its children should inherit that permission too') + writable_root._set_member_permission(manager_member_root, 'write') writable._set_member_permission(manager_member_root, 'read', is_based_on=True) self.assertTrue(writable.is_desynchronized, 'Permission: when removing a member having inherited rights it has be be desynchronized') self.assertMembers(writable, 'write', {self.partner_portal: 'none', self.partner_employee_manager: 'read'}) + for child in writable_children: + self.assertEqual( + child._get_article_member_permissions()[child.id][self.partner_employee_manager.id]['permission'], + 'read', + 'Share Panel: when downgrading a member having inherited rights, the member should be downgraded from the children that inherited that right') + + # adding a member to parent, should not be inherited by children + writable_root._add_members(self.partner_employee2, 'read') + self.assertNotIn( + self.partner_employee2.id, + writable._get_article_member_permissions()[writable.id], + 'Share Panel: when adding a member on a parent of a desynced article, the member should not be added on the desynced article') + for child in writable_children: + self.assertNotIn( + self.partner_employee2.id, + child._get_article_member_permissions()[child.id], + 'Share Panel: when adding a member on a parent of a desynced article, the member should not be added on the children of the desynced article') @mute_logger('odoo.addons.base.models.ir_rule') @users('employee')