From 741a7e376e563d1c59bec5cb84bcd9a7d06bbf4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 7 Jun 2022 07:21:55 +0000 Subject: [PATCH 1/4] [IMP] sales_team: improve default team fetch tests Purpose of this commit is to improve a bit tests about default team computation. Coverage is improved, notably about default context value usage that is about to be updated. Task-2852947 X-original-commit: 8e18a27352d2fd9e68cd2b54e49dfca6fb658eaa Part-of: odoo/odoo#94045 --- addons/sales_team/tests/test_sales_team.py | 109 +++++++++++++++++---- 1 file changed, 92 insertions(+), 17 deletions(-) diff --git a/addons/sales_team/tests/test_sales_team.py b/addons/sales_team/tests/test_sales_team.py index 3556f5ade6c..ad563bb998b 100644 --- a/addons/sales_team/tests/test_sales_team.py +++ b/addons/sales_team/tests/test_sales_team.py @@ -13,6 +13,26 @@ class TestDefaultTeam(TestSalesCommon): def setUpClass(cls): """Set up data for default team tests.""" super(TestDefaultTeam, cls).setUpClass() + cls.env['ir.config_parameter'].set_param('sales_team.membership_multi', True) + + # Salesmen organization + # ------------------------------------------------------------ + # Role: M (team member) R (team manager) + # SALESMAN---------------sales_team_1---C2Team1---LowSequ---Team3 + # admin------------------M-------------- --------- --------- + # user_sales_manager-----R-------------- --------- ---------R + # user_sales_leads-------M-------------- ---------M--------- + # user_sales_salesman----/-------------- --------- --------- + + # Sales teams organization + # ------------------------------------------------------------ + # SALESTEAM-----------SEQU-----COMPANY + # LowSequence---------0--------False + # C2Team1-------------1--------C2 + # Team3---------------3--------Main + # sales_team_1--------5--------False + # data----------------9999-----?? + cls.company_2 = cls.env['res.company'].create({ 'name': 'New Test Company', 'email': 'company.2@test.example.com', @@ -22,35 +42,31 @@ class TestDefaultTeam(TestSalesCommon): 'name': 'C2 Team1', 'sequence': 1, 'company_id': cls.company_2.id, + 'user_id': False, }) cls.team_sequence = cls.env['crm.team'].create({ - 'name': 'Team LowSequence', - 'sequence': 0, 'company_id': False, + 'name': 'Team LowSequence', + 'member_ids': [(4, cls.user_sales_leads.id)], + 'sequence': 0, + 'user_id': False, }) cls.team_responsible = cls.env['crm.team'].create({ + 'company_id': cls.company_main.id, 'name': 'Team 3', 'user_id': cls.user_sales_manager.id, 'sequence': 3, - 'company_id': cls.company_main.id }) - def test_default_team_member(self): - with self.with_user('user_sales_leads'): - team = self.env['crm.team']._get_default_team_id() - self.assertEqual(team, self.sales_team_1) - - # responsible with lower sequence better than member with higher sequence - self.team_responsible.user_id = self.user_sales_leads.id - with self.with_user('user_sales_leads'): - team = self.env['crm.team']._get_default_team_id() - self.assertEqual(team, self.team_responsible) - def test_default_team_fallback(self): - """ Test fallback: domain, order """ + """ Test fallbacks when computing default team without any memberships: + domain, order """ self.sales_team_1.member_ids = [(5,)] - self.sales_team_1.flush_model() + self.team_sequence.member_ids = [(5,)] + (self.sales_team_1 + self.team_sequence).flush_model() + self.assertFalse(self.env['crm.team.member'].search([('user_id', '=', self.user_sales_leads.id)])) + # default is better sequence matching company criterion with self.with_user('user_sales_leads'): team = self.env['crm.team']._get_default_team_id() self.assertEqual(team, self.team_sequence) @@ -66,11 +82,70 @@ class TestDefaultTeam(TestSalesCommon): 'company_id': self.company_2.id, }) # multi company: switch company - self.user_sales_leads.write({'company_id': self.company_2.id}) + self.user_sales_leads.write({ + 'company_id': self.company_2.id, + 'company_ids': [(4, self.company_2.id)], + }) with self.with_user('user_sales_leads'): team = self.env['crm.team']._get_default_team_id() self.assertEqual(team, self.team_c2) + def test_default_team_member(self): + """ Test default team choice based on sequence, when having several + possible choices due to membership """ + with self.with_user('user_sales_leads'): + team = self.env['crm.team']._get_default_team_id() + self.assertEqual(team, self.team_sequence) + + self.team_sequence.member_ids = [(5,)] + self.team_sequence.flush_model() + with self.with_user('user_sales_leads'): + team = self.env['crm.team']._get_default_team_id() + self.assertEqual(team, self.sales_team_1) + + # responsible with lower sequence better than member with higher sequence + self.team_responsible.user_id = self.user_sales_leads.id + with self.with_user('user_sales_leads'): + team = self.env['crm.team']._get_default_team_id() + self.assertEqual(team, self.team_responsible) + + # in case of same sequence: take latest team + self.team_responsible.sequence = self.sales_team_1.sequence + with self.with_user('user_sales_leads'): + team = self.env['crm.team']._get_default_team_id() + self.assertEqual(team, self.team_responsible) + + def test_default_team_wcontext(self): + """ Test default team choice when having a value in context """ + with self.with_user('user_sales_leads'): + team = self.env['crm.team']._get_default_team_id() + self.assertEqual(team, self.team_sequence) + + team = self.env['crm.team'].with_context( + default_team_id=self.sales_team_1.id + )._get_default_team_id() + self.assertEqual( + team, self.team_sequence, + 'SalesTeam: default not taken into account if member / responsible' + ) + + # remove all memberships + self.sales_team_1.member_ids = [(5,)] + self.team_sequence.member_ids = [(5,)] + (self.sales_team_1 + self.team_sequence).flush_model() + self.assertFalse(self.env['crm.team.member'].search([('user_id', '=', self.user_sales_leads.id)])) + + with self.with_user('user_sales_leads'): + team = self.env['crm.team']._get_default_team_id() + self.assertEqual(team, self.team_sequence) + + team = self.env['crm.team'].with_context( + default_team_id=self.sales_team_1.id + )._get_default_team_id() + self.assertEqual( + team, self.sales_team_1, + 'SalesTeam: default taken into account when no member / responsible' + ) class TestMultiCompany(TestSalesMC): """Tests to check multi company management with sales team and their From deb66c90d86fbe94926f97df463e8f13167db339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 7 Jun 2022 07:22:42 +0000 Subject: [PATCH 2/4] [FIX] sales_team: ensure determinism when finding sales team Current sales team ordering is based on sequence. However several sales teams with same sequence may exist. This leads to a not deterministic behavior as we are unsure how database will choose the ordering. This is fixed with this commit: when having teams with same sequence we take the newest one first. Task-2852947 X-original-commit: 34da99f6bec92553febfae7bc9c80ccebb38f70f Part-of: odoo/odoo#94045 --- addons/sales_team/models/crm_team.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/sales_team/models/crm_team.py b/addons/sales_team/models/crm_team.py index 8dc485d4873..43f13185192 100644 --- a/addons/sales_team/models/crm_team.py +++ b/addons/sales_team/models/crm_team.py @@ -17,7 +17,7 @@ class CrmTeam(models.Model): _name = "crm.team" _inherit = ['mail.thread'] _description = "Sales Team" - _order = "sequence" + _order = "sequence ASC, create_date DESC, id DESC" _check_company_auto = True def _get_default_team_id(self, user_id=None, domain=None): @@ -52,7 +52,7 @@ class CrmTeam(models.Model): team = self.env['crm.team'] teams = self.env['crm.team'].search([ ('company_id', 'in', valid_cids), - '|', ('user_id', '=', user.id), ('member_ids', 'in', [user.id]), + '|', ('user_id', '=', user.id), ('member_ids', 'in', [user.id]) ]) if teams and domain: team = teams.filtered_domain(domain)[:1] From ac6957700a4456f6e666be93086370462c99bdff Mon Sep 17 00:00:00 2001 From: mafo-odoo Date: Thu, 2 Jun 2022 12:29:39 +0000 Subject: [PATCH 3/4] [FIX] sales_team: better support default_team_id Purpose of this commit is to better support default_team_id in context when computing default team. Indeed currently default key is ignored if the user is member or responsible of any team. It is used only when no membership exists as a fallback. However when giving a default_team_id in a default-like method we think we should better try to match this value. When having memberships the final team is the default one if present in the subset of teams. Instead of taking the first found one in the teams set (filtered on a domain or not) we first check for the default team presence, then fallback on the ordering based on sequence. Task-2852947 opw-2830913 X-original-commit: fb060c86daf0c1d737abc9228002cdfa280e1d2c Part-of: odoo/odoo#94045 --- addons/sales_team/models/crm_team.py | 34 +++++++++++++++------- addons/sales_team/tests/test_sales_team.py | 4 +-- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/addons/sales_team/models/crm_team.py b/addons/sales_team/models/crm_team.py index 43f13185192..756da3846ee 100644 --- a/addons/sales_team/models/crm_team.py +++ b/addons/sales_team/models/crm_team.py @@ -25,13 +25,16 @@ class CrmTeam(models.Model): method is not called by default_get as it takes some additional parameters and is meant to be called by other default methods. - Heuristic (when multiple match: take first sequence ordered) + Heuristic (when multiple match: take from default context value or first + sequence ordered) - 1- any of my teams (member OR responsible) matching domain - 2- any of my teams (member OR responsible) + 1- any of my teams (member OR responsible) matching domain, either from + context or based on _order; + 2- any of my teams (member OR responsible), either from context or based + on _order; 3- default from context - 4- any team matching my company and domain - 5- any team matching my company + 4- any team matching my company and domain (based on company rule) + 5- any team matching my company (based on company rule) Note: ResPartner.team_id field is explicitly not taken into account. We think this field causes a lot of noises compared to its added value. @@ -45,6 +48,9 @@ class CrmTeam(models.Model): user = self.env.user else: user = self.env['res.users'].sudo().browse(user_id) + default_team = self.env['crm.team'].browse( + self.env.context['default_team_id'] + ) if self.env.context.get('default_team_id') else self.env['crm.team'] valid_cids = [False] + [c for c in user.company_ids.ids if c in self.env.companies.ids] # 1- find in user memberships - note that if current user in C1 searches @@ -55,20 +61,28 @@ class CrmTeam(models.Model): '|', ('user_id', '=', user.id), ('member_ids', 'in', [user.id]) ]) if teams and domain: - team = teams.filtered_domain(domain)[:1] + filtered_teams = teams.filtered_domain(domain) + if default_team and default_team in filtered_teams: + team = default_team + else: + team = filtered_teams[:1] # 2- any of my teams if not team: - team = teams[:1] + if default_team and default_team in teams: + team = default_team + else: + team = teams[:1] # 3- default: context - if not team and 'default_team_id' in self.env.context: - team = self.env['crm.team'].browse(self.env.context.get('default_team_id')) + if not team and default_team: + team = default_team - # 4- default: first one matching domain, then first one if not team: teams = self.env['crm.team'].search([('company_id', 'in', valid_cids)]) + # 4- default: based on company rule, first one matching domain if teams and domain: team = teams.filtered_domain(domain)[:1] + # 5- default: based on company rule, first one if not team: team = teams[:1] diff --git a/addons/sales_team/tests/test_sales_team.py b/addons/sales_team/tests/test_sales_team.py index ad563bb998b..06953959e31 100644 --- a/addons/sales_team/tests/test_sales_team.py +++ b/addons/sales_team/tests/test_sales_team.py @@ -125,8 +125,8 @@ class TestDefaultTeam(TestSalesCommon): default_team_id=self.sales_team_1.id )._get_default_team_id() self.assertEqual( - team, self.team_sequence, - 'SalesTeam: default not taken into account if member / responsible' + team, self.sales_team_1, + 'SalesTeam: default takes over ordering when member / responsible' ) # remove all memberships From 6a0ab5f3c66f985f35a192a12fc97a2a3be25909 Mon Sep 17 00:00:00 2001 From: mafo-odoo Date: Mon, 2 May 2022 09:49:16 +0000 Subject: [PATCH 4/4] [FIX] sale,sales_team: set lead team when creating quotation Steps to reproduce: -Install sales and crm -Apply multiteam option in crm settings -Create a user with two teams A and B -Create with this user a lead with team B -From this lead create a quotation Current behavior: The quotation could have team A (the team of the lead is ignored) Expected behavior: The quotation always has team B Problem when creating the quotation the team id is not propagated in the context to the final function _get_default_team_id and in this function the default context team is only check if no teams have been found for the user, so the function always selects the first team of the user. To solve the issue we propagate the team id to the default function and we change it so that if the context default team is in the user teams it is this one that is going to be selected and no other. opw-2830913 X-original-commit: 0b6c63f2be90928f5d6806ef489c369a1310be04 Part-of: odoo/odoo#94045 --- addons/sale/models/sale_order.py | 2 +- addons/sales_team/models/crm_team.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/addons/sale/models/sale_order.py b/addons/sale/models/sale_order.py index 28f79b0c3a9..ba3362fe7fa 100644 --- a/addons/sale/models/sale_order.py +++ b/addons/sale/models/sale_order.py @@ -409,7 +409,7 @@ class SaleOrder(models.Model): def _compute_team_id(self): cached_teams = {} for order in self: - default_team_id = (order.team_id or order.partner_id.team_id).id + default_team_id = self.env.context.get('default_team_id', False) or order.team_id.id or order.partner_id.team_id.id user_id = order.user_id.id company_id = order.company_id.id key = (default_team_id, user_id, company_id) diff --git a/addons/sales_team/models/crm_team.py b/addons/sales_team/models/crm_team.py index 756da3846ee..ad28f94ffe2 100644 --- a/addons/sales_team/models/crm_team.py +++ b/addons/sales_team/models/crm_team.py @@ -66,6 +66,7 @@ class CrmTeam(models.Model): team = default_team else: team = filtered_teams[:1] + # 2- any of my teams if not team: if default_team and default_team in teams: