[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) <tde@openerp.com>

4 files changed, 81 insertions, 26 deletions

diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py
index c9f9d826c8d..3af252bda6e 100644
--- a/addons/website/models/website_visitor.py
+++ b/addons/website/models/website_visitor.py
@@ -70,16 +70,16 @@ class WebsiteVisitor(models.Model):
             (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 9b0b4c52c34..6c996df424c 100644
--- a/addons/website_crm/models/website_visitor.py
+++ b/addons/website_crm/models/website_visitor.py
@@ -19,30 +19,17 @@ class WebsiteVisitor(models.Model):
     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 1600ebf1a27..ba4af1674ec 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 00000000000..ce2fcec924e
--- /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" <test@test.example.com>',
+            '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 <beaufort@test.example.com',
+            'visitor_ids': [(4, visitor.id)],
+        })
+        self.assertEqual(visitor.email, lead_1.email_normalized)
+        self.assertFalse(visitor.mobile)
+
+        # second lead created -> 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 <brie@test.example.com',
+            'country_id': self.env.ref('base.be').id,
+            'mobile': '+32456001122',
+            'visitor_ids': [(4, visitor.id)],
+        })
+        self.assertEqual(visitor.email, lead_1.email_normalized)
+        self.assertEqual(visitor.mobile, lead_2.mobile)
+
+        # partner win on leads
+        visitor_sudo.write({'partner_id': self.test_partner.id})
+        self.assertEqual(visitor.email, customer.email_normalized)
+        self.assertEqual(visitor.mobile, customer.mobile)
+
+        # partner updated -> fallback on leads
+        customer.write({'mobile': False})
+        self.assertEqual(visitor.email, customer.email_normalized)
+        self.assertEqual(visitor.mobile, lead_2.mobile)