From 11bef11248022a98aebcc9f7555313c70ad75313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Warnon?= Date: Tue, 24 May 2022 10:05:32 +0000 Subject: [PATCH] [FW][FIX] knowledge: correctly copy members when desynchronizing on archive Currently, when archiving an article, if you don't have write access to some of the children of the article, we do not archive them and do some extra post-processing. Meaning: - They become independent from the archived parent - They are moved as "root articles" - The potential specific member access from their parent(s) are copied To avoid loosing access if specific members where configured on the parent(s) of those articles, since they will no-longer inherit from them. This last part was not working properly and has been fixed. Existing tests have been adapted to ensure the behavior is kept. Task-2859616 X-original-commit: odoo/odoo@b35f2ace22899f00c090801e476889b8d5cdbe03 Part-of: odoo/odoo#95471 --- addons/knowledge/models/knowledge_article.py | 25 +++++++++------ .../tests/test_knowledge_article_business.py | 32 ++++++++++++++++--- .../tests/test_knowledge_performance.py | 2 +- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/addons/knowledge/models/knowledge_article.py b/addons/knowledge/models/knowledge_article.py index 29090181b59..1e9bf3b1287 100644 --- a/addons/knowledge/models/knowledge_article.py +++ b/addons/knowledge/models/knowledge_article.py @@ -1102,7 +1102,7 @@ class Article(models.Model): """ self.ensure_one() new_internal_permission = force_internal_permission or self.inherited_permission - members_commands = self._copy_access_from_parents( + members_commands = self._copy_access_from_parents_commands( force_partners=force_partners, force_member_permission=force_member_permission ) @@ -1113,17 +1113,20 @@ class Article(models.Model): 'is_desynchronized': True, }) - def _copy_access_from_parents(self, force_partners=False, force_member_permission=False): - """ Copies copy all inherited accesses from parents on the article and - de-synchronize the article from its parent, allowing custom access management. + def _copy_access_from_parents_commands(self, force_partners=False, force_member_permission=False): + """ Prepares commands for all inherited accesses from parents on the given + article. It allows to de-synchronize the article from its parent and + allows custom access management. We allow to force permission of given + partners, bypassing inherited ones. - We allow to force permission of given partners. - - :param string force_internal_permission: force a new internal permission - for the article. Otherwise fallback on inherited computed internal - permission; :param force_partners: force permission of new members related to those partners; + :param str force_member_permission: force a new permission to partners + given by force_partners. Otherwise fallback on inherited computed + internal permission; + + :return list member_commands: commands to be applied on 'article_member_ids' + field; """ self.ensure_one() members_permission = self._get_article_member_permissions()[self.id] @@ -1165,7 +1168,9 @@ class Article(models.Model): # copy rights to allow breaking the hierarchy while keeping access for members for article_sudo in other_descendants_sudo: - article_sudo._copy_access_from_parents() + article_sudo.write({ + 'article_member_ids': article_sudo._copy_access_from_parents_commands() + }) # create new root articles: direct children of these articles new_roots_woperm = other_descendants_sudo.filtered(lambda article: article.parent_id in self and not article.internal_permission) diff --git a/addons/knowledge/tests/test_knowledge_article_business.py b/addons/knowledge/tests/test_knowledge_article_business.py index bc7b4d78014..aebe0c8abcb 100644 --- a/addons/knowledge/tests/test_knowledge_article_business.py +++ b/addons/knowledge/tests/test_knowledge_article_business.py @@ -79,11 +79,10 @@ class TestKnowledgeArticleBusiness(KnowledgeCommonWData): wkspace_grandchildren = self.wkspace_grandchildren.with_env(self.env) wkspace_grandgrandchildren = self.wkspace_grandgrandchildren.with_env(self.env) - # TDE FIXME: currently failing # no write access -> cracboum - # with self.assertRaises(exceptions.AccessError, - # msg='Employee can read thus not archive'): - # article_shared.action_archive() + with self.assertRaises(exceptions.AccessError, + msg='Employee can read thus not archive'): + article_shared.action_archive() # set the root + children inactive article_workspace.action_archive() @@ -109,6 +108,13 @@ class TestKnowledgeArticleBusiness(KnowledgeCommonWData): def test_article_archive_mixed_rights(self): """ Test archive in case of mixed rights """ # give write access to shared section, but have children in read or none + # and add a customer on top of shared articles to check propagation + self.article_shared.write({ + 'article_member_ids': [(0, 0, { + 'partner_id': self.customer.id, + 'permission': 'read', + })] + }) self.article_shared.article_member_ids.sudo().filtered( lambda article: article.partner_id == self.partner_employee ).write({'permission': 'write'}) @@ -132,7 +138,6 @@ class TestKnowledgeArticleBusiness(KnowledgeCommonWData): self.assertTrue(writable_child.user_has_write_access) self.assertEqual(shared_children, writable_child + readonly_child, 'Should see only two first children') - # TDE FIXME: currently failing article_shared.action_archive() # check writable articles have been archived, readonly or hidden not self.assertFalse(article_shared.active) @@ -147,6 +152,23 @@ class TestKnowledgeArticleBusiness(KnowledgeCommonWData): self.assertFalse(hidden_child_su.parent_id, 'Archive: article should be extracted in archive process as non writable') self.assertEqual(hidden_child_su.root_article_id, hidden_child_su) + # verify that the child that was not accessible was moved as a root article... + self.assertTrue(hidden_child_su.active) + self.assertEqual(hidden_child_su.category, 'shared') + self.assertEqual(hidden_child_su.internal_permission, 'none') + self.assertFalse(hidden_child_su.parent_id) + # ... and kept his access rights: still member for employee / admin and + # copied customer access from the archived parent + self.assertMembers( + hidden_child_su, + 'none', + {self.user_admin.partner_id: 'write', + self.partner_employee_manager: 'read', + self.partner_employee: 'none', + self.customer: 'read', + } + ) + @mute_logger('odoo.addons.base.models.ir_rule') @users('employee') def test_article_create(self): diff --git a/addons/knowledge/tests/test_knowledge_performance.py b/addons/knowledge/tests/test_knowledge_performance.py index 9ae054dbd92..953668e4f53 100644 --- a/addons/knowledge/tests/test_knowledge_performance.py +++ b/addons/knowledge/tests/test_knowledge_performance.py @@ -81,7 +81,7 @@ class KnowledgePerformanceCase(KnowledgeCommonWData): @users('employee') @warmup def test_article_invite_members(self): - with self.assertQueryCount(employee=84): # knowledge only: 84 + with self.assertQueryCount(employee=85): # knowledge only: 84 shared_article = self.shared_children[0].with_env(self.env) partners = (self.customer + self.partner_employee_manager + self.partner_employee2).with_env(self.env) shared_article.invite_members(partners, 'write')