diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 0e5ce514c78bd..f335a74541c4c 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -213,6 +213,7 @@ class Lead(models.Model): automated_probability = fields.Float('Automated Probability', compute='_compute_probabilities', readonly=True, store=True) is_automated_probability = fields.Boolean('Is automated probability?', compute="_compute_is_automated_probability") # Won/Lost + is_won = fields.Boolean(related='stage_id.is_won') lost_reason = fields.Many2one( 'crm.lost.reason', string='Lost Reason', index=True, ondelete='restrict', tracking=True) @@ -688,7 +689,7 @@ def write(self, vals): elif 'probability' in vals: vals['date_closed'] = False - if any(field in ['active', 'stage_id'] for field in vals): + if any(field in ['active', 'stage_id', 'probability'] for field in vals): self._handle_won_lost(vals) write_result = super(Lead, self).write(vals) @@ -782,7 +783,12 @@ def _handle_won_lost(self, vals): - From lost to Won : We need to decrement corresponding lost count + increment corresponding won count in scoring frequency table. - From won to lost : We need to decrement corresponding won count + increment corresponding lost count - in scoring frequency table.""" + in scoring frequency table. + + A lead is WON when in won stage (and probability = 100% but that is implied) + A lead is LOST when active = False AND probability = 0 + In every other case, the lead is not won nor lost. + """ Lead = self.env['crm.lead'] leads_reach_won = Lead leads_leave_won = Lead @@ -791,19 +797,35 @@ def _handle_won_lost(self, vals): won_stage_ids = self.env['crm.stage'].search([('is_won', '=', True)]).ids for lead in self: if 'stage_id' in vals: - if vals['stage_id'] in won_stage_ids: - if lead.probability == 0: + if vals['stage_id'] in won_stage_ids: # set to won stage + if lead.probability == 0 and not lead.active: leads_leave_lost |= lead leads_reach_won |= lead - elif lead.stage_id.id in won_stage_ids and lead.active: # a lead can be lost at won_stage + elif lead.stage_id.id in won_stage_ids: # remove from won stage leads_leave_won |= lead if 'active' in vals: - if not vals['active'] and lead.active: # archive lead - if lead.stage_id.id in won_stage_ids and lead not in leads_leave_won: - leads_leave_won |= lead + if not vals['active'] and lead.active and (lead.probability == 0 or vals.get('probability', 1) == 0): # archive lead + if lead.stage_id.id in won_stage_ids: # a lead archived in won stage is still won. + continue leads_reach_lost |= lead - elif vals['active'] and not lead.active: # restore lead - leads_leave_lost |= lead + # As "mark as lost" in writing active = False first and then probability afterwards, handle reach lost. + if vals.get('probability', 1) == 0 and not lead.active and not lead.stage_id.id in won_stage_ids: + leads_reach_lost |= lead + + # Leaving lost is a bit tricky as writing active = True and probability = x + # will not be done at same time in most cases. As lost need both conditions (active=False AND P=0), changing + # one of the two should trigger 'leave lost'. But this should not be triggered a second time if we modify + # the other condition. So the lead should leave lost only if both conditions are valid before writing and + # not after (if lead is lost before writing and not after). + # But this is only valid outside of the won_stage as the lead has already been removed from lost when + # arriving in won stage. + + # restore and set p=x OR set p=x and is archived OR restore and p was 0 + if (lead.stage_id.id not in won_stage_ids or lead not in leads_leave_lost) and \ + vals.get('probability', 0) != 0 and ('active' in vals and vals['active']) or \ + vals.get('probability', 0) != 0 and not lead.active or \ + ('active' in vals and vals['active']) and lead.probability == 0: + leads_leave_lost |= lead leads_reach_won._pls_increment_frequencies(to_state='won') leads_leave_won._pls_increment_frequencies(from_state='won') @@ -887,27 +909,26 @@ def _stage_find(self, team_id=False, domain=None, order='sequence', limit=1): # ------------------------------------------------------------ def toggle_active(self): - """ When archiving: mark probability as 0. When re-activating - update probability again, for leads and opportunities. """ + """ When archiving: do nothing more. A lead can be archived but not lost. + When re-activating, force update probability, for leads and opportunities. """ res = super(Lead, self).toggle_active() activated = self.filtered(lambda lead: lead.active) - archived = self.filtered(lambda lead: not lead.active) if activated: activated.write({'lost_reason': False}) activated._compute_probabilities() - if archived: - archived.write({'probability': 0, 'automated_probability': 0}) return res def action_set_lost(self, **additional_values): - """ Lost semantic: probability = 0 or active = False """ + """ Lost semantic: probability = 0 AND active = False """ res = self.action_archive() - if additional_values: - self.write(dict(additional_values)) + additional_values = dict(additional_values) if additional_values else {} + # as archiving lead is not setting the probability to 0 anymore, force it here + additional_values.update({'probability': 0, 'automated_probability': 0}) + self.write(additional_values) return res def action_set_won(self): - """ Won semantic: probability = 100 (active untouched) """ + """ Won semantic: probability = 100 AND stage.is_won """ self.action_unarchive() # group the leads by team_id, in order to write once by values couple (each write leads to frequency increment) leads_by_won_stage = {} diff --git a/addons/crm/report/crm_opportunity_report_views.xml b/addons/crm/report/crm_opportunity_report_views.xml index 8f5aa7f780a0c..364393ca46ddb 100644 --- a/addons/crm/report/crm_opportunity_report_views.xml +++ b/addons/crm/report/crm_opportunity_report_views.xml @@ -68,7 +68,7 @@ + domain="[('probability', '=', 100), ('is_won', '=', True)]"/> diff --git a/addons/crm/static/tests/tours/crm_forecast_tour.js b/addons/crm/static/tests/tours/crm_forecast_tour.js index 730c9c82ac077..af60aadb2e7d6 100644 --- a/addons/crm/static/tests/tours/crm_forecast_tour.js +++ b/addons/crm/static/tests/tours/crm_forecast_tour.js @@ -78,9 +78,8 @@ tour.register('crm_forecast', { content: "wait for date_picker to disappear", run: function () {}, }, { - trigger: "input[name=probability]", - content: "max out probability", - run: "text 100" + trigger: "button[name=action_set_won_rainbowman]", + content: "win the lead", }, { trigger: '.o_back_button', content: 'navigate back to the kanban view', diff --git a/addons/crm/tests/test_crm_pls.py b/addons/crm/tests/test_crm_pls.py index b34cc0c682472..7e9dbea7d476f 100644 --- a/addons/crm/tests/test_crm_pls.py +++ b/addons/crm/tests/test_crm_pls.py @@ -238,9 +238,14 @@ def test_predictive_lead_scoring(self): self.assertEqual(lead_9_country_freq.lost_count, 0.0) # frequency does not exist self.assertEqual(lead_9_email_state_freq.lost_count, 2.1) + # NOTE: if we touch to active, stage_id and probability, the cache needs to be invalidated in order to get the + # correct lead values during the won/lost increment. This is because increment calls _pls_get_lead_pls_values() + # that uses SQL queries and not the ORM cache. + # B. Test Live Increment leads[4].action_set_lost() leads[9].action_set_won() + leads.invalidate_cache() # re-get frequencies that did not exists before lead_9_country_freq = LeadScoringFrequency.search([('team_id', '=', leads[9].team_id.id), ('variable', '=', 'country_id'), ('value', '=', leads[9].country_id.id)]) @@ -273,6 +278,7 @@ def test_predictive_lead_scoring(self): # Restore -> Should decrease lost leads[4].toggle_active() + leads[4].invalidate_cache() self.assertEqual(lead_4_stage_0_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_stage_won_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_country_freq.won_count, 0.1) # unchanged @@ -293,6 +299,7 @@ def test_predictive_lead_scoring(self): # set to won stage -> Should increase won leads[4].stage_id = won_stage_id + leads[4].invalidate_cache() self.assertEqual(lead_4_stage_0_freq.won_count, 2.1) # + 1 self.assertEqual(lead_4_stage_won_freq.won_count, 2.1) # + 1 self.assertEqual(lead_4_country_freq.won_count, 1.1) # + 1 @@ -302,42 +309,59 @@ def test_predictive_lead_scoring(self): self.assertEqual(lead_4_country_freq.lost_count, 1.1) # unchanged self.assertEqual(lead_4_email_state_freq.lost_count, 2.1) # unchanged - # Archive (was won, now lost) -> Should decrease won and increase lost + # Archive in won stage -> Should NOT decrease won NOR increase lost + # as lost = archived + 0% and WON = won_stage (+ 100%) leads[4].toggle_active() - self.assertEqual(lead_4_stage_0_freq.won_count, 1.1) # - 1 - self.assertEqual(lead_4_stage_won_freq.won_count, 1.1) # - 1 - self.assertEqual(lead_4_country_freq.won_count, 0.1) # - 1 - self.assertEqual(lead_4_email_state_freq.won_count, 1.1) # - 1 - self.assertEqual(lead_4_stage_0_freq.lost_count, 3.1) # + 1 - self.assertEqual(lead_4_stage_won_freq.lost_count, 1.1) # consider stages with <= sequence when lostand as stage is won.. even won_stage lost_count is increased by 1 - self.assertEqual(lead_4_country_freq.lost_count, 2.1) # + 1 - self.assertEqual(lead_4_email_state_freq.lost_count, 3.1) # + 1 + leads[4].invalidate_cache() + self.assertEqual(lead_4_stage_0_freq.won_count, 2.1) # unchanged + self.assertEqual(lead_4_stage_won_freq.won_count, 2.1) # unchanged + self.assertEqual(lead_4_country_freq.won_count, 1.1) # unchanged + self.assertEqual(lead_4_email_state_freq.won_count, 2.1) # unchanged + self.assertEqual(lead_4_stage_0_freq.lost_count, 2.1) # unchanged + self.assertEqual(lead_4_stage_won_freq.lost_count, 0.1) # unchanged + self.assertEqual(lead_4_country_freq.lost_count, 1.1) # unchanged + self.assertEqual(lead_4_email_state_freq.lost_count, 2.1) # unchanged - # Move to original stage -> Should do nothing (as lead is still lost) + # Move to original stage -> lead is not won anymore but not lost as probability != 0 leads[4].stage_id = stage_ids[0] + leads[4].invalidate_cache() + self.assertEqual(lead_4_stage_0_freq.won_count, 1.1) # -1 + self.assertEqual(lead_4_stage_won_freq.won_count, 1.1) # -1 + self.assertEqual(lead_4_country_freq.won_count, 0.1) # -1 + self.assertEqual(lead_4_email_state_freq.won_count, 1.1) # -1 + self.assertEqual(lead_4_stage_0_freq.lost_count, 2.1) # unchanged + self.assertEqual(lead_4_stage_won_freq.lost_count, 0.1) # unchanged + self.assertEqual(lead_4_country_freq.lost_count, 1.1) # unchanged + self.assertEqual(lead_4_email_state_freq.lost_count, 2.1) # unchanged + + # force proba to 0% -> as already archived, will be lost (lost = archived AND 0%) + leads[4].probability = 0 + leads[4].invalidate_cache() self.assertEqual(lead_4_stage_0_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_stage_won_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_country_freq.won_count, 0.1) # unchanged self.assertEqual(lead_4_email_state_freq.won_count, 1.1) # unchanged - self.assertEqual(lead_4_stage_0_freq.lost_count, 3.1) # unchanged - self.assertEqual(lead_4_stage_won_freq.lost_count, 1.1) # unchanged - self.assertEqual(lead_4_country_freq.lost_count, 2.1) # unchanged - self.assertEqual(lead_4_email_state_freq.lost_count, 3.1) # unchanged + self.assertEqual(lead_4_stage_0_freq.lost_count, 3.1) # +1 + self.assertEqual(lead_4_stage_won_freq.lost_count, 0.1) # unchanged - should not increase lost frequency of won stage. + consider stages with <= sequence when lost + self.assertEqual(lead_4_country_freq.lost_count, 2.1) # +1 + self.assertEqual(lead_4_email_state_freq.lost_count, 3.1) # +1 # Restore -> Should decrease lost - at the end, frequencies should be like first frequencyes tests (except for 0.0 -> 0.1) leads[4].toggle_active() + leads[4].invalidate_cache() self.assertEqual(lead_4_stage_0_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_stage_won_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_country_freq.won_count, 0.1) # unchanged self.assertEqual(lead_4_email_state_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_stage_0_freq.lost_count, 2.1) # - 1 - self.assertEqual(lead_4_stage_won_freq.lost_count, 1.1) # unchanged - consider stages with <= sequence when lost + self.assertEqual(lead_4_stage_won_freq.lost_count, 0.1) # unchanged - consider stages with <= sequence when lost self.assertEqual(lead_4_country_freq.lost_count, 1.1) # - 1 self.assertEqual(lead_4_email_state_freq.lost_count, 2.1) # - 1 # Probabilities should only be recomputed after modifying the lead itself. leads[3].stage_id = stage_ids[0] # probability should only change a bit as frequencies are almost the same (except 0.0 -> 0.1) leads[8].stage_id = stage_ids[0] # probability should change quite a lot + leads.invalidate_cache() # Test frequencies (should not have changed) self.assertEqual(lead_4_stage_0_freq.won_count, 1.1) # unchanged @@ -345,7 +369,7 @@ def test_predictive_lead_scoring(self): self.assertEqual(lead_4_country_freq.won_count, 0.1) # unchanged self.assertEqual(lead_4_email_state_freq.won_count, 1.1) # unchanged self.assertEqual(lead_4_stage_0_freq.lost_count, 2.1) # unchanged - self.assertEqual(lead_4_stage_won_freq.lost_count, 1.1) # unchanged + self.assertEqual(lead_4_stage_won_freq.lost_count, 0.1) # unchanged self.assertEqual(lead_4_country_freq.lost_count, 1.1) # unchanged self.assertEqual(lead_4_email_state_freq.lost_count, 2.1) # unchanged @@ -360,6 +384,7 @@ def test_predictive_lead_scoring(self): # Continue to test probability computation leads[3].probability = 40 + leads[3].invalidate_cache() self.assertEqual(leads[3].is_automated_probability, False) self.assertEqual(leads[8].is_automated_probability, True) diff --git a/addons/crm/views/crm_lead_views.xml b/addons/crm/views/crm_lead_views.xml index 277d07edb8083..e4650f2d8b1f1 100644 --- a/addons/crm/views/crm_lead_views.xml +++ b/addons/crm/views/crm_lead_views.xml @@ -9,14 +9,18 @@