[17.0][MIG] sales_team_security_crm: Migration to 17.0 #3429

lef-adhoc · 2024-11-25T19:08:56Z

I migrated the module through this PR #2461

cvinh

LGTM

carlos-lopez-tecnativa

LGTM just a minor change

TT52441

@Tecnativa

carlos-lopez-tecnativa · 2025-01-07T16:57:31Z

sales_team_security_crm/security/security.xml

+        <field name="model_id" ref="crm.model_crm_lead" />
+        <field
+            name="domain_force"
+        >['|', '|', ('user_id', '=', user.id), ('user_id', '=', False), '|', ('team_id', '=', user.sale_team_id.id),


I think this can be simplified to: What do you think?

Suggested change

>['|', '|', ('user_id', '=', user.id), ('user_id', '=', False), '|', ('team_id', '=', user.sale_team_id.id),

>['|', ('user_id', 'in', [user.id, False]), ('team_id', 'in', [user.sale_team_id.id, False]),

Moreover, you must take into account the multi team that has been introduced in odoo v15. In that case, user.sale_team_id can be false if a user is in several teams (ie https://github.com/odoo/odoo/blob/b7abee1f64d6812d66c5ac61d1d8f841310c81d7/addons/sales_team/models/res_users.py#L32)
IMHO the correct rule should be
['|', '|', ('user_id', 'in', [user.id, False]), ('team_id', '=', user.sale_team_id.id), ('team_id', 'in', user.crm_team_ids.ids)]

in our case, we did not want the users in Team Only group to access to leads without a user_id so we finally did
['|', '|', ('user_id', '=', user.id), ('team_id', '=', user.sale_team_id.id), ('team_id', 'in', user.crm_team_ids.ids)]

The original module should include leads without user.

The original module should include leads without user.

You are right considering that odoo standard has the same policy with crm ('my document only' group includes lead without user)
Anyway, the rule can be customized after on.
But natively, please include crm_team_ids because if not, the rule does not work out of the box in multi-team configuration

carlos-lopez-tecnativa · 2025-01-07T16:58:39Z

sales_team_security_crm/security/security.xml

+        <field name="model_id" ref="crm.model_crm_activity_report" />
+        <field
+            name="domain_force"
+        >['|', '|', ('user_id', '=', user.id), ('user_id', '=', False), '|', ('team_id', '=', user.sale_team_id.id),


the same here

same here
['|', '|', ('user_id', 'in', [user.id, False]), ('team_id', '=', user.sale_team_id.id), ('team_id', 'in', user.crm_team_ids.ids)]

cvinh · 2025-01-08T22:33:54Z

sales_team_security_crm/security/security.xml

+        <field name="model_id" ref="crm.model_crm_lead" />
+        <field
+            name="domain_force"
+        >['|', '|', ('user_id', '=', user.id), ('user_id', '=', False), '|', ('team_id', '=', user.sale_team_id.id),


Moreover, you must take into account the multi team that has been introduced in odoo v15. In that case, user.sale_team_id can be false if a user is in several teams (ie https://github.com/odoo/odoo/blob/b7abee1f64d6812d66c5ac61d1d8f841310c81d7/addons/sales_team/models/res_users.py#L32)
IMHO the correct rule should be
['|', '|', ('user_id', 'in', [user.id, False]), ('team_id', '=', user.sale_team_id.id), ('team_id', 'in', user.crm_team_ids.ids)]

cvinh · 2025-01-08T22:33:57Z

sales_team_security_crm/security/security.xml

+        <field name="model_id" ref="crm.model_crm_activity_report" />
+        <field
+            name="domain_force"
+        >['|', '|', ('user_id', '=', user.id), ('user_id', '=', False), '|', ('team_id', '=', user.sale_team_id.id),


same here
['|', '|', ('user_id', 'in', [user.id, False]), ('team_id', '=', user.sale_team_id.id), ('team_id', 'in', user.crm_team_ids.ids)]

cvinh · 2025-01-08T22:35:16Z

sales_team_security_crm/security/security.xml

+    <record id="sales_team_team_rule" model="ir.rule">
+        <field name="name">Own Sale Teams</field>
+        <field name="model_id" ref="sales_team.model_crm_team" />
+        <field name="domain_force">[('id', '=', user.sale_team_id.id)]</field>


and here
['|', ('id', '=', user.sale_team_id.id), ('id', 'in', user.crm_team_ids.ids)]

lef-adhoc · 2025-01-09T16:40:51Z

@cvinh the tests fail with the domains you gave me, could you check it?

cvinh · 2025-01-09T17:41:19Z

@cvinh the tests fail with the domains you gave me, could you check it?

Humm there should be something to do in sales_team_security

cvinh · 2025-01-09T17:56:13Z

sales_team_security_crm/tests/test_sales_team_security_crm.py

+                "team_id": cls.team.id,
+            }
+        )
+


Tests with the case multi-team should be added and see why this one fails

cvinh · 2025-01-09T17:57:16Z

@cvinh the tests fail with the domains you gave me, could you check it?

Can you commit again to see where it fails ?

lef-adhoc · 2025-01-09T18:11:53Z

@cvinh The failure occurs in the following line https://github.com/adhoc-dev/sale-workflow/blob/4cd5e8211dceceece3b54121c9b0e6c7c7420e63/sales_team_security/tests/common.py#L88

When searching with the self.user in the following line it does not find anything https://github.com/adhoc-dev/sale-workflow/blob/4cd5e8211dceceece3b54121c9b0e6c7c7420e63/sales_team_security/tests/common.py#L69

lef-adhoc · 2025-01-10T11:54:59Z

@cvinh I think that's it, I also added a multi-team test, what do you think?

cvinh

I think if a user is in one team only, the crm_team_ids are empty
If I'm wrong then it's ok as you did

cvinh · 2025-01-10T17:08:26Z

sales_team_security_crm/security/security.xml

+    <record id="sales_team_team_rule" model="ir.rule">
+        <field name="name">Own Sale Teams</field>
+        <field name="model_id" ref="sales_team.model_crm_team" />
+        <field name="domain_force">[('id', 'in', user.crm_team_ids.ids)]</field>


Suggested change

<field name="domain_force">[('id', 'in', user.crm_team_ids.ids)]</field>

<field name="domain_force">['|', ('id', '=', 'user.sale_team_id.id'), ('id', 'in', user.crm_team_ids.ids)]</field>

ivantodorovich and others added 7 commits November 25, 2024 15:01

[ADD] sales_team_security_crm

a6173d1

[IMP] sales_team_security: Auto-followers - Adapt to 14.0

de23f1e

[MIG] sales_team_security_crm: Migration to 15.0

50aef32

[UPD] Update sales_team_security_crm.pot

26dd399

[UPD] README.rst

86830cd

[MIG] sales_team_security_crm: Migration to 16.0

63d96fb

[IMP] sales_team_security_crm: pre-commit execution

3451ee1

lef-adhoc force-pushed the 17.0-mig-sales_team_security_crm branch from b4b9174 to 5dbaa96 Compare December 12, 2024 13:36

cvinh approved these changes Dec 17, 2024

View reviewed changes

carlos-lopez-tecnativa suggested changes Jan 7, 2025

View reviewed changes

cvinh suggested changes Jan 8, 2025

View reviewed changes

lef-adhoc force-pushed the 17.0-mig-sales_team_security_crm branch 3 times, most recently from 205ce85 to 4cd5e82 Compare January 9, 2025 16:40

cvinh reviewed Jan 9, 2025

View reviewed changes

lef-adhoc force-pushed the 17.0-mig-sales_team_security_crm branch from 4cd5e82 to f0d2461 Compare January 10, 2025 11:51

[MIG] sales_team_security_crm: Migration to 17.0

5825dd1

lef-adhoc force-pushed the 17.0-mig-sales_team_security_crm branch from f0d2461 to 5825dd1 Compare January 10, 2025 12:11

cvinh reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[17.0][MIG] sales_team_security_crm: Migration to 17.0 #3429

[17.0][MIG] sales_team_security_crm: Migration to 17.0 #3429

lef-adhoc commented Nov 25, 2024 •

edited

Loading

cvinh left a comment

carlos-lopez-tecnativa left a comment

carlos-lopez-tecnativa Jan 7, 2025

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 9, 2025

pedrobaeza Jan 9, 2025

cvinh Jan 9, 2025 •

edited

Loading

carlos-lopez-tecnativa Jan 7, 2025

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

lef-adhoc commented Jan 9, 2025

cvinh commented Jan 9, 2025

cvinh Jan 9, 2025

cvinh commented Jan 9, 2025

lef-adhoc commented Jan 9, 2025 •

edited

Loading

lef-adhoc commented Jan 10, 2025

cvinh left a comment

cvinh Jan 10, 2025 •

edited

Loading

	>['\|', '\|', ('user_id', '=', user.id), ('user_id', '=', False), '\|', ('team_id', '=', user.sale_team_id.id),
	>['\|', ('user_id', 'in', [user.id, False]), ('team_id', 'in', [user.sale_team_id.id, False]),

	<field name="domain_force">[('id', 'in', user.crm_team_ids.ids)]</field>
	<field name="domain_force">['\|', ('id', '=', 'user.sale_team_id.id'), ('id', 'in', user.crm_team_ids.ids)]</field>

[17.0][MIG] sales_team_security_crm: Migration to 17.0 #3429

Are you sure you want to change the base?

[17.0][MIG] sales_team_security_crm: Migration to 17.0 #3429

Conversation

lef-adhoc commented Nov 25, 2024 • edited Loading

cvinh left a comment

Choose a reason for hiding this comment

carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

carlos-lopez-tecnativa Jan 7, 2025

Choose a reason for hiding this comment

cvinh Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

cvinh Jan 9, 2025

Choose a reason for hiding this comment

pedrobaeza Jan 9, 2025

Choose a reason for hiding this comment

cvinh Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

carlos-lopez-tecnativa Jan 7, 2025

Choose a reason for hiding this comment

cvinh Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

cvinh Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

cvinh Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

cvinh Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

lef-adhoc commented Jan 9, 2025

cvinh commented Jan 9, 2025

cvinh Jan 9, 2025

Choose a reason for hiding this comment

cvinh commented Jan 9, 2025

lef-adhoc commented Jan 9, 2025 • edited Loading

lef-adhoc commented Jan 10, 2025

cvinh left a comment

Choose a reason for hiding this comment

cvinh Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

lef-adhoc commented Nov 25, 2024 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 9, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

cvinh Jan 8, 2025 •

edited

Loading

lef-adhoc commented Jan 9, 2025 •

edited

Loading

cvinh Jan 10, 2025 •

edited

Loading