pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] {mass_mailing_,}sms: delay sms deletion

Browse Source 
Also impacts test_mail_sms.

Purpose: let a cron do delete records to end the
sending transaction sooner.

Removing the foreign key between sms_sms and mailing_trace
is necessary as traces may have to be updated due to
delivery reports.
This is why we do it here and not with notifications
because only traces will trigger updates of a possibly
massive number of records and repeated concurrent updates.

We also replace the now obsolete "sms_sms_X" prefix as there
are not many "sms_id" to distinguish from.

Task-2560666

Part-of: odoo/odoo#133392
This commit is contained in:
Florian Charlier
2023-10-24 12:38:13 +00:00
parent bd5a68a1a7
commit e8878a8b4c
15 changed files with 82 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mass_mailing_sms/controllers/main.py
+4 -4
View File
@@ -89,10 +89,10 @@ class MailingSMSController(http.Controller):
'unsubscribe_error': unsubscribe_error,
})
@http.route('/r/<string:code>/s/<int:sms_sms_id>', type='http', auth="public")
def sms_short_link_redirect(self, code, sms_sms_id, **post):
if sms_sms_id:
trace_id = request.env['mailing.trace'].sudo().search([('sms_sms_id_int', '=', int(sms_sms_id))]).id
@http.route('/r/<string:code>/s/<int:sms_id_int>', type='http', auth="public")
def sms_short_link_redirect(self, code, sms_id_int, **post):
if sms_id_int:
trace_id = request.env['mailing.trace'].sudo().search([('sms_id_int', '=', int(sms_id_int))]).id
else:
trace_id = False
addons/mass_mailing_sms/models/mailing_trace.py
+15 -5
View File
@@ -16,9 +16,9 @@ class MailingTrace(models.Model):
trace_type = fields.Selection(selection_add=[
('sms', 'SMS')
], ondelete={'sms': 'set default'})
sms_sms_id = fields.Many2one('sms.sms', string='SMS', index='btree_not_null', ondelete='set null')
sms_sms_id_int = fields.Integer(
string='SMS ID (tech)',
sms_id = fields.Many2one('sms.sms', string='SMS', store=False, compute='_compute_sms_id')
sms_id_int = fields.Integer(
string='SMS ID',
index='btree_not_null'
# Integer because the related sms.sms can be deleted separately from its statistics.
# However, the ID is needed for several action and controllers.
@@ -46,11 +46,21 @@ class MailingTrace(models.Model):
('sms_rejected', 'Rejected'),
])
@api.depends('sms_id_int', 'trace_type')
def _compute_sms_id(self):
self.sms_id = False
sms_traces = self.filtered(lambda t: t.trace_type == 'sms' and bool(t.sms_id_int))
if not sms_traces:
return
existing_sms_ids = self.env['sms.sms'].sudo().search([
('id', 'in', sms_traces.mapped('sms_id_int')), ('to_delete', '!=', True)
]).ids
for sms_trace in sms_traces.filtered(lambda n: n.sms_id_int in set(existing_sms_ids)):
sms_trace.sms_id = sms_trace.sms_id_int
@api.model_create_multi
def create(self, values_list):
for values in values_list:
if 'sms_sms_id' in values:
values['sms_sms_id_int'] = values['sms_sms_id']
if values.get('trace_type') == 'sms' and not values.get('sms_code'):
values['sms_code'] = self._get_random_code()
return super(MailingTrace, self).create(values_list)
addons/mass_mailing_sms/models/sms_sms.py
+4 -1
View File
@@ -10,7 +10,10 @@ class SmsSms(models.Model):
_inherit = ['sms.sms']
mailing_id = fields.Many2one('mailing.mailing', string='Mass Mailing')
mailing_trace_ids = fields.One2many('mailing.trace', 'sms_sms_id', string='Statistics')
# Linking to another field than the comodel id allows to use the ORM to create
# "linked" records (see _prepare_sms_values) without adding a foreign key.
# See commit message for why this is useful.
mailing_trace_ids = fields.One2many('mailing.trace', 'sms_id_int', string='Statistics')
def _update_body_short_links(self):
""" Override to tweak shortened URLs by adding statistics ids, allowing to
addons/mass_mailing_sms/tests/common.py
+3 -3
View File
@@ -90,7 +90,7 @@ class MassSMSCase(SMSCase, MockLinkTracker):
)
self.assertTrue(len(trace) == 1,
'SMS: found %s notification for number %s, (status: %s) (1 expected)' % (len(trace), number, status))
self.assertTrue(bool(trace.sms_sms_id_int))
self.assertTrue(bool(trace.sms_id_int))
if check_sms:
if status in {'process', 'pending', 'sent'}:
@@ -156,8 +156,8 @@ class MassSMSCase(SMSCase, MockLinkTracker):
if '/r/' in url: # shortened link, like 'http://localhost:8069/r/LBG/s/53'
parsed_url = werkzeug.urls.url_parse(url)
path_items = parsed_url.path.split('/')
code, sms_sms_id = path_items[2], int(path_items[4])
trace_id = self.env['mailing.trace'].sudo().search([('sms_sms_id_int', '=', sms_sms_id)]).id
code, sms_id_int = path_items[2], int(path_items[4])
trace_id = self.env['mailing.trace'].sudo().search([('sms_id_int', '=', sms_id_int)]).id
self.env['link.tracker.click'].sudo().add_click(
code,
addons/mass_mailing_sms/views/mailing_trace_views.xml
+5 -5
View File
@@ -6,8 +6,8 @@
<field name="inherit_id" ref="mass_mailing.mailing_trace_view_search"/>
<field name="arch" type="xml">
<xpath expr="//field[@name='email']" position="after">
<field name="sms_sms_id_int"/>
<field name="sms_sms_id"/>
<field name="sms_id_int"/>
<field name="sms_id"/>
<field name="sms_number"/>
</xpath>
</field>
@@ -68,7 +68,7 @@
<field name="sms_number" invisible="trace_type != 'sms'"/>
</xpath>
<xpath expr="//field[@name='message_id']" position="after">
<field name="sms_sms_id_int" string="SMS ID"
<field name="sms_id_int" string="SMS ID"
invisible="trace_type != 'sms'"
groups="base.group_no_one"/>
<field name="sms_code" invisible="trace_type != 'sms'"
@@ -118,14 +118,14 @@
<field name="trace_type" invisible="1"/>
<field name="sms_number"/>
<field name="mass_mailing_id"/>
<field name="sms_sms_id_int" string="SMS ID" groups="base.group_no_one"/>
<field name="sms_id_int" string="SMS ID" groups="base.group_no_one"/>
<field name="sms_code" groups="base.group_no_one"/>
</group>
<group string="Marketing">
<field name="campaign_id" groups="mass_mailing.group_mass_mailing_campaign"/>
<field name="medium_id"/>
<field name="source_id"/>
<field name="sms_sms_id" groups="base.group_no_one"/>
<field name="sms_id" groups="base.group_no_one" invisible="not sms_id"/>
</group>
</group>
</sheet>
addons/sms/controllers/main.py
+1 -1
View File
@@ -36,7 +36,7 @@ class SmsController(Controller):
else:
sms_trackers_sudo._action_update_from_provider_error(iap_status)
all_uuids += uuids
http.request.env['sms.sms'].sudo().search([('uuid', 'in', all_uuids)]).unlink()
request.env['sms.sms'].sudo().search([('uuid', 'in', all_uuids), ('to_delete', '=', False)]).to_delete = True
return 'OK'
@staticmethod
addons/sms/models/mail_notification.py
+16 -2
View File
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from odoo import fields, models
from odoo import api, fields, models
class MailNotification(models.Model):
@@ -10,7 +10,9 @@ class MailNotification(models.Model):
notification_type = fields.Selection(selection_add=[
('sms', 'SMS')
], ondelete={'sms': 'cascade'})
sms_id = fields.Many2one('sms.sms', string='SMS', index='btree_not_null', ondelete='set null')
sms_id_int = fields.Integer('SMS ID', index='btree_not_null')
# Used to give links on form view without foreign key. In most cases, you'd want to use sms_id_int or sms_tracker_ids.sms_uuid.
sms_id = fields.Many2one('sms.sms', string='SMS', store=False, compute='_compute_sms_id')
sms_tracker_ids = fields.One2many('sms.tracker', 'mail_notification_id', string="SMS Trackers")
sms_number = fields.Char('SMS Number')
failure_type = fields.Selection(selection_add=[
@@ -28,3 +30,15 @@ class MailNotification(models.Model):
('sms_not_delivered', 'Not Delivered'),
('sms_rejected', 'Rejected'),
])
@api.depends('sms_id_int', 'notification_type')
def _compute_sms_id(self):
self.sms_id = False
sms_notifications = self.filtered(lambda n: n.notification_type == 'sms' and bool(n.sms_id_int))
if not sms_notifications:
return
existing_sms_ids = self.env['sms.sms'].sudo().search([
('id', 'in', sms_notifications.mapped('sms_id_int')), ('to_delete', '!=', True)
]).ids
for sms_notification in sms_notifications.filtered(lambda n: n.sms_id_int in set(existing_sms_ids)):
sms_notification.sms_id = sms_notification.sms_id_int
addons/sms/models/mail_thread.py
+2 -2
View File
@@ -305,7 +305,7 @@ class MailThread(models.AbstractModel):
'res_partner_id': sms.partner_id.id,
'sms_number': sms.number,
'notification_type': 'sms',
'sms_id': sms.id,
'sms_id_int': sms.id,
'sms_tracker_ids': [Command.create({'sms_uuid': sms.uuid})] if sms.state == 'outgoing' else False,
'is_read': True, # discard Inbox notification
'notification_status': 'ready' if sms.state == 'outgoing' else 'exception',
@@ -323,7 +323,7 @@ class MailThread(models.AbstractModel):
notif.write({
'notification_type': 'sms',
'notification_status': 'ready',
'sms_id': sms.id,
'sms_id_int': sms.id,
'sms_tracker_ids': [Command.create({'sms_uuid': sms.uuid})],
'sms_number': sms.number,
})
addons/sms/models/sms_sms.py
+17 -13
View File
@@ -63,6 +63,10 @@ class SmsSms(models.Model):
('sms_optout', 'Opted Out'),
], copy=False)
sms_tracker_id = fields.Many2one('sms.tracker', string='SMS trackers', compute='_compute_sms_tracker_id')
to_delete = fields.Boolean(
'Marked for deletion', default=False,
help='Will automatically be deleted, while notifications will not be deleted in any case.'
)
_sql_constraints = [
('uuid_unique', 'unique(uuid)', 'UUID must be unique'),
@@ -93,7 +97,7 @@ class SmsSms(models.Model):
:param auto_commit: commit after each batch of SMS;
:param raise_exception: raise if there is an issue contacting IAP;
"""
self = self.filtered(lambda sms: sms.state == 'outgoing')
self = self.filtered(lambda sms: sms.state == 'outgoing' and not sms.to_delete)
for batch_ids in self._split_batch():
self.browse(batch_ids)._send(unlink_failed=unlink_failed, unlink_sent=unlink_sent, raise_exception=raise_exception)
# auto-commit if asked except in testing mode
@@ -101,7 +105,7 @@ class SmsSms(models.Model):
self._cr.commit()
def resend_failed(self):
sms_to_send = self.filtered(lambda sms: sms.state == 'error')
sms_to_send = self.filtered(lambda sms: sms.state == 'error' and not sms.to_delete)
sms_to_send.state = 'outgoing'
notification_title = _('Warning')
notification_type = 'danger'
@@ -135,7 +139,7 @@ class SmsSms(models.Model):
:param list ids: optional list of emails ids to send. If passed no search
is performed, and these ids are used instead.
"""
domain = [('state', '=', 'outgoing')]
domain = [('state', '=', 'outgoing'), ('to_delete', '!=', True)]
filtered_ids = self.search(domain, limit=10000).ids # TDE note: arbitrary limit we might have to update
if ids:
@@ -177,30 +181,30 @@ class SmsSms(models.Model):
results_uuids = [result['uuid'] for result in results]
all_sms_sudo = self.env['sms.sms'].sudo().search([('uuid', 'in', results_uuids)]).with_context(sms_skip_msg_notification=True)
mail_message_ids_sudo = all_sms_sudo.mail_message_id
for iap_state, results_group in tools.groupby(results, key=lambda result: result['state']):
sms_sudo = all_sms_sudo.filtered(lambda s: s.uuid in {result['uuid'] for result in results_group})
if success_state := self.IAP_TO_SMS_STATE_SUCCESS.get(iap_state):
sms_sudo.sms_tracker_id._action_update_from_sms_state(success_state)
if unlink_sent:
sms_sudo.unlink()
else:
sms_sudo.write({'state': success_state, 'failure_type': False})
to_delete = {'to_delete': True} if unlink_sent else {}
sms_sudo.write({'state': success_state, 'failure_type': False, **to_delete})
else:
failure_type = self.IAP_TO_SMS_FAILURE_TYPE.get(iap_state, 'unknown')
if failure_type != 'unknown':
sms_sudo.sms_tracker_id._action_update_from_sms_state('error', failure_type=failure_type)
else:
sms_sudo.sms_tracker_id._action_update_from_provider_error(iap_state)
if unlink_failed:
sms_sudo.unlink()
else:
sms_sudo.write({'state': 'error', 'failure_type': failure_type})
to_delete = {'to_delete': True} if unlink_failed else {}
sms_sudo.write({'state': 'error', 'failure_type': failure_type, **to_delete})
mail_message_ids_sudo._notify_message_notification_update()
all_sms_sudo.mail_message_id._notify_message_notification_update()
def _update_sms_state_and_trackers(self, new_state, failure_type=None):
"""Update sms state update and related tracking records (notifications, traces)."""
self.write({'state': new_state, 'failure_type': failure_type})
self.sms_tracker_id._action_update_from_sms_state(new_state, failure_type=failure_type)
@api.autovacuum
def _gc_device(self):
self._cr.execute("DELETE FROM sms_sms WHERE to_delete = TRUE")
_logger.info("GC'd %d sms marked for deletion", self._cr.rowcount)
addons/sms/tests/common.py
-4
View File
@@ -138,10 +138,6 @@ class SMSCase(MockSMS):
raise NotImplementedError()
return sms
def assertNoSMS(self):
""" Check no sms persisted during mock. """
self.assertEqual(len(self._new_sms.exists()), 0)
def assertSMSIapSent(self, numbers, content=None):
""" Check sent SMS. Order is not checked. Each number should have received
the same content. Useful to check batch sending.
addons/sms/views/sms_sms_views.xml
+3 -1
View File
@@ -6,7 +6,8 @@
<field name="arch" type="xml">
<form string="SMS">
<header>
<button name="send" string="Send Now" type="object" invisible="state != 'outgoing'" class="oe_highlight"/>
<field name="to_delete" invisible="1"/>
<button name="send" string="Send Now" type="object" invisible="state != 'outgoing' or to_delete" class="oe_highlight"/>
<button name="action_set_outgoing" string="Retry" type="object" invisible="state not in ('error', 'canceled')"/>
<button name="action_set_canceled" string="Cancel" type="object" invisible="state not in ('error', 'outgoing')"/>
<field name="state" widget="statusbar" statusbar_visible="outgoing,sent,error,canceled"/>
@@ -62,6 +63,7 @@
<field name="name">SMS</field>
<field name="res_model">sms.sms</field>
<field name="view_mode">tree,form</field>
<field name="domain">[('to_delete', '!=', True)]</field>
</record>
<menuitem id="sms_sms_menu"
addons/test_mail_sms/tests/test_sms_composer.py
+4 -6
View File
@@ -46,9 +46,8 @@ class TestSMSComposerComment(SMSCommon, TestSMSRecipients):
with self.mockSMSGateway(sms_allow_unlink=True):
composer._action_send_sms()
# sms.sms was deleted as successfully sent
self.assertNoSMS()
self.assertSMSIapSent(self.random_numbers_san, self._test_body)
for number in self.random_numbers_san:
self.assertSMS(self.env['res.partner'], number, 'pending', content=self._test_body, fields_values={'to_delete': True})
def test_composer_comment_default(self):
with self.with_user('employee'):
@@ -262,9 +261,8 @@ class TestSMSComposerComment(SMSCommon, TestSMSRecipients):
with self.mockSMSGateway(sms_allow_unlink=True):
composer._action_send_sms()
# sms.sms was deleted as successfully sent
self.assertNoSMS()
self.assertSMSIapSent(self.random_numbers_san, self._test_body)
for number in self.random_numbers_san:
self.assertSMS(self.env['res.partner'], number, 'pending', content=self._test_body, fields_values={'to_delete': True})
def test_composer_sending_with_no_number_field(self):
test_record = self.env['mail.test.sms.partner'].create({'name': 'Test'})
addons/test_mail_sms/tests/test_sms_management.py
+2 -2
View File
@@ -32,7 +32,7 @@ class TestSMSActionsCommon(SMSCommon, TestSMSRecipients):
'author_id': cls.msg.author_id.id,
'mail_message_id': cls.msg.id,
'res_partner_id': cls.partner_1.id,
'sms_id': cls.sms_p1.id,
'sms_id_int': cls.sms_p1.id,
'sms_number': cls.partner_1.mobile,
'sms_tracker_ids': [Command.create({'sms_uuid': cls.sms_p1.uuid})],
'notification_type': 'sms',
@@ -52,7 +52,7 @@ class TestSMSActionsCommon(SMSCommon, TestSMSRecipients):
'author_id': cls.msg.author_id.id,
'mail_message_id': cls.msg.id,
'res_partner_id': cls.partner_2.id,
'sms_id': cls.sms_p2.id,
'sms_id_int': cls.sms_p2.id,
'sms_number': cls.partner_2.mobile,
'sms_tracker_ids': [Command.create({'sms_uuid': cls.sms_p2.uuid})],
'notification_type': 'sms',
addons/test_mail_sms/tests/test_sms_performance.py
+2 -2
View File
@@ -117,7 +117,7 @@ class TestSMSMassPerformance(BaseMailPerformance, sms_common.MockSMS):
'mass_keep_log': False,
})
with self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=55):
with self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=54):
composer.action_send_sms()
@mute_logger('odoo.addons.sms.models.sms_sms')
@@ -133,5 +133,5 @@ class TestSMSMassPerformance(BaseMailPerformance, sms_common.MockSMS):
'mass_keep_log': True,
})
with self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=58):
with self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=57):
composer.action_send_sms()
addons/test_mail_sms/tests/test_sms_sms.py
+4 -4
View File
@@ -46,7 +46,7 @@ class TestSMSPost(SMSCommon, MockLinkTracker):
def test_sms_send_delete_all(self):
with self.mockSMSGateway(sms_allow_unlink=True, sim_error='jsonrpc_exception'):
self.env['sms.sms'].browse(self.sms_all.ids).send(unlink_failed=True, unlink_sent=True, raise_exception=False)
self.assertFalse(len(self.sms_all.exists()))
self.assertFalse(len(self.sms_all.exists().filtered(lambda s: not s.to_delete)))
def test_sms_send_delete_default(self):
""" Test default send behavior: keep failed SMS, remove sent. """
@@ -57,7 +57,7 @@ class TestSMSPost(SMSCommon, MockLinkTracker):
'+32456000044': 'unregistered',
}):
self.env['sms.sms'].browse(self.sms_all.ids).send(raise_exception=False)
remaining = self.sms_all.exists()
remaining = self.sms_all.exists().filtered(lambda s: not s.to_delete)
self.assertEqual(len(remaining), 4)
self.assertEqual(set(remaining.mapped('state')), {'error'})
@@ -67,7 +67,7 @@ class TestSMSPost(SMSCommon, MockLinkTracker):
'+32456000022': 'wrong_number_format',
}):
self.env['sms.sms'].browse(self.sms_all.ids).send(unlink_failed=True, unlink_sent=False, raise_exception=False)
remaining = self.sms_all.exists()
remaining = self.sms_all.exists().filtered(lambda s: not s.to_delete)
self.assertEqual(len(remaining), 8)
self.assertEqual(set(remaining.mapped('state')), {'pending'})
@@ -89,7 +89,7 @@ class TestSMSPost(SMSCommon, MockLinkTracker):
'+32456000022': 'wrong_number_format',
}):
self.env['sms.sms'].browse(self.sms_all.ids).send(unlink_failed=False, unlink_sent=True, raise_exception=False)
remaining = self.sms_all.exists()
remaining = self.sms_all.exists().filtered(lambda s: not s.to_delete)
self.assertEqual(len(remaining), 2)
self.assertEqual(set(remaining.mapped('state')), {'error'})