diff --git a/addons/im_livechat/models/mail_channel.py b/addons/im_livechat/models/mail_channel.py index 26d3e4c70e5..79bb445f835 100644 --- a/addons/im_livechat/models/mail_channel.py +++ b/addons/im_livechat/models/mail_channel.py @@ -148,7 +148,7 @@ class MailChannel(models.Model): 'id': message.id, 'body': message.body, }) - super()._message_update_content_after_hook(message=message) + return super()._message_update_content_after_hook(message=message) def _get_visitor_leave_message(self, operator=False, cancel=False): return _('Visitor has left the conversation.') diff --git a/addons/mail/controllers/discuss.py b/addons/mail/controllers/discuss.py index 7a748fea508..ebeacc8a6a0 100644 --- a/addons/mail/controllers/discuss.py +++ b/addons/mail/controllers/discuss.py @@ -241,7 +241,13 @@ class DiscussController(http.Controller): message_sudo = guest.env['mail.message'].browse(message_id).sudo().exists() if not message_sudo.is_current_user_or_guest_author and not guest.env.user._is_admin(): raise NotFound() - message_sudo._update_content(body=body, attachment_ids=attachment_ids) + if not message_sudo.model or not message_sudo.res_id: + raise NotFound() + request.env[message_sudo.model].browse([message_sudo.res_id])._message_update_content( + message_sudo, + body, + attachment_ids=attachment_ids + ) return { 'id': message_sudo.id, 'body': message_sudo.body, diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 5f86648fcf7..a8bb8e81e6c 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -808,25 +808,6 @@ class Message(models.Model): reaction.unlink() self.env[self.model].browse(self.res_id)._message_remove_reaction_after_hook(message=self, content=content) - def _update_content(self, body, attachment_ids): - self.ensure_one() - thread = self.env[self.model].browse(self.res_id) - thread._check_can_update_message_content(self) - self.body = body - if not attachment_ids: - self.attachment_ids._delete_and_notify() - else: - message_values = { - 'model': self.model, - 'body': body, - 'res_id': self.res_id, - } - attachement_values = thread._message_post_process_attachments([], attachment_ids, message_values) - self.update(attachement_values) - # Cleanup related message data if the message is empty - self.sudo()._filter_empty()._cleanup_side_records() - thread._message_update_content_after_hook(self) - # ------------------------------------------------------ # MESSAGE READ / FETCH / FAILURE API # ------------------------------------------------------ diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 661742115a2..c62648d5255 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -433,14 +433,20 @@ class MailThread(models.AbstractModel): return self.with_context(lang=self.env.user.lang) return self - def _check_can_update_message_content(self, message): - """" Checks that the current user can update the content of the message. """ + def _check_can_update_message_content(self, messages): + """" Checks that the current user can update the content of the message. + Current heuristic is + + * limited to note; + * if no tracking; + * only for user generated content; + """ note_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note') - if not message.subtype_id.id == note_id: + if any(message.subtype_id.id != note_id for message in messages): raise exceptions.UserError(_("Only logged notes can have their content updated on model '%s'", self._name)) - if message.tracking_value_ids: + if messages.tracking_value_ids: raise exceptions.UserError(_("Messages with tracking values cannot be modified")) - if not message.message_type == 'comment': + if any(message.message_type != 'comment' for message in messages): raise exceptions.UserError(_("Only messages type comment can have their content updated")) # ------------------------------------------------------ @@ -3122,8 +3128,52 @@ class MailThread(models.AbstractModel): msg_not_comment.write(msg_vals) return True + def _message_update_content(self, message, body, attachment_ids=None, + strict=True): + """ Update message content. Currently does not support attachments + specific code (see ``_message_post_process_attachments``), to be added + when necessary. + + Private method to use for tooling, do not expose to interface as editing + messages should be avoided at all costs (think of: notifications already + sent, ...). + + :param message: message to update, should be linked to self through + model and res_id; + :param str body: new body (None to skip its update); + :param list attachment_ids: list of new attachments IDs, replacing old one (None + to skip its update); + :param bool strict: whether to check for allowance before updating + content. This should be skipped only when really necessary as it + creates issues with already-sent notifications, lack of content + tracking, ... + """ + self.ensure_one() + if strict: + self._check_can_update_message_content(message.sudo()) + + msg_values = {'body': body} if body is not None else {} + if attachment_ids: + msg_values.update( + self._message_post_process_attachments([], attachment_ids, { + 'body': body, + 'model': self._name, + 'res_id': self.id, + }) + ) + elif attachment_ids is not None: # None means "no update" + message.attachment_ids._delete_and_notify() + if msg_values: + message.write(msg_values) + + # cleanup related message data if the message is empty + message.sudo()._filter_empty()._cleanup_side_records() + + return self._message_update_content_after_hook(message) + def _message_update_content_after_hook(self, message): """ Hook to add custom behavior after having updated the message content. """ + return True # ------------------------------------------------------ # CONTROLLERS diff --git a/addons/test_mail/tests/test_mail_message.py b/addons/test_mail/tests/test_mail_message.py index d43489c58f6..5de0cdc9de7 100644 --- a/addons/test_mail/tests/test_mail_message.py +++ b/addons/test_mail/tests/test_mail_message.py @@ -65,13 +65,13 @@ class TestMessageValues(TestMailCommon): self.assertFalse(message.sudo().tracking_value_ids) # Reset body case - message._update_content('


', attachment_ids=message.attachment_ids.ids) + record._message_update_content(message, '


', attachment_ids=message.attachment_ids.ids) self.assertTrue(is_html_empty(message.body)) self.assertFalse(message.sudo()._filter_empty(), 'Still having attachments') # Subtype content note_subtype.write({'description': 'Very important discussions'}) - message._update_content('', None) + record._message_update_content(message, '', []) self.assertFalse(message.attachment_ids) self.assertEqual(message.notified_partner_ids, self.partner_admin) self.assertEqual(message.starred_partner_ids, self.partner_admin) @@ -80,7 +80,7 @@ class TestMessageValues(TestMailCommon): # Completely void now note_subtype.write({'description': ''}) self.assertEqual(message.sudo()._filter_empty(), message) - message._update_content('', None) + record._message_update_content(message, '', []) self.assertFalse(message.notified_partner_ids) self.assertFalse(message.starred_partner_ids) @@ -93,7 +93,7 @@ class TestMessageValues(TestMailCommon): self.assertFalse(tracking_message.subtype_id.description) self.assertFalse(tracking_message.sudo()._filter_empty(), 'Has tracking values') with self.assertRaises(UserError, msg='Tracking values prevent from updating content'): - tracking_message._update_content('', None) + record._message_update_content(tracking_message, '', []) @mute_logger('odoo.models.unlink') def test_mail_message_format(self): diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py index 7ec34dcc778..0a0cb046dad 100644 --- a/addons/test_mail/tests/test_mail_thread_internals.py +++ b/addons/test_mail/tests/test_mail_thread_internals.py @@ -12,6 +12,94 @@ from odoo.tests.common import tagged, HttpCase, users from odoo.tools import mute_logger +@tagged('mail_thread') +class TestAPI(TestMailCommon, TestRecipients): + + @classmethod + def setUpClass(cls): + super(TestAPI, cls).setUpClass() + cls.ticket_record = cls.env['mail.test.ticket'].with_context(cls._test_context).create({ + 'email_from': '"Paulette Vachette" ', + 'name': 'Test', + 'user_id': cls.user_employee.id, + }) + + @mute_logger('openerp.addons.mail.models.mail_mail') + @users('employee') + def test_message_update_content(self): + """ Test updating message content. """ + ticket_record = self.ticket_record.with_env(self.env) + attachments = self.env['ir.attachment'].create( + self._generate_attachments_data(2, 'mail.compose.message', 0) + ) + + # post a note + message = ticket_record.message_post( + attachment_ids=attachments.ids, + body="

Initial Body

", + message_type="comment", + partner_ids=self.partner_1.ids, + ) + self.assertEqual(message.attachment_ids, attachments) + self.assertEqual(set(message.mapped('attachment_ids.res_id')), set(ticket_record.ids)) + self.assertEqual(set(message.mapped('attachment_ids.res_model')), set([ticket_record._name])) + self.assertEqual(message.body, "

Initial Body

") + self.assertEqual(message.subtype_id, self.env.ref('mail.mt_note')) + + # update the content with new attachments + new_attachments = self.env['ir.attachment'].create( + self._generate_attachments_data(2, 'mail.compose.message', 0) + ) + ticket_record._message_update_content( + message, "

New Body

", + attachment_ids=new_attachments.ids + ) + self.assertEqual(message.attachment_ids, attachments + new_attachments) + self.assertEqual(set(message.mapped('attachment_ids.res_id')), set(ticket_record.ids)) + self.assertEqual(set(message.mapped('attachment_ids.res_model')), set([ticket_record._name])) + self.assertEqual(message.body, "

New Body

") + + # void attachments + ticket_record._message_update_content( + message, "

Another Body, void attachments

", + attachment_ids=[] + ) + self.assertFalse(message.attachment_ids) + self.assertFalse((attachments + new_attachments).exists()) + self.assertEqual(message.body, "

Another Body, void attachments

") + + @mute_logger('openerp.addons.mail.models.mail_mail') + @users('employee') + def test_message_update_content_check(self): + """ Test cases where updating content should be prevented """ + ticket_record = self.ticket_record.with_env(self.env) + + # cannot edit user comments (subtype) + message = ticket_record.message_post( + body="

Initial Body

", + message_type="comment", + subtype_id=self.env.ref('mail.mt_comment').id, + ) + with self.assertRaises(exceptions.UserError): + ticket_record._message_update_content( + message, "

New Body

" + ) + + message.sudo().write({'subtype_id': self.env.ref('mail.mt_note')}) + ticket_record._message_update_content( + message, "

New Body

" + ) + + # cannot edit notifications + for message_type in ['notification', 'user_notification', 'email']: + message.sudo().write({'message_type': message_type}) + with self.assertRaises(exceptions.UserError): + ticket_record._message_update_content( + message, "

New Body

" + ) + + +@tagged('mail_thread') class TestChatterTweaks(TestMailCommon, TestRecipients): @classmethod