[FW][FIX] phone_validation: extend implementation of phone_mobile_search
PURPOSE Extend implementation of ``phone_mobile_search`` that is currently limited to performing an ilike with a term containing at least 3 characters. Any operator is automatically considered as being an 'ilike'. SPECIFICATIONS Correctly support boolean check (is set / is not set), with its own small implementation easier than the remaining computation. Correctly propagate operator in generated SQL: like, ilike, =, those operators should not be automatically considered as always ilike. Correctly support negative operators. Supporting only positive search is a bit limited. Add tests. Task-3012789 closes odoo/odoo#104111 Forward-port-of: odoo/odoo#103354 Forward-port-of: odoo/odoo#100724 Related: odoo/enterprise#33211 Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -735,10 +735,14 @@ class TestCRMLead(TestCrmCommon):
|
||||
lead_3,
|
||||
'Should behave like a text field'
|
||||
)
|
||||
self.assertEqual(
|
||||
self.assertFalse(
|
||||
self.env['crm.lead'].search([('phone_mobile_search', 'like', 'Hello')]),
|
||||
'Should behave like a text field (case sensitive)'
|
||||
)
|
||||
self.assertEqual(
|
||||
self.env['crm.lead'].search([('phone_mobile_search', 'ilike', 'Hello')]),
|
||||
lead_3,
|
||||
'Should behave like a text field'
|
||||
'Should behave like a text field (case insensitive)'
|
||||
)
|
||||
self.assertEqual(
|
||||
self.env['crm.lead'].search([('phone_mobile_search', 'like', 'hello123')]),
|
||||
|
||||
@@ -248,6 +248,12 @@ msgstr ""
|
||||
msgid "Messages"
|
||||
msgstr ""
|
||||
|
||||
#. module: phone_validation
|
||||
#: code:addons/phone_validation/models/mail_thread_phone.py:0
|
||||
#, python-format
|
||||
msgid "Missing definition of phone fields."
|
||||
msgstr ""
|
||||
|
||||
#. module: phone_validation
|
||||
#: model:ir.model.constraint,message:phone_validation.constraint_phone_blacklist_unique_number
|
||||
msgid "Number already exists"
|
||||
|
||||
@@ -6,6 +6,7 @@ import re
|
||||
from odoo import api, fields, models, _
|
||||
from odoo.addons.phone_validation.tools import phone_validation
|
||||
from odoo.exceptions import AccessError, UserError
|
||||
from odoo.osv import expression
|
||||
|
||||
|
||||
class PhoneMixin(models.AbstractModel):
|
||||
@@ -47,47 +48,73 @@ class PhoneMixin(models.AbstractModel):
|
||||
phone_mobile_search = fields.Char("Phone/Mobile", store=False, search='_search_phone_mobile_search')
|
||||
|
||||
def _search_phone_mobile_search(self, operator, value):
|
||||
value = value.strip()
|
||||
value = value.strip() if isinstance(value, str) else value
|
||||
phone_fields = [
|
||||
fname for fname in self._phone_get_number_fields()
|
||||
if fname in self._fields and self._fields[fname].store
|
||||
]
|
||||
if not phone_fields:
|
||||
raise UserError(_('Missing definition of phone fields.'))
|
||||
|
||||
# search if phone/mobile is set or not
|
||||
if (value is True or not value) and operator in ('=', '!='):
|
||||
if value:
|
||||
# inverse the operator
|
||||
operator = '=' if operator == '!=' else '!='
|
||||
op = expression.AND if operator == '=' else expression.OR
|
||||
return op([[(phone_field, operator, False)] for phone_field in phone_fields])
|
||||
|
||||
if len(value) < 3:
|
||||
raise UserError(_('Please enter at least 3 characters when searching a Phone/Mobile number.'))
|
||||
|
||||
pattern = r'[\s\\./\(\)\-]'
|
||||
sql_operator = {'=like': 'LIKE', '=ilike': 'ILIKE'}.get(operator, operator)
|
||||
|
||||
if value.startswith('+') or value.startswith('00'):
|
||||
# searching on +32485112233 should also finds 0032485112233 (and vice versa)
|
||||
# we therefore remove it from input value and search for both of them in db
|
||||
query = f"""
|
||||
SELECT model.id
|
||||
FROM {self._table} model
|
||||
WHERE
|
||||
model.phone IS NOT NULL AND (
|
||||
REGEXP_REPLACE(model.phone, %s, '', 'g') ILIKE %s OR
|
||||
REGEXP_REPLACE(model.phone, %s, '', 'g') ILIKE %s
|
||||
) OR
|
||||
model.mobile IS NOT NULL AND (
|
||||
REGEXP_REPLACE(model.mobile, %s, '', 'g') ILIKE %s OR
|
||||
REGEXP_REPLACE(model.mobile, %s, '', 'g') ILIKE %s
|
||||
);
|
||||
"""
|
||||
term = re.sub(pattern, '', value[1 if value.startswith('+') else 2:]) + '%'
|
||||
self._cr.execute(query, (
|
||||
pattern, '00' + term,
|
||||
pattern, '+' + term,
|
||||
pattern, '00' + term,
|
||||
pattern, '+' + term
|
||||
))
|
||||
if operator in expression.NEGATIVE_TERM_OPERATORS:
|
||||
# searching on +32485112233 should also finds 0032485112233 (and vice versa)
|
||||
# we therefore remove it from input value and search for both of them in db
|
||||
where_str = ' AND '.join(
|
||||
f"""model.{phone_field} IS NULL OR (
|
||||
REGEXP_REPLACE(model.{phone_field}, %s, '', 'g') {sql_operator} %s OR
|
||||
REGEXP_REPLACE(model.{phone_field}, %s, '', 'g') {sql_operator} %s
|
||||
)"""
|
||||
for phone_field in phone_fields
|
||||
)
|
||||
else:
|
||||
# searching on +32485112233 should also finds 0032485112233 (and vice versa)
|
||||
# we therefore remove it from input value and search for both of them in db
|
||||
where_str = ' OR '.join(
|
||||
f"""model.{phone_field} IS NOT NULL AND (
|
||||
REGEXP_REPLACE(model.{phone_field}, %s, '', 'g') {sql_operator} %s OR
|
||||
REGEXP_REPLACE(model.{phone_field}, %s, '', 'g') {sql_operator} %s
|
||||
)"""
|
||||
for phone_field in phone_fields
|
||||
)
|
||||
query = f"SELECT model.id FROM {self._table} model WHERE {where_str};"
|
||||
|
||||
term = re.sub(pattern, '', value[1 if value.startswith('+') else 2:])
|
||||
if operator not in ('=', '!='): # for like operators
|
||||
term = f'{term}%'
|
||||
self._cr.execute(
|
||||
query, (pattern, '00' + term, pattern, '+' + term) * len(phone_fields)
|
||||
)
|
||||
else:
|
||||
query = f"""
|
||||
SELECT model.id
|
||||
FROM {self._table} model
|
||||
WHERE
|
||||
REGEXP_REPLACE(model.phone, %s, '', 'g') ILIKE %s OR
|
||||
REGEXP_REPLACE(model.mobile, %s, '', 'g') ILIKE %s;
|
||||
"""
|
||||
term = '%' + re.sub(pattern, '', value) + '%'
|
||||
self._cr.execute(query, (
|
||||
pattern, term,
|
||||
pattern, term
|
||||
))
|
||||
if operator in expression.NEGATIVE_TERM_OPERATORS:
|
||||
where_str = ' AND '.join(
|
||||
f"(model.{phone_field} IS NULL OR REGEXP_REPLACE(model.{phone_field}, %s, '', 'g') {sql_operator} %s)"
|
||||
for phone_field in phone_fields
|
||||
)
|
||||
else:
|
||||
where_str = ' OR '.join(
|
||||
f"(model.{phone_field} IS NOT NULL AND REGEXP_REPLACE(model.{phone_field}, %s, '', 'g') {sql_operator} %s)"
|
||||
for phone_field in phone_fields
|
||||
)
|
||||
query = f"SELECT model.id FROM {self._table} model WHERE {where_str};"
|
||||
term = re.sub(pattern, '', value)
|
||||
if operator not in ('=', '!='): # for like operators
|
||||
term = f'%{term}%'
|
||||
self._cr.execute(query, (pattern, term) * len(phone_fields))
|
||||
res = self._cr.fetchall()
|
||||
if not res:
|
||||
return [(0, '=', 1)]
|
||||
|
||||
@@ -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 MailTestSMS(models.Model):
|
||||
@@ -39,10 +39,20 @@ class MailTestSMSBL(models.Model):
|
||||
name = fields.Char()
|
||||
subject = fields.Char()
|
||||
email_from = fields.Char()
|
||||
phone_nbr = fields.Char()
|
||||
mobile_nbr = fields.Char()
|
||||
phone_nbr = fields.Char(compute='_compute_phone_nbr', readonly=False, store=True)
|
||||
mobile_nbr = fields.Char(compute='_compute_mobile_nbr', readonly=False, store=True)
|
||||
customer_id = fields.Many2one('res.partner', 'Customer')
|
||||
|
||||
@api.depends('customer_id')
|
||||
def _compute_mobile_nbr(self):
|
||||
for phone_record in self.filtered(lambda rec: not rec.mobile_nbr and rec.customer_id):
|
||||
phone_record.mobile_nbr = phone_record.customer_id.mobile
|
||||
|
||||
@api.depends('customer_id')
|
||||
def _compute_phone_nbr(self):
|
||||
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 _sms_get_partner_fields(self):
|
||||
return ['customer_id']
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from . import test_mail_thread_phone
|
||||
from . import test_phone_blacklist
|
||||
from . import test_sms_composer
|
||||
from . import test_sms_management
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from odoo.addons.test_mail_sms.tests.common import TestSMSCommon, TestSMSRecipients
|
||||
from odoo.tests import tagged, users
|
||||
|
||||
|
||||
@tagged('mail_thread')
|
||||
class TestSMSActionsCommon(TestSMSCommon, TestSMSRecipients):
|
||||
""" Test mail.thread.phone mixin, its tools and API """
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
super().setUpClass()
|
||||
cls.test_phone_records, cls.test_phone_partners = cls._create_records_for_batch(
|
||||
'mail.test.sms.bl',
|
||||
5,
|
||||
)
|
||||
cls.test_phone_records += cls.env['mail.test.sms.bl'].create([
|
||||
{
|
||||
'phone_nbr': '+32475110505',
|
||||
'mobile_nbr': '+32475000505',
|
||||
}, {
|
||||
'phone_nbr': '0032475110606',
|
||||
'mobile_nbr': '0032475000606',
|
||||
}, {
|
||||
'phone_nbr': '0032475110707',
|
||||
'mobile_nbr': False,
|
||||
}, {
|
||||
'phone_nbr': False,
|
||||
'mobile_nbr': False,
|
||||
},
|
||||
])
|
||||
|
||||
def test_initial_data(self):
|
||||
""" Test initial data for this class, allowing to be sure of I/O of tests. """
|
||||
self.assertEqual(
|
||||
self.test_phone_records.mapped('mobile_nbr'),
|
||||
['0475000000', '0475000101', '0475000202', '0475000303', '0475000404',
|
||||
'+32475000505', '0032475000606',
|
||||
False, False,
|
||||
]
|
||||
)
|
||||
self.assertEqual(
|
||||
self.test_phone_records.mapped('phone_nbr'),
|
||||
[False] * 5 + ['+32475110505', '0032475110606', '0032475110707', False]
|
||||
)
|
||||
|
||||
@users('employee')
|
||||
def test_search_phone_mobile_search_boolean(self):
|
||||
test_phone_records = self.test_phone_records.with_env(self.env)
|
||||
|
||||
# test Falsy -> is set / is not set
|
||||
for test_values in [False, '', ' ']:
|
||||
# test is not set -> both fields should be not set
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '=', test_values)])
|
||||
self.assertEqual(results, test_phone_records[-1],
|
||||
'Search on phone_mobile_search: = False: record with two void values')
|
||||
# test is set -> at least one field should be set
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '!=', test_values)])
|
||||
self.assertEqual(results, test_phone_records[:-1],
|
||||
'Search on phone_mobile_search: != False: record at least one value set')
|
||||
|
||||
# test Truthy -> is set / is not set
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '=', True)])
|
||||
self.assertEqual(results, test_phone_records[:-1],
|
||||
'Search on phone_mobile_search: = True: record at least one value set')
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '!=', True)])
|
||||
self.assertEqual(results, test_phone_records[-1],
|
||||
'Search on phone_mobile_search: != True: record with two void values')
|
||||
|
||||
@users('employee')
|
||||
def test_search_phone_mobile_search_equal(self):
|
||||
""" Test searching by phone/mobile with direct search """
|
||||
test_phone_records = self.test_phone_records.with_env(self.env)
|
||||
|
||||
# test "=" search
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '=', '0475')])
|
||||
self.assertFalse(results, 'Search on phone_mobile_search: = should return only matching results')
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '=', '0475000000')])
|
||||
self.assertEqual(results, test_phone_records[0])
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '=', '0032475110606')])
|
||||
self.assertEqual(results, test_phone_records[6])
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', '=', '+32475110606')])
|
||||
self.assertEqual(results, test_phone_records[6])
|
||||
|
||||
@users('employee')
|
||||
def test_search_phone_mobile_search_ilike(self):
|
||||
""" Test searching by phone/mobile on various ilike combinations """
|
||||
test_phone_records = self.test_phone_records.with_env(self.env)
|
||||
|
||||
# test ilike search
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', 'ilike', '0475')])
|
||||
self.assertEqual(results, test_phone_records[:5])
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', 'ilike', '101')])
|
||||
self.assertEqual(results, test_phone_records[1])
|
||||
|
||||
# test search using +32/0032
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', 'ilike', '+32475')])
|
||||
self.assertEqual(results, test_phone_records[5:8],
|
||||
'Search on phone_mobile_search: +32/0032 likeliness')
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', 'ilike', '0032475')])
|
||||
self.assertEqual(results, test_phone_records[5:8],
|
||||
'Search on phone_mobile_search: +32/0032 likeliness')
|
||||
|
||||
# test inverse ilike search
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', 'not ilike', '0475')])
|
||||
self.assertEqual(results, test_phone_records - test_phone_records[:5])
|
||||
results = self.env['mail.test.sms.bl'].search([('phone_mobile_search', 'not ilike', '101')])
|
||||
self.assertEqual(results, test_phone_records - test_phone_records[1])
|
||||
Reference in New Issue
Block a user