From 54aaf3a0a55f2f4a2cbb162007cc9694708fe164 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Thu, 26 Oct 2023 14:05:00 +0200 Subject: [PATCH 1/5] [IMP] mail: display activities without record access in the systray When scheduling plans on employees, activities may be assigned to people who don't have access to the record. It was managed locally in HR through a dummy model aggregating activities. However, it was removed and we replace it here by a generic solution that adds an entry in the systray: "Other activities". When clicking on that entry, it displays a list view of the activities assigned to the user for which he has no access to the related record. Those activities were not visible to the user before. Technical note: to allow displaying such records, we have modified the custom acl defined in python to allow users to read activities assigned to them even if they don't have access to the related record. Task-3562098 Part-of: odoo/odoo#139917 --- addons/mail/models/mail_activity.py | 16 ++++++--- addons/mail/models/res_users.py | 35 ++++++++++++------- .../mail/static/src/core/web/activity_menu.js | 14 ++++++-- addons/test_mail/tests/test_mail_activity.py | 20 +++++++++++ 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 7ac69f4dc8f..5483d98af7d 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -218,6 +218,9 @@ class MailActivity(models.Model): valid = super(MailActivity, self)._filter_access_rules(operation) if valid and valid == self: return self + elif operation == 'read': + # Not in the ACL otherwise it would break the custom _search method + valid = self.sudo().filtered_domain([('user_id', '=', self.env.uid)]) else: valid = self.env[self._name] return self._filter_access_rules_remaining(valid, operation, '_filter_access_rules') @@ -228,6 +231,8 @@ class MailActivity(models.Model): valid = super(MailActivity, self)._filter_access_rules_python(operation) if valid and valid == self: return self + elif operation == 'read': + valid = self.sudo().filtered_domain([('user_id', '=', self.env.uid)]) else: valid = self.env[self._name] return self._filter_access_rules_remaining(valid, operation, '_filter_access_rules_python') @@ -413,14 +418,17 @@ class MailActivity(models.Model): f'"{self._table}"."id"', f'"{self._table}"."res_model"', f'"{self._table}"."res_id"', + f'"{self._table}"."user_id"', ) self.env.cr.execute(query_str, params) rows = self.env.cr.fetchall() # group res_ids by model, and determine accessible records + # Note: the user can read all activities assigned to him (see at the end of the method) model_ids = defaultdict(set) - for _id, res_model, res_id in rows: - model_ids[res_model].add(res_id) + for __, res_model, res_id, user_id in rows: + if user_id != self.env.uid: + model_ids[res_model].add(res_id) allowed_ids = defaultdict(set) for res_model, res_ids in model_ids.items(): @@ -433,8 +441,8 @@ class MailActivity(models.Model): activities = self.browse( id_ - for id_, res_model, res_id in rows - if res_id in allowed_ids[res_model] + for id_, res_model, res_id, user_id in rows + if user_id == self.env.uid or res_id in allowed_ids[res_model] ) return activities._as_query(order) diff --git a/addons/mail/models/res_users.py b/addons/mail/models/res_users.py index 04a1cbb35ee..6c8df4d163a 100644 --- a/addons/mail/models/res_users.py +++ b/addons/mail/models/res_users.py @@ -278,14 +278,22 @@ class Users(models.Model): for activity in activities: record = self.env[activity.res_model].browse(activity.res_id) activities_by_record_by_model_name[activity.res_model][record] += activity - model_ids = list({self.env["ir.model"]._get(name).id for name in activities_by_record_by_model_name.keys()}) - user_activities = {} + activities_by_model_name = defaultdict(lambda: self.env["mail.activity"]) for model_name, activities_by_record in activities_by_record_by_model_name.items(): - domain = [("id", "in", list({r.id for r in activities_by_record.keys()}))] + if self.env[model_name].check_access_rights('read', raise_exception=False): + res_ids = [r.id for r in activities_by_record] + allowed_records = self.env[model_name].browse(res_ids)._filter_access_rules('read') + else: + allowed_records = self.env[model_name] + for record, activities in activities_by_record.items(): + if record not in allowed_records: + activities_by_model_name['mail.activity'] += activities + else: + activities_by_model_name[model_name] += activities + model_ids = [self.env["ir.model"]._get_id(name) for name in activities_by_model_name] + user_activities = {} + for model_name, activities in activities_by_model_name.items(): Model = self.env[model_name] - allowed_records = Model.search(domain) - if not allowed_records: - continue module = Model._original_module icon = module and modules.module.get_module_icon(module) model = self.env["ir.model"]._get(model_name).with_prefetch(model_ids) @@ -301,11 +309,12 @@ class Users(models.Model): "planned_count": 0, "view_type": getattr(Model, '_systray_view', 'list'), } - for record, activities in activities_by_record.items(): - if record not in allowed_records: - continue - for activity in activities: - user_activities[model_name]["%s_count" % activity.state] += 1 - if activity.state in ("today", "overdue"): - user_activities[model_name]["total_count"] += 1 + if model_name == 'mail.activity': + user_activities[model_name]['activity_ids'] = activities.ids + for activity in activities: + user_activities[model_name]["%s_count" % activity.state] += 1 + if activity.state in ("today", "overdue"): + user_activities[model_name]["total_count"] += 1 + if "mail.activity" in user_activities: + user_activities["mail.activity"]["name"] = _("Other activities") return list(user_activities.values()) diff --git a/addons/mail/static/src/core/web/activity_menu.js b/addons/mail/static/src/core/web/activity_menu.js index 72aa7b2e191..bd8e04eec49 100644 --- a/addons/mail/static/src/core/web/activity_menu.js +++ b/addons/mail/static/src/core/web/activity_menu.js @@ -31,8 +31,13 @@ export class ActivityMenu extends Component { this.sortActivityGroups(); } + /** + * Sort by model ID ASC but always place the activity group for "mail.activity" model at the end (other activities). + */ sortActivityGroups() { - this.store.activityGroups.sort((g1, g2) => g1.id - g2.id); + const getSortId = (activityGroup) => + activityGroup.model === "mail.activity" ? Number.MAX_VALUE : activityGroup.id; + this.store.activityGroups.sort((g1, g2) => getSortId(g1) - getSortId(g2)); } onBeforeOpen() { @@ -55,7 +60,12 @@ export class ActivityMenu extends Component { // So, duplicates are faking the count and "Load more" doesn't show up force_search_count: 1, }; - let domain = [["activity_user_id", "=", this.userId]]; + let domain; + if (group.model !== "mail.activity") { + domain = [["activity_user_id", "=", this.userId]]; + } else { + domain = [["id", "in", group.activity_ids]]; + } if (group.domain) { domain = Domain.and([domain, group.domain]).toList(); } diff --git a/addons/test_mail/tests/test_mail_activity.py b/addons/test_mail/tests/test_mail_activity.py index f64a2ba6edd..aae2d186f37 100644 --- a/addons/test_mail/tests/test_mail_activity.py +++ b/addons/test_mail/tests/test_mail_activity.py @@ -141,6 +141,26 @@ class TestActivityRights(TestActivityCommon): 'test_mail.mail_act_test_todo', user_id=self.user_admin.id) + test_activity.user_id = self.user_employee + test_activity.flush_recordset() + + # user can read activities assigned to him even if he has no access to the document + with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash): + found = self.env['mail.activity'].with_user(self.user_employee).search( + [('id', '=', test_activity.id)]) + self.assertEqual(found, test_activity) + found.read(['summary']) + + # user can read_group activities assigned to him even if he has no access to the document + with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash): + read_group_result = self.env['mail.activity'].with_user(self.user_employee).read_group( + [('id', '=', test_activity.id)], + ['summary'], + ['summary'], + ) + self.assertEqual(1, read_group_result[0]['summary_count']) + self.assertEqual('Summary', read_group_result[0]['summary']) + @tests.tagged('mail_activity') class TestActivityFlow(TestActivityCommon): From e91814667042f3f267dc10ade94b192ce90090e6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Thu, 2 Nov 2023 15:29:05 +0100 Subject: [PATCH 2/5] [IMP] mail: allow to mark as done activity without record access When scheduling plans on employees, activities may be assigned to people who don't have access to the record, but they still need to mark them as done. And as marking as done (action_done on mail_activity) triggers a message post on the related record, that action cannot be executed by a user without access to the record. We change that here to allow user having delete/archive right on the activity, to be able as well to mark it as done (as the activity is archived/deleted at the end of the method _action_done, the user must at least have the right to do that to be able to mark as done the activity). Task-3562098 Part-of: odoo/odoo#139917 --- addons/mail/models/mail_activity.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 5483d98af7d..01256dfe141 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -552,17 +552,20 @@ class MailActivity(models.Model): activity_attachments[activity_id].append(attachment['id']) for model, activity_data in self._classify_by_model().items(): - records = self.env[model].browse(activity_data['record_ids']) - for record, activity in zip(records, activity_data['activities']): + # Allow user without access to the record to "mark as done" activities assigned to them. At the end of the + # method, the activity is unlinked or archived which ensure the user has enough right on the activities. + records_sudo = self.env[model].sudo().browse(activity_data['record_ids']) + for record_sudo, activity in zip(records_sudo, activity_data['activities']): # extract value to generate next activities if activity.chaining_type == 'trigger': vals = activity.with_context(activity_previous_deadline=activity.date_deadline)._prepare_next_activity_values() next_activities_values.append(vals) # post message on activity, before deleting it - activity_message = record.message_post_with_source( + activity_message = record_sudo.message_post_with_source( 'mail.message_activity_done', attachment_ids=attachment_ids, + author_id=self.env.user.partner_id.id, render_values={ 'activity': activity, 'feedback': feedback, From c5bd3f6b2f55ed383f01123324a0027a52de8f93 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Fri, 27 Oct 2023 09:31:08 +0200 Subject: [PATCH 3/5] [IMP] mail: add custom view for "other activities" When clicking on "Other activities" in the systray, a list of activities assigned to the user for which he has no access to underlying record is displayed. The default list allows to delete, archive, duplicate, ... activities but what we want is a readonly view. We add here a new view that doesn't allow any modification on the activities by removing those actions. Unfortunately, archive and unarchive action is not controllable with an attribute (nor by setting the active field as readonly in the view), so we create a very simple extension of the list view to disable it. We also add in the list view a button to mark an activity as done and a simple form view to see the details of the activity (note, ...). The "mark as done is not added in the form view as the activity is deleted causing an error because the form is still displayed and the record is gone. Note that those action are only removed from the client but the actions are not forbidden on the server side. Task-3562098 Part-of: odoo/odoo#139917 --- addons/mail/i18n/mail.pot | 10 +++ .../mail/static/src/core/web/activity_menu.js | 13 ++-- .../list/archive_disabled_list_controller.js | 10 +++ .../web/list/archive_disabled_list_view.js | 12 ++++ addons/mail/views/mail_activity_views.xml | 70 +++++++++++++++++++ 5 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 addons/mail/static/src/views/web/list/archive_disabled_list_controller.js create mode 100644 addons/mail/static/src/views/web/list/archive_disabled_list_view.js diff --git a/addons/mail/i18n/mail.pot b/addons/mail/i18n/mail.pot index af5344b1b30..da5ad0af1e5 100644 --- a/addons/mail/i18n/mail.pot +++ b/addons/mail/i18n/mail.pot @@ -651,6 +651,7 @@ msgstr "" #: model_terms:ir.ui.view,arch_db:mail.mail_activity_plan_template_view_form #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_calendar #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_popup +#: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_without_record_access #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_search #, python-format msgid "Activity" @@ -5163,11 +5164,13 @@ msgstr "" #. module: mail #: model_terms:ir.ui.view,arch_db:mail.mail_activity_schedule_view_form #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_popup +#: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_without_record_access msgid "Log a note..." msgstr "" #. module: mail #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_popup +#: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_without_record_access msgid "Log an Activity" msgstr "" @@ -5455,6 +5458,7 @@ msgstr "" #. module: mail #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_popup +#: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_tree_without_record_access msgid "Mark as Done" msgstr "" @@ -6031,6 +6035,11 @@ msgstr "" msgid "No Record" msgstr "" +#. module: mail +#: model_terms:ir.actions.act_window,help:mail.mail_activity_without_access_action +msgid "No activities." +msgstr "" + #. module: mail #. odoo-javascript #: code:addons/mail/static/src/discuss/core/web/command_palette.js:0 @@ -10532,6 +10541,7 @@ msgstr "" #. module: mail #: model_terms:ir.ui.view,arch_db:mail.mail_activity_schedule_view_form #: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_popup +#: model_terms:ir.ui.view,arch_db:mail.mail_activity_view_form_without_record_access #: model_terms:ir.ui.view,arch_db:mail.view_server_action_form_template msgid "e.g. Discuss proposal" msgstr "" diff --git a/addons/mail/static/src/core/web/activity_menu.js b/addons/mail/static/src/core/web/activity_menu.js index bd8e04eec49..d81c69eb466 100644 --- a/addons/mail/static/src/core/web/activity_menu.js +++ b/addons/mail/static/src/core/web/activity_menu.js @@ -60,12 +60,15 @@ export class ActivityMenu extends Component { // So, duplicates are faking the count and "Load more" doesn't show up force_search_count: 1, }; - let domain; - if (group.model !== "mail.activity") { - domain = [["activity_user_id", "=", this.userId]]; - } else { - domain = [["id", "in", group.activity_ids]]; + if (group.model === "mail.activity") { + this.action.doAction("mail.mail_activity_without_access_action", { + additionalContext: { + active_ids: group.activity_ids, + }, + }); + return; } + let domain = [["activity_user_id", "=", this.userId]]; if (group.domain) { domain = Domain.and([domain, group.domain]).toList(); } diff --git a/addons/mail/static/src/views/web/list/archive_disabled_list_controller.js b/addons/mail/static/src/views/web/list/archive_disabled_list_controller.js new file mode 100644 index 00000000000..da0270c4bda --- /dev/null +++ b/addons/mail/static/src/views/web/list/archive_disabled_list_controller.js @@ -0,0 +1,10 @@ +/** @odoo-module */ + +import { ListController } from "@web/views/list/list_controller"; + +export class ArchiveDisabledListController extends ListController { + setup() { + super.setup(); + this.archiveEnabled = false; + } +} diff --git a/addons/mail/static/src/views/web/list/archive_disabled_list_view.js b/addons/mail/static/src/views/web/list/archive_disabled_list_view.js new file mode 100644 index 00000000000..cdd4c97d330 --- /dev/null +++ b/addons/mail/static/src/views/web/list/archive_disabled_list_view.js @@ -0,0 +1,12 @@ +/** @odoo-module **/ + +import { registry } from "@web/core/registry"; +import { listView } from "@web/views/list/list_view"; +import { ArchiveDisabledListController } from "./archive_disabled_list_controller"; + +export const archiveDisabledListView = { + ...listView, + Controller: ArchiveDisabledListController, +}; + +registry.category("views").add("archive_disabled_activity_list", archiveDisabledListView); diff --git a/addons/mail/views/mail_activity_views.xml b/addons/mail/views/mail_activity_views.xml index be0a6340b98..c82095e77c7 100644 --- a/addons/mail/views/mail_activity_views.xml +++ b/addons/mail/views/mail_activity_views.xml @@ -194,6 +194,30 @@ + + mail.activity.view.form.without.record.access + mail.activity + 32 + +
+ + + + + + + + + + + + + + +
+ + + mail.activity.view.search mail.activity @@ -235,6 +259,25 @@ + + mail.activity.view.tree.without.record.access + mail.activity + + primary + 32 + + + 0 + 0 + 0 + archive_disabled_activity_list + + +