From 1e45ec7b367bc628f7d228ca8d9405a197ac5104 Mon Sep 17 00:00:00 2001 From: Jay Savaliya Date: Mon, 4 Dec 2023 16:32:52 +0100 Subject: [PATCH] [FIX] crm: fix date_{last_stage_update/open} update issues Before this commit: * when a user changes 'user_id' to set the same previous 'user_id', the assign date 'date_open' is updated but it should not as the responsible did not change; * when a user changes the salesperson 'user_id' of a crm lead, it triggers a recompute of 'team_id' that triggers a recompute of 'stage_id' that updates 'date_last_stage_update' even if the stage does not change, which happens frequently when changing leads within a given team (new assign, salesperson on holidays, ...) * when merging opportunities, 'user_id' can be set on the main opportunity which triggers a recomputation of both 'date_last_stage_update' and 'date_open' as explained in above points; Reason: The 'date_last_stage_update' field depends on 'stage_id' which depends on 'team_id' which depends on 'user_id'. As a result, when 'user_id' changes, 'date_last_stage_update' also updates. Moreover those fields are implemented using editable stored computed fields which are triggered everytime a value is given to those fields, even when the same value is given. After this commit: 'date_last_stage_update' and 'date_open' will only update when there are real changes. Task-3515225 X-original-commit: 8dc18806847e5240dba6f05bdd80d313bf466ebf Part-of: odoo/odoo#161918 --- addons/crm/models/crm_lead.py | 44 +++++++++++++++++++++++-------- addons/crm/tests/test_crm_lead.py | 20 +++++++------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 068f1623bdd..b93151994ec 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -297,7 +297,8 @@ class Lead(models.Model): continue team_domain = [('use_leads', '=', True)] if lead.type == 'lead' else [('use_opportunities', '=', True)] team = self.env['crm.team']._get_default_team_id(user_id=user.id, domain=team_domain) - lead.team_id = team.id + if lead.team_id != team: + lead.team_id = team.id @api.depends('user_id', 'team_id', 'partner_id') def _compute_company_id(self): @@ -345,12 +346,14 @@ class Lead(models.Model): @api.depends('user_id') def _compute_date_open(self): for lead in self: - lead.date_open = self.env.cr.now() if lead.user_id else False + if not lead.date_open and lead.user_id: + lead.date_open = self.env.cr.now() @api.depends('stage_id') def _compute_date_last_stage_update(self): for lead in self: - lead.date_last_stage_update = self.env.cr.now() + if not lead.date_last_stage_update: + lead.date_last_stage_update = self.env.cr.now() @api.depends('create_date', 'date_open') def _compute_day_open(self): @@ -767,13 +770,27 @@ class Lead(models.Model): if vals.get('website'): vals['website'] = self.env['res.partner']._clean_website(vals['website']) - stage_updated, stage_is_won = vals.get('stage_id'), False - # stage change: update date_last_stage_update - if stage_updated: - stage = self.env['crm.stage'].browse(vals['stage_id']) - if stage.is_won: - vals.update({'probability': 100, 'automated_probability': 100}) - stage_is_won = True + now = self.env.cr.now() + stage_updated, stage_is_won = False, False + # stage change (or reset): update date_last_stage_update if at least one + # lead does not have the same stage + if 'stage_id' in vals: + stage_updated = any(lead.stage_id.id != vals['stage_id'] for lead in self) + if stage_updated: + vals['date_last_stage_update'] = now + if stage_updated and vals.get('stage_id'): + stage = self.env['crm.stage'].browse(vals['stage_id']) + if stage.is_won: + vals.update({'probability': 100, 'automated_probability': 100}) + stage_is_won = True + # user change; update date_open if at least one lead does not + # have the same user + if 'user_id' in vals and not vals.get('user_id'): + vals['date_open'] = False + elif vals.get('user_id'): + user_updated = any(lead.user_id.id != vals['user_id'] for lead in self) + if user_updated: + vals['date_open'] = now # stage change with new stage: update probability and date_closed if vals.get('probability', 0) >= 100 or not vals.get('active', True): @@ -1432,7 +1449,12 @@ class Lead(models.Model): if merged_data.get('stage_id') not in team_stage_ids.ids: merged_data['stage_id'] = team_stage_ids[0].id if team_stage_ids else False - # write merged data into first opportunity + # write merged data into first opportunity; remove some keys if already + # set on opp to avoid useless recomputes + if 'user_id' in merged_data and opportunities_head.user_id.id == merged_data['user_id']: + merged_data.pop('user_id') + if 'team_id' in merged_data and opportunities_head.team_id.id == merged_data['team_id']: + merged_data.pop('team_id') opportunities_head.write(merged_data) # delete tail opportunities diff --git a/addons/crm/tests/test_crm_lead.py b/addons/crm/tests/test_crm_lead.py index 75126ea567b..adc584d7180 100644 --- a/addons/crm/tests/test_crm_lead.py +++ b/addons/crm/tests/test_crm_lead.py @@ -680,17 +680,17 @@ class TestCRMLead(TestCrmCommon): self.assertEqual(lead.stage_id, self.stage_team1_1) self.assertEqual(lead.team_id, self.sales_team_1) self.assertEqual( - leads[0].date_last_stage_update, updated_time, - 'FIXME: set same stage when changing user_id, should not update') + leads[0].date_last_stage_update, first_now, + 'Setting same stage when changing user_id, should not update') self.assertEqual( leads[0].date_open, updated_time, 'User assigned -> assign date updated') self.assertEqual( - leads[1].date_last_stage_update, updated_time, - 'FIXME: set same stage when changing user_id, should not update') + leads[1].date_last_stage_update, first_now, + 'Setting same stage when changing user_id, should not update') self.assertEqual( leads[1].date_open, updated_time, - 'FIXME: Should not update date_open, was already the same user_id') + 'Should not update date_open, was already the same user_id, but done in batch so ...') # set won changes stage -> update date_last_stage_update newer_time = datetime(2023, 11, 26, 8, 0, 0) @@ -713,16 +713,16 @@ class TestCRMLead(TestCrmCommon): auto_unlink=False, ) leads.flush_recordset() - self.assertEqual(leads[0].date_last_stage_update, updated_time) + self.assertEqual(leads[0].date_last_stage_update, first_now) self.assertEqual(leads[0].date_open, updated_time) self.assertEqual(leads[0].stage_id, self.stage_team1_1) self.assertEqual(leads[0].team_id, self.sales_team_1) self.assertEqual( - leads[1].date_last_stage_update, last_time, - 'FIXME: should not rewrite when setting same stage') + leads[1].date_last_stage_update, newer_time, + 'Should not rewrite when setting same stage') self.assertEqual( - leads[1].date_open, last_time, - 'FIXME: should not rewrite when setting same user_id') + leads[1].date_open, updated_time, + 'Should not rewrite when setting same user_id') self.assertEqual(leads[1].stage_id, self.stage_gen_won) self.assertEqual(leads[1].team_id, self.sales_team_1) self.assertEqual(leads[1].user_id, self.user_sales_salesman)