[FIX] calendar: read([]) as a non-admin would not return all fields
Because private fields would automatically add a bunch of fields to
the user's request (in order to do their own post-treatment), unlike
normal behavior `read([])` would be completed to `read(['privacy',
'user_id'])` and would fail to trigger the "select all the fields"
behavior of `BaseModel.read`.
Move the privacy management to a `_read` override instead. Also update
the code to be more linear and straightforward, without a bunch of
intermediate helper function.
This is also a small performance optimisation, although apparently
much more minor for 14 (best case of about 5%) than on 15.0 (where it
seemed to reach 30). The gain is mostly for private events being read
for non-participants: because the non-public fields get neutered
before computation (rather than after), fields are computed on a
neutered basis and thus largely do nothing. This is especially salient
with computations relying on relations (in this case
`attendee_status`), as from an empty starting point they essentially
do not do anything.
This leads to the effort (and cost) of private events being read by
non-participants to be about the same as the cost of public events,
whereas before the change there is a visible overhead. The signal is
quite noisy though.
NOTE: backported the addition of `privacy` to the public fields from
2f4a91c0ce as that is better
behavior and makes the tests more logical (and easier to forward
port probably)
OPW-2831113
closes odoo/odoo#89936
X-original-commit: b0c6f2969596fb715e0a6c62865b5c7dbc0bcc78
Related: odoo/enterprise#26699
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
This commit is contained in:
@@ -1,10 +1,12 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
import logging
|
||||
import math
|
||||
from collections import defaultdict
|
||||
from datetime import timedelta
|
||||
import math
|
||||
import logging
|
||||
from itertools import repeat
|
||||
|
||||
import pytz
|
||||
import uuid
|
||||
|
||||
@@ -461,56 +463,34 @@ class Meeting(models.Model):
|
||||
|
||||
return events
|
||||
|
||||
def read(self, fields=None, load='_classic_read'):
|
||||
def hide(field, value):
|
||||
"""
|
||||
:param field: field name
|
||||
:param value: field value
|
||||
:return: obfuscated field value
|
||||
"""
|
||||
if field in {'name', 'display_name'}:
|
||||
return _('Busy')
|
||||
return [] if isinstance(value, list) else False
|
||||
def _read(self, fields):
|
||||
if self.env.is_system():
|
||||
super()._read(fields)
|
||||
return
|
||||
|
||||
def split_privacy(events):
|
||||
"""
|
||||
:param events: list of event values (dict)
|
||||
:return: tuple(private events, public events)
|
||||
"""
|
||||
private = [event for event in events if event.get('privacy') == 'private']
|
||||
public = [event for event in events if event.get('privacy') != 'private']
|
||||
return private, public
|
||||
fields = set(fields)
|
||||
private_fields = fields - self._get_public_fields()
|
||||
if not private_fields:
|
||||
super()._read(fields)
|
||||
return
|
||||
|
||||
def split_visibility(events):
|
||||
"""
|
||||
:param events: list of event values (dict)
|
||||
:return: tuple(my events, other events)
|
||||
"""
|
||||
current_partner_id = self.env.user.partner_id.id
|
||||
visible_events = []
|
||||
other_events = []
|
||||
for event in events:
|
||||
if (event.get('user_id') and event.get('user_id')[0] == self.env.uid) or current_partner_id in event.get('partner_ids'):
|
||||
visible_events.append(event)
|
||||
else:
|
||||
other_events.append(event)
|
||||
return visible_events, other_events
|
||||
private_fields.add('partner_ids')
|
||||
super()._read(fields | {'privacy', 'user_id', 'partner_ids'})
|
||||
current_partner_id = self.env.user.partner_id
|
||||
others_private_events = self.filtered(
|
||||
lambda e: e.privacy == 'private' \
|
||||
and e.user_id != self.env.user \
|
||||
and current_partner_id not in e.partner_ids
|
||||
)
|
||||
if not others_private_events:
|
||||
return
|
||||
|
||||
def obfuscated(events):
|
||||
"""
|
||||
:param events: list of event values (dict)
|
||||
:return: events with private field values obfuscated
|
||||
"""
|
||||
public_fields = self._get_public_fields()
|
||||
return [{
|
||||
field: hide(field, value) if field not in public_fields else value
|
||||
for field, value in event.items()
|
||||
} for event in events]
|
||||
|
||||
events = super().read(fields=fields + ['privacy', 'user_id', 'partner_ids'], load=load)
|
||||
private_events, public_events = split_privacy(events)
|
||||
my_private_events, others_private_events = split_visibility(private_events)
|
||||
return public_events + my_private_events + obfuscated(others_private_events)
|
||||
for field_name in private_fields:
|
||||
field = self._fields[field_name]
|
||||
replacement = field.convert_to_cache(
|
||||
_('Busy') if field_name == 'name' else False,
|
||||
others_private_events)
|
||||
self.env.cache.update(others_private_events, field, repeat(replacement))
|
||||
|
||||
def write(self, values):
|
||||
detached_events = self.env['calendar.event']
|
||||
|
||||
@@ -5,54 +5,82 @@ from datetime import datetime
|
||||
|
||||
from odoo.tests.common import TransactionCase, new_test_user
|
||||
from odoo.exceptions import AccessError
|
||||
from odoo.tools import mute_logger
|
||||
|
||||
|
||||
class TestAccessRights(TransactionCase):
|
||||
|
||||
@classmethod
|
||||
@mute_logger('odoo.tests', 'odoo.addons.auth_signup.models.res_users')
|
||||
def setUpClass(cls):
|
||||
super().setUpClass()
|
||||
cls.john = new_test_user(cls.env, login='john', groups='base.group_user')
|
||||
cls.raoul = new_test_user(cls.env, login='raoul', groups='base.group_user')
|
||||
cls.george = new_test_user(cls.env, login='george', groups='base.group_user')
|
||||
cls.portal = new_test_user(cls.env, login='pot', groups='base.group_portal')
|
||||
|
||||
def create_event(self, user, **values):
|
||||
return self.env['calendar.event'].with_user(user).create(dict({
|
||||
return self.env['calendar.event'].with_user(user).create({
|
||||
'name': 'Event',
|
||||
'start': datetime(2020, 2, 2, 8, 0),
|
||||
'stop': datetime(2020, 2, 2, 18, 0),
|
||||
'user_id': user.id,
|
||||
}, **values))
|
||||
'partner_ids': [(4, self.george.partner_id.id, 0)],
|
||||
**values
|
||||
})
|
||||
|
||||
def read_event(self, user, events, field):
|
||||
data = events.with_user(user).read([field])
|
||||
if len(events) == 1:
|
||||
return data[0][field]
|
||||
mapped_data = {record['id']: record for record in data}
|
||||
# Keep the same order
|
||||
return [mapped_data[eid][field] for eid in events.ids]
|
||||
return [r[field] for r in data]
|
||||
|
||||
def test_private_read_name(self):
|
||||
# don't spam logs with ACL failures from portal
|
||||
@mute_logger('odoo.addons.base.models.ir_rule')
|
||||
def test_privacy(self):
|
||||
event = self.create_event(
|
||||
self.john,
|
||||
privacy='private',
|
||||
name='my private event',
|
||||
location='in the Sky'
|
||||
)
|
||||
self.assertEqual(self.read_event(self.john, event, 'name'), 'my private event', "Owner should be able to read the event")
|
||||
self.assertEqual(self.read_event(self.raoul, event, 'name'), 'Busy', "Private value should be obfuscated")
|
||||
with self.assertRaises(AccessError):
|
||||
self.read_event(self.portal, event, 'name')
|
||||
|
||||
def test_private_other_field(self):
|
||||
event = self.create_event(
|
||||
self.john,
|
||||
privacy='private',
|
||||
location='in the Sky',
|
||||
)
|
||||
self.assertEqual(self.read_event(self.john, event, 'location'), 'in the Sky', "Owner should be able to read the event")
|
||||
self.assertEqual(self.read_event(self.raoul, event, 'location'), False, "Private value should be obfuscated")
|
||||
with self.assertRaises(AccessError):
|
||||
self.read_event(self.portal, event, 'location')
|
||||
for user, field, expect, error in [
|
||||
# public field, any employee can read
|
||||
(self.john, 'privacy', 'private', None),
|
||||
(self.george, 'privacy', 'private', None),
|
||||
(self.raoul, 'privacy', 'private', None),
|
||||
(self.portal, 'privacy', None, AccessError),
|
||||
# substituted private field, only owner and invitees can read, other
|
||||
# employees get substitution
|
||||
(self.john, 'name', 'my private event', None),
|
||||
(self.george, 'name', 'my private event', None),
|
||||
(self.raoul, 'name', 'Busy', None),
|
||||
(self.portal, 'name', None, AccessError),
|
||||
# computed from private field
|
||||
(self.john, 'display_name', 'my private event', None),
|
||||
(self.george, 'display_name', 'my private event', None),
|
||||
(self.raoul, 'display_name', 'Busy', None),
|
||||
(self.portal, 'display_name', None, AccessError),
|
||||
# non-substituted private field, only owner and invitees can read,
|
||||
# other employees get an empty field
|
||||
(self.john, 'location', 'in the Sky', None),
|
||||
(self.george, 'location', 'in the Sky', None),
|
||||
(self.raoul, 'location', False, None),
|
||||
(self.portal, 'location', None, AccessError),
|
||||
# non-substituted sequence field
|
||||
(self.john, 'partner_ids', self.george.partner_id, None),
|
||||
(self.george, 'partner_ids', self.george.partner_id, None),
|
||||
(self.raoul, 'partner_ids', self.env['res.partner'], None),
|
||||
(self.portal, 'partner_ids', None, AccessError),
|
||||
]:
|
||||
event.invalidate_cache()
|
||||
with self.subTest("private read", user=user.display_name, field=field, error=error):
|
||||
e = event.with_user(user)
|
||||
if error:
|
||||
with self.assertRaises(error):
|
||||
_ = e[field]
|
||||
else:
|
||||
self.assertEqual(e[field], expect)
|
||||
|
||||
def test_private_and_public(self):
|
||||
private = self.create_event(
|
||||
|
||||
Reference in New Issue
Block a user