From 52ac7f2ddb5c27a177f9bd0c088f2a8a08ab676f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 13 Feb 2020 12:59:47 +0000 Subject: [PATCH] [FIX] website(_crm): fix email and phone computation of visitor Currently there are several limitations in visitor email and phone computation, notably phone of partner is not taken into account (only mobile) and computation does not work in website_crm if there are no leads. In this commit we * support both mobile and phone fields of partner. Indeed generally all SMS flows check for those two fields and not only for mobile; * replace the SQL by standard ORM computation. As those fields are not stored and displayed in list views ORM should be capable of managing them; * avoid crash when creating visitors through the UI (no ids -> don't go into raw SQL); * correctly keep partner-based values when no lead is linked to the visitor; Tests are also added in order to ensure behavior. Task ID 2196869 closes odoo/odoo#48369 X-original-commit: 02ceb42e204be2b40b334b4e554eef6dfaffd228 Signed-off-by: Thibault Delavallee (tde) --- addons/website/models/website_visitor.py | 6 +- addons/website_crm/models/website_visitor.py | 33 +++------ addons/website_crm/tests/__init__.py | 1 + .../website_crm/tests/test_website_visitor.py | 67 +++++++++++++++++++ 4 files changed, 81 insertions(+), 26 deletions(-) create mode 100644 addons/website_crm/tests/test_website_visitor.py diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py index c9f9d826c8d5b..3af252bda6ee6 100644 --- a/addons/website/models/website_visitor.py +++ b/addons/website/models/website_visitor.py @@ -70,16 +70,16 @@ def name_get(self): (record.name or _('Website Visitor #%s') % record.id) ) for record in self] - @api.depends('partner_id.email_normalized', 'partner_id.mobile') + @api.depends('partner_id.email_normalized', 'partner_id.mobile', 'partner_id.phone') def _compute_email_phone(self): results = self.env['res.partner'].search_read( [('id', 'in', self.partner_id.ids)], - ['id', 'email_normalized', 'mobile'], + ['id', 'email_normalized', 'mobile', 'phone'], ) mapped_data = { result['id']: { 'email_normalized': result['email_normalized'], - 'mobile': result['mobile'] + 'mobile': result['mobile'] if result['mobile'] else result['phone'] } for result in results } diff --git a/addons/website_crm/models/website_visitor.py b/addons/website_crm/models/website_visitor.py index 9b0b4c52c3465..6c996df424cca 100644 --- a/addons/website_crm/models/website_visitor.py +++ b/addons/website_crm/models/website_visitor.py @@ -19,30 +19,17 @@ def _compute_lead_count(self): def _compute_email_phone(self): super(WebsiteVisitor, self)._compute_email_phone() self.flush() - sql = """ SELECT v.id as visitor_id, l.id as lead_id, - CASE WHEN p.email_normalized is not null THEN p.email_normalized ELSE l.email_normalized END as email, - CASE WHEN p.mobile is not null THEN p.mobile WHEN l.mobile is not null THEN l.mobile ELSE l.phone END as mobile - FROM website_visitor v - JOIN crm_lead_website_visitor_rel lv on lv.website_visitor_id = v.id - JOIN crm_lead l ON lv.crm_lead_id = l.id - LEFT JOIN res_partner p on p.id = v.partner_id - WHERE v.id in %s - ORDER BY l.create_date ASC""" - self.env.cr.execute(sql, (tuple(self.ids),)) - results = self.env.cr.dictfetchall() - mapped_data = {} - for result in results: - visitor_info = mapped_data.get(result['visitor_id'], {'email': '', 'mobile': ''}) - if result['email']: - visitor_info['email'] = result['email'] - if result['mobile']: - visitor_info['mobile'] = result['mobile'] - mapped_data[result['visitor_id']] = visitor_info - for visitor in self: - email = mapped_data.get(visitor.id, {}).get('email') - visitor.email = email[:-1] if email else False - visitor.mobile = mapped_data.get(visitor.id, {}).get('mobile') + left_visitors = self.filtered(lambda visitor: not visitor.email or not visitor.mobile) + leads = left_visitors.mapped('lead_ids').sorted('create_date', reverse=True) + visitor_to_lead_ids = dict((visitor.id, visitor.lead_ids.ids) for visitor in left_visitors) + + for visitor in left_visitors: + visitor_leads = leads.filtered(lambda lead: lead.id in visitor_to_lead_ids[visitor.id]) + if not visitor.email: + visitor.email = next((lead.email_normalized for lead in visitor_leads if lead.email_normalized), False) + if not visitor.mobile: + visitor.mobile = next((lead.mobile or lead.phone for lead in visitor_leads if lead.mobile or lead.phone), False) def _check_for_message_composer(self): check = super(WebsiteVisitor, self)._check_for_message_composer() diff --git a/addons/website_crm/tests/__init__.py b/addons/website_crm/tests/__init__.py index 1600ebf1a27d9..ba4af1674ece3 100644 --- a/addons/website_crm/tests/__init__.py +++ b/addons/website_crm/tests/__init__.py @@ -2,3 +2,4 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import test_website_crm +from . import test_website_visitor diff --git a/addons/website_crm/tests/test_website_visitor.py b/addons/website_crm/tests/test_website_visitor.py new file mode 100644 index 0000000000000..ce2fcec924e86 --- /dev/null +++ b/addons/website_crm/tests/test_website_visitor.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.addons.crm.tests.common import TestCrmCommon +from odoo.tests.common import users + + +class TestWebsiteVisitor(TestCrmCommon): + + def setUp(self): + super(TestWebsiteVisitor, self).setUp() + self.test_partner = self.env['res.partner'].create({ + 'name': 'Test Customer', + 'email': '"Test Customer" ', + 'country_id': self.env.ref('base.be').id, + 'mobile': '+32456001122' + }) + + @users('user_sales_manager') + def test_compute_email_phone(self): + visitor_sudo = self.env['website.visitor'].sudo().create({ + 'name': 'Mega Visitor', + }) + visitor = visitor_sudo.with_user(self.env.user) # as of 13.0 salesmen cannot create visitors, only read them + customer = self.test_partner.with_user(self.env.user) + self.assertFalse(visitor.email) + self.assertFalse(visitor.mobile) + + # partner information copied on visitor -> behaves like related + visitor_sudo.write({'partner_id': self.test_partner.id}) + self.assertEqual(visitor.email, customer.email_normalized) + self.assertEqual(visitor.mobile, customer.mobile) + + # if reset -> behaves like a related, also reset on visitor + visitor_sudo.write({'partner_id': False}) + self.assertFalse(visitor.email) + self.assertFalse(visitor.mobile) + + # first lead created -> updates email + lead_1 = self.env['crm.lead'].create({ + 'name': 'Test Lead 1', + 'email_from': 'Rambeau Fort keep first email but takes mobile as not defined before + lead_2 = self.env['crm.lead'].create({ + 'name': 'Test Lead 1', + 'email_from': 'Martino Brie fallback on leads + customer.write({'mobile': False}) + self.assertEqual(visitor.email, customer.email_normalized) + self.assertEqual(visitor.mobile, lead_2.mobile)