From 626557147cefe9e8479a57d585138e4a3f6fc412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 20 Jul 2023 13:08:45 +0000 Subject: [PATCH] [REF] various: remove duplicated sms helper tools RATIONALE Simplify field management for mail / phone / sms flows. Make it working out of the box, easier to use and tweak. SPECIFICATIONS Simplify model custom code when dealing with phone and sms by using helpers and standard behavior defined on all models. '_sms_get_partner_fields' was introduced at odoo/odoo@bdebcab0cea467e070c1936fed16173247abafdd to have a generic implementation of finding partners on a record. Since then another version has been added directly in 'mail' module, using '_mail_get_partners'. '_sms_get_number_fields' was introduced at the same time to have a generic implementation of finding numbers on a record. Since then a generic and improved version has been added directly in 'phone_validation' module, see '_phone_get_number_fields'. Those method can therefore be removed, and replaced by the generic ones available on BaseModel. Task-3422449 (Mail, Phone: Move and improve field helpers) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#130468 --- addons/event_sms/models/__init__.py | 1 - addons/event_sms/models/event_registration.py | 13 ------ addons/hr/__manifest__.py | 1 + addons/hr/models/hr_employee.py | 8 ++-- .../models/mailing_mailing.py | 12 ++---- .../models/mail_thread_phone.py | 4 +- addons/sale/models/sale_order.py | 7 +--- addons/sms/models/__init__.py | 1 - addons/sms/models/ir_model.py | 4 +- addons/sms/models/mail_thread.py | 29 +++---------- addons/sms/models/mail_thread_phone.py | 16 ------- addons/stock_sms/models/stock_picking.py | 3 -- .../models/test_mail_sms_models.py | 42 +++++-------------- .../test_mail_sms/tests/test_sms_composer.py | 2 +- .../website_crm/controllers/website_form.py | 11 +---- 15 files changed, 32 insertions(+), 122 deletions(-) delete mode 100644 addons/event_sms/models/event_registration.py delete mode 100644 addons/sms/models/mail_thread_phone.py diff --git a/addons/event_sms/models/__init__.py b/addons/event_sms/models/__init__.py index 57080a42cfc..23d5f066f19 100644 --- a/addons/event_sms/models/__init__.py +++ b/addons/event_sms/models/__init__.py @@ -2,5 +2,4 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import event_mail -from . import event_registration from . import sms_template diff --git a/addons/event_sms/models/event_registration.py b/addons/event_sms/models/event_registration.py deleted file mode 100644 index d8be5a5390a..00000000000 --- a/addons/event_sms/models/event_registration.py +++ /dev/null @@ -1,13 +0,0 @@ -# -*- coding: utf-8 -*- -# Part of Odoo. See LICENSE file for full copyright and licensing details. - -from odoo import models - - -class Registration(models.Model): - _inherit = 'event.registration' - - def _sms_get_number_fields(self): - """ This method returns the fields to use to find the number to use to - send an SMS on a record. """ - return ['mobile', 'phone'] diff --git a/addons/hr/__manifest__.py b/addons/hr/__manifest__.py index 9713f4aa208..e107ca096f6 100644 --- a/addons/hr/__manifest__.py +++ b/addons/hr/__manifest__.py @@ -13,6 +13,7 @@ ], 'depends': [ 'base_setup', + 'phone_validation', 'mail', 'resource', 'web', diff --git a/addons/hr/models/hr_employee.py b/addons/hr/models/hr_employee.py index c9a03f809d1..a1f78f90ed8 100644 --- a/addons/hr/models/hr_employee.py +++ b/addons/hr/models/hr_employee.py @@ -529,6 +529,9 @@ class HrEmployeePrivate(models.Model): # Messaging # --------------------------------------------------------- + def _phone_get_number_fields(self): + return ['mobile_phone'] + def _message_log(self, **kwargs): return super(HrEmployeePrivate, self._post_author())._message_log(**kwargs) @@ -536,8 +539,5 @@ class HrEmployeePrivate(models.Model): def message_post(self, **kwargs): return super(HrEmployeePrivate, self._post_author()).message_post(**kwargs) - def _sms_get_partner_fields(self): + def _mail_get_partner_fields(self, introspect_fields=False): return ['user_partner_id'] - - def _sms_get_number_fields(self): - return ['mobile_phone'] diff --git a/addons/mass_mailing_sms/models/mailing_mailing.py b/addons/mass_mailing_sms/models/mailing_mailing.py index 3d1ba768b43..e0558636ddb 100644 --- a/addons/mass_mailing_sms/models/mailing_mailing.py +++ b/addons/mass_mailing_sms/models/mailing_mailing.py @@ -171,18 +171,12 @@ class Mailing(models.Model): partner_fields = [] if issubclass(type(target), self.pool['mail.thread.phone']): phone_fields = ['phone_sanitized'] - elif issubclass(type(target), self.pool['mail.thread']): + else: phone_fields = [ - fname for fname in target._sms_get_number_fields() + fname for fname in target._phone_get_number_fields() if fname in target._fields and target._fields[fname].store ] - partner_fields = target._sms_get_partner_fields() - else: - phone_fields = [] - if 'mobile' in target._fields and target._fields['mobile'].store: - phone_fields.append('mobile') - if 'phone' in target._fields and target._fields['phone'].store: - phone_fields.append('phone') + partner_fields = target._mail_get_partner_fields() partner_field = next( (fname for fname in partner_fields if target._fields[fname].store and target._fields[fname].type == 'many2one'), False diff --git a/addons/phone_validation/models/mail_thread_phone.py b/addons/phone_validation/models/mail_thread_phone.py index 33a3e5aaf32..184e79e1b90 100644 --- a/addons/phone_validation/models/mail_thread_phone.py +++ b/addons/phone_validation/models/mail_thread_phone.py @@ -12,9 +12,9 @@ from odoo.osv import expression class PhoneMixin(models.AbstractModel): """ Purpose of this mixin is to offer two services - * compute a sanitized phone number based on ´´_sms_get_number_fields´´. + * compute a sanitized phone number based on _phone_get_number_fields. It takes first sanitized value, trying each field returned by the - method (see ``MailThread._sms_get_number_fields()´´ for more details + method (see ``BaseModel._phone_get_number_fields()´´ for more details about the usage of this method); * compute blacklist state of records. It is based on phone.blacklist model and give an easy-to-use field and API to manipulate blacklisted diff --git a/addons/sale/models/sale_order.py b/addons/sale/models/sale_order.py index 1792b01dbf8..51b6a58d89e 100644 --- a/addons/sale/models/sale_order.py +++ b/addons/sale/models/sale_order.py @@ -1423,14 +1423,11 @@ class SaleOrder(models.Model): render_context['subtitles'] = subtitles return render_context - def _sms_get_number_fields(self): + def _phone_get_number_fields(self): """ No phone or mobile field is available on sale model. Instead SMS will - fallback on partner-based computation using ``_sms_get_partner_fields``. """ + fallback on partner-based computation using ``_mail_get_partner_fields``. """ return [] - def _sms_get_partner_fields(self): - return ['partner_id'] - def _track_subtype(self, init_values): self.ensure_one() if 'state' in init_values and self.state == 'sale': diff --git a/addons/sms/models/__init__.py b/addons/sms/models/__init__.py index c767c8d7427..d0d09a915fd 100644 --- a/addons/sms/models/__init__.py +++ b/addons/sms/models/__init__.py @@ -7,7 +7,6 @@ from . import mail_followers from . import mail_message from . import mail_notification from . import mail_thread -from . import mail_thread_phone from . import res_partner from . import sms_api from . import sms_sms diff --git a/addons/sms/models/ir_model.py b/addons/sms/models/ir_model.py index 1b1db6a9bae..5dc23787cf7 100644 --- a/addons/sms/models/ir_model.py +++ b/addons/sms/models/ir_model.py @@ -18,7 +18,7 @@ class IrModel(models.Model): for model in self: if model.is_mail_thread: ModelObject = self.env[model.model] - potential_fields = ModelObject._sms_get_number_fields() + ModelObject._sms_get_partner_fields() + potential_fields = ModelObject._phone_get_number_fields() + ModelObject._mail_get_partner_fields() if any(fname in ModelObject._fields for fname in potential_fields): model.is_mail_thread_sms = True continue @@ -31,7 +31,7 @@ class IrModel(models.Model): if model.model not in self.env: continue ModelObject = self.env[model.model] - potential_fields = ModelObject._sms_get_number_fields() + ModelObject._sms_get_partner_fields() + potential_fields = ModelObject._phone_get_number_fields() + ModelObject._mail_get_partner_fields() if any(fname in ModelObject._fields for fname in potential_fields): valid_models |= model diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py index 0c1f3a02e8c..7c6d6bc000a 100644 --- a/addons/sms/models/mail_thread.py +++ b/addons/sms/models/mail_thread.py @@ -42,34 +42,15 @@ class MailThread(models.AbstractModel): def _search_message_has_sms_error(self, operator, operand): return ['&', ('message_ids.has_sms_error', operator, operand), ('message_ids.author_id', '=', self.env.user.partner_id.id)] - def _sms_get_partner_fields(self): - """ This method returns the fields to use to find the contact to link - whensending an SMS. Having partner is not necessary, having only phone - number fields is possible. However it gives more flexibility to - notifications management when having partners. """ - fields = [] - if hasattr(self, 'partner_id'): - fields.append('partner_id') - if hasattr(self, 'partner_ids'): - fields.append('partner_ids') - return fields - def _sms_get_default_partners(self): """ This method will likely need to be overridden by inherited models. :returns partners: recordset of res.partner """ partners = self.env['res.partner'] - for fname in self._sms_get_partner_fields(): + for fname in self._mail_get_partner_fields(): partners = partners.union(*self.mapped(fname)) # ensure ordering return partners - def _sms_get_number_fields(self): - """ This method returns the fields to use to find the number to use to - send an SMS on a record. """ - if 'mobile' in self: - return ['mobile'] - return [] - def _sms_get_recipients_info(self, force_field=False, partner_fallback=True): """" Get SMS recipient information on current record set. This method checks for numbers and sanitation in order to centralize computation. @@ -82,7 +63,7 @@ class MailThread(models.AbstractModel): customer, force its number to found field number or fallback on customer fields; :param force_field: either give a specific field to find phone number, either - generic heuristic is used to find one based on ``_sms_get_number_fields``; + generic heuristic is used to find one based on ``_phone_get_number_fields``; :param partner_fallback: if no value found in the record, check its customer values based on ``_sms_get_default_partners``; @@ -96,11 +77,11 @@ class MailThread(models.AbstractModel): False it means number comes from the record itself, even if linked to a customer; 'field_store': field in which the number has been found (generally mobile or - phone, see ``_sms_get_number_fields``); + phone, see ``_phone_get_number_fields``); } for each record in self """ result = dict.fromkeys(self.ids, False) - tocheck_fields = [force_field] if force_field else self._sms_get_number_fields() + tocheck_fields = [force_field] if force_field else self._phone_get_number_fields() for record in self: all_numbers = [record[fname] for fname in tocheck_fields if fname in record] all_partners = record._sms_get_default_partners() @@ -122,7 +103,7 @@ class MailThread(models.AbstractModel): elif all_partners and partner_fallback: partner = self.env['res.partner'] for partner in all_partners: - for fname in self.env['res.partner']._sms_get_number_fields(): + for fname in self.env['res.partner']._phone_get_number_fields(): valid_number = phone_validation.phone_sanitize_numbers_w_record([partner[fname]], record)[partner[fname]]['sanitized'] if valid_number: break diff --git a/addons/sms/models/mail_thread_phone.py b/addons/sms/models/mail_thread_phone.py deleted file mode 100644 index 30c2342224f..00000000000 --- a/addons/sms/models/mail_thread_phone.py +++ /dev/null @@ -1,16 +0,0 @@ -# -*- coding: utf-8 -*- -# Part of Odoo. See LICENSE file for full copyright and licensing details. - -from odoo import models - - -class PhoneMixin(models.AbstractModel): - _inherit = 'mail.thread.phone' - - def _sms_get_number_fields(self): - """ Add fields coming from mail.thread.phone implementation. """ - phone_fields = self._phone_get_number_fields() - sms_fields = super(PhoneMixin, self)._sms_get_number_fields() - for fname in (f for f in sms_fields if f not in phone_fields): - phone_fields.append(fname) - return phone_fields diff --git a/addons/stock_sms/models/stock_picking.py b/addons/stock_sms/models/stock_picking.py index f7afe2448a2..42480a10494 100644 --- a/addons/stock_sms/models/stock_picking.py +++ b/addons/stock_sms/models/stock_picking.py @@ -45,9 +45,6 @@ class Picking(models.Model): 'context': self.env.context, } - def _sms_get_partner_fields(self): - return ['partner_id'] - def _send_confirmation_email(self): super(Picking, self)._send_confirmation_email() if not self.env.context.get('skip_sms') and not getattr(threading.current_thread(), 'testing', False) and not self.env.registry.in_test_mode(): diff --git a/addons/test_mail_sms/models/test_mail_sms_models.py b/addons/test_mail_sms/models/test_mail_sms_models.py index c01630b42c8..114738750c7 100644 --- a/addons/test_mail_sms/models/test_mail_sms_models.py +++ b/addons/test_mail_sms/models/test_mail_sms_models.py @@ -20,15 +20,12 @@ class MailTestSMS(models.Model): mobile_nbr = fields.Char() customer_id = fields.Many2one('res.partner', 'Customer') + def _phone_get_number_fields(self): + return ['phone_nbr', 'mobile_nbr'] + def _mail_get_partner_fields(self, introspect_fields=False): return ['customer_id'] - def _sms_get_partner_fields(self): - return ['customer_id'] - - def _sms_get_number_fields(self): - return ['phone_nbr', 'mobile_nbr'] - class MailTestSMSBL(models.Model): """ A model inheriting from mail.thread.phone allowing to test auto formatting @@ -56,15 +53,12 @@ class MailTestSMSBL(models.Model): for phone_record in self.filtered(lambda rec: not rec.phone_nbr and rec.customer_id): phone_record.phone_nbr = phone_record.customer_id.phone - def _mail_get_partner_fields(self, introspect_fields=False): - return ['customer_id'] - - def _sms_get_partner_fields(self): - return ['customer_id'] - def _phone_get_number_fields(self): return ['phone_nbr', 'mobile_nbr'] + def _mail_get_partner_fields(self, introspect_fields=False): + return ['customer_id'] + class MailTestSMSBLActivity(models.Model): """ A model inheriting from mail.thread.phone allowing to test auto formatting @@ -96,6 +90,9 @@ class MailTestSMSOptout(models.Model): customer_id = fields.Many2one('res.partner', 'Customer') opt_out = fields.Boolean() + def _phone_get_number_fields(self): + return ['phone_nbr', 'mobile_nbr'] + def _mail_get_partner_fields(self, introspect_fields=False): return ['customer_id'] @@ -106,13 +103,6 @@ class MailTestSMSOptout(models.Model): ('opt_out', '=', True) ]).ids - def _sms_get_partner_fields(self): - return ['customer_id'] - - def _sms_get_number_fields(self): - # TDE note: should override _phone_get_number_fields but ok as sms in dependencies - return ['phone_nbr', 'mobile_nbr'] - class MailTestSMSPartner(models.Model): """ A model like sale order having only a customer, not specific phone @@ -136,12 +126,6 @@ class MailTestSMSPartner(models.Model): ('opt_out', '=', True) ]).ids - def _sms_get_partner_fields(self): - return ['customer_id'] - - def _sms_get_number_fields(self): - return [] - class MailTestSMSPartner2Many(models.Model): """ A model like sale order having only a customer, not specific phone @@ -156,7 +140,7 @@ class MailTestSMSPartner2Many(models.Model): opt_out = fields.Boolean() def _mail_get_partner_fields(self, introspect_fields=False): - return ['customer_id'] + return ['customer_ids'] def _mailing_get_opt_out_list_sms(self, mailing): res_ids = mailing._get_recipients() @@ -164,9 +148,3 @@ class MailTestSMSPartner2Many(models.Model): ('id', 'in', res_ids), ('opt_out', '=', True) ]).ids - - def _sms_get_partner_fields(self): - return ['customer_ids'] - - def _sms_get_number_fields(self): - return [] diff --git a/addons/test_mail_sms/tests/test_sms_composer.py b/addons/test_mail_sms/tests/test_sms_composer.py index c03cc948eb9..28fa8d2172c 100644 --- a/addons/test_mail_sms/tests/test_sms_composer.py +++ b/addons/test_mail_sms/tests/test_sms_composer.py @@ -212,7 +212,7 @@ class TestSMSComposerComment(SMSCommon, TestSMSRecipients): 'phone_nbr': False, 'mobile_nbr': False, }) - default_field_name = self.env['mail.test.sms']._sms_get_number_fields()[0] + default_field_name = self.env['mail.test.sms']._phone_get_number_fields()[0] with self.with_user('employee'): composer = self.env['sms.composer'].with_context( diff --git a/addons/website_crm/controllers/website_form.py b/addons/website_crm/controllers/website_form.py index 7f9d43b666f..8071b663903 100644 --- a/addons/website_crm/controllers/website_form.py +++ b/addons/website_crm/controllers/website_form.py @@ -21,17 +21,10 @@ class WebsiteForm(form.WebsiteForm): return request.env['res.country'].sudo().search([('code', '=', country_code)], limit=1) return request.env['res.country'] - def _get_phone_fields_to_validate(self): - return ['phone', 'mobile'] - # Check and insert values from the form on the model + validation phone fields def _handle_website_form(self, model_name, **kwargs): model_record = request.env['ir.model'].sudo().search([('model', '=', model_name), ('website_form_access', '=', True)]) - if model_record and hasattr(request.env[model_name], '_phone_format') or hasattr(request.env[model_name], 'phone_get_sanitized_number'): - # filter on either custom _phone_format method, either phone_get_sanitized_number but directly - # call phone_format from phone validation herebelow to simplify things as we don't have real - # records but a dictionary of value at this point (record.phone_get_sanitized_number would - # not work) + if model_record: try: data = self.extract_data(model_record, request.params) except: @@ -39,7 +32,7 @@ class WebsiteForm(form.WebsiteForm): pass else: record = data.get('record', {}) - phone_fields = self._get_phone_fields_to_validate() + phone_fields = request.env[model_name]._phone_get_number_fields() country = request.env['res.country'].browse(record.get('country_id')) contact_country = country if country.exists() else self._get_country() for phone_field in phone_fields: