From f49d09decdd916c5824e39b2ff7a3939a7c68449 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 3 Feb 2022 15:21:49 +0000 Subject: [PATCH] [FIX] crm: avoid indeterminism linked to create_date filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test `test_assign_count` may fail randomly leading to the error: ``` self.assertEqual(self.sales_team_1_m3.lead_month_count, 14) AssertionError: 12 != 14 ``` Since BUNDLE_HOURS_DELAY is 0 (during tests), the domain becomes `[('create_date', '<', fields.Datetime.now()]` Depending of the time when the transaction starts, and the tests is executed, the domain could match more or less records when executed one second earlier or later. This is especially True for single module build where there is less logic and the execution is faster. The domain may be tested earlier and match less records. This error can be reproduced deterministically by setting `BUNDLE_HOURS_DELAY = 1/3600` A fix could be to ignore ignore the create_date domain if the BUNDLE_HOURS_DELAY is 0, we want to match all records in theory (except if some of them are created in the future...) The chosen solution is to use cr.now and use a <= instead. Some assertions where also added to the test in order to identify why the count was different and give a more precise message when it fails. ``` AssertionError: Lists differ: [] != ['TestLeadInitial_0000', 'TestLeadInitial_[97 chars]006'] Second list contains 6 additional elements. First extra element 0: 'TestLeadInitial_0000' - [] + ['TestLeadInitial_0000', + 'TestLeadInitial_0001', + 'TestLeadInitial_0002', + 'TestLeadInitial_0004', + 'TestLeadInitial_0005', + 'TestLeadInitial_0006'] ``` task-2796579 closes odoo/odoo#87107 X-original-commit: 4a5b68f05c57ba36560d43733dfd04b67c3297a8 Signed-off-by: Thibault Delavallee (tde) Signed-off-by: Xavier Dollé (xdo) --- addons/crm/models/crm_team.py | 4 +-- addons/crm/tests/common.py | 4 +-- addons/crm/tests/test_crm_lead_assignment.py | 27 +++++++++++++++++--- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/addons/crm/models/crm_team.py b/addons/crm/models/crm_team.py index 737b0f47628..c0f0d5bce8e 100644 --- a/addons/crm/models/crm_team.py +++ b/addons/crm/models/crm_team.py @@ -448,7 +448,7 @@ class Team(models.Model): auto_commit = not getattr(threading.currentThread(), 'testing', False) # leads - max_create_dt = fields.Datetime.now() - datetime.timedelta(hours=BUNDLE_HOURS_DELAY) + max_create_dt = self.env.cr.now() - datetime.timedelta(hours=BUNDLE_HOURS_DELAY) duplicates_lead_cache = dict() # teams data @@ -459,7 +459,7 @@ class Team(models.Model): lead_domain = expression.AND([ literal_eval(team.assignment_domain or '[]'), - [('create_date', '<', max_create_dt)], + [('create_date', '<=', max_create_dt)], ['&', ('team_id', '=', False), ('user_id', '=', False)], ['|', ('stage_id', '=', False), ('stage_id.is_won', '=', False)] ]) diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index 542235340d9..8da73db8597 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -298,7 +298,7 @@ class TestCrmCommon(TestSalesCommon, MailCase): def _create_leads_batch(self, lead_type='lead', count=10, email_dup_count=0, partner_count=0, partner_ids=None, user_ids=None, - country_ids=None, probabilities=None): + country_ids=None, probabilities=None, suffix=''): """ Helper tool method creating a batch of leads, useful when dealing with batch processes. Please update me. @@ -313,7 +313,7 @@ class TestCrmCommon(TestSalesCommon, MailCase): """ types = ['lead', 'opportunity'] leads_data = [{ - 'name': 'TestLead_%04d' % (x), + 'name': f'TestLead{suffix}_{x:04d}', 'type': lead_type if lead_type else types[x % 2], 'priority': '%s' % (x % 3), } for x in range(count)] diff --git a/addons/crm/tests/test_crm_lead_assignment.py b/addons/crm/tests/test_crm_lead_assignment.py index d39d18cff38..e528b5a87cb 100644 --- a/addons/crm/tests/test_crm_lead_assignment.py +++ b/addons/crm/tests/test_crm_lead_assignment.py @@ -114,7 +114,8 @@ class TestLeadAssign(TestLeadAssignCommon): user_ids=[False], partner_ids=[False, False, False, self.contact_1.id], probabilities=[30], - count=8 + count=8, + suffix='Initial', ) # commit probability and related fields leads.flush() @@ -137,7 +138,8 @@ class TestLeadAssign(TestLeadAssignCommon): existing_leads = self._create_leads_batch( lead_type='lead', user_ids=[self.user_sales_salesman.id], probabilities=[10], - count=14) + count=14, + suffix='Existing') self.assertEqual(existing_leads.team_id, self.sales_team_1, "Team should have lower sequence") existing_leads[0].active = False # lost existing_leads[1].probability = 100 # not won @@ -158,7 +160,26 @@ class TestLeadAssign(TestLeadAssignCommon): # sales_team_1_m2 is opt-out (new field in 14.3) -> even with max, no lead assigned self.sales_team_1_m2.update({'assignment_max': 45, 'assignment_optout': True}) with self.with_user('user_sales_manager'): - self.env['crm.team'].browse(self.sales_team_1.ids)._action_assign_leads(work_days=4) + teams_data, members_data = self.sales_team_1._action_assign_leads(work_days=4) + + Leads = self.env['crm.lead'] + + self.assertEqual( + sorted(Leads.browse(teams_data[self.sales_team_1]['assigned']).mapped('name')), + ['TestLeadInitial_0000', 'TestLeadInitial_0001', 'TestLeadInitial_0002', + 'TestLeadInitial_0004', 'TestLeadInitial_0005', 'TestLeadInitial_0006'] + ) + self.assertEqual( + Leads.browse(teams_data[self.sales_team_1]['merged']).mapped('name'), + ['TestLeadInitial_0003'] + ) + + self.assertEqual(len(teams_data[self.sales_team_1]['duplicates']), 1) + + self.assertEqual( + sorted(members_data[self.sales_team_1_m3]['assigned'].mapped('name')), + ['TestLeadInitial_0000', 'TestLeadInitial_0005'] + ) # salespersons assign self.members.invalidate_cache(fnames=['lead_month_count'])