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

2 files changed, 234 insertions, 24 deletions

diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py
index fa2c77a2307..2fbfcc0affa 100644
--- a/addons/crm/tests/common.py
+++ b/addons/crm/tests/common.py
@@ -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()
diff --git a/addons/crm/tests/test_crm_lead_convert.py b/addons/crm/tests/test_crm_lead_convert.py
index ec587d10c4f..7e589598c41 100644
--- a/addons/crm/tests/test_crm_lead_convert.py
+++ b/addons/crm/tests/test_crm_lead_convert.py
@@ -85,6 +85,52 @@ class TestLeadConvert(crm_common.TestLeadConvertCommon):
         # 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 """
         # ensure initial data to avoid spaghetti test update afterwards
@@ -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_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': 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):
-        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_ids': self.assign_users.ids,