[FW][FIX] knowledge: correctly propagate to children when moving to private
Currently, when moving an article to the private section, its children are untouched and keep their access rights. This is incorrect: when making an article private, all (accessible) children should be made private as well. See 'knowledge.article#_make_private' for details. An exception is made for children articles to which the user does not have a write access to. Those are handled as the following (same as archiving flow): - They become independent from the 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. Tests were added to enforce this behavior. Task-2859616 X-original-commit: odoo/odoo@82af404c5b Part-of: odoo/odoo#95471
This commit is contained in:
committed by
Thibault Delavallée
parent
11bef11248
commit
57d04db7e1
@@ -8,7 +8,7 @@ from markupsafe import Markup
|
||||
from werkzeug.urls import url_join
|
||||
|
||||
from odoo import Command, fields, models, api, _
|
||||
from odoo.exceptions import AccessError, ValidationError
|
||||
from odoo.exceptions import AccessError, ValidationError, UserError
|
||||
from odoo.osv import expression
|
||||
from odoo.tools import get_lang
|
||||
|
||||
@@ -1190,6 +1190,10 @@ class Article(models.Model):
|
||||
with write access. Requires a sudo to bypass member ACLs after checking
|
||||
write access on the article.
|
||||
|
||||
Children articles to which the user also has a write access to are made
|
||||
private as well. Other articles are detached, see '_detach_unwritable_descendants'
|
||||
for details.
|
||||
|
||||
:param <knowledge.article> parent: an optional parent to move the article under;
|
||||
:param <knowledge.article> before_article: article before which the article
|
||||
should be moved. Otherwise it is put as last parent children;
|
||||
@@ -1200,47 +1204,68 @@ class Article(models.Model):
|
||||
parent = parent if parent is not False else self.env['knowledge.article']
|
||||
before_article = before_article if before_article is not False else self.env['knowledge.article']
|
||||
|
||||
article_values = {'parent_id': parent.id}
|
||||
try:
|
||||
self.check_access_rights('write')
|
||||
(self + parent).check_access_rule('write')
|
||||
except (AccessError, UserError):
|
||||
if parent:
|
||||
raise AccessError(
|
||||
_("You are not allowed to move '%(article_name)s' under '%(parent_name)s'.",
|
||||
article_name=self.display_name,
|
||||
parent_name=parent.display_name)
|
||||
)
|
||||
raise AccessError(
|
||||
_("You are not allowed to make '%(article_name)s' private.", article_name=self.display_name)
|
||||
)
|
||||
|
||||
# first detach unwritable children (see ``_detach_unwritable_descendants``)
|
||||
writable_descendants = self._detach_unwritable_descendants()
|
||||
|
||||
article_values = {
|
||||
# reset internal permission if parent is set (will inherit) or force private (aka 'none')
|
||||
'internal_permission': False if parent else 'none',
|
||||
# cannot be desync when made private as we wipe members access
|
||||
'is_desynchronized': False,
|
||||
'parent_id': parent.id if parent else False,
|
||||
}
|
||||
if before_article:
|
||||
article_values['sequence'] = before_article.sequence
|
||||
|
||||
# when moving as root: imply member manipulations hence sudo and forced ACL check
|
||||
writable_self = self
|
||||
self_sudo = self.sudo()
|
||||
# remove members as the article is moved to private
|
||||
members_to_remove = self_sudo.article_member_ids
|
||||
self_member_command = []
|
||||
if not parent:
|
||||
try:
|
||||
self.check_access_rights('write')
|
||||
(self + parent).check_access_rule('write')
|
||||
except:
|
||||
raise AccessError(
|
||||
_("You are not allowed to move this article under article %(parent_name)s",
|
||||
parent_name=parent.display_name)
|
||||
)
|
||||
|
||||
# be sure to have an internal permission on the article if moved outside
|
||||
# of an hierarchy
|
||||
article_values.update({
|
||||
'internal_permission': 'none',
|
||||
'is_desynchronized': False,
|
||||
})
|
||||
|
||||
writable_self = self.sudo()
|
||||
self_member = writable_self.article_member_ids.filtered(lambda m: m.partner_id == self.env.user.partner_id)
|
||||
|
||||
# first add or update the new membership with 'write' access
|
||||
# make sure the current user is the only member left with write access to the article
|
||||
# if we have a parent, this is not necessary as we will inherit members access from them
|
||||
self_member = self_sudo.article_member_ids.filtered(lambda m: m.partner_id == self.env.user.partner_id)
|
||||
if self_member:
|
||||
write_member_command = [(1, self_member.id, {'permission': 'write'})]
|
||||
self_member_command = [(1, self_member.id, {'permission': 'write'})]
|
||||
# keep current member
|
||||
members_to_remove = members_to_remove.filtered(
|
||||
lambda member: member.partner_id != self.env.user.partner_id
|
||||
)
|
||||
else:
|
||||
write_member_command = [(0, 0, {
|
||||
self_member_command = [(0, 0, {
|
||||
'partner_id': self.env.user.partner_id.id,
|
||||
'permission': 'write'
|
||||
})]
|
||||
# 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)
|
||||
article_values['article_member_ids'] = [
|
||||
(2, member.id)
|
||||
for member in members_to_remove
|
||||
] + self_member_command
|
||||
|
||||
res = self_sudo.with_context(knowledge_member_skip_writable_check=True).write(article_values)
|
||||
|
||||
# remove all specific memberships configurations on children. They now inherit
|
||||
# the only 'write' member from their parent now, making them private.
|
||||
writable_descendants.with_context(knowledge_member_skip_writable_check=True).write({
|
||||
'internal_permission': False,
|
||||
'article_member_ids': [(5, 0)],
|
||||
})
|
||||
|
||||
return res
|
||||
|
||||
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
|
||||
|
||||
@@ -476,6 +476,165 @@ class TestKnowledgeArticleBusiness(KnowledgeCommonWData):
|
||||
playground_articles_asmanager = playground_articles.with_user(self.user_employee_manager)
|
||||
self.assertEqual(playground_articles_asmanager.mapped('is_user_favorite'), [False, False, False])
|
||||
|
||||
@mute_logger('odoo.addons.base.models.ir_rule', 'odoo.models.unlink')
|
||||
@users('employee')
|
||||
def test_article_make_private(self):
|
||||
""" Testing the API that makes an article 'private'. Making an article
|
||||
private generally:
|
||||
- sets internal_permission 'none';
|
||||
- sets current environment user as only write member;
|
||||
|
||||
A lot of extra post-processing is applied, see ``KnowledgeArticle.
|
||||
_move_and_make_private()`` for details.
|
||||
|
||||
Specific setup for this test
|
||||
# - Playground workspace w+ (customer-r+)
|
||||
# - Child1 " " (customer-r+)
|
||||
# - Gd Child1 " "
|
||||
# - Gd Child2 " "
|
||||
# - GdGd Child1 " "
|
||||
# - Child2 " "
|
||||
# - Gd Child1 " "
|
||||
# - GdGd Child " "
|
||||
# - ReadMember Child " w+ (employee-r+)
|
||||
# - ReadInternal Child " r+ (employee2-w+)
|
||||
# - Hidden Child " w+ (employee-no)
|
||||
"""
|
||||
article_workspace = self.article_workspace.with_env(self.env)
|
||||
workspace_children = self.workspace_children.with_env(self.env)
|
||||
wkspace_grandchildren = self.wkspace_grandchildren.with_env(self.env)
|
||||
wkspace_grandgrandchildren = self.wkspace_grandgrandchildren.with_env(self.env)
|
||||
|
||||
# add an additional member on 'article_workspace' and one of its children for further checks
|
||||
(self.article_workspace + self.workspace_children[0]).write({
|
||||
'article_member_ids': [(0, 0, {
|
||||
'partner_id': self.customer.id,
|
||||
'permission': 'read',
|
||||
})]
|
||||
})
|
||||
|
||||
# add 3 extra articles for further checks
|
||||
# - one to which 'employee' only has 'read' access (as member)
|
||||
# - one to which 'employee' only has 'read' access (internal)
|
||||
# - one invisible, "employee2" has access to that one in write mode
|
||||
[wkspace_child_read_member_access,
|
||||
wkspace_child_read_internal_access,
|
||||
wkspace_child_no_access] = self.env['knowledge.article'].sudo().create([
|
||||
{
|
||||
'article_member_ids': [(0, 0, {
|
||||
'partner_id': self.partner_employee.id,
|
||||
'permission': 'read',
|
||||
})],
|
||||
'internal_permission': 'write',
|
||||
'name': 'Read Member Child',
|
||||
'parent_id': article_workspace.id,
|
||||
}, {
|
||||
'article_member_ids': [(0, 0, {
|
||||
'partner_id': self.partner_employee2.id,
|
||||
'permission': 'write',
|
||||
})],
|
||||
'internal_permission': 'read',
|
||||
'name': 'Read Internal Child',
|
||||
'parent_id': article_workspace.id,
|
||||
}, {
|
||||
'article_member_ids': [(0, 0, {
|
||||
'partner_id': self.partner_employee.id,
|
||||
'permission': 'none',
|
||||
})],
|
||||
'internal_permission': 'write',
|
||||
'name': 'Hidden Child',
|
||||
'parent_id': article_workspace.id,
|
||||
}
|
||||
])
|
||||
with self.assertRaises(exceptions.AccessError):
|
||||
wkspace_child_no_access.with_env(self.env).body
|
||||
article_workspace._move_and_make_private()
|
||||
|
||||
# 1. main article was correctly moved to private
|
||||
self.assertEqual(article_workspace.category, 'private')
|
||||
self.assertEqual(article_workspace.internal_permission, 'none')
|
||||
self.assertMembers(
|
||||
article_workspace,
|
||||
'none',
|
||||
{self.partner_employee: 'write'}
|
||||
)
|
||||
|
||||
# 2. accessible children were correctly moved to private
|
||||
for workspace_descendant, parent_id in zip(
|
||||
workspace_children + wkspace_grandchildren + wkspace_grandgrandchildren,
|
||||
[article_workspace, article_workspace, workspace_children[0], workspace_children[0],
|
||||
workspace_children[1], wkspace_grandchildren[1], wkspace_grandchildren[2]]
|
||||
):
|
||||
self.assertEqual(workspace_descendant.category, 'private')
|
||||
self.assertEqual(workspace_descendant.inherited_permission_parent_id, article_workspace)
|
||||
self.assertEqual(workspace_descendant.parent_id, parent_id)
|
||||
# all specific members should have been wiped, no permission
|
||||
self.assertMembers(
|
||||
workspace_descendant,
|
||||
False,
|
||||
{}
|
||||
)
|
||||
|
||||
# 3.children that were not writable are moved as a root articles and that
|
||||
# members / internal permissions are kept
|
||||
self.assertEqual(wkspace_child_read_member_access.category, 'workspace')
|
||||
self.assertFalse(wkspace_child_read_member_access.parent_id)
|
||||
self.assertMembers(
|
||||
wkspace_child_read_member_access,
|
||||
'write',
|
||||
{self.partner_employee: 'read',
|
||||
self.customer: 'read'}
|
||||
)
|
||||
self.assertEqual(wkspace_child_read_internal_access.category, 'workspace')
|
||||
self.assertFalse(wkspace_child_read_internal_access.parent_id)
|
||||
self.assertMembers(
|
||||
wkspace_child_read_internal_access,
|
||||
'read',
|
||||
{self.partner_employee2: 'write',
|
||||
self.customer: 'read'}
|
||||
)
|
||||
self.assertEqual(wkspace_child_no_access.category, 'workspace')
|
||||
self.assertFalse(wkspace_child_no_access.parent_id)
|
||||
self.assertMembers(
|
||||
wkspace_child_no_access,
|
||||
'write',
|
||||
{self.partner_employee: 'none',
|
||||
self.customer: 'read'}
|
||||
)
|
||||
|
||||
# 'Hidden Child' is still not accessible for employee
|
||||
with self.assertRaises(exceptions.AccessError):
|
||||
wkspace_child_no_access.with_env(self.env).body
|
||||
wkspace_child_no_access.with_user(self.user_employee2).body
|
||||
|
||||
@mute_logger('odoo.addons.base.models.ir_rule', 'odoo.models.unlink')
|
||||
@users('employee_manager')
|
||||
def test_article_make_private_w_parent(self):
|
||||
""" Test a special case when making private: moving under an existing private parent. """
|
||||
article_shared = self.article_shared.with_env(self.env)
|
||||
article_private_manager = self.article_private_manager.with_env(self.env)
|
||||
|
||||
# first test that making 'article_shared' fails since 'employee_manager'
|
||||
# does not have write access to it (only read)
|
||||
with self.assertRaises(exceptions.AccessError):
|
||||
article_shared._move_and_make_private(parent=article_private_manager)
|
||||
|
||||
# then grant write access to test the flow
|
||||
article_shared.sudo().article_member_ids.filtered(
|
||||
lambda member: member.partner_id == self.partner_employee_manager
|
||||
).write({'permission': 'write'})
|
||||
|
||||
article_shared._move_and_make_private(parent=article_private_manager)
|
||||
|
||||
self.assertEqual(article_shared.category, 'private')
|
||||
# the internal permission should not be set as we inherit from our private parent
|
||||
# members should be wiped as we inherit from our private parent
|
||||
self.assertMembers(
|
||||
article_shared,
|
||||
False,
|
||||
{}
|
||||
)
|
||||
|
||||
@mute_logger('odoo.addons.base.models.ir_rule')
|
||||
@users('employee')
|
||||
def test_article_move_to(self):
|
||||
|
||||
Reference in New Issue
Block a user