Skip to content

Commit

Permalink
[FIX] website(_crm): fix email and phone computation of visitor
Browse files Browse the repository at this point in the history
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 #48369

X-original-commit: 02ceb42
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
  • Loading branch information
tde-banana-odoo committed Mar 25, 2020
1 parent a2e5514 commit 52ac7f2
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 26 deletions.
6 changes: 3 additions & 3 deletions addons/website/models/website_visitor.py
Expand Up @@ -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
}

Expand Down
33 changes: 10 additions & 23 deletions addons/website_crm/models/website_visitor.py
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions addons/website_crm/tests/__init__.py
Expand Up @@ -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
67 changes: 67 additions & 0 deletions 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)

0 comments on commit 52ac7f2

Please sign in to comment.