diff --git a/addons/knowledge/models/knowledge_article.py b/addons/knowledge/models/knowledge_article.py index dcc410c4755..29090181b59 100644 --- a/addons/knowledge/models/knowledge_article.py +++ b/addons/knowledge/models/knowledge_article.py @@ -140,17 +140,7 @@ class Article(models.Model): Ǹote: computation is done in Py instead of using optimized SQL queries because value are not yet in DB at this point.""" for article in self: - def has_write_member(a, child_members=False): - if not child_members: - child_members = self.env['knowledge.article.member'] - article_members = a.article_member_ids - if any(m.permission == 'write' and m.partner_id not in child_members.mapped('partner_id') - for m in article_members): - return True - if a.parent_id and not a.is_desynchronized: - return has_write_member(a.parent_id, article_members | child_members) - return False - if article.inherited_permission != 'write' and not has_write_member(article): + if article.inherited_permission != 'write' and not article._has_write_member(): raise ValidationError(_("The article '%s' needs at least one member with 'Write' access.", article.display_name)) @api.constrains('parent_id') @@ -1230,19 +1220,55 @@ class Article(models.Model): writable_self = self.sudo() self_member = writable_self.article_member_ids.filtered(lambda m: m.partner_id == self.env.user.partner_id) - member_command = [(2, member.id) for member in writable_self.article_member_ids if member.partner_id != self.env.user.partner_id] + # first add or update the new membership with 'write' access if self_member: - member_command.append((1, self_member.id, {'permission': 'write'})) + write_member_command = [(1, self_member.id, {'permission': 'write'})] else: - member_command.append((0, 0, { + write_member_command = [(0, 0, { 'partner_id': self.env.user.partner_id.id, 'permission': 'write' - })) - article_values['article_member_ids'] = member_command + })] + # then remove other members as the article is moved to private + article_values['article_member_ids'] = [ + (2, member.id) + for member in writable_self.article_member_ids if member.partner_id != self.env.user.partner_id + ] + write_member_command return writable_self.with_context(knowledge_member_skip_writable_check=True).write(article_values) + def _has_write_member(self, partners_to_exclude=False, members_to_exclude=False): + """ Method allowing to check if this article still has at least one member + with write access. Typically used during constraints checks. + + Please note that this method is *not* optimized and should be avoided by + using ``_get_article_member_permissions`` instead when possible. + + :param partners_to_exclude: used when checking recursively + through article parents, we only check for the most specific access + for a given partner; + :param members_to_exclude: memberships that + should not be considered when checking for a write access, used when + unlinking members that should not be taken into account; + + :return boolean: whether a write member has been found; + """ + self.ensure_one() + partners_to_exclude = partners_to_exclude if partners_to_exclude else self.env['res.partner'] + + article_members = self.article_member_ids + if members_to_exclude: + article_members -= members_to_exclude + + if any(m.permission == 'write' and m.partner_id not in partners_to_exclude + for m in article_members): + return True + if not self.is_desynchronized and self.parent_id: + return self.parent_id._has_write_member( + partners_to_exclude=article_members.partner_id | partners_to_exclude + ) + return False + # ------------------------------------------------------------ # PERMISSIONS BATCH COMPUTATION # ------------------------------------------------------------ diff --git a/addons/knowledge/models/knowledge_article_member.py b/addons/knowledge/models/knowledge_article_member.py index a2155355b58..8f633bb0716 100644 --- a/addons/knowledge/models/knowledge_article_member.py +++ b/addons/knowledge/models/knowledge_article_member.py @@ -61,7 +61,6 @@ class ArticleMember(models.Model): for member in self.filtered(lambda member: member.article_id in articles_to_check): deleted_members_by_article[member.article_id.id] |= member - parents_members_permission = articles_to_check.parent_id._get_article_member_permissions() for article in articles_to_check: # Check on permission on members members_to_check = article.article_member_ids @@ -70,15 +69,8 @@ class ArticleMember(models.Model): if any(m.permission == 'write' for m in members_to_check): continue - # we need to add the members on parents to check the validity - parent_write_members = any( - values['permission'] == 'write' for partner_id, values - in parents_members_permission[article.parent_id.id].items() - if article.parent_id and not article.is_desynchronized - and partner_id not in article.article_member_ids.partner_id.ids - ) - - if not parent_write_members: + members_to_exclude = deleted_members_by_article[article.id] if on_unlink else False + if not article._has_write_member(members_to_exclude=members_to_exclude): raise ValidationError( _("Article '%s' should always have a writer: inherit write permission, or have a member with write access", article.display_name) diff --git a/addons/knowledge/tests/test_knowledge_article_constraints.py b/addons/knowledge/tests/test_knowledge_article_constraints.py index 2541b77bf83..6c9757bc671 100644 --- a/addons/knowledge/tests/test_knowledge_article_constraints.py +++ b/addons/knowledge/tests/test_knowledge_article_constraints.py @@ -319,6 +319,13 @@ class TestKnowledgeArticleConstraints(KnowledgeCommon): with self.assertRaises(exceptions.ValidationError, msg='Cannot remove the last writer on an article'): article_private._add_members(membership_sudo.partner_id, 'none') + # moving the article to private will remove the second member + # but should not trigger an error since we also add 'employee' as a write member + article_workspace = self.article_workspace.with_env(self.env) + article_workspace.move_to(is_private=True) + self.assertEqual(article_workspace.category, 'private') + self.assertTrue(article_workspace._has_write_member()) + @mute_logger('odoo.sql_db') @users('employee') def test_favourite_uniqueness(self): diff --git a/addons/knowledge/tests/test_knowledge_performance.py b/addons/knowledge/tests/test_knowledge_performance.py index 4427045deac..9ae054dbd92 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=89): # knowledge only: 89 + with self.assertQueryCount(employee=84): # 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')