[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>

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
         }
 

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()

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

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)