diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index db90f984acd..494ffe94c79 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -49,7 +49,7 @@ EMPTY = object() class AccountMove(models.Model): _name = "account.move" - _inherit = ['portal.mixin', 'mail.thread', 'mail.activity.mixin', 'sequence.mixin'] + _inherit = ['portal.mixin', 'mail.thread.main.attachment', 'mail.activity.mixin', 'sequence.mixin'] _description = "Journal Entry" _order = 'date desc, name desc, id desc' _mail_post_access = 'read' diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index 20eb353866c..c8634590845 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -7,7 +7,7 @@ from odoo.tools.misc import format_date, formatLang class AccountPayment(models.Model): _name = "account.payment" _inherits = {'account.move': 'move_id'} - _inherit = ['mail.thread', 'mail.activity.mixin'] + _inherit = ['mail.thread.main.attachment', 'mail.activity.mixin'] _description = "Payments" _order = "date desc, name desc" _check_company_auto = True diff --git a/addons/crm/data/crm_lead_demo.xml b/addons/crm/data/crm_lead_demo.xml index 8cbc3c03a4a..31385b1349d 100644 --- a/addons/crm/data/crm_lead_demo.xml +++ b/addons/crm/data/crm_lead_demo.xml @@ -444,10 +444,6 @@ Andrew

]]> - - - - Re: Plan to buy RedHat servers crm.lead diff --git a/addons/hr/models/hr_employee.py b/addons/hr/models/hr_employee.py index a851e18d937..81c807f801a 100644 --- a/addons/hr/models/hr_employee.py +++ b/addons/hr/models/hr_employee.py @@ -26,7 +26,7 @@ class HrEmployeePrivate(models.Model): _name = "hr.employee" _description = "Employee" _order = 'name' - _inherit = ['hr.employee.base', 'mail.thread', 'mail.activity.mixin', 'resource.mixin', 'avatar.mixin'] + _inherit = ['hr.employee.base', 'mail.thread.main.attachment', 'mail.activity.mixin', 'resource.mixin', 'avatar.mixin'] _mail_post_access = 'read' # resource and user diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py index 2910a4db5cc..bdb4654c6a5 100644 --- a/addons/hr_expense/models/hr_expense.py +++ b/addons/hr_expense/models/hr_expense.py @@ -15,7 +15,7 @@ from odoo.tools import email_split, float_repr, float_compare, is_html_empty class HrExpense(models.Model): _name = "hr.expense" - _inherit = ['mail.thread', 'mail.activity.mixin', 'analytic.mixin'] + _inherit = ['mail.thread.main.attachment', 'mail.activity.mixin', 'analytic.mixin'] _description = "Expense" _order = "date desc, id desc" _check_company_auto = True @@ -784,7 +784,7 @@ class HrExpenseSheet(models.Model): ================================================================================= """ _name = "hr.expense.sheet" - _inherit = ['mail.thread', 'mail.activity.mixin'] + _inherit = ['mail.thread.main.attachment', 'mail.activity.mixin'] _description = "Expense Report" _order = "accounting_date desc, id desc" _check_company_auto = True diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index a03bc41a9f7..9612ed30749 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -72,7 +72,7 @@ class HolidaysRequest(models.Model): _name = "hr.leave" _description = "Time Off" _order = "date_from desc" - _inherit = ['mail.thread', 'mail.activity.mixin'] + _inherit = ['mail.thread.main.attachment', 'mail.activity.mixin'] _mail_post_access = 'read' @api.model diff --git a/addons/hr_recruitment/models/hr_applicant.py b/addons/hr_recruitment/models/hr_applicant.py index 0a091639ba7..e9730b74a46 100644 --- a/addons/hr_recruitment/models/hr_applicant.py +++ b/addons/hr_recruitment/models/hr_applicant.py @@ -22,7 +22,7 @@ class Applicant(models.Model): _name = "hr.applicant" _description = "Applicant" _order = "priority desc, id desc" - _inherit = ['mail.thread.cc', 'mail.activity.mixin', 'utm.mixin'] + _inherit = ['mail.thread.cc', 'mail.thread.main.attachment', 'mail.activity.mixin', 'utm.mixin'] _mailing_enabled = True _primary_email = 'email_from' diff --git a/addons/mail/models/__init__.py b/addons/mail/models/__init__.py index 03bcd6eb3c1..aebd5ca5fbe 100644 --- a/addons/mail/models/__init__.py +++ b/addons/mail/models/__init__.py @@ -13,6 +13,7 @@ from . import mail_composer_mixin from . import mail_thread from . import mail_thread_blacklist from . import mail_thread_cc +from . import mail_thread_main_attachment from . import template_reset_mixin # mail models diff --git a/addons/mail/models/ir_attachment.py b/addons/mail/models/ir_attachment.py index 277d7c35004..487aef52fb7 100644 --- a/addons/mail/models/ir_attachment.py +++ b/addons/mail/models/ir_attachment.py @@ -1,10 +1,13 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import contextlib + from odoo import models from odoo.exceptions import AccessError from odoo.http import request + class IrAttachment(models.Model): _inherit = 'ir.attachment' @@ -18,23 +21,22 @@ class IrAttachment(models.Model): def register_as_main_attachment(self, force=True): """ Registers this attachment as the main one of the model it is attached to. + + :param bool force: if set, the method always updates the existing main attachment + otherwise it only sets the main attachment if there is none. """ self.ensure_one() - if not self.res_model: + if not self.res_model or not self.res_id: return related_record = self.env[self.res_model].browse(self.res_id) - if not related_record.check_access_rights('write', raise_exception=False): + if not related_record or \ + not related_record.check_access_rights('write', raise_exception=False) or \ + not hasattr(related_record, 'message_main_attachment_id'): return - # message_main_attachment_id field can be empty, that's why we compare to False; - # we are just checking that it exists on the model before writing it - if related_record and hasattr(related_record, 'message_main_attachment_id'): - if force or not related_record.message_main_attachment_id: - #Ignore AccessError, if you don't have access to modify the document - #Just don't set the value - try: - related_record.message_main_attachment_id = self - except AccessError: - pass + + if force or not related_record.message_main_attachment_id: + with contextlib.suppress(AccessError): + related_record.message_main_attachment_id = self def _delete_and_notify(self): for attachment in self: diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index f9d3da88868..2b3ed38fc6b 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -107,7 +107,6 @@ class MailThread(models.AbstractModel): 'Number of errors', compute='_compute_message_has_error', help="Number of messages with delivery error") message_attachment_count = fields.Integer('Attachment Count', compute='_compute_message_attachment_count', groups="base.group_user") - message_main_attachment_id = fields.Many2one(string="Main Attachment", comodel_name='ir.attachment', copy=False) @api.depends('message_follower_ids') def _compute_message_partner_ids(self): @@ -1971,11 +1970,6 @@ class MailThread(models.AbstractModel): """ Hook to add custom behavior after having posted the message. Both message and computed value are given, to try to lessen query count by using already-computed values instead of having to rebrowse things. """ - # Set main attachment field if necessary - self._message_set_main_attachment_id([ - attachment_command[1] - for attachment_command in (msg_values['attachment_ids'] or []) - ]) def _message_mail_after_hook(self, mails): """ Hook to add custom behavior after having sent an mass mailing. @@ -1983,16 +1977,6 @@ class MailThread(models.AbstractModel): :param mail.mail mails: mail.mail records about to be sent""" return - def _message_set_main_attachment_id(self, attachment_ids): - if self._name == 'mail.thread': - return - if attachment_ids and not self.message_main_attachment_id: - all_attachments = self.env['ir.attachment'].browse(attachment_ids) - prioritary_attachments = all_attachments.filtered(lambda x: x.mimetype.endswith('pdf')) \ - or all_attachments.filtered(lambda x: x.mimetype.startswith('image')) \ - or all_attachments - self.sudo().with_context(tracking_disable=True).message_main_attachment_id = prioritary_attachments[0].id - def _process_attachments_for_post(self, attachments, attachment_ids, message_values): """ Preprocess attachments for MailTread.message_post() or MailMail.create(). Purpose is to @@ -3797,7 +3781,6 @@ class MailThread(models.AbstractModel): res['activities'] = self.activity_ids.activity_format() if 'attachments' in request_list: res['attachments'] = self._get_mail_thread_data_attachments()._attachment_format() - res['mainAttachment'] = {'id': self.message_main_attachment_id.id} if self.message_main_attachment_id else [('clear',)] if 'followers' in request_list: res['followers'] = [{ 'id': follower.id, diff --git a/addons/mail/models/mail_thread_main_attachment.py b/addons/mail/models/mail_thread_main_attachment.py new file mode 100644 index 00000000000..82ab9b7306f --- /dev/null +++ b/addons/mail/models/mail_thread_main_attachment.py @@ -0,0 +1,36 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import fields, models + + +class MailMainAttachmentMixin(models.AbstractModel): + """ Mixin that adds main attachment support to the MailThread class. """ + + _name = 'mail.thread.main.attachment' + _inherit = 'mail.thread' + _description = 'Mail Main Attachment management' + + message_main_attachment_id = fields.Many2one(string="Main Attachment", comodel_name='ir.attachment', copy=False) + + def _message_post_after_hook(self, message, msg_values): + """ Set main attachment field if necessary """ + super()._message_post_after_hook(message, msg_values) + self._message_set_main_attachment_id([ + attachment_command[1] + for attachment_command in (msg_values['attachment_ids'] or []) + ]) + + def _message_set_main_attachment_id(self, attachment_ids): + if attachment_ids and not self.message_main_attachment_id: + # Assign one of the attachments as the main according to the following priority: pdf, image, other types. + self.sudo().with_context(tracking_disable=True).message_main_attachment_id = max( + self.env['ir.attachment'].browse(attachment_ids), + key=lambda r: (r.mimetype.endswith('pdf'), r.mimetype.startswith('image')) + ).id + + def _get_mail_thread_data(self, request_list): + res = super()._get_mail_thread_data(request_list) + if 'attachments' in request_list: + res['mainAttachment'] = {'id': self.message_main_attachment_id.id} if self.message_main_attachment_id else [('clear',)] + return res diff --git a/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js b/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js index 7b94b9f2b46..71d39bb58d9 100644 --- a/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js +++ b/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js @@ -398,9 +398,12 @@ patch(MockServer.prototype, "mail/controllers/discuss", { res["attachments"] = this._mockIrAttachment_attachmentFormat( attachments.map((attachment) => attachment.id) ); - res["mainAttachment"] = thread.message_main_attachment_id - ? { id: thread.message_main_attachment_id[0] } - : [["clear"]]; + // Specific implementation of mail.thread.main.attachment + if (this.models[thread_model].fields["message_main_attachment_id"]) { + res["mainAttachment"] = thread.message_main_attachment_id + ? { id: thread.message_main_attachment_id[0] } + : [["clear"]]; + } } if (request_list.includes("followers")) { const followers = this.pyEnv["mail.followers"].searchRead([ diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py index 62f2494e1c4..d479cea1ce0 100644 --- a/addons/test_mail/models/test_mail_models.py +++ b/addons/test_mail/models/test_mail_models.py @@ -32,6 +32,12 @@ class MailTestSimple(models.Model): return headers +class MailTestSimpleWithMainAttachment(models.Model): + _description = 'Simple Chatter Model With Main Attachment Management' + _name = 'mail.test.simple.main.attachment' + _inherit = ['mail.test.simple', 'mail.thread.main.attachment'] + + class MailTestGateway(models.Model): """ A very simple model only inheriting from mail.thread to test pure mass mailing features and base performances. """ diff --git a/addons/test_mail/security/ir.model.access.csv b/addons/test_mail/security/ir.model.access.csv index 0c36bef86ce..94d31f29d8a 100644 --- a/addons/test_mail/security/ir.model.access.csv +++ b/addons/test_mail/security/ir.model.access.csv @@ -3,6 +3,8 @@ access_mail_performance_thread,access_mail_performance_thread,model_mail_perform access_mail_performance_tracking_user,mail.performance.tracking,model_mail_performance_tracking,base.group_user,1,1,1,1 access_mail_test_simple_portal,mail.test.simple.portal,model_mail_test_simple,base.group_portal,1,0,0,0 access_mail_test_simple_user,mail.test.simple.user,model_mail_test_simple,base.group_user,1,1,1,1 +access_mail_test_simple_main_attachment_portal,mail.test.simple.main.attachment.portal,model_mail_test_simple_main_attachment,base.group_portal,1,0,0,0 +access_mail_test_simple_main_attachment_user,mail.test.simple.main.attachment.user,model_mail_test_simple_main_attachment,base.group_user,1,1,1,1 access_mail_test_gateway_portal,mail.test.gateway.portal,model_mail_test_gateway,base.group_portal,1,0,0,0 access_mail_test_gateway_user,mail.test.gateway.user,model_mail_test_gateway,base.group_user,1,1,1,1 access_mail_test_gateway_groups_portal,mail.test.gateway.groups.portal,model_mail_test_gateway_groups,base.group_portal,1,0,0,0 diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 881d60c2199..7d90526ceea 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -919,7 +919,13 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): @users('employee') @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.addons.mail.models.mail_message_schedule') - def test_message_post_w_attachments(self): + def test_message_post_w_attachments_on_main_attachment_model(self): + """ Test posting a message with attachments on a model inheriting from + the mixin mail.thread.main.attachment. + + As the mixin inherits from mail.thread, we test mainly features from + mail.thread but with the ones added of the main attachment mixin. + """ _attachments = [ ('List1', b'My first attachment'), ('List2', b'My second attachment'), @@ -929,7 +935,12 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): ) _attachment_records[1].write({'mimetype': 'image/png'}) # to test message_main_attachment heuristic - test_record = self.env['mail.test.simple'].browse(self.test_record.ids) + test_record = self.env['mail.test.simple.main.attachment'].with_context(self._test_context).create({ + 'name': 'Test', + 'email_from': 'ignasse@example.com', + }) + self._reset_mail_context(test_record) + self.test_message.model = test_record._name self.assertFalse(test_record.message_main_attachment_id) with self.mock_mail_gateway(): @@ -949,8 +960,8 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): # message attachments self.assertEqual(len(msg.attachment_ids), 5) - self.assertEqual(set(msg.attachment_ids.mapped('res_model')), set([self.test_record._name])) - self.assertEqual(set(msg.attachment_ids.mapped('res_id')), set([self.test_record.id])) + self.assertEqual(set(msg.attachment_ids.mapped('res_model')), {test_record._name}) + self.assertEqual(set(msg.attachment_ids.mapped('res_id')), {test_record.id}) self.assertEqual(set(base64.b64decode(x) for x in msg.attachment_ids.mapped('datas')), set([b'AttContent_00', b'AttContent_01', b'AttContent_02', _attachments[0][1], _attachments[1][1]])) self.assertTrue(set(_attachment_records.ids).issubset(msg.attachment_ids.ids),