[IMP] crm: add tests for convert wizards

PURPOSE

As CRM is going to be updated (onchange -> compute, code improvements, better
management of sales teams) cleaning and improving code is a necessary first
step.

SPECIFICATIONS

Add tests. Amazing. Notably about matching partner / duplicate based on email
as there is some weird behavior.

closes odoo/odoo#45477

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Thibault Delavallée
2020-02-19 11:43:53 +00:00
parent 7e9918dbe3
commit e3a87d3511
2 changed files with 235 additions and 25 deletions
+71 -11
View File
@@ -39,7 +39,7 @@ Cheers,
Somebody."""
class TestCrmCommon(MailCase, SavepointCase):
class TestCrmCommon(SavepointCase, MailCase):
@classmethod
def setUpClass(cls):
@@ -112,10 +112,28 @@ class TestCrmCommon(MailCase, SavepointCase):
'type': 'lead',
'user_id': cls.user_sales_leads.id,
'team_id': cls.sales_team_1.id,
'partner_id': False,
'contact_name': 'Amy Wong',
'email_from': 'amy.wong@test.example.com',
})
# create an history for new team
cls.lead_team_1_won = cls.env['crm.lead'].create({
'name': 'Already Won',
'type': 'lead',
'user_id': cls.user_sales_leads.id,
'team_id': cls.sales_team_1.id,
})
cls.lead_team_1_won.action_set_won()
cls.lead_team_1_lost = cls.env['crm.lead'].create({
'name': 'Already Won',
'type': 'lead',
'user_id': cls.user_sales_leads.id,
'team_id': cls.sales_team_1.id,
})
cls.lead_team_1_lost.action_set_lost()
(cls.lead_team_1_won | cls.lead_team_1_lost).flush()
cls.contact_company_1 = cls.env['res.partner'].create({
'name': 'Planet Express',
'email': 'planet.express@test.example.com',
@@ -148,18 +166,60 @@ class TestCrmCommon(MailCase, SavepointCase):
'zip': '97648',
})
def _create_duplicates(self, lead, count=2):
leads = self.env['crm.lead']
for x in range(count):
leads |= self.env['crm.lead'].create({
'name': 'Dup-%02d-%s' % (x+1, lead.name),
'type': 'lead',
'user_id': False,
def _create_duplicates(self, lead, create_opp=True):
""" Helper tool method creating, based on a given lead
* a customer (res.partner) based on lead email (to test partner finding)
-> FIXME: using same normalized email does not work currently, only exact email works
* a lead with same email_from
* a lead with same email_normalized (other email_from)
* a lead with customer but another email
* a lost opportunity with same email_from
"""
self.customer = self.env['res.partner'].create({
'name': 'Lead1 Email Customer',
'email': lead.email_from,
})
self.lead_email_from = self.env['crm.lead'].create({
'name': 'Duplicate: same email_from',
'type': 'lead',
'team_id': lead.team_id.id,
'email_from': lead.email_from,
})
# self.lead_email_normalized = self.env['crm.lead'].create({
# 'name': 'Duplicate: email_normalize comparison',
# 'type': 'lead',
# 'team_id': lead.team_id.id,
# 'stage_id': lead.stage_id.id,
# 'email_from': 'CUSTOMER WITH NAME <%s>' % lead.email_normalized.upper(),
# })
self.lead_partner = self.env['crm.lead'].create({
'name': 'Duplicate: customer ID',
'type': 'lead',
'team_id': lead.team_id.id,
'partner_id': self.customer.id,
'email_from': 'another.email@test.example.com',
})
if create_opp:
self.opp_lost = self.env['crm.lead'].create({
'name': 'Duplicate: lost opportunity',
'type': 'opportunity',
'team_id': lead.team_id.id,
'contact_name': 'Duplicate %02d of %s' % (x+1, lead.contact_name),
'stage_id': lead.stage_id.id,
'email_from': lead.email_from,
})
return leads
self.opp_lost.action_set_lost()
else:
self.opp_lost = self.env['crm.lead']
# self.assertEqual(self.lead_email_from.email_normalized, self.lead_email_normalized.email_normalized)
# self.assertTrue(lead.email_from != self.lead_email_normalized.email_from)
# self.assertFalse(self.opp_lost.active)
# new_lead = self.lead_email_from | self.lead_email_normalized | self.lead_partner | self.opp_lost
new_leads = self.lead_email_from | self.lead_partner | self.opp_lost
new_leads.flush() # compute notably probability
return new_leads
class TestLeadConvertCommon(TestCrmCommon):
@@ -280,4 +340,4 @@ class TestLeadConvertMassCommon(TestLeadConvertCommon):
})
for l in (cls.lead_w_partner | cls.lead_w_partner_company | cls.lead_w_contact | cls.lead_w_email | cls.lead_w_email_lost):
l._onchange_user_id()
l.flush()
(cls.lead_w_partner | cls.lead_w_partner_company | cls.lead_w_contact | cls.lead_w_email | cls.lead_w_email_lost).flush()
+164 -14
View File
@@ -84,6 +84,52 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
# TDE FIXME: convert does not recompute stage based on team
# self.assertEqual(lead.stage_id, self.stage_team_convert_1)
@users('user_sales_manager')
def test_lead_convert_corner_cases_crud(self):
""" Test Lead._find_matching_partner() """
# email formatting
other_lead = self.lead_1.copy()
other_lead.write({'partner_id': self.contact_1.id})
# TDE FIXME: default_lead_id should be correctly supported by wizard -> currently completely broken
# convert = self.env['crm.lead2opportunity.partner'].with_context({
# 'default_lead_id': other_lead.id,
# }).create({})
# self.assertEqual(convert.lead_id, other_lead)
# self.assertEqual(convert.partner_id, self.contact_1)
# self.assertEqual(convert.action, 'exist')
convert = self.env['crm.lead2opportunity.partner'].with_context({
'default_lead_id': other_lead.id,
'active_model': 'crm.lead',
'active_id': self.lead_1.id,
}).create({})
# TDE FIXME: active_id should be a fallback, not the only one way to work with a standard wizard
self.assertEqual(convert.lead_id, self.lead_1)
# self.assertEqual(convert.lead_id, other_lead)
# self.assertEqual(convert.partner_id, self.contact_1)
# self.assertEqual(convert.action, 'exist')
@users('user_sales_manager')
def test_lead_convert_corner_cases_matching(self):
""" Test Lead._find_matching_partner() """
# email formatting
self.lead_1.write({
'email_from': 'Amy Wong <amy.wong@test.example.com>'
})
customer = self.env['res.partner'].create({
'name': 'Different Name',
'email': 'Wong AMY <AMY.WONG@test.example.com>'
})
convert = self.env['crm.lead2opportunity.partner'].with_context({
'active_model': 'crm.lead',
'active_id': self.lead_1.id,
'active_ids': self.lead_1.ids,
}).create({})
# TDE FIXME: should take into account normalized email version, not encoded one
# self.assertEqual(convert.partner_id, customer)
@users('user_sales_manager')
def test_lead_convert_internals(self):
""" Test internals of convert wizard """
@@ -101,7 +147,7 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
}).create({})
# test internals of convert wizard
# self.assertEqual(convert.lead_id, self.lead_1)
self.assertEqual(convert.lead_id, self.lead_1)
self.assertEqual(convert.user_id, self.lead_1.user_id)
self.assertEqual(convert.team_id, self.lead_1.team_id)
self.assertFalse(convert.partner_id)
@@ -164,7 +210,14 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
date = Datetime.from_string('2020-01-20 16:00:00')
self.crm_lead_dt_mock.now.return_value = date
leads = self._create_duplicates(self.lead_1)
leads = self.env['crm.lead']
for x in range(2):
leads |= self.env['crm.lead'].create({
'name': 'Dup-%02d-%s' % (x+1, self.lead_1.name),
'type': 'lead', 'user_id': False, 'team_id': self.lead_1.team_id.id,
'contact_name': 'Duplicate %02d of %s' % (x+1, self.lead_1.contact_name),
'email_from': self.lead_1.email_from,
})
convert = self.env['crm.lead2opportunity.partner'].with_context({
'active_model': 'crm.lead',
@@ -188,6 +241,69 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
convert.action_apply()
self.assertEqual(self.lead_1.type, 'opportunity')
@users('user_sales_manager')
def test_lead_merge_duplicates(self):
""" Test Lead._get_duplicated_leads_by_emails() """
# Check: partner / email fallbacks
self._create_duplicates(self.lead_1)
self.lead_1.write({
'partner_id': self.customer.id,
})
self.customer.write({'email': False})
convert = self.env['crm.lead2opportunity.partner'].with_context({
'active_model': 'crm.lead',
'active_id': self.lead_1.id,
'active_ids': self.lead_1.ids,
}).create({})
self.assertEqual(convert.partner_id, self.customer)
# TDE FIXME: should not give priority to partner email if it is void
# self.assertEqual(convert.opportunity_ids, self.lead_1 | lead_email_from | lead_email_normalized | lead_partner)
self.assertEqual(convert.opportunity_ids, self.env['crm.lead'])
# Check: partner fallbacks
self.lead_1.write({
'email_from': False,
'partner_id': self.customer.id,
})
self.customer.write({'email': False})
convert = self.env['crm.lead2opportunity.partner'].with_context({
'active_model': 'crm.lead',
'active_id': self.lead_1.id,
'active_ids': self.lead_1.ids,
}).create({})
self.assertEqual(convert.partner_id, self.customer)
# TDE FIXME: CHECKME: 2917b38f28d5c2d6c53c706e613da7b8e2ad7b52
# self.assertEqual(convert.opportunity_ids, self.lead_1 | lead_partner)
self.assertEqual(convert.opportunity_ids, self.env['crm.lead'])
@users('user_sales_manager')
def test_lead_merge_duplicates_flow(self):
""" Test Lead._get_duplicated_leads_by_emails() + merge with active_test """
# Check: email formatting
self.lead_1.write({
'email_from': 'Amy Wong <amy.wong@test.example.com>'
})
self._create_duplicates(self.lead_1)
convert = self.env['crm.lead2opportunity.partner'].with_context({
'active_model': 'crm.lead',
'active_id': self.lead_1.id,
'active_ids': self.lead_1.ids,
}).create({})
self.assertEqual(convert.partner_id, self.customer)
# TDE FIXME: should check for email_normalized -> lead_email_normalized not correctly found
# self.assertEqual(convert.opportunity_ids, self.lead_1 | lead_email_from | lead_email_normalized | lead_partner | opp_lost)
# TDE FIXME: add a active test context on opportunity_ids fields
self.assertEqual(convert.with_context(active_test=False).opportunity_ids, self.lead_1 | self.lead_email_from | self.lead_partner | self.opp_lost)
convert.action_apply()
self.assertEqual(
# (self.lead_1 | self.lead_email_from | self.lead_email_normalized | self.lead_partner | self.opp_lost).exists(),
(self.lead_1 | self.lead_email_from | self.lead_partner | self.opp_lost).exists(),
self.opp_lost)
@tagged('lead_manage')
class TestLeadConvertBatch(crm_common.TestLeadConvertMassCommon):
@@ -264,22 +380,20 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon):
super(TestLeadConvertMass, cls).setUpClass()
cls.leads = cls.lead_1 + cls.lead_w_partner + cls.lead_w_email_lost
# reset some assigned users to test salesmen assign
(cls.lead_w_partner | cls.lead_w_email_lost).write({
'user_id': False
})
cls.assign_users = cls.user_sales_manager + cls.user_sales_leads_convert + cls.user_sales_salesman
@users('user_sales_manager')
def test_mass_convert_internals(self):
""" Test internals mass converted in convert mode, without duplicate management """
leads = self.leads
# reset some assigned users to test salesmen assign
(self.lead_w_partner | self.lead_w_email_lost).write({
'user_id': False
})
mass_convert = self.env['crm.lead2opportunity.partner.mass'].with_context({
'active_model': 'crm.lead',
'active_ids': leads.ids,
'active_id': leads.ids[0]
'active_ids': self.leads.ids,
'active_id': self.leads.ids[0]
}).create({
'deduplicate': False,
'user_id': self.user_sales_salesman.id,
@@ -324,13 +438,49 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon):
# TDE FIXME: partner creation is done even on lost leads because not checked in wizard
# self.assertEqual(self.lead_w_email_lost.partner_id, self.env['res.partner'])
def test_mass_convert_w_salesmen(self):
leads = self.leads
def test_mass_convert_deduplicate(self):
""" Test opportunity_ids fields having another behavior in mass convert
because why not. Its use is: among leads under convert, store those with
duplicates if deduplicate is set to True. """
lead_1_dups = self._create_duplicates(self.lead_1, create_opp=False)
lead_1_final = self.lead_1 # after merge: same but with lower ID
lead_1_dups_partner = lead_1_dups[1] # copy with a partner_id set but another email -> not correctly taken into account
lead_w_partner_dups = self._create_duplicates(self.lead_w_partner, create_opp=False)
lead_w_partner_final = lead_w_partner_dups[0] # lead_w_partner has no stage -> lower in sort by confidence
lead_w_partner_dups_partner = lead_w_partner_dups[1] # copy with a partner_id set but another email -> not correctly taken into account
mass_convert = self.env['crm.lead2opportunity.partner.mass'].with_context({
'active_model': 'crm.lead',
'active_ids': leads.ids,
'active_id': leads.ids[0]
'active_ids': self.leads.ids,
}).create({
'deduplicate': True,
})
mass_convert._onchange_deduplicate()
self.assertEqual(mass_convert.action, 'each_exist_or_create')
self.assertEqual(mass_convert.name, 'convert')
self.assertEqual(mass_convert.opportunity_ids, self.lead_1 | self.lead_w_partner)
mass_convert.mass_convert()
self.assertEqual(
(lead_1_dups | lead_w_partner_dups).exists(),
lead_1_dups_partner | lead_w_partner_final | lead_w_partner_dups_partner
)
for lead in lead_1_final | lead_w_partner_final:
self.assertTrue(lead.active)
self.assertEqual(lead.type, 'opportunity')
def test_mass_convert_w_salesmen(self):
# reset some assigned users to test salesmen assign
(self.lead_w_partner | self.lead_w_email_lost).write({
'user_id': False
})
mass_convert = self.env['crm.lead2opportunity.partner.mass'].with_context({
'active_model': 'crm.lead',
'active_ids': self.leads.ids,
'active_id': self.leads.ids[0]
}).create({
'deduplicate': False,
'user_ids': self.assign_users.ids,