From 07fb34691993a83fce31a6850a00659adfac8936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 31 Jan 2022 08:30:00 +0000 Subject: [PATCH 01/23] [IMP] various: update counters Update some counters at latest runbot results, just to have updated counters. Note that some counters are somehow not deterministic, hence not updating everything. Task-2712450 (Mail/Sale: Improve 'Pay Now' notification template) Part-of: odoo/odoo#82167 --- addons/google_calendar/tests/test_sync_odoo2google.py | 4 ++-- addons/hr_holidays/tests/test_company_leave.py | 2 +- addons/sale_stock/tests/test_create_perf.py | 4 ++-- odoo/addons/base/tests/test_ir_actions.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/addons/google_calendar/tests/test_sync_odoo2google.py b/addons/google_calendar/tests/test_sync_odoo2google.py index c69dfdeac86..75374bf6412 100644 --- a/addons/google_calendar/tests/test_sync_odoo2google.py +++ b/addons/google_calendar/tests/test_sync_odoo2google.py @@ -70,7 +70,7 @@ class TestSyncOdoo2Google(TestSyncGoogle): }) partner_model = self.env.ref('base.model_res_partner') partner = self.env['res.partner'].search([], limit=1) - with self.assertQueryCount(__system__=1211): + with self.assertQueryCount(__system__=1112): events = self.env['calendar.event'].create([{ 'name': "Event %s" % (i), 'start': datetime(2020, 1, 15, 8, 0), @@ -102,7 +102,7 @@ class TestSyncOdoo2Google(TestSyncGoogle): }) partner_model = self.env.ref('base.model_res_partner') partner = self.env['res.partner'].search([], limit=1) - with self.assertQueryCount(__system__=3635): + with self.assertQueryCount(__system__=2916): event = self.env['calendar.event'].create({ 'name': "Event", 'start': datetime(2020, 1, 15, 8, 0), diff --git a/addons/hr_holidays/tests/test_company_leave.py b/addons/hr_holidays/tests/test_company_leave.py index 2a7b78b4b67..73acca240e1 100644 --- a/addons/hr_holidays/tests/test_company_leave.py +++ b/addons/hr_holidays/tests/test_company_leave.py @@ -323,7 +323,7 @@ class TestCompanyLeave(TransactionCase): }) company_leave._compute_date_from_to() - with self.assertQueryCount(__system__=773, admin=865): + with self.assertQueryCount(__system__=659, admin=865): # Original query count: 1987 # Without tracking/activity context keys: 5154 company_leave.action_validate() diff --git a/addons/sale_stock/tests/test_create_perf.py b/addons/sale_stock/tests/test_create_perf.py index c53810c0393..50e2a83d0de 100644 --- a/addons/sale_stock/tests/test_create_perf.py +++ b/addons/sale_stock/tests/test_create_perf.py @@ -160,6 +160,6 @@ class TestPERF(common.TransactionCase): ], } for i in range(self.ENTITIES)] - # 1592 locally, 1593 in nightly runbot - with self.assertQueryCount(admin=1593): + # 1592 locally, 1593 in nightly runbot, 1954 sometimes + with self.assertQueryCount(admin=1594): self.env["sale.order"].create(vals_list) diff --git a/odoo/addons/base/tests/test_ir_actions.py b/odoo/addons/base/tests/test_ir_actions.py index 53a1384bc56..379af5f6b78 100644 --- a/odoo/addons/base/tests/test_ir_actions.py +++ b/odoo/addons/base/tests/test_ir_actions.py @@ -503,7 +503,7 @@ class TestCustomFields(common.TransactionCase): # create a non-computed field, and assert how many queries it takes model_id = self.env['ir.model']._get_id('res.partner') - query_count = 44 + query_count = 42 with self.assertQueryCount(query_count): self.env.registry.clear_caches() self.env['ir.model.fields'].create({ From 0b952df5e012403434181fe74b1d0c25b04875e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 19 Jan 2022 16:21:12 +0000 Subject: [PATCH 02/23] [MOV] account, purchase: move thread code in its own section Purpose is to better isolate MailThread related methods in those huge files containing lot of code. Better have them located in a sub-section in order to have all mail code at the same place. Task-2710804 (Mail: Clean Mail.Thread API) Part-of: odoo/odoo#82167 --- addons/account/models/account_move.py | 228 +++++++++++++------------- addons/purchase/models/purchase.py | 44 +++-- 2 files changed, 142 insertions(+), 130 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index a8484dba4d2..d7b9de5bd9e 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -2346,41 +2346,6 @@ class AccountMove(models.Model): result.append((move.id, name)) return result - def _creation_subtype(self): - # OVERRIDE - if self.move_type in ('out_invoice', 'out_refund', 'out_receipt'): - return self.env.ref('account.mt_invoice_created') - else: - return super(AccountMove, self)._creation_subtype() - - def _track_subtype(self, init_values): - # OVERRIDE to add custom subtype depending of the state. - self.ensure_one() - - if not self.is_invoice(include_receipts=True): - if self.payment_id and 'state' in init_values: - self.payment_id._message_track(['state'], {self.payment_id.id: init_values}) - return super(AccountMove, self)._track_subtype(init_values) - - if 'payment_state' in init_values and self.payment_state == 'paid': - return self.env.ref('account.mt_invoice_paid') - elif 'state' in init_values and self.state == 'posted' and self.is_sale_document(include_receipts=True): - return self.env.ref('account.mt_invoice_validated') - return super(AccountMove, self)._track_subtype(init_values) - - def _creation_message(self): - # OVERRIDE - if not self.is_invoice(include_receipts=True): - return super()._creation_message() - return { - 'out_invoice': _('Invoice Created'), - 'out_refund': _('Credit Note Created'), - 'in_invoice': _('Vendor Bill Created'), - 'in_refund': _('Refund Created'), - 'out_receipt': _('Sales Receipt Created'), - 'in_receipt': _('Purchase Receipt Created'), - }[self.move_type] - # ------------------------------------------------------------------------- # RECONCILIATION METHODS # ------------------------------------------------------------------------- @@ -2857,59 +2822,6 @@ class AccountMove(models.Model): 'views': [(self.env.ref('account.view_move_tree').id, 'tree'), (False, 'form')], } - @api.model - def message_new(self, msg_dict, custom_values=None): - # OVERRIDE - # Add custom behavior when receiving a new invoice through the mail's gateway. - if (custom_values or {}).get('move_type', 'entry') not in ('out_invoice', 'in_invoice'): - return super().message_new(msg_dict, custom_values=custom_values) - - def is_internal_partner(partner): - # Helper to know if the partner is an internal one. - return partner.user_ids and all(user.has_group('base.group_user') for user in partner.user_ids) - - extra_domain = False - if custom_values.get('company_id'): - extra_domain = ['|', ('company_id', '=', custom_values['company_id']), ('company_id', '=', False)] - - # Search for partners in copy. - cc_mail_addresses = email_split(msg_dict.get('cc', '')) - followers = [partner for partner in self._mail_find_partner_from_emails(cc_mail_addresses, extra_domain) if partner] - - # Search for partner that sent the mail. - from_mail_addresses = email_split(msg_dict.get('from', '')) - senders = partners = [partner for partner in self._mail_find_partner_from_emails(from_mail_addresses, extra_domain) if partner] - - # Search for partners using the user. - if not senders: - senders = partners = list(self._mail_search_on_user(from_mail_addresses)) - - if partners: - # Check we are not in the case when an internal user forwarded the mail manually. - if is_internal_partner(partners[0]): - # Search for partners in the mail's body. - body_mail_addresses = set(email_re.findall(msg_dict.get('body'))) - partners = [partner for partner in self._mail_find_partner_from_emails(body_mail_addresses, extra_domain) if not is_internal_partner(partner)] - - # Little hack: Inject the mail's subject in the body. - if msg_dict.get('subject') and msg_dict.get('body'): - msg_dict['body'] = '

%s

%s
' % (msg_dict['subject'], msg_dict['body']) - - # Create the invoice. - values = { - 'name': '/', # we have to give the name otherwise it will be set to the mail's subject - 'invoice_source_email': from_mail_addresses[0], - 'partner_id': partners and partners[0].id or False, - } - move_ctx = self.with_context(default_move_type=custom_values['move_type'], default_journal_id=custom_values['journal_id']) - move = super(AccountMove, move_ctx).message_new(msg_dict, custom_values=values) - move._compute_name() # because the name is given, we need to recompute in case it is the first invoice of the journal - - # Assign followers. - all_followers_ids = set(partner.id for partner in followers + senders + partners if is_internal_partner(partner)) - move.message_subscribe(list(all_followers_ids)) - return move - def post(self): warnings.warn( "RedirectWarning method 'post()' is a deprecated alias to 'action_post()' or _post()", @@ -3403,6 +3315,92 @@ class AccountMove(models.Model): return rslt + def _get_create_invoice_from_attachment_decoders(self): + """ Returns a list of method that are able to create an invoice from an attachment and a priority. + + :returns: A list of tuples (priority, method) where method takes an attachment as parameter. + """ + return [] + + def _get_update_invoice_from_attachment_decoders(self, invoice): + """ Returns a list of method that are able to create an invoice from an attachment and a priority. + + :param invoice: The invoice on which to update the data. + :returns: A list of tuples (priority, method) where method takes an attachment as parameter. + """ + return [] + + @api.depends('move_type', 'partner_id', 'company_id') + def _compute_narration(self): + use_invoice_terms = self.env['ir.config_parameter'].sudo().get_param('account.use_invoice_terms') + for move in self.filtered(lambda am: not am.narration): + if not use_invoice_terms or not move.is_sale_document(include_receipts=True): + move.narration = False + else: + if not move.company_id.terms_type == 'html': + narration = move.company_id.invoice_terms if not is_html_empty(move.company_id.invoice_terms) else '' + else: + baseurl = self.env.company.get_base_url() + '/terms' + narration = _('Terms & Conditions: %s', baseurl) + move.narration = narration or False + + # ------------------------------------------------------------ + # MAIL.THREAD + # ------------------------------------------------------------ + + @api.model + def message_new(self, msg_dict, custom_values=None): + # OVERRIDE + # Add custom behavior when receiving a new invoice through the mail's gateway. + if (custom_values or {}).get('move_type', 'entry') not in ('out_invoice', 'in_invoice'): + return super().message_new(msg_dict, custom_values=custom_values) + + def is_internal_partner(partner): + # Helper to know if the partner is an internal one. + return partner.user_ids and all(user.has_group('base.group_user') for user in partner.user_ids) + + extra_domain = False + if custom_values.get('company_id'): + extra_domain = ['|', ('company_id', '=', custom_values['company_id']), ('company_id', '=', False)] + + # Search for partners in copy. + cc_mail_addresses = email_split(msg_dict.get('cc', '')) + followers = [partner for partner in self._mail_find_partner_from_emails(cc_mail_addresses, extra_domain) if partner] + + # Search for partner that sent the mail. + from_mail_addresses = email_split(msg_dict.get('from', '')) + senders = partners = [partner for partner in self._mail_find_partner_from_emails(from_mail_addresses, extra_domain) if partner] + + # Search for partners using the user. + if not senders: + senders = partners = list(self._mail_search_on_user(from_mail_addresses)) + + if partners: + # Check we are not in the case when an internal user forwarded the mail manually. + if is_internal_partner(partners[0]): + # Search for partners in the mail's body. + body_mail_addresses = set(email_re.findall(msg_dict.get('body'))) + partners = [partner for partner in self._mail_find_partner_from_emails(body_mail_addresses, extra_domain) if not is_internal_partner(partner)] + + # Little hack: Inject the mail's subject in the body. + if msg_dict.get('subject') and msg_dict.get('body'): + msg_dict['body'] = '

%s

%s
' % (msg_dict['subject'], msg_dict['body']) + + # Create the invoice. + values = { + 'name': '/', # we have to give the name otherwise it will be set to the mail's subject + 'invoice_source_email': from_mail_addresses[0], + 'partner_id': partners and partners[0].id or False, + } + move_ctx = self.with_context(default_move_type=custom_values['move_type'], default_journal_id=custom_values['journal_id']) + move = super(AccountMove, move_ctx).message_new(msg_dict, custom_values=values) + move._compute_name() # because the name is given, we need to recompute in case it is the first invoice of the journal + + # Assign followers. + all_followers_ids = set(partner.id for partner in followers + senders + partners if is_internal_partner(partner)) + move.message_subscribe(list(all_followers_ids)) + return move + def _message_post_after_hook(self, new_message, message_values): # OVERRIDE # When posting a message, check the attachment to see if it's an invoice and update with the imported data. @@ -3437,34 +3435,40 @@ class AccountMove(models.Model): return res - def _get_create_invoice_from_attachment_decoders(self): - """ Returns a list of method that are able to create an invoice from an attachment and a priority. + def _creation_subtype(self): + # OVERRIDE + if self.move_type in ('out_invoice', 'out_refund', 'out_receipt'): + return self.env.ref('account.mt_invoice_created') + else: + return super(AccountMove, self)._creation_subtype() - :returns: A list of tuples (priority, method) where method takes an attachment as parameter. - """ - return [] + def _track_subtype(self, init_values): + # OVERRIDE to add custom subtype depending of the state. + self.ensure_one() - def _get_update_invoice_from_attachment_decoders(self, invoice): - """ Returns a list of method that are able to create an invoice from an attachment and a priority. + if not self.is_invoice(include_receipts=True): + if self.payment_id and 'state' in init_values: + self.payment_id._message_track(['state'], {self.payment_id.id: init_values}) + return super(AccountMove, self)._track_subtype(init_values) - :param invoice: The invoice on which to update the data. - :returns: A list of tuples (priority, method) where method takes an attachment as parameter. - """ - return [] + if 'payment_state' in init_values and self.payment_state == 'paid': + return self.env.ref('account.mt_invoice_paid') + elif 'state' in init_values and self.state == 'posted' and self.is_sale_document(include_receipts=True): + return self.env.ref('account.mt_invoice_validated') + return super(AccountMove, self)._track_subtype(init_values) - @api.depends('move_type', 'partner_id', 'company_id') - def _compute_narration(self): - use_invoice_terms = self.env['ir.config_parameter'].sudo().get_param('account.use_invoice_terms') - for move in self.filtered(lambda am: not am.narration): - if not use_invoice_terms or not move.is_sale_document(include_receipts=True): - move.narration = False - else: - if not move.company_id.terms_type == 'html': - narration = move.company_id.invoice_terms if not is_html_empty(move.company_id.invoice_terms) else '' - else: - baseurl = self.env.company.get_base_url() + '/terms' - narration = _('Terms & Conditions: %s', baseurl) - move.narration = narration or False + def _creation_message(self): + # OVERRIDE + if not self.is_invoice(include_receipts=True): + return super()._creation_message() + return { + 'out_invoice': _('Invoice Created'), + 'out_refund': _('Credit Note Created'), + 'in_invoice': _('Vendor Bill Created'), + 'in_refund': _('Refund Created'), + 'out_receipt': _('Sales Receipt Created'), + 'in_receipt': _('Purchase Receipt Created'), + }[self.move_type] class AccountMoveLine(models.Model): diff --git a/addons/purchase/models/purchase.py b/addons/purchase/models/purchase.py index 3d2dc6d67ca..e3fece18edf 100644 --- a/addons/purchase/models/purchase.py +++ b/addons/purchase/models/purchase.py @@ -289,18 +289,6 @@ class PurchaseOrder(models.Model): del line[2]['date_planned'] return result - def _track_subtype(self, init_values): - self.ensure_one() - if 'state' in init_values and self.state == 'purchase': - if init_values['state'] == 'to approve': - return self.env.ref('purchase.mt_rfq_approved') - return self.env.ref('purchase.mt_rfq_confirmed') - elif 'state' in init_values and self.state == 'to approve': - return self.env.ref('purchase.mt_rfq_confirmed') - elif 'state' in init_values and self.state == 'done': - return self.env.ref('purchase.mt_rfq_done') - return super(PurchaseOrder, self)._track_subtype(init_values) - def _get_report_base_filename(self): self.ensure_one() return 'Purchase Order-%s' % (self.name) @@ -353,6 +341,32 @@ class PurchaseOrder(models.Model): return {'warning': warning} return {} + # ------------------------------------------------------------ + # MAIL.THREAD + # ------------------------------------------------------------ + + @api.returns('mail.message', lambda value: value.id) + def message_post(self, **kwargs): + if self.env.context.get('mark_rfq_as_sent'): + self.filtered(lambda o: o.state == 'draft').write({'state': 'sent'}) + return super(PurchaseOrder, self.with_context(mail_post_autofollow=self.env.context.get('mail_post_autofollow', True))).message_post(**kwargs) + + def _track_subtype(self, init_values): + self.ensure_one() + if 'state' in init_values and self.state == 'purchase': + if init_values['state'] == 'to approve': + return self.env.ref('purchase.mt_rfq_approved') + return self.env.ref('purchase.mt_rfq_confirmed') + elif 'state' in init_values and self.state == 'to approve': + return self.env.ref('purchase.mt_rfq_confirmed') + elif 'state' in init_values and self.state == 'done': + return self.env.ref('purchase.mt_rfq_done') + return super(PurchaseOrder, self)._track_subtype(init_values) + + # ------------------------------------------------------------ + # ACTIONS + # ------------------------------------------------------------ + def action_rfq_send(self): ''' This function opens a window to compose an email, with the edi purchase template message loaded by default @@ -410,12 +424,6 @@ class PurchaseOrder(models.Model): 'context': ctx, } - @api.returns('mail.message', lambda value: value.id) - def message_post(self, **kwargs): - if self.env.context.get('mark_rfq_as_sent'): - self.filtered(lambda o: o.state == 'draft').write({'state': 'sent'}) - return super(PurchaseOrder, self.with_context(mail_post_autofollow=self.env.context.get('mail_post_autofollow', True))).message_post(**kwargs) - def print_quotation(self): self.write({'state': "sent"}) return self.env.ref('purchase.report_purchase_quotation').report_action(self) From 1f7c83cc3e97c7c81893c4c90f69727242c92eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 22 Sep 2021 13:38:46 +0000 Subject: [PATCH 03/23] [REF] mail, various: clean thread _notify API PURPOSE Global purpose is to rename some methods and add some docstrings to clean API of notification methods used in mail thread. Notably use _notify_thread prefix for main methods, and _notify_by_'mean' for tool sub-methods. SPECIFICATIONS Remove unused arguments coming from old implementation and usage. They were introduced notably for performance reason when cache was more often invalidated which is not the case anymore. Anyway when parameters are unused it is always better to remove them. Remove ``notify_by_email`` parameter in ``_notify_thread``. It is only used in channels to avoid notifying people of some automated notifications. The same behavior has been cleanly implemented at odoo/odoo@018820d . Rename methods, starting with ``_notify(_records)`` to ease their grouping and understanding. Add some additional prefixes like ``_notify_by_'mean'`` and ``_notify_get_recipients`` for recipients related computation. Add some docstrings, notably when parameters usage is not clear. Some linting is also performed in updated actions, just to lessen styling issues notably on runbot. This commit should not change anything functionally as it contains only some renaming and docstrings updates as well as some outdated parameters removal. Task-2710804 (Mail: Clean Mail.Thread API) Part-of: odoo/odoo#82167 --- addons/crm/models/crm_lead.py | 17 +- addons/hr_holidays/models/hr_leave.py | 11 +- .../hr_holidays/models/hr_leave_allocation.py | 11 +- .../hr_recruitment/models/hr_recruitment.py | 6 +- addons/mail/models/mail_channel.py | 16 +- addons/mail/models/mail_thread.py | 391 ++++++++++-------- addons/mail/models/models.py | 25 +- addons/mail/tests/common.py | 2 +- addons/mail/wizard/mail_compose_message.py | 4 +- addons/mail/wizard/mail_resend_message.py | 6 +- addons/mail/wizard/mail_wizard_invite.py | 17 +- addons/mail_group/models/mail_group.py | 2 +- addons/portal/models/portal_mixin.py | 4 +- addons/project/models/project.py | 14 +- addons/sms/models/mail_thread.py | 28 +- addons/sms/wizard/sms_resend.py | 19 +- addons/test_mail/tests/test_message_post.py | 14 +- addons/test_mail/tests/test_message_track.py | 4 +- addons/test_mail_full/tests/test_sms_post.py | 2 +- addons/website_blog/models/website_blog.py | 12 +- addons/website_forum/models/forum.py | 12 +- addons/website_slides/models/slide_slide.py | 6 +- .../wizard/slide_channel_invite.py | 2 +- 23 files changed, 354 insertions(+), 271 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 9bcd56bd1cd..d7b909347fb 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -1751,10 +1751,10 @@ class Lead(models.Model): return self.env.ref('crm.mt_lead_lost') return super(Lead, self)._track_subtype(init_values) - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ Handle salesman recipients that can convert leads into opportunities and set opportunities as won / lost. """ - groups = super(Lead, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(Lead, self)._notify_get_recipients_groups(msg_vals=msg_vals) local_msg_vals = dict(msg_vals or {}) self.ensure_one() @@ -1777,19 +1777,20 @@ class Lead(models.Model): salesman_group_id = self.env.ref('sales_team.group_sale_salesman').id new_group = ( - 'group_sale_salesman', lambda pdata: pdata['type'] == 'user' and salesman_group_id in pdata['groups'], { - 'actions': salesman_actions, - }) + 'group_sale_salesman', + lambda pdata: pdata['type'] == 'user' and salesman_group_id in pdata['groups'], + {'actions': salesman_actions} + ) return [new_group] + groups - def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): + def _notify_get_reply_to(self, default=None): """ Override to set alias of lead and opportunities to their sales team if any. """ - aliases = self.mapped('team_id').sudo()._notify_get_reply_to(default=default, records=None, company=company, doc_names=None) + aliases = self.mapped('team_id').sudo()._notify_get_reply_to(default=default) res = {lead.id: aliases.get(lead.team_id.id) for lead in self} leftover = self.filtered(lambda rec: not rec.team_id) if leftover: - res.update(super(Lead, leftover)._notify_get_reply_to(default=default, records=None, company=company, doc_names=doc_names)) + res.update(super(Lead, leftover)._notify_get_reply_to(default=default)) return res def _message_get_default_recipients(self): diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 1d38a5057bf..7d8a4b8dcfc 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -1430,10 +1430,10 @@ class HolidaysRequest(models.Model): return leave_notif_subtype or self.env.ref('hr_holidays.mt_leave') return super(HolidaysRequest, self)._track_subtype(init_values) - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ Handle HR users and officers recipients that can validate or refuse holidays directly from email. """ - groups = super(HolidaysRequest, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(HolidaysRequest, self)._notify_get_recipients_groups(msg_vals=msg_vals) local_msg_vals = dict(msg_vals or {}) self.ensure_one() @@ -1447,9 +1447,10 @@ class HolidaysRequest(models.Model): holiday_user_group_id = self.env.ref('hr_holidays.group_hr_holidays_user').id new_group = ( - 'group_hr_holidays_user', lambda pdata: pdata['type'] == 'user' and holiday_user_group_id in pdata['groups'], { - 'actions': hr_actions, - }) + 'group_hr_holidays_user', + lambda pdata: pdata['type'] == 'user' and holiday_user_group_id in pdata['groups'], + {'actions': hr_actions} + ) return [new_group] + groups diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index c204e89d82e..fa80f799a33 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -743,10 +743,10 @@ class HolidaysAllocation(models.Model): return allocation_notif_subtype_id or self.env.ref('hr_holidays.mt_leave_allocation') return super(HolidaysAllocation, self)._track_subtype(init_values) - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ Handle HR users and officers recipients that can validate or refuse holidays directly from email. """ - groups = super(HolidaysAllocation, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(HolidaysAllocation, self)._notify_get_recipients_groups(msg_vals=msg_vals) local_msg_vals = dict(msg_vals or {}) self.ensure_one() @@ -760,9 +760,10 @@ class HolidaysAllocation(models.Model): holiday_user_group_id = self.env.ref('hr_holidays.group_hr_holidays_user').id new_group = ( - 'group_hr_holidays_user', lambda pdata: pdata['type'] == 'user' and holiday_user_group_id in pdata['groups'], { - 'actions': hr_actions, - }) + 'group_hr_holidays_user', + lambda pdata: pdata['type'] == 'user' and holiday_user_group_id in pdata['groups'], + {'actions': hr_actions} + ) return [new_group] + groups diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index ae3a479aeff..8dd4e0ed81a 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -500,13 +500,13 @@ class Applicant(models.Model): return self.env.ref('hr_recruitment.mt_applicant_stage_changed') return super(Applicant, self)._track_subtype(init_values) - def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): + def _notify_get_reply_to(self, default=None): """ Override to set alias of applicants to their job definition if any. """ - aliases = self.mapped('job_id')._notify_get_reply_to(default=default, records=None, company=company, doc_names=None) + aliases = self.mapped('job_id')._notify_get_reply_to(default=default) res = {app.id: aliases.get(app.job_id.id) for app in self} leftover = self.filtered(lambda rec: not rec.job_id) if leftover: - res.update(super(Applicant, leftover)._notify_get_reply_to(default=default, records=None, company=company, doc_names=doc_names)) + res.update(super(Applicant, leftover)._notify_get_reply_to(default=default)) return res def _message_get_suggested_recipients(self): diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 5bb4f27cd89..3986b4d8da6 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -349,14 +349,14 @@ class Channel(models.Model): new_partner_id=channel_partner.partner_id.id, new_partner_name=channel_partner.partner_id.name, ) - channel_partner.channel_id.message_post(body=notification, message_type="notification", subtype_xmlid="mail.mt_comment", notify_by_email=False) + channel_partner.channel_id.message_post(body=notification, message_type="notification", subtype_xmlid="mail.mt_comment") members_data.append({ 'id': channel_partner.partner_id.id, 'im_status': channel_partner.partner_id.im_status, 'name': channel_partner.partner_id.name, }) for channel_partner in new_members.filtered(lambda channel_partner: channel_partner.guest_id): - channel_partner.channel_id.message_post(body=_('
joined the channel
'), message_type="notification", subtype_xmlid="mail.mt_comment", notify_by_email=False) + channel_partner.channel_id.message_post(body=_('
joined the channel
'), message_type="notification", subtype_xmlid="mail.mt_comment") guest_members_data.append({ 'id': channel_partner.guest_id.id, 'name': channel_partner.guest_id.name, @@ -479,13 +479,13 @@ class Channel(models.Model): return False return super(Channel, self)._alias_get_error_message(message, message_dict, alias) - def _notify_compute_recipients(self, message, msg_vals): + def _notify_get_recipients(self, message, msg_vals): """ Override recipients computation as channel is not a standard mail.thread document. Indeed there are no followers on a channel. Instead of followers it has members that should be notified. - :param message: see ``MailThread._notify_compute_recipients()``; - :param msg_vals: see ``MailThread._notify_compute_recipients()``; + :param message: see ``MailThread._notify_get_recipients()``; + :param msg_vals: see ``MailThread._notify_get_recipients()``; :return recipients: structured data holding recipients data. See ``MailThread._notify_thread()`` for more details about its content @@ -536,13 +536,13 @@ class Channel(models.Model): return recipients_data - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ All recipients of a message on a channel are considered as partners. This means they will receive a minimal email, without a link to access in the backend. Mailing lists should indeed send minimal emails to avoid the noise. """ - groups = super(Channel, self)._notify_get_groups(msg_vals=msg_vals) - for (index, (group_name, group_func, group_data)) in enumerate(groups): + groups = super(Channel, self)._notify_get_recipients_groups(msg_vals=msg_vals) + for (index, (group_name, _group_func, group_data)) in enumerate(groups): if group_name != 'customer': groups[index] = (group_name, lambda partner: False, group_data) return groups diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index bd2ac656927..84139ad1726 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1638,9 +1638,9 @@ class MailThread(models.AbstractModel): ]).write({'author_id': partner.id}) return result - # ------------------------------------------------------ + # ------------------------------------------------------------ # MESSAGE POST MAIN - # ------------------------------------------------------ + # ------------------------------------------------------------ def _message_post_process_attachments(self, attachments, attachment_ids, message_values): """ Preprocess attachments for mail_thread.message_post() or mail_mail.create(). @@ -1868,9 +1868,9 @@ class MailThread(models.AbstractModel): message and computed value are given, to try to lessen query count by using already-computed values instead of having to rebrowse things. """ - # ------------------------------------------------------ + # ------------------------------------------------------------ # MESSAGE POST API / WRAPPERS - # ------------------------------------------------------ + # ------------------------------------------------------------ def _message_compose_with_view(self, views_or_xmlid, message_log=False, **kwargs): """ Helper method to send a mail / post a message / log a note using @@ -1970,7 +1970,7 @@ class MailThread(models.AbstractModel): 'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'), 'is_internal': True, 'record_name': False, - 'reply_to': MailThread._notify_get_reply_to(default=email_from, records=None)[False], + 'reply_to': MailThread._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), } values.update(msg_kwargs) @@ -2003,7 +2003,7 @@ class MailThread(models.AbstractModel): 'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'), 'is_internal': True, 'record_name': False, - 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from, records=None)[False], + 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify } message_values.update(kwargs) @@ -2026,7 +2026,7 @@ class MailThread(models.AbstractModel): 'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'), 'is_internal': True, 'record_name': False, - 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from, records=None)[False], + 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify } values_list = [dict(base_message_values, @@ -2035,6 +2035,10 @@ class MailThread(models.AbstractModel): for record in self] return self.sudo()._message_create(values_list) + # ------------------------------------------------------------ + # MAIL.MESSAGE HELPERS + # ------------------------------------------------------------ + def _message_compute_author(self, author_id=None, email_from=None, raise_exception=True): """ Tool method computing author information for messages. Purpose is to ensure maximum coherence between author / current user / email_from @@ -2097,43 +2101,57 @@ class MailThread(models.AbstractModel): # NOTIFICATION API # ------------------------------------------------------ - def _notify_thread(self, message, msg_vals=False, notify_by_email=True, **kwargs): + def _notify_thread(self, message, msg_vals=False, **kwargs): """ Main notification method. This method basically does two things - * call ``_notify_compute_recipients`` that computes recipients to + * call ``_notify_get_recipients`` that computes recipients to notify based on message record or message creation values if given (to optimize performance if we already have data computed); * performs the notification process by calling the various notification methods implemented; - :param message: mail.message record to notify; - :param msg_vals: dictionary of values used to create the message. If given - it is used instead of accessing ``self`` to lessen query count in some - simple cases where no notification is actually required; + :param message: ``mail.message`` record to notify; + :param msg_vals: dictionary of values used to create the message. If given it + may be used to access values related to ``message`` without accessing it + directly. It lessens query count in some optimized use cases by avoiding + access message content in db; Kwargs allow to pass various parameters that are given to sub notification methods. See those methods for more details about the additional parameters. - Parameters used for email-style notifications + + :return: recipients data (see ``MailThread._notify_get_recipients()``) """ msg_vals = msg_vals if msg_vals else {} - rdata = self._notify_compute_recipients(message, msg_vals) - if not rdata: - return rdata + recipients_data = self._notify_get_recipients(message, msg_vals) + if not recipients_data: + return recipients_data - self._notify_record_by_inbox(message, rdata, msg_vals=msg_vals, **kwargs) - if notify_by_email: - self._notify_record_by_email(message, rdata, msg_vals=msg_vals, **kwargs) + self._notify_thread_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) + self._notify_thread_by_email(message, recipients_data, msg_vals=msg_vals, **kwargs) - return rdata + return recipients_data - def _notify_record_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): - """ Notification method: inbox. Do two main things + def _notify_thread_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): + """ Notification method: inbox. Does two main things : - * create an inbox notification for users; + * create inbox notifications for users; * send bus notifications; - TDE/XDO TODO: flag rdata directly, with for example r['notif'] = 'ocn_client' and r['needaction']=False - and correctly override notify_recipients + :param message: ``mail.message`` record to notify; + :param recipients_data: list of recipients information (based on res.partner + records), formatted like + [{'active': partner.active; + 'id': id of the res.partner being recipient to notify; + 'groups': res.group IDs if linked to a user; + 'notif': 'inbox', 'email', 'sms' (SMS App); + 'share': partner.partner_share; + 'type': 'customer', 'portal', 'user;' + }, {...}]. + See ``MailThread._notify_get_recipients``; + :param msg_vals: dictionary of values used to create the message. If given it + may be used to access values related to ``message`` without accessing it + directly. It lessens query count in some optimized use cases by avoiding + access message content in db; """ bus_notifications = [] inbox_pids = [r['id'] for r in recipients_data if r['notif'] == 'inbox'] @@ -2151,19 +2169,34 @@ class MailThread(models.AbstractModel): bus_notifications.append((self.env['res.partner'].browse(partner_id), 'mail.message/inbox', dict(message_format_values))) self.env['bus.bus'].sudo()._sendmany(bus_notifications) - def _notify_record_by_email(self, message, recipients_data, msg_vals=False, - model_description=False, mail_auto_delete=True, check_existing=False, - force_send=True, send_after_commit=True, + def _notify_thread_by_email(self, message, recipients_data, msg_vals=False, + mail_auto_delete=True, # mail.mail + model_description=False, # rendering + check_existing=False, force_send=True, send_after_commit=True, # email send **kwargs): """ Method to send email linked to notified messages. - :param message: mail.message record to notify; - :param recipients_data: see ``_notify_thread``; - :param msg_vals: see ``_notify_thread``; + :param message: ``mail.message`` record to notify; + :param recipients_data: list of recipients information (based on res.partner + records), formatted like + [{'active': partner.active; + 'id': id of the res.partner being recipient to notify; + 'groups': res.group IDs if linked to a user; + 'notif': 'inbox', 'email', 'sms' (SMS App); + 'share': partner.partner_share; + 'type': 'customer', 'portal', 'user;' + }, {...}]. + See ``MailThread._notify_get_recipients``; + :param msg_vals: dictionary of values used to create the message. If given it + may be used to access values related to ``message`` without accessing it + directly. It lessens query count in some optimized use cases by avoiding + access message content in db; + + :param mail_auto_delete: delete notification emails once sent; :param model_description: model description used in email notification process (computed if not given); - :param mail_auto_delete: delete notification emails once sent; + :param check_existing: check for existing notifications to update based on mailed recipient, otherwise create new notifications; @@ -2177,13 +2210,13 @@ class MailThread(models.AbstractModel): model = msg_vals.get('model') if msg_vals else message.model model_name = model_description or (self._fallback_lang().env['ir.model']._get(model).display_name if model else False) # one query for display name - recipients_groups_data = self._notify_classify_recipients(partners_data, model_name, msg_vals=msg_vals) + recipients_groups_data = self._notify_get_recipients_classify(partners_data, model_name, msg_vals=msg_vals) if not recipients_groups_data: return True force_send = self.env.context.get('mail_notify_force_send', force_send) - template_values = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) # 10 queries + template_values = self._notify_by_email_prepare_rendering_context(message, msg_vals, model_description=model_description) # 10 queries email_layout_xmlid = msg_vals.get('email_layout_xmlid') if msg_vals else message.email_layout_xmlid template_xmlid = email_layout_xmlid if email_layout_xmlid else 'mail.message_notification_email' @@ -2205,7 +2238,7 @@ class MailThread(models.AbstractModel): 'references': message.parent_id.sudo().message_id if message.parent_id else False, 'subject': mail_subject, } - base_mail_values = self._notify_by_email_add_values(base_mail_values) + base_mail_values = self._notify_by_email_add_mail_values(base_mail_values) # Clean the context to get rid of residual default_* keys that could cause issues during # the mail.mail creation. @@ -2235,7 +2268,7 @@ class MailThread(models.AbstractModel): # create email for recipients_ids_chunk in split_every(recipients_max, recipients_ids): - recipient_values = self._notify_email_recipient_values(recipients_ids_chunk) + recipient_values = self._notify_by_email_get_recipients_values(recipients_ids_chunk) email_to = recipient_values['email_to'] recipient_ids = recipient_values['recipient_ids'] @@ -2302,8 +2335,25 @@ class MailThread(models.AbstractModel): return True @api.model - def _notify_prepare_template_context(self, message, msg_vals, model_description=False, mail_auto_delete=True): - # compute send user and its related signature + def _notify_by_email_prepare_rendering_context(self, message, msg_vals, model_description=False): + """ Prepare rendering context for notification email. + + Signature: if asked a default signature is computed based on author. Either + it has an user and we use the user's signature. Either we do not find any + user and we compute a default one based on the author's name. + + Company: either there is one defined on the record (company_id field set + with a value), either we use env.company. + + Lang: when calling this method, ``_fallback_lang`` should already been + called, or a lang set in context with another way. A wild guess is done + based on templates to try to retrieve the recipient's language when a flow + like "send by email" is performed. Lang is used to try to have the + notification layout in the same language as the email content. + + :param model_description: model description used in email notification process + (computed if not given); + """ signature = '' user = self.env.user author = message.env['res.partner'].browse(msg_vals.get('author_id')) if msg_vals else message.author_id @@ -2323,17 +2373,12 @@ class MailThread(models.AbstractModel): if add_sign: signature = "

--
%s

" % author.name - # company value should fall back on env.company if: - # - no company_id field on record - # - company_id field available but not set company = self.company_id.sudo() if self and 'company_id' in self and self.company_id else self.env.company if company.website: website_url = 'http://%s' % company.website if not company.website.lower().startswith(('http:', 'https:')) else company.website else: website_url = False - # Retrieve the language in which the template was rendered, in order to render the custom - # layout in the same language. # TDE FIXME: this whole brol should be cleaned ! lang = self.env.context.get('lang') if {'default_template_id', 'default_model', 'default_res_id'} <= self.env.context.keys(): @@ -2369,7 +2414,7 @@ class MailThread(models.AbstractModel): 'lang': lang, } - def _notify_by_email_add_values(self, base_mail_values): + def _notify_by_email_add_mail_values(self, base_mail_values): """ Add model-specific values to the dictionary used to create the notification email. Its base behavior is to compute model-specific headers. @@ -2377,14 +2422,40 @@ class MailThread(models.AbstractModel): :param dict base_mail_values: base mail.mail values, holding message to notify (mail_message_id and its fields), server, references, subject. """ - headers = self._notify_email_headers() + headers = self._notify_by_email_get_headers() if headers: - base_mail_values['headers'] = headers + base_mail_values['headers'] = repr(headers) return base_mail_values - def _notify_compute_recipients(self, message, msg_vals): + def _notify_by_email_get_recipients_values(self, recipient_ids): + """ Format email notification recipient values to store on the notification + mail.mail. Basic method just set the recipient partners as mail_mail + recipients. Override to generate other mail values like email_to or + email_cc. + :param recipient_ids: res.partner recordset to notify + """ + return { + 'email_to': False, + 'recipient_ids': recipient_ids, + } + + def _notify_get_recipients(self, message, msg_vals): """ Compute recipients to notify based on subtype and followers. This - method returns data structured as expected for ``_notify_recipients``. """ + method returns data structured as expected for ``_notify_recipients``. + + TDE/XDO TODO: flag rdata directly, with for example r['notif'] = 'ocn_client' and r['needaction']=False + and correctly override _notify_get_recipients + + :return list recipients_data: this is a list of recipients information + composed of a dictionary { + 'active': partner.active; + 'id': id of the res.partner; + 'groups': res.group IDs if linked to a user; + 'notif': 'inbox', 'email', 'sms' (SMS App); + 'share': partner.partner_share; + 'type': 'customer', 'portal', 'user;' + } + """ msg_sudo = message.sudo() # get values from msg_vals or from message if msg_vals doen't exists pids = msg_vals.get('partner_ids', []) if msg_vals else msg_sudo.partner_ids.ids @@ -2416,6 +2487,111 @@ class MailThread(models.AbstractModel): return recipients_data + def _notify_get_recipients_groups(self, msg_vals=None): + """ Return groups used to classify recipients of a notification email. + Groups is a list of tuple containing of form (group_name, group_func, + group_data) where + * group_name is an identifier used only to be able to override and manipulate + groups. Default groups are user (recipients linked to an employee user), + portal (recipients linked to a portal user) and customer (recipients not + linked to any user). An example of override use would be to add a group + linked to a res.groups like Hr Officers to set specific action buttons to + them. + * group_func is a function pointer taking a partner record as parameter. This + method will be applied on recipients to know whether they belong to a given + group or not. Only first matching group is kept. Evaluation order is the + list order. + * group_data is a dict containing parameters for the notification email + * has_button_access: whether to display Access in email. True + by default for new groups, False for portal / customer. + * button_access: dict with url and title of the button + * actions: list of action buttons to display in the notification email. + Each action is a dict containing url and title of the button. + Groups has a default value that you can find in mail_thread + ``_notify_get_recipients_classify`` method. + """ + return [ + ( + 'user', + lambda pdata: pdata['type'] == 'user', + {'has_button_access': True} + ), ( + 'portal', + lambda pdata: pdata['type'] == 'portal', + {'has_button_access': False} + ), ( + 'customer', + lambda pdata: True, + {'has_button_access': False} + ) + ] + + def _notify_get_recipients_classify(self, recipient_data, model_name, msg_vals=None): + """ Classify recipients to be notified of a message in groups to have + specific rendering depending on their group. For example users could + have access to buttons customers should not have in their emails. + Module-specific grouping should be done by overriding ``_notify_get_recipients_groups`` + method defined here-under. + :param recipient_data:todo xdo UPDATE ME + return example: + [{ + 'actions': [], + 'button_access': {'title': 'View Simple Chatter Model', + 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, + 'has_button_access': False, + 'recipients': [11] + }, + { + 'actions': [], + 'button_access': {'title': 'View Simple Chatter Model', + 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, + 'has_button_access': False, + 'recipients': [4, 5, 6] + }, + { + 'actions': [], + 'button_access': {'title': 'View Simple Chatter Model', + 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, + 'has_button_access': True, + 'recipients': [10, 11, 12] + }] + only return groups with recipients + """ + # keep a local copy of msg_vals as it may be modified to include more information about groups or links + local_msg_vals = dict(msg_vals) if msg_vals else {} + groups = self._notify_get_recipients_groups(msg_vals=local_msg_vals) + access_link = self._notify_get_action_link('view', **local_msg_vals) + + if model_name: + view_title = _('View %s', model_name) + else: + view_title = _('View') + + # fill group_data with default_values if they are not complete + for group_name, group_func, group_data in groups: + group_data.setdefault('notification_group_name', group_name) + group_data.setdefault('notification_is_customer', False) + group_data.setdefault('has_button_access', True) + group_button_access = group_data.setdefault('button_access', {}) + group_button_access.setdefault('url', access_link) + group_button_access.setdefault('title', view_title) + group_data.setdefault('actions', list()) + group_data.setdefault('recipients', list()) + + # classify recipients in each group + for recipient in recipient_data: + for group_name, group_func, group_data in groups: + if group_func(recipient): + group_data['recipients'].append(recipient['id']) + break + + result = [] + for group_name, _group_method, group_data in groups: + if group_data['recipients']: + result.append(group_data) + + return result + @api.model def _notify_encode_link(self, base_link, params): secret = self.env['ir.config_parameter'].sudo().get_param('database.secret') @@ -2456,123 +2632,6 @@ class MailThread(models.AbstractModel): return link - def _notify_get_groups(self, msg_vals=None): - """ Return groups used to classify recipients of a notification email. - Groups is a list of tuple containing of form (group_name, group_func, - group_data) where - * group_name is an identifier used only to be able to override and manipulate - groups. Default groups are user (recipients linked to an employee user), - portal (recipients linked to a portal user) and customer (recipients not - linked to any user). An example of override use would be to add a group - linked to a res.groups like Hr Officers to set specific action buttons to - them. - * group_func is a function pointer taking a partner record as parameter. This - method will be applied on recipients to know whether they belong to a given - group or not. Only first matching group is kept. Evaluation order is the - list order. - * group_data is a dict containing parameters for the notification email - * has_button_access: whether to display Access in email. True - by default for new groups, False for portal / customer. - * button_access: dict with url and title of the button - * actions: list of action buttons to display in the notification email. - Each action is a dict containing url and title of the button. - Groups has a default value that you can find in mail_thread - ``_notify_classify_recipients`` method. - """ - return [ - ( - 'user', - lambda pdata: pdata['type'] == 'user', - {'has_button_access': True} - ), ( - 'portal', - lambda pdata: pdata['type'] == 'portal', - {'has_button_access': False} - ), ( - 'customer', - lambda pdata: True, - {'has_button_access': False} - ) - ] - - def _notify_classify_recipients(self, recipient_data, model_name, msg_vals=None): - """ Classify recipients to be notified of a message in groups to have - specific rendering depending on their group. For example users could - have access to buttons customers should not have in their emails. - Module-specific grouping should be done by overriding ``_notify_get_groups`` - method defined here-under. - :param recipient_data:todo xdo UPDATE ME - return example: - [{ - 'actions': [], - 'button_access': {'title': 'View Simple Chatter Model', - 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, - 'has_button_access': False, - 'recipients': [11] - }, - { - 'actions': [], - 'button_access': {'title': 'View Simple Chatter Model', - 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, - 'has_button_access': False, - 'recipients': [4, 5, 6] - }, - { - 'actions': [], - 'button_access': {'title': 'View Simple Chatter Model', - 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, - 'has_button_access': True, - 'recipients': [10, 11, 12] - }] - only return groups with recipients - """ - # keep a local copy of msg_vals as it may be modified to include more information about groups or links - local_msg_vals = dict(msg_vals) if msg_vals else {} - groups = self._notify_get_groups(msg_vals=local_msg_vals) - access_link = self._notify_get_action_link('view', **local_msg_vals) - - if model_name: - view_title = _('View %s', model_name) - else: - view_title = _('View') - - # fill group_data with default_values if they are not complete - for group_name, group_func, group_data in groups: - group_data.setdefault('notification_group_name', group_name) - group_data.setdefault('notification_is_customer', False) - group_data.setdefault('has_button_access', True) - group_button_access = group_data.setdefault('button_access', {}) - group_button_access.setdefault('url', access_link) - group_button_access.setdefault('title', view_title) - group_data.setdefault('actions', list()) - group_data.setdefault('recipients', list()) - - # classify recipients in each group - for recipient in recipient_data: - for group_name, group_func, group_data in groups: - if group_func(recipient): - group_data['recipients'].append(recipient['id']) - break - - result = [] - for group_name, group_method, group_data in groups: - if group_data['recipients']: - result.append(group_data) - - return result - - def _notify_email_recipient_values(self, recipient_ids): - """ Format email notification recipient values to store on the notification - mail.mail. Basic method just set the recipient partners as mail_mail - recipients. Override to generate other mail values like email_to or - email_cc. - :param recipient_ids: res.partner recordset to notify - """ - return { - 'email_to': False, - 'recipient_ids': recipient_ids, - } - # ------------------------------------------------------ # FOLLOWERS API # ------------------------------------------------------ diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 2e31ed89bd7..335f144bd2e 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -84,7 +84,7 @@ class BaseModel(models.AbstractModel): res[record.id] = {'partner_ids': recipient_ids, 'email_to': email_to, 'email_cc': email_cc} return res - def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): + def _notify_get_reply_to(self, default=None): """ Returns the preferred reply-to email address when replying to a thread on documents. This method is a generic implementation available for all models as we could send an email through mail templates on models @@ -104,19 +104,9 @@ class BaseModel(models.AbstractModel): An example would be tasks taking their reply-to alias from their project. :param default: default email if no alias or catchall is found; - :param records: DEPRECATED, self should be a valid record set or an - empty recordset if a generic reply-to is required; - :param company: used to compute company name part of the from name; provide - it if already known, otherwise fall back on user company; - :param doc_names: dict(res_id, doc_name) used to compute doc name part of - the from name; provide it if already known to avoid queries, otherwise - name_get on document will be performed; :return result: dictionary. Keys are record IDs and value is formatted like an email "Company_name Document_name "/ """ - if records: - raise ValueError('Use of records is deprecated as this method is available on BaseModel.') - _records = self model = _records._name if _records and _records._name != 'mail.thread' else False res_ids = _records.ids if _records and model else [] @@ -125,7 +115,7 @@ class BaseModel(models.AbstractModel): alias_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") result = dict.fromkeys(_res_ids, False) result_email = dict() - doc_names = doc_names if doc_names else dict() + doc_names = dict() if alias_domain: if model and res_ids: @@ -148,7 +138,7 @@ class BaseModel(models.AbstractModel): result_email.update(dict((rid, '%s@%s' % (catchall, alias_domain)) for rid in left_ids)) # compute name of reply-to - TDE tocheck: quotes and stuff like that - company_name = company.name if company else self.env.company.name + company_name = self.env.company.name for res_id in result_email: name = '%s%s%s' % (company_name, ' ' if doc_names.get(res_id) else '', doc_names.get(res_id, '')) result[res_id] = tools.formataddr((name, result_email[res_id])) @@ -203,16 +193,11 @@ class BaseModel(models.AbstractModel): '&', ('hidden', '=', False), '|', ('res_model', '=', self._name), ('res_model', '=', False)]) - def _notify_email_headers(self): - """ - Generate the email headers based on record - """ + def _notify_by_email_get_headers(self): + """ Generate the email headers based on record """ if not self: return {} self.ensure_one() - return repr(self._notify_email_header_dict()) - - def _notify_email_header_dict(self): return { 'X-Odoo-Objects': "%s-%s" % (self._name, self.id), } diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 78311e2b6b8..4a732903395 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -607,7 +607,7 @@ class MailCase(MockEmail): some notification methods, notably testing links or group-based notification details. - See notably ``MailThread._notify_compute_recipients()``. + See notably ``MailThread._notify_get_recipients()``. """ return [ {'id': partner.id, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index b0acedd9705..b7393a58f61 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -252,7 +252,7 @@ class MailComposer(models.TransientModel): # 'purchase.order' which is used for a RFQ and and PO. To avoid confusion, we must use a # different wording depending on the state of the object. # Therefore, we can set the description in the context from the beginning to avoid falling - # back on the regular display_name retrieved in '_notify_prepare_template_context'. + # back on the regular display_name retrieved in ``_notify_by_email_prepare_rendering_context()``. model_description = self._context.get('model_description') for wizard in self: @@ -395,7 +395,7 @@ class MailComposer(models.TransientModel): # mass mailing: rendering override wizard static values if mass_mail_mode and self.model: record = self.env[self.model].browse(res_id) - mail_values['headers'] = record._notify_email_headers() + mail_values['headers'] = repr(record._notify_by_email_get_headers()) # keep a copy unless specifically requested, reset record name (avoid browsing records) mail_values.update(is_notification=not self.auto_delete_message, model=self.model, res_id=res_id, record_name=False) # auto deletion of mail_mail diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py index 411126f1362..f7763eb0272 100644 --- a/addons/mail/wizard/mail_resend_message.py +++ b/addons/mail/wizard/mail_resend_message.py @@ -78,7 +78,11 @@ class MailResendMessage(models.TransientModel): else: # has no user, is therefore customer email_partners_data.append(dict(pdata, type='customer')) - record._notify_record_by_email(message, email_partners_data, check_existing=True, send_after_commit=False) + record._notify_thread_by_email( + message, email_partners_data, + check_existing=True, + send_after_commit=False + ) self.mail_message_id._notify_message_notification_update() return {'type': 'ir.actions.act_window_close'} diff --git a/addons/mail/wizard/mail_wizard_invite.py b/addons/mail/wizard/mail_wizard_invite.py index d1200264c59..bb2e1f6559d 100644 --- a/addons/mail/wizard/mail_wizard_invite.py +++ b/addons/mail/wizard/mail_wizard_invite.py @@ -71,9 +71,15 @@ class Invite(models.TransientModel): 'add_sign': True, }) partners_data = [] - recipient_data = self.env['mail.followers']._get_recipient_data(document, 'comment', False, pids=new_partners.ids) - for pid, active, pshare, notif, groups in recipient_data: - pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups or []} + recipients_data = self.env['mail.followers']._get_recipient_data(document, 'comment', False, pids=new_partners.ids) + for pid, active, pshare, notif, groups in recipients_data: + pdata = { + 'active': active, + 'id': pid, + 'groups': groups or [], + 'notif': 'email', + 'share': pshare, + } if not pshare and notif: # has an user and is not shared, is therefore user partners_data.append(dict(pdata, type='user')) elif pshare and notif: # has an user and is shared, is therefore portal @@ -81,7 +87,10 @@ class Invite(models.TransientModel): else: # has no user, is therefore customer partners_data.append(dict(pdata, type='customer')) - document._notify_record_by_email(message, partners_data, send_after_commit=False) + document._notify_thread_by_email( + message, partners_data, + send_after_commit=False + ) # in case of failure, the web client must know the message was # deleted to discard the related failure notification self.env['bus.bus']._sendone(self.env.user.partner_id, 'mail.message/delete', {'message_ids': message.ids}) diff --git a/addons/mail_group/models/mail_group.py b/addons/mail_group/models/mail_group.py index e1de925ba2e..7cbfa2cc1e1 100644 --- a/addons/mail_group/models/mail_group.py +++ b/addons/mail_group/models/mail_group.py @@ -424,7 +424,7 @@ class MailGroup(models.Model): # SMTP headers related to the subscription email_url_encoded = urls.url_quote(email_member) headers = { - ** self._notify_email_header_dict(), + ** self._notify_by_email_get_headers(), 'List-Archive': f'<{base_url}/groups/{slug(self)}>', 'List-Subscribe': f'<{base_url}/groups?email={email_url_encoded}>', 'List-Unsubscribe': f'<{base_url}/groups?unsubscribe&email={email_url_encoded}>', diff --git a/addons/portal/models/portal_mixin.py b/addons/portal/models/portal_mixin.py index be4f9e762cb..1e0e146bd4e 100644 --- a/addons/portal/models/portal_mixin.py +++ b/addons/portal/models/portal_mixin.py @@ -60,9 +60,9 @@ class PortalMixin(models.AbstractModel): return '%s?%s' % ('/mail/view' if redirect else self.access_url, url_encode(params)) - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): access_token = self._portal_ensure_token() - groups = super(PortalMixin, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(PortalMixin, self)._notify_get_recipients_groups(msg_vals=msg_vals) local_msg_vals = dict(msg_vals or {}) if access_token and 'partner_id' in self._fields and self['partner_id']: diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 99e9211d9d7..8bd5d765a9c 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -1951,12 +1951,12 @@ class Task(models.Model): res -= dependency_subtype return res - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ Handle project users and managers recipients that can assign tasks and create new one directly from notification emails. Also give access button to portal users and portal customers. If they are notified they should probably have access to the document. """ - groups = super(Task, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(Task, self)._notify_get_recipients_groups(msg_vals=msg_vals) local_msg_vals = dict(msg_vals or {}) self.ensure_one() @@ -1983,13 +1983,13 @@ class Task(models.Model): return groups - def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): + def _notify_get_reply_to(self, default=None): """ Override to set alias of tasks to their project if any. """ - aliases = self.sudo().mapped('project_id')._notify_get_reply_to(default=default, records=None, company=company, doc_names=None) + aliases = self.sudo().mapped('project_id')._notify_get_reply_to(default=default) res = {task.id: aliases.get(task.project_id.id) for task in self} leftover = self.filtered(lambda rec: not rec.project_id) if leftover: - res.update(super(Task, leftover)._notify_get_reply_to(default=default, records=None, company=company, doc_names=doc_names)) + res.update(super(Task, leftover)._notify_get_reply_to(default=default)) return res def email_split(self, msg): @@ -2044,8 +2044,8 @@ class Task(models.Model): task._message_add_suggested_recipient(recipients, email=task.email_from, reason=_('Customer Email')) return recipients - def _notify_email_header_dict(self): - headers = super(Task, self)._notify_email_header_dict() + def _notify_by_email_get_headers(self): + headers = super(Task, self)._notify_by_email_get_headers() if self.project_id: current_objects = [h for h in headers.get('X-Odoo-Objects', '').split(',') if h] current_objects.insert(0, 'project.project-%s, ' % self.project_id.id) diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py index c8a80d38288..7954b523dba 100644 --- a/addons/sms/models/mail_thread.py +++ b/addons/sms/models/mail_thread.py @@ -204,8 +204,8 @@ class MailThread(models.AbstractModel): :param partner_ids: if set is a record set of partners to notify; :param number_field: if set is a name of field to use on current record to compute a number to notify; - :param sms_numbers: see ``_notify_record_by_sms``; - :param sms_pid_to_number: see ``_notify_record_by_sms``; + :param sms_numbers: see ``_notify_thread_by_sms``; + :param sms_pid_to_number: see ``_notify_thread_by_sms``; """ self.ensure_one() sms_pid_to_number = sms_pid_to_number if sms_pid_to_number is not None else {} @@ -237,17 +237,29 @@ class MailThread(models.AbstractModel): def _notify_thread(self, message, msg_vals=False, **kwargs): recipients_data = super(MailThread, self)._notify_thread(message, msg_vals=msg_vals, **kwargs) - self._notify_record_by_sms(message, recipients_data, msg_vals=msg_vals, **kwargs) + self._notify_thread_by_sms(message, recipients_data, msg_vals=msg_vals, **kwargs) return recipients_data - def _notify_record_by_sms(self, message, recipients_data, msg_vals=False, + def _notify_thread_by_sms(self, message, recipients_data, msg_vals=False, sms_numbers=None, sms_pid_to_number=None, check_existing=False, put_in_queue=False, **kwargs): """ Notification method: by SMS. - :param message: mail.message record to notify; - :param recipients_data: see ``_notify_thread``; - :param msg_vals: see ``_notify_thread``; + :param message: ``mail.message`` record to notify; + :param recipients_data: list of recipients information (based on res.partner + records), formatted like + [{'active': partner.active; + 'id': id of the res.partner being recipient to notify; + 'groups': res.group IDs if linked to a user; + 'notif': 'inbox', 'email', 'sms' (SMS App); + 'share': partner.partner_share; + 'type': 'customer', 'portal', 'user;' + }, {...}]. + See ``MailThread._notify_get_recipients``; + :param msg_vals: dictionary of values used to create the message. If given it + may be used to access values related to ``message`` without accessing it + directly. It lessens query count in some optimized use cases by avoiding + access message content in db; :param sms_numbers: additional numbers to notify in addition to partners and classic recipients; @@ -264,7 +276,7 @@ class MailThread(models.AbstractModel): sms_all = self.env['sms.sms'].sudo() # pre-compute SMS data - body = msg_vals['body'] if msg_vals and msg_vals.get('body') else message.body + body = msg_vals['body'] if msg_vals and 'body' in msg_vals else message.body sms_base_vals = { 'body': html2plaintext(body), 'mail_message_id': message.id, diff --git a/addons/sms/wizard/sms_resend.py b/addons/sms/wizard/sms_resend.py index 25882a91871..5cb2ab995f3 100644 --- a/addons/sms/wizard/sms_resend.py +++ b/addons/sms/wizard/sms_resend.py @@ -90,15 +90,22 @@ class SMSResend(models.TransientModel): pids = list(sms_pid_to_number.keys()) numbers = [r.sms_number for r in self.recipient_ids if r.resend and not r.partner_id] - rdata = [] + recipients_data = [] for pid, active, pshare, notif, groups in self.env['mail.followers']._get_recipient_data(record, 'sms', False, pids=pids): if pid and notif == 'sms': - rdata.append({'id': pid, 'share': pshare, 'active': active, 'notif': notif, 'groups': groups or [], 'type': 'customer' if pshare else 'user'}) - if rdata or numbers: - record._notify_record_by_sms( - self.mail_message_id, rdata, check_existing=True, + recipients_data.append({ + 'active': active, + 'groups': groups or [], + 'id': pid, + 'notif': notif, + 'share': pshare, + 'type': 'customer' if pshare else 'user', + }) + if recipients_data or numbers: + record._notify_thread_by_sms( + self.mail_message_id, recipients_data, sms_numbers=numbers, sms_pid_to_number=sms_pid_to_number, - put_in_queue=False + check_existing=True, put_in_queue=False ) self.mail_message_id._notify_message_notification_update() diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 1f0b66c51aa..2d7e2b9e72f 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -55,7 +55,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): 'mail.mail_notification_paynow']: test_message.write({'email_layout_xmlid': email_xmlid}) with self.mock_mail_gateway(): - test_record._notify_record_by_email( + test_record._notify_thread_by_email( test_message, recipients_data, force_send=False @@ -71,7 +71,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): self.assertTrue(user_email) @users('employee') - def test_notify_mail_add_signature(self): + def test_notify_by_mail_add_signature(self): self.test_track = self.env['mail.test.track'].with_context(self._test_context).with_user(self.user_employee).create({ 'name': 'Test', 'email_from': 'ignasse@example.com' @@ -96,7 +96,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): self.assertEqual(found_mail.body_html.count(signature), 0) @users('employee') - def test_notify_prepare_template_context_company_value(self): + def test_notify_by_email_prepare_rendering_contextt(self): """ Verify that the template context company value is right after switching the env company or if a company_id is set on mail record. @@ -113,7 +113,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): # self.env.company.id = Main Company AND test_record.company_id = False self.assertEqual(self.env.company.id, main_company.id) self.assertEqual(test_record.company_id.id, False) - template_values = test_record._notify_prepare_template_context(test_record.message_ids, {}) + template_values = test_record._notify_by_email_prepare_rendering_context(test_record.message_ids, {}) self.assertEqual(template_values.get('company').id, self.env.company.id) # self.env.company.id = Other Company AND test_record.company_id = False @@ -121,7 +121,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): test_record = self.env['mail.test.multi.company'].browse(test_record.id) self.assertEqual(self.env.company.id, other_company.id) self.assertEqual(test_record.company_id.id, False) - template_values = test_record._notify_prepare_template_context(test_record.message_ids, {}) + template_values = test_record._notify_by_email_prepare_rendering_context(test_record.message_ids, {}) self.assertEqual(template_values.get('company').id, self.env.company.id) # self.env.company.id = Other Company AND test_record.company_id = Main Company @@ -129,7 +129,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): test_record = self.env['mail.test.multi.company'].browse(test_record.id) self.assertEqual(self.env.company.id, other_company.id) self.assertEqual(test_record.company_id.id, main_company.id) - template_values = test_record._notify_prepare_template_context(test_record.message_ids, {}) + template_values = test_record._notify_by_email_prepare_rendering_context(test_record.message_ids, {}) self.assertEqual(template_values.get('company').id, main_company.id) def test_notify_recipients_internals(self): @@ -147,7 +147,7 @@ class TestMessagePost(TestMailCommon, TestRecipients): 'auth_login': 'auth_login_val', } notify_msg_vals = dict(msg_vals, **link_vals) - classify_res = self.env[self.test_record._name]._notify_classify_recipients(pdata, 'My Custom Model Name', msg_vals=notify_msg_vals) + classify_res = self.env[self.test_record._name]._notify_get_recipients_classify(pdata, 'My Custom Model Name', msg_vals=notify_msg_vals) # find back information for each recipients partner_info = next(item for item in classify_res if item['recipients'] == self.partner_1.ids) emp_info = next(item for item in classify_res if item['recipients'] == self.partner_employee.ids) diff --git a/addons/test_mail/tests/test_message_track.py b/addons/test_mail/tests/test_message_track.py index 4be1f2cacae..bc3226a03e8 100644 --- a/addons/test_mail/tests/test_message_track.py +++ b/addons/test_mail/tests/test_message_track.py @@ -349,8 +349,8 @@ class TestTrackingInternals(TestMailCommon): self.assertFalse(msg_emp[0].get('tracking_value_ids'), "should not have protected tracking values") self.assertTrue(msg_sudo[0].get('tracking_value_ids'), "should have protected tracking values") - msg_emp = self.record._notify_prepare_template_context(self.record.message_ids, {}) - msg_sudo = self.record.sudo()._notify_prepare_template_context(self.record.message_ids, {}) + msg_emp = self.record._notify_by_email_prepare_rendering_context(self.record.message_ids, {}) + msg_sudo = self.record.sudo()._notify_by_email_prepare_rendering_context(self.record.message_ids, {}) self.assertFalse(msg_emp.get('tracking_values'), "should not have protected tracking values") self.assertTrue(msg_sudo.get('tracking_values'), "should have protected tracking values") diff --git a/addons/test_mail_full/tests/test_sms_post.py b/addons/test_mail_full/tests/test_sms_post.py index a52f33b7734..c8619d84281 100644 --- a/addons/test_mail_full/tests/test_sms_post.py +++ b/addons/test_mail_full/tests/test_sms_post.py @@ -41,7 +41,7 @@ class TestSMSPost(TestMailFullCommon, TestMailFullRecipients): with self.with_user('employee'), self.mockSMSGateway(): test_record = self.env['mail.test.sms'].browse(self.test_record.id) - test_record._notify_record_by_sms(messages, [{'id': self.partner_1.id, 'notif': 'sms'}], check_existing=True) + test_record._notify_thread_by_sms(messages, [{'id': self.partner_1.id, 'notif': 'sms'}], check_existing=True) self.assertSMSNotification([{'partner': self.partner_1}], self._test_body, messages) def test_message_sms_internals_sms_numbers(self): diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py index b1c5b02dd89..98a4e491b73 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -273,23 +273,25 @@ class BlogPost(models.Model): 'res_id': self.id, } - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ Add access button to everyone if the document is published. """ - groups = super(BlogPost, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(BlogPost, self)._notify_get_recipients_groups(msg_vals=msg_vals) if self.website_published: - for group_name, group_method, group_data in groups: + for _group_name, _group_method, group_data in groups: group_data['has_button_access'] = True return groups - def _notify_record_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): + def _notify_thread_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): """ Override to avoid keeping all notified recipients of a comment. We avoid tracking needaction on post comments. Only emails should be sufficient. """ + if msg_vals is None: + msg_vals = {} if msg_vals.get('message_type', message.message_type) == 'comment': return - return super(BlogPost, self)._notify_record_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) + return super(BlogPost, self)._notify_thread_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) def _default_website_meta(self): res = super(BlogPost, self)._default_website_meta() diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index 1c9d83a5a79..de7fdb92a83 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -922,12 +922,12 @@ class Post(models.Model): 'res_id': self.id, } - def _notify_get_groups(self, msg_vals=None): + def _notify_recipients_get_groups(self, msg_vals=None): """ Add access button to everyone if the document is active. """ - groups = super(Post, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(Post, self)._notify_recipients_get_groups(msg_vals=msg_vals) if self.state == 'active': - for group_name, group_method, group_data in groups: + for _group_name, _group_method, group_data in groups: group_data['has_button_access'] = True return groups @@ -954,13 +954,15 @@ class Post(models.Model): kwargs['record_name'] = self.parent_id.name return super(Post, self).message_post(message_type=message_type, **kwargs) - def _notify_record_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): + def _notify_thread_by_inbox(self, message, recipients_data, msg_vals=False, **kwargs): """ Override to avoid keeping all notified recipients of a comment. We avoid tracking needaction on post comments. Only emails should be sufficient. """ + if msg_vals is None: + msg_vals = {} if msg_vals.get('message_type', message.message_type) == 'comment': return - return super(Post, self)._notify_record_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) + return super(Post, self)._notify_thread_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs) def _compute_website_url(self): return '/forum/{forum}/{post}{anchor}'.format( diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index 631945e371d..b614508bc1f 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -662,12 +662,12 @@ class Slide(models.Model): } return super(Slide, self).get_access_action(access_uid) - def _notify_get_groups(self, msg_vals=None): + def _notify_get_recipients_groups(self, msg_vals=None): """ Add access button to everyone if the document is active. """ - groups = super(Slide, self)._notify_get_groups(msg_vals=msg_vals) + groups = super(Slide, self)._notify_get_recipients_groups(msg_vals=msg_vals) if self.website_published: - for group_name, group_method, group_data in groups: + for _group_name, _group_method, group_data in groups: group_data['has_button_access'] = True return groups diff --git a/addons/website_slides/wizard/slide_channel_invite.py b/addons/website_slides/wizard/slide_channel_invite.py index bae1be76d4b..2aee778a3ce 100644 --- a/addons/website_slides/wizard/slide_channel_invite.py +++ b/addons/website_slides/wizard/slide_channel_invite.py @@ -89,7 +89,7 @@ class SlideChannelInvite(models.TransientModel): except ValueError: _logger.warning('QWeb template %s not found when sending slide channel mails. Sending without layout.', email_layout_xmlid) else: - # could be great to use _notify_prepare_template_context someday + # could be great to use ``_notify_by_email_prepare_rendering_context`` someday template_ctx = { 'message': self.env['mail.message'].sudo().new(dict(body=mail_values['body_html'], record_name=self.channel_id.name)), 'model_description': self.env['ir.model']._get('slide.channel').display_name, From 517df2661399caed15d8fdc30a7ee22988c610c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 28 Jan 2022 12:43:37 +0000 Subject: [PATCH 04/23] [REF] mail: cleanup notification email values generation PURPOSE Purpose of this commit is to clean notification email values generation while keeping possibility to override and tweak values. It now has clean hooks on base mail value and final mail values. SPECIFICATIONS Base mail value are values independent from recipients. It allows notably to tweak values depending on current record without having to browse and access record for every recipient, saving unnecessary computation. Final mail values are recipients-dependent and are the values that will be use to create the final ``mail.mail``. With a mechanism of optional additional values + hooks for base and final values it should be possible to keep any custom behavior already implemented with a cleaner interface and code flow. See notably odoo/odoo@b1a5bf95f8ce26944959c97b760239d008efec95 for example of why some code exist just for hook purpose. As a side note, light variable renaming is performed (email -> new_email) to avoid shadowing email. Task-2710804 (Mail: Clean Mail.Thread API) Part-of: odoo/odoo#82167 --- addons/mail/models/mail_thread.py | 72 +++++++++++++++---------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 84139ad1726..357c2948013 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2226,19 +2226,7 @@ class MailThread(models.AbstractModel): _logger.warning('QWeb template %s not found when sending notification emails. Sending without layouting.' % (template_xmlid)) base_template = False - mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # in cache, no queries - # Replace new lines by spaces to conform to email headers requirements - mail_subject = ' '.join((mail_subject or '').splitlines()) - # prepare notification mail values - base_mail_values = { - 'mail_message_id': message.id, - 'mail_server_id': message.mail_server_id.id, # 2 query, check acces + read, may be useless, Falsy, when will it be used? - 'auto_delete': mail_auto_delete, - # due to ir.rule, user have no right to access parent message if message is not published - 'references': message.parent_id.sudo().message_id if message.parent_id else False, - 'subject': mail_subject, - } - base_mail_values = self._notify_by_email_add_mail_values(base_mail_values) + base_mail_values = self._notify_by_email_get_base_mail_values(message, additional_values={'auto_delete': mail_auto_delete}) # Clean the context to get rid of residual default_* keys that could cause issues during # the mail.mail creation. @@ -2268,22 +2256,15 @@ class MailThread(models.AbstractModel): # create email for recipients_ids_chunk in split_every(recipients_max, recipients_ids): - recipient_values = self._notify_by_email_get_recipients_values(recipients_ids_chunk) - email_to = recipient_values['email_to'] - recipient_ids = recipient_values['recipient_ids'] + mail_values = self._notify_by_email_get_final_mail_values( + recipients_ids_chunk, + base_mail_values, + additional_values={'body_html': mail_body} + ) + new_email = SafeMail.create(mail_values) - create_values = { - 'body_html': mail_body, - 'subject': mail_subject, - 'recipient_ids': [Command.link(pid) for pid in recipient_ids], - } - if email_to: - create_values['email_to'] = email_to - create_values.update(base_mail_values) # mail_message_id, mail_server_id, auto_delete, references, headers - email = SafeMail.create(create_values) - - if email and recipient_ids: - tocreate_recipient_ids = list(recipient_ids) + if new_email and recipients_ids_chunk: + tocreate_recipient_ids = list(recipients_ids_chunk) if check_existing: existing_notifications = self.env['mail.notification'].sudo().search([ ('mail_message_id', '=', message.id), @@ -2291,20 +2272,20 @@ class MailThread(models.AbstractModel): ('res_partner_id', 'in', tocreate_recipient_ids) ]) if existing_notifications: - tocreate_recipient_ids = [rid for rid in recipient_ids if rid not in existing_notifications.mapped('res_partner_id.id')] + tocreate_recipient_ids = [rid for rid in recipients_ids_chunk if rid not in existing_notifications.mapped('res_partner_id.id')] existing_notifications.write({ 'notification_status': 'ready', - 'mail_mail_id': email.id, + 'mail_mail_id': new_email.id, }) notif_create_values += [{ 'mail_message_id': message.id, 'res_partner_id': recipient_id, 'notification_type': 'email', - 'mail_mail_id': email.id, + 'mail_mail_id': new_email.id, 'is_read': True, # discard Inbox notification 'notification_status': 'ready', } for recipient_id in tocreate_recipient_ids] - emails |= email + emails |= new_email if notif_create_values: SafeNotification.create(notif_create_values) @@ -2414,7 +2395,7 @@ class MailThread(models.AbstractModel): 'lang': lang, } - def _notify_by_email_add_mail_values(self, base_mail_values): + def _notify_by_email_get_base_mail_values(self, message, additional_values=None): """ Add model-specific values to the dictionary used to create the notification email. Its base behavior is to compute model-specific headers. @@ -2422,22 +2403,37 @@ class MailThread(models.AbstractModel): :param dict base_mail_values: base mail.mail values, holding message to notify (mail_message_id and its fields), server, references, subject. """ + mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # in cache, no queries + # Replace new lines by spaces to conform to email headers requirements + mail_subject = ' '.join((mail_subject or '').splitlines()) + # prepare notification mail values + base_mail_values = { + 'mail_message_id': message.id, + 'mail_server_id': message.mail_server_id.id, # 2 query, check acces + read, may be useless, Falsy, when will it be used? + # due to ir.rule, user have no right to access parent message if message is not published + 'references': message.parent_id.sudo().message_id if message.parent_id else False, + 'subject': mail_subject, + } + if additional_values: + base_mail_values.update(additional_values) + headers = self._notify_by_email_get_headers() if headers: base_mail_values['headers'] = repr(headers) return base_mail_values - def _notify_by_email_get_recipients_values(self, recipient_ids): + def _notify_by_email_get_final_mail_values(self, recipient_ids, base_mail_values, additional_values=None): """ Format email notification recipient values to store on the notification mail.mail. Basic method just set the recipient partners as mail_mail recipients. Override to generate other mail values like email_to or email_cc. :param recipient_ids: res.partner recordset to notify """ - return { - 'email_to': False, - 'recipient_ids': recipient_ids, - } + final_mail_values = dict(base_mail_values) + final_mail_values['recipient_ids'] = [Command.link(pid) for pid in recipient_ids] + if additional_values: + final_mail_values.update(additional_values) + return final_mail_values def _notify_get_recipients(self, message, msg_vals): """ Compute recipients to notify based on subtype and followers. This From 3eb96806022705f5b667e149ffa2240760097278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 3 Jan 2022 15:13:48 +0000 Subject: [PATCH 05/23] [REF] mail: improve propagation and setup of render context when posting messages PURPOSE Purpose of this commit is to cleanup flow of values going through message_post and its sub methods until email generation for notifications. SPECIFICATIONS Ensure some values are given directly when creating message linked to post methods to avoid browsing message when value is not known. Notably make signature propagation (add_sign) more explicit in message_post and its sub methods (notify, log, ...). Prepare future language related improvements by allowing to force company and lang values for rendering context. Also add ``is_html_empty`` tool method to use it in notification templates. It will be used notably to check for empty html blocks, e.g. user signature. Rename some internal variables to better understand their purpose. Finally update some outdated and/or badly indented docstrings. Task-2726501 (Mail: Propagate message values in post methods) Part-of: odoo/odoo#82167 --- addons/mail/models/mail_template.py | 13 +- addons/mail/models/mail_thread.py | 240 +++++++++++++++++----------- 2 files changed, 163 insertions(+), 90 deletions(-) diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index e831054da18..c970cea4bdf 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -7,6 +7,7 @@ import logging from odoo import _, api, fields, models, tools, Command from odoo.exceptions import UserError +from odoo.tools import is_html_empty _logger = logging.getLogger(__name__) @@ -301,10 +302,20 @@ class MailTemplate(models.Model): model = model.with_context(lang=lang) template_ctx = { + # message 'message': self.env['mail.message'].sudo().new(dict(body=values['body_html'], record_name=record.display_name)), + 'subtype': self.env['mail.message.subtype'].sudo(), + # record 'model_description': model.display_name, - 'company': 'company_id' in record and record['company_id'] or self.env.company, 'record': record, + 'record_name': False, + # user / environment + 'company': 'company_id' in record and record['company_id'] or self.env.company, + 'email_add_signature': False, + 'signature': '', + 'website_url': '', + # tools + 'is_html_empty': is_html_empty, } body = template._render(template_ctx, engine='ir.qweb', minimal_qcontext=True) values['body_html'] = self.env['mail.render.mixin']._replace_local_links(body) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 357c2948013..cc26032c0fd 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -26,7 +26,7 @@ from xmlrpc import client as xmlrpclib from odoo import _, api, exceptions, fields, models, tools, registry, SUPERUSER_ID, Command from odoo.exceptions import MissingError from odoo.osv import expression - +from odoo.tools import is_html_empty from odoo.tools.misc import clean_context, split_every _logger = logging.getLogger(__name__) @@ -1644,12 +1644,22 @@ class MailThread(models.AbstractModel): def _message_post_process_attachments(self, attachments, attachment_ids, message_values): """ Preprocess attachments for mail_thread.message_post() or mail_mail.create(). + Purpose is to - :param list attachments: list of attachment tuples in the form ``(name,content)``, #todo xdo update that - where content is NOT base64 encoded - :param list attachment_ids: a list of attachment ids, not in tomany command form - :param dict message_data: model: the model of the attachments parent record, - res_id: the id of the attachments parent record + * transfer attachments given by ``attachment_ids`` from the composer to + the record (if any); + * limit attachments manipulation when being a shared user; + * create attachments from ``attachments``. If those are linked to the + content (body) through CIDs body is updated accordingly; + + :param list(tuple(str,str), tuple(str,str, dict)) attachments : list of attachment + tuples in the form ``(name,content)`` or ``(name,content, info)`` where content + is NOT base64 encoded; + :param list attachment_ids: list of existing attachments to link to this message; + :param message_values: dictionary of values that will be used to create the + message. It is used to find back record- or content- context; + + :return dict: new values for message: 'attachment_ids' and optionally 'body' """ return_values = {} body = message_values.get('body') @@ -1703,7 +1713,7 @@ class MailThread(models.AbstractModel): content = content.as_bytes() elif content is None: continue - attachement_values= { + attachement_values = { 'name': name, 'datas': base64.b64encode(content), 'type': 'binary', @@ -1753,29 +1763,37 @@ class MailThread(models.AbstractModel): email_from=None, author_id=None, parent_id=False, subtype_xmlid=None, subtype_id=False, partner_ids=None, attachments=None, attachment_ids=None, - add_sign=True, record_name=False, **kwargs): - """ Post a new message in an existing thread, returning the new - mail.message ID. - :param str body: body of the message, usually raw HTML that will - be sanitized - :param str subject: subject of the message - :param str message_type: see mail_message.message_type field. Can be anything but - user_notification, reserved for message_notify - :param int parent_id: handle thread formation - :param int subtype_id: subtype_id of the message, used mainly use for - followers notification mechanism; - :param list(int) partner_ids: partner_ids to notify in addition to partners - computed based on subtype / followers matching; - :param list(tuple(str,str), tuple(str,str, dict) or int) attachments : list of attachment tuples in the form - ``(name,content)`` or ``(name,content, info)``, where content is NOT base64 encoded - :param list id attachment_ids: list of existing attachement to link to this message - -Should only be setted by chatter - -Attachement object attached to mail.compose.message(0) will be attached - to the related document. - Extra keyword arguments will be used as default column values for the - new mail.message record. - :return int: ID of newly created mail.message + """ Post a new message in an existing thread, returning the new mail.message. + + :param str body: body of the message, usually raw HTML that will + be sanitized + :param str subject: subject of the message + :param str message_type: see mail_message.message_type field. Can be anything but + user_notification, reserved for message_notify + :param str email_from: from address of the author. See ``_message_compute_author`` + that uses it to make email_from / author_id coherent; + :param int author_id: optional ID of partner record being the author. See + ``_message_compute_author`` that uses it to make email_from / author_id coherent; + :param int parent_id: handle thread formation + :param int subtype_id: subtype_id of the message, used mainly for followers + notification mechanism; + :param list(int) partner_ids: partner_ids to notify in addition to partners + computed based on subtype / followers matching; + :param list(tuple(str,str), tuple(str,str, dict)) attachments : list of attachment + tuples in the form ``(name,content)`` or ``(name,content, info)`` where content + is NOT base64 encoded; + :param list attachment_ids: list of existing attachments to link to this message + -Should only be set by chatter + -Attachment object attached to mail.compose.message(0) will be attached + to the related document. + + Extra keyword arguments will be used either + * as default column values for the new mail.message record if they match + mail.message fields; + * propagated to notification methods; + + :return record: newly create mail.message """ self.ensure_one() # should always be posted on a record, use message_notify if no record # split message additional values from notify additional values @@ -1795,12 +1813,11 @@ class MailThread(models.AbstractModel): if any(not isinstance(pc_id, int) for pc_id in partner_ids): raise ValueError(_('message_post partner_ids and must be integer list, not commands.')) - self = self._fallback_lang() # add lang to context imediatly since it will be usefull in various flows latter. + self = self._fallback_lang() # add lang to context immediately since it will be useful in various flows latter. # Explicit access rights check, because display_name is computed as sudo. self.check_access_rights('read') self.check_access_rule('read') - record_name = record_name or self.display_name # Find the message's author if self.env.user._is_public() and 'guest' in self.env.context: @@ -1821,38 +1838,43 @@ class MailThread(models.AbstractModel): parent_id = self._message_compute_parent_id(parent_id) - values = dict(msg_kwargs) - values.update({ + msg_values = dict(msg_kwargs) + if 'add_sign' not in msg_values: + msg_values['add_sign'] = True + if not msg_values.get('record_name'): + msg_values['record_name'] = self.display_name + msg_values.update({ 'author_id': author_id, 'author_guest_id': author_guest_id, 'email_from': email_from, 'model': self._name, 'res_id': self.id, + # content 'body': body, 'subject': subject or False, 'message_type': message_type, 'parent_id': parent_id, 'subtype_id': subtype_id, + # recipients 'partner_ids': partner_ids, - 'add_sign': add_sign, - 'record_name': record_name, }) + attachments = attachments or [] attachment_ids = attachment_ids or [] - attachement_values = self._message_post_process_attachments(attachments, attachment_ids, values) - values.update(attachement_values) # attachement_ids, [body] + attachement_values = self._message_post_process_attachments(attachments, attachment_ids, msg_values) + msg_values.update(attachement_values) # attachement_ids, [body] - new_message = self._message_create(values) + new_message = self._message_create(msg_values) # Set main attachment field if necessary - self._message_set_main_attachment_id(values['attachment_ids']) + self._message_set_main_attachment_id(msg_values['attachment_ids']) - if values['author_id'] and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower - self._message_subscribe(partner_ids=[values['author_id']]) + if msg_values['author_id'] and msg_values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): + if self.env['res.partner'].browse(msg_values['author_id']).active: # we dont want to add odoobot/inactive as a follower + self._message_subscribe(partner_ids=[msg_values['author_id']]) - self._message_post_after_hook(new_message, values) - self._notify_thread(new_message, values, **notif_kwargs) + self._message_post_after_hook(new_message, msg_values) + self._notify_thread(new_message, msg_values, **notif_kwargs) return new_message def _message_set_main_attachment_id(self, attachment_ids): # todo move this out of mail.thread @@ -1957,7 +1979,7 @@ class MailThread(models.AbstractModel): res_id = False MailThread = self.env['mail.thread'] - values = { + msg_values = { 'parent_id': parent_id, 'model': self._name if self else model, 'res_id': self.id if self else res_id, @@ -1973,9 +1995,12 @@ class MailThread(models.AbstractModel): 'reply_to': MailThread._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), } - values.update(msg_kwargs) - new_message = MailThread._message_create(values) - MailThread._notify_thread(new_message, values, **notif_kwargs) + msg_values.update(msg_kwargs) + if 'add_sign' not in msg_values: + msg_values['add_sign'] = True + + new_message = MailThread._message_create(msg_values) + MailThread._notify_thread(new_message, msg_values, **notif_kwargs) return new_message def _message_log_with_view(self, views_or_xmlid, **kwargs): @@ -1992,7 +2017,7 @@ class MailThread(models.AbstractModel): self.ensure_one() author_id, email_from = self._message_compute_author(author_id, email_from, raise_exception=False) - message_values = { + msg_values = { 'subject': subject, 'body': body, 'author_id': author_id, @@ -2005,9 +2030,10 @@ class MailThread(models.AbstractModel): 'record_name': False, 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify + 'add_sign': False, # False as no notification -> no need to compute signature } - message_values.update(kwargs) - return self.sudo()._message_create(message_values) + msg_values.update(kwargs) + return self.sudo()._message_create(msg_values) def _message_log_batch(self, bodies, author_id=None, email_from=None, subject=False, message_type='notification'): """ Shortcut allowing to post notes on a batch of documents. It achieve the @@ -2028,6 +2054,7 @@ class MailThread(models.AbstractModel): 'record_name': False, 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify + 'add_sign': False, } values_list = [dict(base_message_values, res_id=record.id, @@ -2121,6 +2148,9 @@ class MailThread(models.AbstractModel): :return: recipients data (see ``MailThread._notify_get_recipients()``) """ + # add lang to context immediately since it will be useful in various rendering later + self = self._fallback_lang() + msg_vals = msg_vals if msg_vals else {} recipients_data = self._notify_get_recipients(message, msg_vals) if not recipients_data: @@ -2170,8 +2200,8 @@ class MailThread(models.AbstractModel): self.env['bus.bus'].sudo()._sendmany(bus_notifications) def _notify_thread_by_email(self, message, recipients_data, msg_vals=False, - mail_auto_delete=True, # mail.mail - model_description=False, # rendering + mail_auto_delete=True, # mail.mail + model_description=False, force_email_company=False, force_email_lang=False, # rendering check_existing=False, force_send=True, send_after_commit=True, # email send **kwargs): """ Method to send email linked to notified messages. @@ -2196,10 +2226,11 @@ class MailThread(models.AbstractModel): :param model_description: model description used in email notification process (computed if not given); + :param force_email_company: see ``_notify_by_email_prepare_rendering_context``; + :param force_email_lang: see ``_notify_by_email_prepare_rendering_context``; :param check_existing: check for existing notifications to update based on mailed recipient, otherwise create new notifications; - :param force_send: send emails directly instead of using queue; :param send_after_commit: if force_send, tells whether to send emails after the transaction has been committed using a post-commit hook; @@ -2209,14 +2240,18 @@ class MailThread(models.AbstractModel): return True model = msg_vals.get('model') if msg_vals else message.model - model_name = model_description or (self._fallback_lang().env['ir.model']._get(model).display_name if model else False) # one query for display name + model_name = model_description or (self.env['ir.model']._get(model).display_name if model else False) # one query for display name recipients_groups_data = self._notify_get_recipients_classify(partners_data, model_name, msg_vals=msg_vals) if not recipients_groups_data: return True force_send = self.env.context.get('mail_notify_force_send', force_send) - template_values = self._notify_by_email_prepare_rendering_context(message, msg_vals, model_description=model_description) # 10 queries + template_values = self._notify_by_email_prepare_rendering_context( + message, msg_vals=msg_vals, model_description=model_description, + force_email_company=force_email_company, + force_email_lang=force_email_lang, + ) # 10 queries email_layout_xmlid = msg_vals.get('email_layout_xmlid') if msg_vals else message.email_layout_xmlid template_xmlid = email_layout_xmlid if email_layout_xmlid else 'mail.message_notification_email' @@ -2316,7 +2351,8 @@ class MailThread(models.AbstractModel): return True @api.model - def _notify_by_email_prepare_rendering_context(self, message, msg_vals, model_description=False): + def _notify_by_email_prepare_rendering_context(self, message, msg_vals=False, model_description=False, + force_email_company=False, force_email_lang=False): """ Prepare rendering context for notification email. Signature: if asked a default signature is computed based on author. Either @@ -2324,75 +2360,101 @@ class MailThread(models.AbstractModel): user and we compute a default one based on the author's name. Company: either there is one defined on the record (company_id field set - with a value), either we use env.company. + with a value), either we use env.company. A new parameter allows to force + its value. Lang: when calling this method, ``_fallback_lang`` should already been called, or a lang set in context with another way. A wild guess is done based on templates to try to retrieve the recipient's language when a flow like "send by email" is performed. Lang is used to try to have the - notification layout in the same language as the email content. + notification layout in the same language as the email content. A new + parameter allows to force its value. + :param msg_vals: dictionary of values used to create the message. If given it + may be used to access values related to ``message`` without accessing it + directly. It lessens query count in some optimized use cases by avoiding + access message content in db; :param model_description: model description used in email notification process (computed if not given); + :param force_email_company: res.company record used when rendering notification + layout. Otherwise computed based on current record; + :param force_email_lang: lang used when rendering content, used notably to + compute model name; """ + if msg_vals is False: + msg_vals = {} + + # compute send user and its related signature; try to use self.env.user instead of browsing + # user_ids if he is the author will give a sudo user, improving access performances and cache usage. signature = '' - user = self.env.user - author = message.env['res.partner'].browse(msg_vals.get('author_id')) if msg_vals else message.author_id - model = msg_vals.get('model') if msg_vals else message.model - add_sign = msg_vals.get('add_sign') if msg_vals else message.add_sign - subtype_id = msg_vals.get('subtype_id') if msg_vals else message.subtype_id.id - message_id = message.id - record_name = msg_vals.get('record_name') if msg_vals else message.record_name - author_user = user if user.partner_id == author else author.user_ids[0] if author and author.user_ids else False - # trying to use user (self.env.user) instead of browing user_ids if he is the author will give a sudo user, - # improving access performances and cache usage. - if author_user: - user = author_user - if add_sign: - signature = user.signature - else: - if add_sign: + add_sign = msg_vals.get('add_sign') if 'add_sign' in msg_vals else message.add_sign + if add_sign: + author = message.env['res.partner'].browse(msg_vals.get('author_id')) if 'author_id' in msg_vals else message.author_id + author_user = self.env.user if self.env.user.partner_id == author else author.user_ids[0] if author and author.user_ids else False + if author_user: + signature = author_user.signature + else: signature = "

--
%s

" % author.name - company = self.company_id.sudo() if self and 'company_id' in self and self.company_id else self.env.company + if force_email_company: + company = force_email_company + else: + company = self.company_id.sudo() if self and 'company_id' in self and self.company_id else self.env.company if company.website: website_url = 'http://%s' % company.website if not company.website.lower().startswith(('http:', 'https:')) else company.website else: website_url = False - # TDE FIXME: this whole brol should be cleaned ! - lang = self.env.context.get('lang') - if {'default_template_id', 'default_model', 'default_res_id'} <= self.env.context.keys(): + # compute lang in which content was rendered or typed + lang = False + if force_email_lang: + lang = force_email_lang + elif {'default_template_id', 'default_model', 'default_res_id'} <= self.env.context.keys(): + # TDE FIXME: this whole brol should be cleaned ! template = self.env['mail.template'].browse(self.env.context['default_template_id']) if template and template.lang: lang = template._render_lang([self.env.context['default_res_id']])[self.env.context['default_res_id']] + if not lang: + lang = self.env.context.get('lang') - if not model_description and model: - model_description = self.env['ir.model'].with_context(lang=lang)._get(model).display_name + # record, model + if not model_description: + model = msg_vals.get('model') if 'model' in msg_vals else message.model + if model: + model_description = self.env['ir.model'].with_context(lang=lang)._get(model).display_name + record_name = msg_vals.get('record_name') if 'record_name' in msg_vals else message.record_name + # tracking tracking = [] if msg_vals.get('tracking_value_ids', True) if msg_vals else bool(self): # could be tracking for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): groups = tracking_value.field_groups if not groups or self.env.is_superuser() or self.user_has_groups(groups): tracking.append((tracking_value.field_desc, - tracking_value.get_old_display_value()[0], - tracking_value.get_new_display_value()[0])) + tracking_value.get_old_display_value()[0], + tracking_value.get_new_display_value()[0])) + subtype_id = msg_vals.get('subtype_id') if msg_vals and 'subtype_id' in msg_vals else message.subtype_id.id is_discussion = subtype_id == self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment') return { + # message + 'is_discussion': is_discussion, 'message': message, - 'signature': signature, - 'website_url': website_url, - 'company': company, + 'subtype': message.subtype_id, + 'tracking_values': tracking, + # record 'model_description': model_description, 'record': self, 'record_name': record_name, - 'tracking_values': tracking, - 'is_discussion': is_discussion, - 'subtype': message.subtype_id, + # user / environment + 'add_sign': add_sign, + 'company': company, 'lang': lang, + 'signature': signature, + 'website_url': website_url, + # tools + 'is_html_empty': is_html_empty, } def _notify_by_email_get_base_mail_values(self, message, additional_values=None): From 86db4f56c822494400f649ed2fd51c7078db24fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 6 Jan 2022 16:20:52 +0000 Subject: [PATCH 06/23] [IMP] (test_)mail: add tests for translation of notification layout Purpose of this commit is to add tests for language-based tweaks of email notification layouts and composer usage. Purpose is to assess current behavior before going into some cleaning or refactoring. Notably * check layout content is translated; * check "view document" button is translated; * check action buttons are translated; * check content based on templates is translated; This is currently mainly available through the "lang" context key being correctly set. Partial support of translations when posting based on template is tested (content and model description but not notification layout). Failing support in mass mail mode is tested. Future improvements will be done in order to improve language support. Note that ``MailTemplate.send_mail()`` email sending method language support is tested in ``test_mail_template`` file. Those tests are updated to share a common base with new tests about language setup. Task-2712450 (Mail/Sale: Improve 'Pay Now' notification template) Part-of: odoo/odoo#82167 --- addons/mail/tests/common.py | 166 ++++++++++++++++++ .../models/test_mail_corner_case_models.py | 14 +- addons/test_mail/tests/test_mail_composer.py | 15 -- addons/test_mail/tests/test_mail_template.py | 89 ++-------- .../tests/test_mail_template_preview.py | 4 +- addons/test_mail/tests/test_message_post.py | 130 ++++++++++++++ 6 files changed, 326 insertions(+), 92 deletions(-) diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 4a732903395..9478fd0fb4f 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import base64 import email import email.policy import time @@ -940,3 +941,168 @@ class MailCommon(common.TransactionCase, MailCase): signature='--\nEnguerrand' ) cls.partner_employee_c2 = cls.user_employee_c2.partner_id + + @classmethod + def _activate_multi_lang(cls, lang_code='es_ES', layout_arch_db=None, test_record=False, test_template=False): + """ Summary of es_ES matching done here (a bit hardcoded to ease tests) + + * layout + * 'English Layout for' -> Spanish Layout para + * model + * description: English: Lang Chatter Model (depends on test_record._name) + translated: Spanish description + * module + * _('TestStuff') -> TestSpanishStuff (used as link button name in layout) + * _('View %s') -> SpanishView %s + * template + * body: English:

EnglishBody for

(depends on test_template.body) + translated:

SpanishBody for

+ * subject: English: EnglishSubject for {{ object.name }} (depends on test_template.subject) + translated: SpanishSubject for {{ object.name }} + """ + # activate translations + cls.env['res.lang']._activate_lang(lang_code) + cls.env.ref('base.module_base')._update_translations([lang_code]) + + # Make sure Spanish translations have not been altered + if test_record: + description_translations = cls.env['ir.translation'].search([ + ('module', '=', 'test_mail'), + ('src', '=', test_record._description), + ('lang', '=', lang_code) + ]) + if description_translations: + description_translations.update({'value': 'Spanish description'}) + else: + description_translations.create({ + 'lang': lang_code, + 'module': 'test_mail', + 'name': 'ir.model,name', + 'res_id': cls.env['ir.model']._get_id(test_record._name), + 'src': test_record._description, + 'state': 'translated', + 'type': 'model', + 'value': 'Spanish description', + }) + + translations_tocreate = [] + # Have a TestStuff always available + test_stuff_translations = cls.env['ir.translation'].search([ + ('module', '=', 'test_mail'), + ('src', '=', 'TestStuff'), + ('lang', '=', lang_code) + ]) + if test_stuff_translations: + test_stuff_translations.update({'value': 'TestSpanishStuff'}) + else: + translations_tocreate.append({ + 'lang': lang_code, + 'name': 'idontknow', + 'module': 'test_mail', + 'res_id': False, + 'src': 'TestStuff', + 'state': 'translated', + 'type': 'code', + 'value': 'TestSpanishStuff', + }) + + view_translations = cls.env['ir.translation'].search([ + ('module', '=', 'mail'), + ('src', '=', 'View %s'), + ('lang', '=', lang_code) + ]) + if view_translations: + view_translations.update({'value': 'SpanishView'}) + else: + translations_tocreate.append({ + 'lang': lang_code, + 'name': 'idontknow', + 'module': 'mail', + 'res_id': False, + 'src': 'View %s', + 'state': 'translated', + 'type': 'code', + 'value': 'SpanishView %s', + }) + + # Prepare some translated value for template if given + if test_template: + translations_tocreate += [{ + 'lang': lang_code, + 'module': 'mail', + 'name': 'mail.template,subject', + 'res_id': test_template.id, + 'state': 'translated', + 'type': 'model', + 'value': 'SpanishSubject for {{ object.name }}', + }, { + 'lang': lang_code, + 'module': 'mail', + 'name': 'mail.template,body_html', + 'res_id': test_template.id, + 'state': 'translated', + 'type': 'model', + 'value': '

SpanishBody for

', + }] + + # create a custom layout for email notification + if not layout_arch_db: + layout_arch_db = """ + +

English Layout for

+ + + + + + + + + + + + +
    +
  • + : -> +
    • +
+
+

Sent by

+""" + view = cls.env['ir.ui.view'].create({ + 'arch_db': layout_arch_db, + 'key': 'test_layout', + 'name': 'test_layout', + 'type': 'qweb', + }) + cls.env['ir.model.data'].create({ + 'model': 'ir.ui.view', + 'module': 'mail', + 'name': 'test_layout', + 'res_id': view.id + }) + translations_tocreate.append({ + 'lang': lang_code, + 'module': 'mail', + 'name': 'ir.ui.view,arch_db', + 'res_id': view.id, + 'src': 'English Layout for', + 'state': 'translated', + 'type': 'model_terms', + 'value': 'Spanish Layout para', + }) + cls.env['ir.translation'].create(translations_tocreate) + + def _generate_attachments_data(self, count, res_model=None, res_id=None): + # attachment visibility depends on what they are attached to + if res_model is None: + res_model = self.template._name + if res_id is None: + res_id = self.template.id + return [{ + 'name': '%02d.txt' % x, + 'datas': base64.b64encode(b'Att%02d' % x), + 'res_model': res_model, + 'res_id': res_id, + } for x in range(count)] diff --git a/addons/test_mail/models/test_mail_corner_case_models.py b/addons/test_mail/models/test_mail_corner_case_models.py index 8272a559d17..a348cbf49d6 100644 --- a/addons/test_mail/models/test_mail_corner_case_models.py +++ b/addons/test_mail/models/test_mail_corner_case_models.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models +from odoo import api, fields, models, _ class MailPerformanceThread(models.Model): @@ -66,6 +66,18 @@ class MailTestLang(models.Model): customer_id = fields.Many2one('res.partner') lang = fields.Char('Lang') + def _notify_get_recipients_groups(self, msg_vals=None): + groups = super(MailTestLang, self)._notify_get_recipients_groups(msg_vals=msg_vals) + local_msg_vals = dict(msg_vals or {}) + + customer_group_opts = next(group for group in groups if group[0] == 'customer')[2] + customer_group_opts['has_button_access'] = True + customer_group_opts['actions'] = [ + {'url': self._notify_get_action_link('controller', controller='/test_mail/do_stuff', **local_msg_vals), + 'title': _('TestStuff')} + ] + return groups + class MailTestTrackCompute(models.Model): _name = 'mail.test.track.compute' diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 6703ae4450f..ffcc01eeeca 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -1,8 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import base64 - from unittest.mock import patch from odoo.addons.mail.tests.common import mail_new_test_user @@ -69,19 +67,6 @@ class TestMailComposer(TestMailCommon, TestRecipients): 'auto_delete': True, }) - def _generate_attachments_data(self, count, res_model=None, res_id=None): - # attachment visibility depends on what they are attached to - if res_model is None: - res_model = self.template._name - if res_id is None: - res_id = self.template.id - return [{ - 'name': '%02d.txt' % x, - 'datas': base64.b64encode(b'Att%02d' % x), - 'res_model': res_model, - 'res_id': res_id, - } for x in range(count)] - def _get_web_context(self, records, add_web=True, **values): """ Helper to generate composer context. Will make tests a bit less verbose. diff --git a/addons/test_mail/tests/test_mail_template.py b/addons/test_mail/tests/test_mail_template.py index 006e9a6706c..5b5016ca39e 100644 --- a/addons/test_mail/tests/test_mail_template.py +++ b/addons/test_mail/tests/test_mail_template.py @@ -8,7 +8,7 @@ from odoo.tests import tagged from odoo.tools import mute_logger -@tagged('mail_template') +@tagged('mail_template', 'multi_lang') class TestMailTemplate(TestMailCommon, TestRecipients): @classmethod @@ -39,81 +39,22 @@ class TestMailTemplate(TestMailCommon, TestRecipients): cls.email_2 = 'test2@example.com' cls.email_3 = cls.partner_1.email - # activate translations - cls.env['res.lang']._activate_lang('es_ES') - cls.env.ref('base.module_base')._update_translations(['es_ES']) - # create a complete test template cls.test_template = cls._create_template('mail.test.lang', { 'attachment_ids': [(0, 0, cls._attachments[0]), (0, 0, cls._attachments[1])], - 'body_html': '

English Body for

', + 'body_html': '

EnglishBody for

', 'lang': '{{ object.customer_id.lang or object.lang }}', 'email_to': '%s, %s' % (cls.email_1, cls.email_2), 'email_cc': '%s' % cls.email_3, 'partner_to': '%s,%s' % (cls.partner_2.id, cls.user_admin.partner_id.id), - 'subject': 'English for {{ object.name }}', + 'subject': 'EnglishSubject for {{ object.name }}', }) - # Make sure Spanish translations have not been altered - description_translations = cls.env['ir.translation'].search([ - ('module', '=', 'test_mail'), - ('src', '=', cls.test_record._description), - ('lang', '=', 'es_ES') - ]) - if description_translations: - description_translations.update({'value': 'Spanish description'}) - else: - description_translations.create({ - 'type': 'model', - 'name': 'ir.model,name', - 'module': 'test_mail', - 'lang': 'es_ES', - 'res_id': cls.env['ir.model']._get_id('mail.test.lang'), - 'src': cls.test_record._description, - 'value': 'Spanish description', - 'state': 'translated', - }) - - cls.env['ir.translation'].create({ - 'type': 'model', - 'name': 'mail.template,subject', - 'module': 'mail', - 'lang': 'es_ES', - 'res_id': cls.test_template.id, - 'value': 'Spanish for {{ object.name }}', - 'state': 'translated', - }) - cls.env['ir.translation'].create({ - 'type': 'model', - 'name': 'mail.template,body_html', - 'module': 'mail', - 'lang': 'es_ES', - 'res_id': cls.test_template.id, - 'value': '

Spanish Body for

', - 'state': 'translated', - }) - view = cls.env['ir.ui.view'].create({ - 'name': 'test_layout', - 'key': 'test_layout', - 'type': 'qweb', - 'arch_db': ' English Layout ' - }) - cls.env['ir.model.data'].create({ - 'name': 'test_layout', - 'module': 'test_mail', - 'model': 'ir.ui.view', - 'res_id': view.id - }) - cls.env['ir.translation'].create({ - 'type': 'model_terms', - 'name': 'ir.ui.view,arch_db', - 'module': 'test_mail', - 'lang': 'es_ES', - 'res_id': view.id, - 'src': 'English Layout', - 'value': 'Spanish Layout', - 'state': 'translated', - }) + # activate translations + cls._activate_multi_lang( + layout_arch_db=' English Layout for ', + test_record=cls.test_record, test_template=cls.test_template + ) # admin should receive emails cls.user_admin.write({'notification_type': 'email'}) @@ -127,7 +68,7 @@ class TestMailTemplate(TestMailCommon, TestRecipients): self.assertEqual(mail.email_cc, self.test_template.email_cc) self.assertEqual(mail.email_to, self.test_template.email_to) self.assertEqual(mail.recipient_ids, self.partner_2 | self.user_admin.partner_id) - self.assertEqual(mail.subject, 'English for %s' % self.test_record.name) + self.assertEqual(mail.subject, 'EnglishSubject for %s' % self.test_record.name) @mute_logger('odoo.addons.mail.models.mail_mail') def test_template_translation_lang(self): @@ -137,11 +78,11 @@ class TestMailTemplate(TestMailCommon, TestRecipients): }) test_template = self.env['mail.template'].browse(self.test_template.ids) - mail_id = test_template.send_mail(test_record.id, email_layout_xmlid='test_mail.test_layout') + mail_id = test_template.send_mail(test_record.id, email_layout_xmlid='mail.test_layout') mail = self.env['mail.mail'].sudo().browse(mail_id) self.assertEqual(mail.body_html, - '

Spanish Body for %s

Spanish Layout Spanish description' % self.test_record.name) - self.assertEqual(mail.subject, 'Spanish for %s' % self.test_record.name) + '

SpanishBody for %s

Spanish Layout para Spanish description' % self.test_record.name) + self.assertEqual(mail.subject, 'SpanishSubject for %s' % self.test_record.name) @mute_logger('odoo.addons.mail.models.mail_mail') def test_template_translation_partner_lang(self): @@ -156,11 +97,11 @@ class TestMailTemplate(TestMailCommon, TestRecipients): }) test_template = self.env['mail.template'].browse(self.test_template.ids) - mail_id = test_template.send_mail(test_record.id, email_layout_xmlid='test_mail.test_layout') + mail_id = test_template.send_mail(test_record.id, email_layout_xmlid='mail.test_layout') mail = self.env['mail.mail'].sudo().browse(mail_id) self.assertEqual(mail.body_html, - '

Spanish Body for %s

Spanish Layout Spanish description' % self.test_record.name) - self.assertEqual(mail.subject, 'Spanish for %s' % self.test_record.name) + '

SpanishBody for %s

Spanish Layout para Spanish description' % self.test_record.name) + self.assertEqual(mail.subject, 'SpanishSubject for %s' % self.test_record.name) def test_template_add_context_action(self): self.test_template.create_action() diff --git a/addons/test_mail/tests/test_mail_template_preview.py b/addons/test_mail/tests/test_mail_template_preview.py index de8a2eadfa1..87becbaaeb1 100644 --- a/addons/test_mail/tests/test_mail_template_preview.py +++ b/addons/test_mail/tests/test_mail_template_preview.py @@ -20,7 +20,7 @@ class TestMailTemplateTools(TestMailTemplate): 'resource_ref': test_record, 'lang': 'es_ES', }) - self.assertEqual(preview.body_html, '

Spanish Body for %s

' % test_record.name) + self.assertEqual(preview.body_html, '

SpanishBody for %s

' % test_record.name) preview.write({'lang': 'en_US'}) - self.assertEqual(preview.body_html, '

English Body for %s

' % test_record.name) + self.assertEqual(preview.body_html, '

EnglishBody for %s

' % test_record.name) diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 2d7e2b9e72f..6228d8047a0 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -528,3 +528,133 @@ class TestMessagePostGlobal(TestMailCommon, TestRecipients): {'body': 'test'} ) self.assertTrue(isinstance(message_id, int)) + + +@tagged('mail_post', 'multi_lang') +class TestMessagePostLang(TestMailCommon, TestRecipients): + + @classmethod + def setUpClass(cls): + super(TestMessagePostLang, cls).setUpClass() + + cls.test_records = cls.env['mail.test.lang'].create([ + {'customer_id': False, + 'email_from': 'test.record.1@test.customer.com', + 'lang': 'es_ES', + 'name': 'TestRecord1', + }, + {'customer_id': cls.partner_2.id, + 'email_from': 'valid.other@gmail.com', + 'name': 'TestRecord2', + }, + ]) + + cls.test_template = cls.env['mail.template'].create({ + 'auto_delete': True, + 'body_html': '

EnglishBody for

', + 'email_from': '{{ user.email_formatted }}', + 'email_to': '{{ (object.email_from if not object.customer_id else "") }}', + 'lang': '{{ object.customer_id.lang or object.lang }}', + 'model_id': cls.env['ir.model']._get('mail.test.lang').id, + 'name': 'TestTemplate', + 'partner_to': '{{ object.customer_id.id if object.customer_id else "" }}', + 'subject': 'EnglishSubject for {{ object.name }}', + }) + cls.user_employee.write({ # add group to create contacts, necessary for templates + 'groups_id': [(4, cls.env.ref('base.group_partner_manager').id)], + }) + + cls._activate_multi_company() + cls._activate_multi_lang(test_record=cls.test_records[0], test_template=cls.test_template) + + cls.partner_2.write({'lang': 'es_ES'}) + + @users('employee') + def test_composer_lang_template(self): + test_records = self.test_records.with_user(self.env.user) + test_template = self.test_template.with_user(self.env.user) + + with self.mock_mail_gateway(): + test_records.message_post_with_template( + test_template.id, + composition_mode='mass_mail', + # email_layout_xmlid='mail.test_layout', Not supported + message_type='comment', + subtype_id=self.env.ref('mail.mt_comment').id, + ) + + record0_customer = self.env['res.partner'].search([('email_normalized', '=', 'test.record.1@test.customer.com')], limit=1) + self.assertTrue(record0_customer, 'Template usage should have created a contact based on record email') + + for record, customer in zip(test_records, record0_customer + self.partner_2): + customer_email = self._find_sent_mail_wemail(customer.email_formatted) + self.assertTrue(customer_email) + body = customer_email['body'] + # check content + # self.assertIn('SpanishBody for %s' % record.name, body, 'Body based on template should be translated') + self.assertIn('EnglishBody for %s' % record.name, body, 'Fixme: this should be translated') + # check subject + # self.assertEqual('SpanishSubject for %s' % record.name, customer_email['subject'], 'Subject based on template should be translated') + self.assertEqual('EnglishSubject for %s' % record.name, customer_email['subject'], 'Fixme: this should be translated') + + @users('employee') + def test_layout_email_lang_context(self): + test_records = self.test_records.with_user(self.env.user).with_context(lang='es_ES') + test_records[1].message_subscribe(self.partner_2.ids) + + with self.mock_mail_gateway(): + test_records[1].message_post( + body='

Hello

', + email_layout_xmlid='mail.test_layout', + message_type='comment', + subject='Subject', + subtype_xmlid='mail.mt_comment', + ) + + customer_email = self._find_sent_mail_wemail(self.partner_2.email_formatted) + self.assertTrue(customer_email) + body = customer_email['body'] + # check notification layout translation + self.assertIn('Spanish Layout para', body, 'Layout content should be translated') + self.assertNotIn('English Layout for', body) + self.assertIn('Spanish Layout para Spanish description', body, 'Model name should be translated') + self.assertIn('SpanishView Spanish description', body, '"View document" should be translated') + self.assertNotIn('View %s' % test_records[1]._description, body) + self.assertIn('TestSpanishStuff', body, 'Groups-based action names should be translated') + self.assertNotIn('TestStuff', body) + # check content + self.assertIn('Hello', body, 'Body of posted message should be present') + + @users('employee') + def test_layout_email_lang_template(self): + test_records = self.test_records.with_user(self.env.user) + test_template = self.test_template.with_user(self.env.user) + + with self.mock_mail_gateway(): + for test_record in test_records: + test_record.message_post_with_template( + test_template.id, + email_layout_xmlid='mail.test_layout', + message_type='comment', + subtype_id=self.env.ref('mail.mt_comment').id, + ) + + record0_customer = self.env['res.partner'].search([('email_normalized', '=', 'test.record.1@test.customer.com')], limit=1) + self.assertTrue(record0_customer, 'Template usage should have created a contact based on record email') + + for record, customer in zip(test_records, record0_customer + self.partner_2): + customer_email = self._find_sent_mail_wemail(customer.email_formatted) + self.assertTrue(customer_email) + body = customer_email['body'] + # check notification layout translation + self.assertIn('Spanish Layout para', body, 'Layout content should be translated') + self.assertNotIn('English Layout for', body) + self.assertIn('Spanish Layout para Spanish description', body, 'Model name should be translated') + # self.assertIn('SpanishView Spanish description', body, '"View document" should be translated') + self.assertIn('View %s' % test_records[1]._description, body, 'Fixme: this should be translated') + # self.assertIn('TestSpanishStuff', body, 'Groups-based action names should be translated') + self.assertIn('TestStuff', body, 'Fixme: groups-based action names should be translated') + # check content + self.assertIn('SpanishBody for %s' % record.name, body, 'Body based on template should be translated') + # check subject + self.assertEqual('SpanishSubject for %s' % record.name, customer_email['subject'], 'Subject based on template should be translated') From 2ad668d578aef91603093dce2a5fadb935f05d3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 18 Jan 2022 15:32:22 +0000 Subject: [PATCH 07/23] [IMP] (test_)mail: add tests for recipients computation on notify Purpose of this commit is to add tests for recipients fetch. Purpose is to assess current behavior before going into some cleaning or refactoring. Notably when dealing with * forced recipients (pids, aka 'additional recipients' in composers); * subtype-based computation (internal, subtype follower); * multi-users per partner (notably as a result of partner merge); Future improvements will modify this method. Adding some low level tests for it will ease development and checking differences. Some tests are also regrouped in the same test class to lessen noise. Task-2739294 (Mail: Batch recipients fetch and improve its usage) Part-of: odoo/odoo#82167 --- addons/test_mail/tests/test_invite.py | 2 + addons/test_mail/tests/test_mail_followers.py | 266 +++++++++++++++--- 2 files changed, 222 insertions(+), 46 deletions(-) diff --git a/addons/test_mail/tests/test_invite.py b/addons/test_mail/tests/test_invite.py index dc543d1963b..718481a3ffd 100644 --- a/addons/test_mail/tests/test_invite.py +++ b/addons/test_mail/tests/test_invite.py @@ -2,9 +2,11 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.addons.test_mail.tests.common import TestMailCommon +from odoo.tests import tagged from odoo.tools import mute_logger +@tagged('mail_followers') class TestInvite(TestMailCommon): @mute_logger('odoo.addons.mail.models.mail_mail') diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py index f06f1c006e5..f7f6003119d 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -1,12 +1,9 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from psycopg2 import IntegrityError - from odoo.addons.test_mail.tests.common import TestMailCommon from odoo.tests import tagged from odoo.tests import users -from odoo.tools.misc import mute_logger @tagged('mail_followers') @@ -417,62 +414,239 @@ class AdvancedResponsibleNotifiedTest(TestMailCommon): self.assertEqual(mail_notification.mail_mail_id.state, 'outgoing') -@tagged('post_install', '-at_install') -class DuplicateNotificationTest(TestMailCommon): - def test_no_duplicate_notification(self): - """ - Check that we only create one mail.notification per partner +@tagged('mail_followers', 'post_install', '-at_install') +class RecipientsNotificationTest(TestMailCommon): + """ Test advanced and complex recipients computation / notification, such + as multiple users, batch computation, ... Post install because we need the + registry to be ready to send notifications.""" - Post install because we need the registery to be ready to send notification - """ - #Simulate case of 2 users that got their partner merged - common_partner = self.env['res.partner'].create({"name": "demo1", "email": "demo1@test.com"}) - user_1 = self.env['res.users'].create({'login': 'demo1', 'partner_id': common_partner.id, 'notification_type': 'email'}) - user_2 = self.env['res.users'].create({'login': 'demo2', 'partner_id': common_partner.id, 'notification_type': 'inbox'}) + @classmethod + def setUpClass(cls): + super(RecipientsNotificationTest, cls).setUpClass() - #Trigger auto subscribe notification - test = self.env['mail.test.track'].create({"name": "Test Track", "user_id": user_2.id}) + # portal user for testing share status / internal subtypes + cls.user_portal = cls._create_portal_user() + cls.partner_portal = cls.user_portal.partner_id + + # simple customer + cls.customer = cls.env['res.partner'].create({ + 'email': 'customer@test.customer.com', + 'name': 'Customer', + 'phone': '+32455778899', + }) + + # Simulate case of 2 users that got their partner merged + cls.common_partner = cls.env['res.partner'].create({ + 'email': 'common.partner@test.customer.com', + 'name': 'Common Partner', + 'phone': '+32455998877', + }) + cls.user_1, cls.user_2 = cls.env['res.users'].with_context(no_reset_password=True).create([ + {'groups_id': [(4, cls.env.ref('base.group_portal').id)], + 'login': '_login_portal', + 'notification_type': 'email', + 'partner_id': cls.common_partner.id, + }, + {'groups_id': [(4, cls.env.ref('base.group_user').id)], + 'login': '_login_internal', + 'notification_type': 'inbox', + 'partner_id': cls.common_partner.id, + } + ]) + + def assertRecipientsData(self, recipients_data, records, partners, partner_to_users=None): + """ Custom assert as recipients structure is custom and may change due + to some implementation choice. Currently supporting only single-record + computation. """ + self.assertEqual(len(recipients_data), len(partners.ids)) + for partner in partners: + partner_data = next(r for r in recipients_data if r[0] == partner.id) + (pid, active, pshare, notif, groups) = partner_data + if partner_to_users and partner_to_users.get(pid): #helps making test explicit + user = partner_to_users[pid] + else: + user = partner.user_ids[-1] if partner.user_ids else self.env['res.users'] + self.assertEqual(active, partner.active) + if user: + self.assertEqual(set(groups), set(user.groups_id.ids)) + self.assertEqual(notif, user.notification_type) + else: + self.assertEqual(set(groups), {None}) + self.assertEqual(notif, None) + self.assertEqual(pshare, partner.partner_share) + + @users('employee') + def test_notification_nodupe(self): + """ Check that we only create one mail.notification per partner. """ + # Trigger auto subscribe notification + test = self.env['mail.test.track'].create({"name": "Test Track", "user_id": self.user_2.id}) mail_message = self.env['mail.message'].search([ - ('res_id', '=', test.id), - ('model', '=', 'mail.test.track'), - ('message_type', '=', 'user_notification') + ('res_id', '=', test.id), + ('model', '=', 'mail.test.track'), + ('message_type', '=', 'user_notification') ]) notif = self.env['mail.notification'].search([ ('mail_message_id', '=', mail_message.id), - ('res_partner_id', '=', common_partner.id) + ('res_partner_id', '=', self.common_partner.id) ]) self.assertEqual(len(notif), 1) self.assertEqual(notif.notification_type, 'email') - subtype = self.env.ref('mail.mt_comment') - res = self.env['mail.followers']._get_recipient_data(test, 'comment', subtype.id, pids=common_partner.ids) - partner_notif = [r for r in res if r[0] == common_partner.id] - self.assertEqual(len(partner_notif), 1) - self.assertEqual(partner_notif[0][3], 'email') + res = self.env['mail.followers']._get_recipient_data( + test, 'comment', self.env.ref('mail.mt_comment').id, + pids=self.common_partner.ids) + self.assertRecipientsData(res, test, self.common_partner + self.partner_employee, + partner_to_users={self.common_partner.id: self.user_1}) -@tagged('post_install', '-at_install') -class UnlinkedNotificationTest(TestMailCommon): - def test_unlinked_notification(self): - """ - Check that we unlink the created user_notification after unlinked the related document - - Post install because we need the registery to be ready to send notification - """ - common_partner = self.env['res.partner'].create({"name": "demo1", "email": "demo1@test.com"}) - user_1 = self.env['res.users'].create({'login': 'demo1', 'partner_id': common_partner.id, 'notification_type': 'inbox'}) - - test = self.env['mail.test.track'].create({"name": "Test Track", "user_id": user_1.id}) - test_id = test.id + @users('employee') + def test_notification_unlink(self): + """ Check that we unlink the created user_notification after unlinked the + related document. """ + test = self.env['mail.test.track'].create({"name": "Test Track", "user_id": self.user_1.id}) mail_message = self.env['mail.message'].search([ - ('res_id', '=', test_id), - ('model', '=', 'mail.test.track'), - ('message_type', '=', 'user_notification') + ('res_id', '=', test.id), + ('model', '=', 'mail.test.track'), + ('message_type', '=', 'user_notification') ]) self.assertEqual(len(mail_message), 1) test.unlink() - mail_message = self.env['mail.message'].search([ - ('res_id', '=', test_id), - ('model', '=', 'mail.test.track'), - ('message_type', '=', 'user_notification') + self.assertEqual( + self.env['mail.message'].search_count([ + ('res_id', '=', test.id), + ('model', '=', 'mail.test.track'), + ('message_type', '=', 'user_notification') + ]), 0 + ) + + @users('employee') + def test_notification_user_choice(self): + """ Check fetching user information when notifying someone with multiple + users (more complex use case). """ + company_other = self.env['res.company'].sudo().create({ + 'currency_id': self.env.ref('base.CAD').id, + 'email': 'company_other@test.example.com', + 'name': 'Company Other', + }) + shared_partner = self.env['res.partner'].sudo().create({ + 'email': 'common.partner@test.customer.com', + 'name': 'Common Partner', + 'phone': '+32455998877', + }) + cids = (company_other + self.company_admin).ids + user_2_1, user_2_2, user_2_3 = self.env['res.users'].sudo().with_context(no_reset_password=True).create([ + {'company_ids': [(6, 0, cids)], + 'company_id': self.company_admin.id, + 'groups_id': [(4, self.env.ref('base.group_portal').id)], + 'login': '_login2_portal', + 'notification_type': 'email', + 'partner_id': shared_partner.id, + }, + {'company_ids': [(6, 0, cids)], + 'company_id': self.company_admin.id, + 'groups_id': [(4, self.env.ref('base.group_user').id)], + 'login': '_login2_internal', + 'notification_type': 'inbox', + 'partner_id': shared_partner.id, + }, + {'company_ids': [(6, 0, cids)], + 'company_id': company_other.id, + 'groups_id': [(4, self.env.ref('base.group_user').id), (4, self.env.ref('base.group_partner_manager').id)], + 'login': '_login2_manager', + 'notification_type': 'inbox', + 'partner_id': shared_partner.id, + } ]) - self.assertEqual(len(mail_message), 0) + # just ensure current share status + self.assertFalse(shared_partner.partner_share) + self.assertTrue(user_2_1.share) + self.assertFalse(user_2_2.share or user_2_3.share) + + test = self.env['mail.test.track'].create({"name": "Test Track", "user_id": False}) + self.assertEqual(test.message_partner_ids, self.partner_employee) + + with self.assertSinglePostNotifications( + [{'group': 'customer', 'partner': shared_partner, + 'status': 'sent', 'type': 'email'}], + message_info={'content': 'User Choice Notification'}): + test.message_post( + body='

User Choice Notification

', + message_type='comment', + partner_ids=shared_partner.ids, + subtype_xmlid='mail.mt_comment', + ) + + recipients_data = self.env['mail.followers']._get_recipient_data( + test, 'comment', self.env.ref('mail.mt_comment').id, + pids=shared_partner.ids) + self.assertRecipientsData(recipients_data, test, self.partner_employee + shared_partner, + partner_to_users={shared_partner.id: user_2_1}) + + @users('employee') + def test_recipients_fetch(self): + """ Test internals of ``_get_recipient_data`` to ease its maintenance. + Even if it is low-level knowing what it produces allows to write short + tests. """ + test_records = self.env['mail.test.simple'].create([ + {'email_from': 'ignasse@example.com', + 'name': 'Test %s' % idx, + } for idx in range(5) + ]) + # make followers listen to notes to use it and check portal will never be notified of it (internal) + test_records.message_follower_ids.sudo().write({'subtype_ids': [(4, self.env.ref('mail.mt_note').id)]}) + for test_record in test_records: + self.assertEqual(test_record.message_partner_ids, self.env.user.partner_id) + + test_records[0].message_subscribe(self.partner_portal.ids) + self.assertNotIn( + self.env.ref('mail.mt_note'), + test_records[0].message_follower_ids.filtered(lambda fol: fol.partner_id == self.partner_portal).subtype_ids, + 'Portal user should not follow notes by default') + + # just fetch followers + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records[0], 'comment', self.env.ref('mail.mt_comment').id, + pids=None + ) + self.assertRecipientsData(recipients_data, test_records[0], self.env.user.partner_id + self.partner_portal) + + # followers + additional recipients + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records[0], 'comment', self.env.ref('mail.mt_comment').id, + pids=(self.customer + self.common_partner + self.partner_admin).ids + ) + self.assertRecipientsData(recipients_data, test_records[0], + self.env.user.partner_id + self.partner_portal + self.customer + self.common_partner + self.partner_admin) + + # ensure filtering on internal: should exclude Portal even if misconfiguration + follower_portal = test_records[0].message_follower_ids.filtered(lambda fol: fol.partner_id == self.partner_portal).sudo() + follower_portal.write({'subtype_ids': [(4, self.env.ref('mail.mt_note').id)]}) + follower_portal.flush() + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records[0], 'comment', self.env.ref('mail.mt_note').id, + pids=(self.common_partner + self.partner_admin).ids + ) + self.assertRecipientsData(recipients_data, test_records[0], self.env.user.partner_id + self.common_partner + self.partner_admin) + + # ensure filtering on subtype: should exclude Portal as it does not follow comment anymore + follower_portal.write({'subtype_ids': [(3, self.env.ref('mail.mt_comment').id)]}) + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records[0], 'comment', self.env.ref('mail.mt_comment').id, + pids=(self.common_partner + self.partner_admin).ids + ) + self.assertRecipientsData(recipients_data, test_records[0], self.env.user.partner_id + self.common_partner + self.partner_admin) + + # check without subtype + recipients_data = self.env['mail.followers']._get_recipient_data( + test_records[0], 'comment', False, + pids=(self.common_partner + self.partner_admin).ids + ) + # TDE FIXME: currently crashes as no user infomation is fetched for PIDs + # self.assertRecipientsData(recipients_data, test_records[0], self.common_partner + self.partner_admin) + self.assertEqual(len(recipients_data), 2) + for partner in self.common_partner + self.partner_admin: + partner_data = next(r for r in recipients_data if r[0] == partner.id) + (_pid, active, pshare, notif, groups) = partner_data + self.assertEqual(active, partner.active) + self.assertEqual(groups, None, 'FIXME: currently not fetching any group information') + self.assertEqual(notif, 'inbox' if partner == self.partner_admin else 'email') + self.assertEqual(pshare, partner.partner_share) From ccc35f5ab6bd44beee861073e16b7b2d3a04d1dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 18 Jan 2022 15:37:52 +0000 Subject: [PATCH 08/23] [FIX] mail: fix issue when computing ``_get_recipient_data`` If we call ``MailFollowers._get_recipient_data()`` with pids parameter being set to None there is a crash that is easily solved( list(None), should take list fallback before hand). Task-2739294 (Mail: Batch recipients fetch and improve its usage) Part-of: odoo/odoo#82167 --- addons/mail/models/mail_followers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 15777f5977b..6f3e661373c 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -136,7 +136,7 @@ SELECT DISTINCT ON (pid) * FROM ( ) AS x ORDER BY pid, notif """ - params = [subtype_id, records._name, tuple(records.ids), list(pids) or []] + params = [subtype_id, records._name, tuple(records.ids), list(pids or [])] self.env.cr.execute(query, tuple(params)) res = self.env.cr.fetchall() elif pids: From 271859b55276a471395fabf9131ee35221b7fa75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 19 Jan 2022 09:03:29 +0000 Subject: [PATCH 09/23] [FIX] mail: support portal redirection in _notify_get_action_link Purpose is to support portal parameters in ``_notify_get_action_link``. This method generates a ``mail/view`` link that may redirect to frontend e.g. with portal. We already support parameters coming from auth_signup and tokens from portal. Supporting pid / hash allowing to post on documents using the frontend chatter is the next step to generic links. This is just adding them to the accepted like, not changing any current behavior. Task-2712450 (Mail/Sale: Improve 'Pay Now' notification template) Part-of: odoo/odoo#82167 --- addons/mail/models/mail_thread.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index cc26032c0fd..adcc5b1f382 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2663,12 +2663,15 @@ class MailThread(models.AbstractModel): 'model': kwargs.get('model', self._name), 'res_id': kwargs.get('res_id', self.ids and self.ids[0] or False), } - # whitelist accepted parameters: action (deprecated), token (assign), access_token - # (view), auth_signup_token and auth_login (for auth_signup support) + # keep only accepted parameters: + # - action (deprecated), token (assign), access_token (view) + # - auth_signup: auth_signup_token and auth_login + # - portal: pid, hash params.update(dict( (key, value) for key, value in kwargs.items() - if key in ('action', 'token', 'access_token', 'auth_signup_token', 'auth_login') + if key in ('action', 'token', 'access_token', 'auth_signup_token', + 'auth_login', 'pid', 'hash') )) if link_type in ['view', 'assign', 'follow', 'unfollow']: From ef7e6009f8033a223bfbcf2e898bf8fadff8d349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 19 Jan 2022 16:26:06 +0000 Subject: [PATCH 10/23] [FIX] mail: remove unnecessary model decorator A method that actually runs on recordsets should not have a model decorator. Even if there is no issue this is may be confusing. Task-2710804 (Mail: Clean Mail.Thread API) Part-of: odoo/odoo#82167 --- addons/mail/models/mail_thread.py | 1 - 1 file changed, 1 deletion(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index adcc5b1f382..50f4f1a9e48 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2350,7 +2350,6 @@ class MailThread(models.AbstractModel): return True - @api.model def _notify_by_email_prepare_rendering_context(self, message, msg_vals=False, model_description=False, force_email_company=False, force_email_lang=False): """ Prepare rendering context for notification email. From a018735ec4cb1c0c230a7599e0fe2a30785878cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 19 Jan 2022 09:15:29 +0000 Subject: [PATCH 11/23] [REF] base, mail, various: make get_access_action private and explicit Purpose of this commit is to make ``get_access_action`` private as it is not necessary to expose it directly. Website management is also made explicit using a ``force_website`` parameter instead of relying on context key of the same name. This allows to better understand the method code flow. Contains also some code fix / improvements : * Website forum: update code to better skip the frontend redirection if the forum is not active and frontend is not forced; * Website slides: respect force website parameter in redirection; Task-2710804 (Mail: Clean Mail.Thread API) Part-of: odoo/odoo#82167 --- addons/mail/controllers/mail.py | 4 ++-- .../mail/data/mail_templates_email_layouts.xml | 4 ++-- addons/portal/controllers/mail.py | 2 +- addons/portal/models/portal_mixin.py | 16 ++++++++++------ addons/website_blog/models/website_blog.py | 8 ++++---- .../models/crm_lead.py | 10 +++++----- .../tests/test_partner_assign.py | 2 +- addons/website_forum/models/forum.py | 4 +++- addons/website_slides/models/slide_slide.py | 6 +++--- odoo/models.py | 12 ++++++++---- 10 files changed, 39 insertions(+), 29 deletions(-) diff --git a/addons/mail/controllers/mail.py b/addons/mail/controllers/mail.py index 209afa1d9a6..57d842c0150 100644 --- a/addons/mail/controllers/mail.py +++ b/addons/mail/controllers/mail.py @@ -93,9 +93,9 @@ class MailController(http.Controller): except AccessError: return cls._redirect_to_messaging() else: - record_action = record_sudo.get_access_action(access_uid=uid) + record_action = record_sudo._get_access_action(access_uid=uid) else: - record_action = record_sudo.get_access_action() + record_action = record_sudo._get_access_action() if record_action['type'] == 'ir.actions.act_url' and record_action.get('target_type') != 'public': return cls._redirect_to_messaging() diff --git a/addons/mail/data/mail_templates_email_layouts.xml b/addons/mail/data/mail_templates_email_layouts.xml index 890ea6c5f25..75a6b3ba426 100644 --- a/addons/mail/data/mail_templates_email_layouts.xml +++ b/addons/mail/data/mail_templates_email_layouts.xml @@ -132,7 +132,7 @@ - + diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index af3ee0a28ce..e2bb31c961b 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -230,7 +230,7 @@ class MailController(mail.MailController): record_sudo.with_user(uid).check_access_rule('read') except AccessError: if record_sudo.access_token and access_token and consteq(record_sudo.access_token, access_token): - record_action = record_sudo.with_context(force_website=True).get_access_action() + record_action = record_sudo._get_access_action(force_website=True) if record_action['type'] == 'ir.actions.act_url': pid = kwargs.get('pid') hash = kwargs.get('hash') diff --git a/addons/portal/models/portal_mixin.py b/addons/portal/models/portal_mixin.py index 1e0e146bd4e..0cf702ffb5c 100644 --- a/addons/portal/models/portal_mixin.py +++ b/addons/portal/models/portal_mixin.py @@ -84,9 +84,9 @@ class PortalMixin(models.AbstractModel): new_group = [] return new_group + groups - def get_access_action(self, access_uid=None): + def _get_access_action(self, access_uid=None, force_website=False): """ Instead of the classic form view, redirect to the online document for - portal users or if force_website=True in the context. """ + portal users or if force_website=True. """ self.ensure_one() user, record = self.env.user, self @@ -95,15 +95,17 @@ class PortalMixin(models.AbstractModel): record.check_access_rights('read') record.check_access_rule("read") except exceptions.AccessError: - return super(PortalMixin, self).get_access_action(access_uid) + return super(PortalMixin, self)._get_access_action( + access_uid=access_uid, force_website=force_website + ) user = self.env['res.users'].sudo().browse(access_uid) + if user.share or force_website: record = self.with_user(user) - if user.share or self.env.context.get('force_website'): try: record.check_access_rights('read') record.check_access_rule('read') except exceptions.AccessError: - if self.env.context.get('force_website'): + if force_website: return { 'type': 'ir.actions.act_url', 'url': record.access_url, @@ -119,7 +121,9 @@ class PortalMixin(models.AbstractModel): 'target': 'self', 'res_id': record.id, } - return super(PortalMixin, self).get_access_action(access_uid) + return super(PortalMixin, self)._get_access_action( + access_uid=access_uid, force_website=force_website + ) @api.model def action_share(self): diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py index 98a4e491b73..3ed2c5fa551 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -258,13 +258,13 @@ class BlogPost(models.Model): default = dict(default or {}, name=name) return super(BlogPost, self).copy_data(default) - def get_access_action(self, access_uid=None): + def _get_access_action(self, access_uid=None, force_website=False): """ Instead of the classic form view, redirect to the post on website directly if user is an employee or if the post is published. """ self.ensure_one() - user = access_uid and self.env['res.users'].sudo().browse(access_uid) or self.env.user - if user.share and not self.sudo().website_published: - return super(BlogPost, self).get_access_action(access_uid) + user = self.env['res.users'].sudo().browse(access_uid) if access_uid else self.env.user + if not force_website and user.share and not self.sudo().website_published: + return super(BlogPost, self)._get_access_action(access_uid=access_uid, force_website=force_website) return { 'type': 'ir.actions.act_url', 'url': self.website_url, diff --git a/addons/website_crm_partner_assign/models/crm_lead.py b/addons/website_crm_partner_assign/models/crm_lead.py index f5038e49a26..31922c121f4 100644 --- a/addons/website_crm_partner_assign/models/crm_lead.py +++ b/addons/website_crm_partner_assign/models/crm_lead.py @@ -283,9 +283,9 @@ class CrmLead(models.Model): # DO NOT FORWARD PORT IN MASTER # instead, crm.lead should implement portal.mixin # - def get_access_action(self, access_uid=None): + def _get_access_action(self, access_uid=None, force_website=False): """ Instead of the classic form view, redirect to the online document for - portal users or if force_website=True in the context. """ + portal users or if force_website=True. """ self.ensure_one() user, record = self.env.user, self @@ -294,10 +294,10 @@ class CrmLead(models.Model): record.check_access_rights('read') record.check_access_rule("read") except AccessError: - return super(CrmLead, self).get_access_action(access_uid) + return super(CrmLead, self)._get_access_action(access_uid=access_uid, force_website=force_website) user = self.env['res.users'].sudo().browse(access_uid) + if user.share or force_website: record = self.with_user(user) - if user.share or self.env.context.get('force_website'): try: record.check_access_rights('read') record.check_access_rule('read') @@ -308,4 +308,4 @@ class CrmLead(models.Model): 'type': 'ir.actions.act_url', 'url': '/my/opportunity/%s' % record.id, } - return super(CrmLead, self).get_access_action(access_uid) + return super(CrmLead, self)._get_access_action(access_uid=access_uid, force_website=force_website) diff --git a/addons/website_crm_partner_assign/tests/test_partner_assign.py b/addons/website_crm_partner_assign/tests/test_partner_assign.py index 43250c16641..60fba253dbe 100644 --- a/addons/website_crm_partner_assign/tests/test_partner_assign.py +++ b/addons/website_crm_partner_assign/tests/test_partner_assign.py @@ -155,6 +155,6 @@ class TestPartnerLeadPortal(TestCrmCommon): self.assertEqual(opportunity.partner_assigned_id, self.user_portal.partner_id, 'Assigned Partner of created opportunity is the (portal) creator.') def test_portal_mixin_url(self): - record_action = self.lead_portal.get_access_action(self.user_portal.id) + record_action = self.lead_portal._get_access_action(access_uid=self.user_portal.id) self.assertEqual(record_action['url'], '/my/opportunity/%s' % self.lead_portal.id) self.assertEqual(record_action['type'], 'ir.actions.act_url') diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index de7fdb92a83..d057a7dcbbd 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -911,9 +911,11 @@ class Post(models.Model): self.ensure_one() return sql.increment_field_skiplock(self, 'views') - def get_access_action(self, access_uid=None): + def _get_access_action(self, access_uid=None, force_website=False): """ Instead of the classic form view, redirect to the post on the website directly """ self.ensure_one() + if not force_website and not self.state == 'active': + return super(Post, self)._get_access_action(access_uid=access_uid, force_website=force_website) return { 'type': 'ir.actions.act_url', 'url': '/forum/%s/%s' % (self.forum_id.id, self.id), diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index b614508bc1f..17207b0b62a 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -649,10 +649,10 @@ class Slide(models.Model): raise AccessError(_('Not enough karma to comment')) return super(Slide, self).message_post(message_type=message_type, **kwargs) - def get_access_action(self, access_uid=None): + def _get_access_action(self, access_uid=None, force_website=False): """ Instead of the classic form view, redirect to website if it is published. """ self.ensure_one() - if self.website_published: + if force_website or self.website_published: return { 'type': 'ir.actions.act_url', 'url': '%s' % self.website_url, @@ -660,7 +660,7 @@ class Slide(models.Model): 'target_type': 'public', 'res_id': self.id, } - return super(Slide, self).get_access_action(access_uid) + return super(Slide, self)._get_access_action(access_uid=access_uid, force_website=force_website) def _notify_get_recipients_groups(self, msg_vals=None): """ Add access button to everyone if the document is active. """ diff --git a/odoo/models.py b/odoo/models.py index 6bd7ea83510..799e8e27abf 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -1767,15 +1767,19 @@ class BaseModel(metaclass=MetaModel): 'context': dict(self._context), } - def get_access_action(self, access_uid=None): + def _get_access_action(self, access_uid=None, force_website=False): """ Return an action to open the document. This method is meant to be overridden in addons that want to give specific access to the document. By default, it opens the formview of the document. - An optional access_uid holds the user that will access the document - that could be different from the current user. + :param integer access_uid: optional access_uid being the user that + accesses the document. May be different from the current user as we + may compute an access for someone else. + :param integer force_website: force frontend redirection if available + on self. Used in overrides, notably with portal / website addons. """ - return self[0].get_formview_action(access_uid=access_uid) + self.ensure_one() + return self.get_formview_action(access_uid=access_uid) @api.model def search_count(self, args): From b78344f99c8116b34a50c93f4e83f2409a954320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 3 Jan 2022 15:06:20 +0000 Subject: [PATCH 12/23] [REF] mail: rename 'add_sign' fields In order to make fields a bit more explicit, let us rename "add_sign" to "email_add_signature". It indicates it is linked to the email notification process, adding the signature depending on the notification layout itself. This is done on both mail.message and mail.compose.message models. It is also updated as rendering value parameter used in notification emails. Task-2712450 (Mail/Sale: Improve 'Pay Now' notification template) Part-of: odoo/odoo#82167 --- addons/mail/__manifest__.py | 2 +- addons/mail/models/mail_message.py | 2 +- addons/mail/models/mail_thread.py | 18 +++++++++--------- addons/mail/wizard/mail_compose_message.py | 4 ++-- addons/mail/wizard/mail_wizard_invite.py | 2 +- addons/test_mail/tests/test_message_post.py | 16 ++++++++++++++-- addons/test_mail/tests/test_performance.py | 2 +- 7 files changed, 29 insertions(+), 17 deletions(-) diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index bf0987a60bc..c4b7a755e7b 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -2,7 +2,7 @@ { 'name': 'Discuss', - 'version': '1.6', + 'version': '1.7', 'category': 'Productivity/Discuss', 'sequence': 145, 'summary': 'Chat, mail gateway and private channels', diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index bc748045b83..fb30d12fd9e 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -166,7 +166,7 @@ class Message(models.Model): mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing mail server') # keep notification layout informations to be able to generate mail again email_layout_xmlid = fields.Char('Layout', copy=False) # xml id of layout - add_sign = fields.Boolean(default=True) + email_add_signature = fields.Boolean(default=True) # `test_adv_activity`, `test_adv_activity_full`, `test_message_assignation_inbox`,... # By setting an inverse for mail.mail_message_id, the number of SQL queries done by `modified` is reduced. # 'mail.mail' inherits from `mail.message`: `_inherits = {'mail.message': 'mail_message_id'}` diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 50f4f1a9e48..28cc003f4e5 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1839,8 +1839,8 @@ class MailThread(models.AbstractModel): parent_id = self._message_compute_parent_id(parent_id) msg_values = dict(msg_kwargs) - if 'add_sign' not in msg_values: - msg_values['add_sign'] = True + if 'email_add_signature' not in msg_values: + msg_values['email_add_signature'] = True if not msg_values.get('record_name'): msg_values['record_name'] = self.display_name msg_values.update({ @@ -1996,8 +1996,8 @@ class MailThread(models.AbstractModel): 'message_id': tools.generate_tracking_message_id('message-notify'), } msg_values.update(msg_kwargs) - if 'add_sign' not in msg_values: - msg_values['add_sign'] = True + if 'email_add_signature' not in msg_values: + msg_values['email_add_signature'] = True new_message = MailThread._message_create(msg_values) MailThread._notify_thread(new_message, msg_values, **notif_kwargs) @@ -2030,7 +2030,7 @@ class MailThread(models.AbstractModel): 'record_name': False, 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify - 'add_sign': False, # False as no notification -> no need to compute signature + 'email_add_signature': False, # False as no notification -> no need to compute signature } msg_values.update(kwargs) return self.sudo()._message_create(msg_values) @@ -2054,7 +2054,7 @@ class MailThread(models.AbstractModel): 'record_name': False, 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify - 'add_sign': False, + 'email_add_signature': False, } values_list = [dict(base_message_values, res_id=record.id, @@ -2386,8 +2386,8 @@ class MailThread(models.AbstractModel): # compute send user and its related signature; try to use self.env.user instead of browsing # user_ids if he is the author will give a sudo user, improving access performances and cache usage. signature = '' - add_sign = msg_vals.get('add_sign') if 'add_sign' in msg_vals else message.add_sign - if add_sign: + email_add_signature = msg_vals.get('email_add_signature') if msg_vals and 'email_add_signature' in msg_vals else message.email_add_signature + if email_add_signature: author = message.env['res.partner'].browse(msg_vals.get('author_id')) if 'author_id' in msg_vals else message.author_id author_user = self.env.user if self.env.user.partner_id == author else author.user_ids[0] if author and author.user_ids else False if author_user: @@ -2447,8 +2447,8 @@ class MailThread(models.AbstractModel): 'record': self, 'record_name': record_name, # user / environment - 'add_sign': add_sign, 'company': company, + 'email_add_signature': email_add_signature, 'lang': lang, 'signature': signature, 'website_url': website_url, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index b7393a58f61..af68dc0bf28 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -101,7 +101,7 @@ class MailComposer(models.TransientModel): 'wizard_id', 'attachment_id', 'Attachments') email_layout_xmlid = fields.Char('Email Notification Layout', copy=False) layout = fields.Char('Layout', copy=False) # xml id of layout - add_sign = fields.Boolean(default=True) + email_add_signature = fields.Boolean(default=True) # origin email_from = fields.Char('From', help="Email address of the sender. This field is set when no matching partner is found and replaces the author_id field in the chatter.") author_id = fields.Many2one( @@ -311,7 +311,7 @@ class MailComposer(models.TransientModel): message_type=wizard.message_type, subtype_id=subtype_id, email_layout_xmlid=wizard.email_layout_xmlid, - add_sign=not bool(wizard.template_id), + email_add_signature=not bool(wizard.template_id) and wizard.email_add_signature, mail_auto_delete=wizard.template_id.auto_delete if wizard.template_id else self._context.get('mail_auto_delete', True), model_description=model_description) post_params.update(mail_values) diff --git a/addons/mail/wizard/mail_wizard_invite.py b/addons/mail/wizard/mail_wizard_invite.py index bb2e1f6559d..b859e1aabd4 100644 --- a/addons/mail/wizard/mail_wizard_invite.py +++ b/addons/mail/wizard/mail_wizard_invite.py @@ -68,7 +68,7 @@ class Invite(models.TransientModel): 'model': wizard.res_model, 'res_id': wizard.res_id, 'reply_to_force_new': True, - 'add_sign': True, + 'email_add_signature': True, }) partners_data = [] recipients_data = self.env['mail.followers']._get_recipient_data(document, 'comment', False, pids=new_partners.ids) diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 6228d8047a0..5552c0af276 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -84,13 +84,25 @@ class TestMessagePost(TestMailCommon, TestRecipients): self.assertIn("record.user_id.sudo().signature", template.arch) with self.mock_mail_gateway(): - self.test_track.message_post(body="Test body", mail_auto_delete=False, add_sign=True, partner_ids=[self.partner_1.id, self.partner_2.id], email_layout_xmlid="mail.mail_notification_paynow") + self.test_track.message_post( + body="Test body", + email_add_signature=True, + email_layout_xmlid="mail.mail_notification_paynow", + mail_auto_delete=False, + partner_ids=[self.partner_1.id, self.partner_2.id], + ) found_mail = self._new_mails self.assertIn(signature, found_mail.body_html) self.assertEqual(found_mail.body_html.count(signature), 1) with self.mock_mail_gateway(): - self.test_track.message_post(body="Test body", mail_auto_delete=False, add_sign=False, partner_ids=[self.partner_1.id, self.partner_2.id], email_layout_xmlid="mail.mail_notification_paynow") + self.test_track.message_post( + body="Test body", + email_add_signature=False, + email_layout_xmlid="mail.mail_notification_paynow", + mail_auto_delete=False, + partner_ids=[self.partner_1.id, self.partner_2.id] + ) found_mail = self._new_mails self.assertNotIn(signature, found_mail.body_html) self.assertEqual(found_mail.body_html.count(signature), 0) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index eec84943fa2..39cda308659 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -1018,7 +1018,7 @@ class TestMailHeavyPerformancePost(BaseMailPerformance): parent_id=False, attachments=attachements, attachment_ids=attachement_ids, - add_sign=True, + email_add_signature=True, model_description=False, mail_auto_delete=True ) From bf6c805b66e4524c84868e8fe62ebe85d2221660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 4 Jan 2022 10:31:48 +0000 Subject: [PATCH 13/23] [FIX] mail, *: add signature in email layouts only if requested and set * = account, purchase, sale Purpose of this commit is to add signature only if really asked. A variable is available for that purpose (``email_add_signature``, recently renamed from ``add_sign``). We also correctly check signature is not void or pseudo-void using tool ``is_html_empty``. Indeed editor may generates pseudo-void content like ``


``. As signature is not added when a template is used (see composer code) we ensure it is defined in mail templates used in "Send by email" flows. Indeed we cannot know if a template already has a signature or not. To avoid having twice a signature no signature is added in email notification layout when a template is involved. Continuation of odoo/odoo#76418 . Task-2712450 (Mail/Sale: Improve 'Pay Now' notification template) Part-of: odoo/odoo#82167 --- addons/account/data/mail_template_data.xml | 6 +++--- addons/mail/data/mail_templates_email_layouts.xml | 6 +++--- addons/purchase/data/mail_template_data.xml | 14 ++++++++++++++ addons/sale/data/mail_template_data.xml | 10 +++++++++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/addons/account/data/mail_template_data.xml b/addons/account/data/mail_template_data.xml index 4ec06155817..4a6a17ec2a3 100644 --- a/addons/account/data/mail_template_data.xml +++ b/addons/account/data/mail_template_data.xml @@ -46,7 +46,7 @@

Do not hesitate to contact us if you have any questions. - +
--
Mitchell Admin @@ -75,7 +75,7 @@ Do not hesitate to contact us if you have any questions.

Best regards, - +
--
Mitchell Admin @@ -119,7 +119,7 @@ from YourCompany.

Do not hesitate to contact us if you have any questions. - +
--
Mitchell Admin diff --git a/addons/mail/data/mail_templates_email_layouts.xml b/addons/mail/data/mail_templates_email_layouts.xml index 75a6b3ba426..c339ac8889c 100644 --- a/addons/mail/data/mail_templates_email_layouts.xml +++ b/addons/mail/data/mail_templates_email_layouts.xml @@ -49,7 +49,7 @@
  • : ->
    • -
    +

    Sent @@ -177,12 +177,12 @@

    - +
    Best regards,
    &nbsp;
    -
    +
    diff --git a/addons/purchase/data/mail_template_data.xml b/addons/purchase/data/mail_template_data.xml index bdd1a4fc9cf..31573688f43 100644 --- a/addons/purchase/data/mail_template_data.xml +++ b/addons/purchase/data/mail_template_data.xml @@ -23,6 +23,10 @@ If you have any questions, please do not hesitate to contact us.

    Best regards, + +
    + --
    Mitchell Admin     +

    @@ -56,6 +60,11 @@

    Could you please acknowledge the receipt of this order? + +
    + --
    Mitchell Admin     +     +

    @@ -90,6 +99,11 @@ undefined.     Could you please confirm it will be delivered on time? + +
    + --
    Mitchell Admin     +     +

    diff --git a/addons/sale/data/mail_template_data.xml b/addons/sale/data/mail_template_data.xml index 481187503a8..d84e9f8239c 100644 --- a/addons/sale/data/mail_template_data.xml +++ b/addons/sale/data/mail_template_data.xml @@ -30,7 +30,11 @@

    Do not hesitate to contact us if you have any questions. -
    + +
    + --
    Mitchell Admin     +     +

    @@ -65,6 +69,10 @@

    Do not hesitate to contact us if you have any questions. + +
    + --
    Mitchell Admin     +

    From 8ebef14256fea1da3d1ec6670af683250b1dc4cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 7 Jan 2022 16:22:07 +0000 Subject: [PATCH 14/23] [REF] mail: remove unnecessary layout field on composer This field is a duplicate of email_layout_xmlid and is actually never used. Followup of odoo/odoo#76418 . Task-2732660 (Mail: Remove duplicate layout field on composer) Part-of: odoo/odoo#82167 --- addons/mail/wizard/mail_compose_message.py | 1 - 1 file changed, 1 deletion(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index af68dc0bf28..2c7499994bc 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -100,7 +100,6 @@ class MailComposer(models.TransientModel): 'ir.attachment', 'mail_compose_message_ir_attachments_rel', 'wizard_id', 'attachment_id', 'Attachments') email_layout_xmlid = fields.Char('Email Notification Layout', copy=False) - layout = fields.Char('Layout', copy=False) # xml id of layout email_add_signature = fields.Boolean(default=True) # origin email_from = fields.Char('From', help="Email address of the sender. This field is set when no matching partner is found and replaces the author_id field in the chatter.") From 75979fccdff49723314ad76d576e65df570de740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 3 Jan 2022 14:01:09 +0000 Subject: [PATCH 15/23] [REF] mail, *: improve 'pay now' notification template * = sale, purchase PURPOSE Purpose of this commit is to improve the 'Pay Now' notification template used notably when using the "Send by email" button on * invoices * sale orders * RFQ and purchase orders SPECIFICATIONS Global specifications * remove gray background that is around the white content (aka have an email with an uniform white background); * move button on top of email like other notification templates (top-left and company logo is top-right); * fix various small wording issues; * fix signature usage; Technical specifications Remove custom definition of access links and labels in 'Pay Now' notification template (``mail_notification_paynow``). It is now done at model level through the ``_notify_get_groups`` that is generic to notification emails. This allows to remove QWeb override in purchase and sale notably. Displaying access links is now controller by the ``has_button_access`` value. Model computes links, access and labels while view only displays what is requested. That way any template can use those values instead of being defined in a template subject to user changes. Sale / Purchase Overrides of those modules is not necessary anymore since button labelling and URLs are managed at model level. Purchase "specific" buttons for Accept / Update dates are now email layout actions, like used in other modules like HR or Project. Continuation of odoo/odoo#76418 . Task-2712450 (Mail/Sale: Improve 'Pay Now' notification template) Part-of: odoo/odoo#82167 --- .../data/mail_templates_email_layouts.xml | 52 +++++++++---------- addons/portal/models/portal_mixin.py | 14 +++-- addons/purchase/data/mail_templates.xml | 35 ------------- addons/purchase/models/purchase.py | 21 ++++++++ addons/sale/__manifest__.py | 1 - addons/sale/data/mail_templates.xml | 27 ---------- addons/sale/models/sale_order.py | 26 ++++++++++ addons/test_mail/models/test_mail_models.py | 5 -- .../tests/test_website_sale_mail.py | 2 + 9 files changed, 84 insertions(+), 99 deletions(-) delete mode 100644 addons/sale/data/mail_templates.xml diff --git a/addons/mail/data/mail_templates_email_layouts.xml b/addons/mail/data/mail_templates_email_layouts.xml index c339ac8889c..e7e5ca38898 100644 --- a/addons/mail/data/mail_templates_email_layouts.xml +++ b/addons/mail/data/mail_templates_email_layouts.xml @@ -131,33 +131,37 @@ - +