From 3ed821be69176cc5f7d8bfa4e7bd60101a86d7b4 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Wed, 13 Nov 2019 11:48:09 +0000 Subject: [PATCH 1/4] [FIX] base_user_role: Improve tests resiliency These 2 tests were checking the exact set of groups a user should have. If these tests are ran in a database where a module is previously installed which adds more groups to the base role, these exact group sets would be inexact, although the behavior that is being tested was actually properly working. With this patch, basically I'm testing if the user contains the groups from the roles, not the exact role set expected. It should work in integration scenarios. @Tecnativa TT20468 --- base_user_role/tests/test_user_role.py | 56 +++++++++++++++++--------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/base_user_role/tests/test_user_role.py b/base_user_role/tests/test_user_role.py index 6f640227..2b0d863c 100644 --- a/base_user_role/tests/test_user_role.py +++ b/base_user_role/tests/test_user_role.py @@ -137,12 +137,17 @@ class TestUserRole(TransactionCase): self.assertEqual(user_group_ids, role_group_ids) def test_role_unlink(self): - # Get role1 groups - role1_group_ids = ( - self.role1_id.implied_ids.ids + self.role1_id.trans_implied_ids.ids + # Get role1 and role2 groups + role1_groups = ( + self.role1_id.implied_ids + | self.role1_id.trans_implied_ids + | self.role1_id.group_id + ) + role2_groups = ( + self.role2_id.implied_ids + | self.role2_id.trans_implied_ids + | self.role2_id.group_id ) - role1_group_ids.append(self.role1_id.group_id.id) - role1_group_ids = sorted(set(role1_group_ids)) # Configure the user with role1 and role2 self.user_id.write( @@ -153,22 +158,32 @@ class TestUserRole(TransactionCase): ] } ) + # Check user has groups from role1 and role2 + self.assertLessEqual(role1_groups, self.user_id.groups_id) + self.assertLessEqual(role2_groups, self.user_id.groups_id) # Remove role2 self.role2_id.unlink() - user_group_ids = sorted({group.id for group in self.user_id.groups_id}) - self.assertEqual(user_group_ids, role1_group_ids) + # Check user has groups from only role1 + self.assertLessEqual(role1_groups, self.user_id.groups_id) + self.assertFalse(role2_groups <= self.user_id.groups_id) # Remove role1 self.role1_id.unlink() - user_group_ids = sorted({group.id for group in self.user_id.groups_id}) - self.assertEqual(user_group_ids, []) + # Check user has no groups from role1 and role2 + self.assertFalse(role1_groups <= self.user_id.groups_id) + self.assertFalse(role2_groups <= self.user_id.groups_id) def test_role_line_unlink(self): - # Get role1 groups - role1_group_ids = ( - self.role1_id.implied_ids.ids + self.role1_id.trans_implied_ids.ids + # Get role1 and role2 groups + role1_groups = ( + self.role1_id.implied_ids + | self.role1_id.trans_implied_ids + | self.role1_id.group_id + ) + role2_groups = ( + self.role2_id.implied_ids + | self.role2_id.trans_implied_ids + | self.role2_id.group_id ) - role1_group_ids.append(self.role1_id.group_id.id) - role1_group_ids = sorted(set(role1_group_ids)) # Configure the user with role1 and role2 self.user_id.write( @@ -179,18 +194,23 @@ class TestUserRole(TransactionCase): ] } ) + # Check user has groups from role1 and role2 + self.assertLessEqual(role1_groups, self.user_id.groups_id) + self.assertLessEqual(role2_groups, self.user_id.groups_id) # Remove role2 from the user self.user_id.role_line_ids.filtered( lambda rl: rl.role_id.id == self.role2_id.id ).unlink() - user_group_ids = sorted({group.id for group in self.user_id.groups_id}) - self.assertEqual(user_group_ids, role1_group_ids) + # Check user has groups from only role1 + self.assertLessEqual(role1_groups, self.user_id.groups_id) + self.assertFalse(role2_groups <= self.user_id.groups_id) # Remove role1 from the user self.user_id.role_line_ids.filtered( lambda rl: rl.role_id.id == self.role1_id.id ).unlink() - user_group_ids = sorted({group.id for group in self.user_id.groups_id}) - self.assertEqual(user_group_ids, []) + # Check user has no groups from role1 and role2 + self.assertFalse(role1_groups <= self.user_id.groups_id) + self.assertFalse(role2_groups <= self.user_id.groups_id) def test_default_user_roles(self): self.default_user.write( From 45ba0950b980e514b61ea683e09af1e530e662cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Alix?= Date: Wed, 28 Sep 2022 09:39:59 +0200 Subject: [PATCH 2/4] fixup! [FIX] base_user_role: Improve tests resiliency --- base_user_role/tests/test_user_role.py | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/base_user_role/tests/test_user_role.py b/base_user_role/tests/test_user_role.py index 2b0d863c..86c9fd42 100644 --- a/base_user_role/tests/test_user_role.py +++ b/base_user_role/tests/test_user_role.py @@ -138,16 +138,8 @@ class TestUserRole(TransactionCase): def test_role_unlink(self): # Get role1 and role2 groups - role1_groups = ( - self.role1_id.implied_ids - | self.role1_id.trans_implied_ids - | self.role1_id.group_id - ) - role2_groups = ( - self.role2_id.implied_ids - | self.role2_id.trans_implied_ids - | self.role2_id.group_id - ) + role1_groups = self.role1_id.trans_implied_ids | self.role1_id.group_id + role2_groups = self.role2_id.trans_implied_ids | self.role2_id.group_id # Configure the user with role1 and role2 self.user_id.write( @@ -174,16 +166,8 @@ class TestUserRole(TransactionCase): def test_role_line_unlink(self): # Get role1 and role2 groups - role1_groups = ( - self.role1_id.implied_ids - | self.role1_id.trans_implied_ids - | self.role1_id.group_id - ) - role2_groups = ( - self.role2_id.implied_ids - | self.role2_id.trans_implied_ids - | self.role2_id.group_id - ) + role1_groups = self.role1_id.trans_implied_ids | self.role1_id.group_id + role2_groups = self.role2_id.trans_implied_ids | self.role2_id.group_id # Configure the user with role1 and role2 self.user_id.write( From 57cdd8886820a764c6025b7b4e42c94df041a0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Thu, 1 Feb 2024 13:17:46 +0100 Subject: [PATCH 3/4] [IMP] base_user_role: Improve tests to avoid false errors (ids in different order) --- base_user_role/tests/test_user_role.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/base_user_role/tests/test_user_role.py b/base_user_role/tests/test_user_role.py index 86c9fd42..63444c01 100644 --- a/base_user_role/tests/test_user_role.py +++ b/base_user_role/tests/test_user_role.py @@ -261,16 +261,16 @@ class TestUserRole(TransactionCase): self.assertFalse(self.user_id.show_alert) def test_group_groups_into_role(self): - user_group_ids = [group.id for group in self.user_id.groups_id] + user_group_ids = self.user_id.groups_id.ids # Check that there is not a role with name: Test Role self.assertFalse(self.role_model.search([("name", "=", "Test Role")])) # Call create_role function to group groups into a role wizard = self.wiz_model.with_context(active_ids=user_group_ids).create( {"name": "Test Role"} ) - wizard.create_role() + res = wizard.create_role() # Check that a role with name: Test Role has been created - new_role = self.role_model.search([("name", "=", "Test Role")]) - self.assertTrue(new_role) - # Check that the role has the correct groups - self.assertEqual(new_role.implied_ids.ids, user_group_ids) + new_role = self.env[res["res_model"]].browse(res["res_id"]) + self.assertEqual(new_role.name, "Test Role") + # Check that the role has the correct groups (even if the order is not equal) + self.assertEqual(set(new_role.implied_ids.ids), set(user_group_ids)) From b2fd4424fc053db521db4604a20dbec010c663a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Alix?= Date: Tue, 28 Jan 2025 14:30:38 +0100 Subject: [PATCH 4/4] oca-port: blacklist PR(s) 237, 262 for base_user_role --- .oca/oca-port/blacklist/base_user_role.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .oca/oca-port/blacklist/base_user_role.json diff --git a/.oca/oca-port/blacklist/base_user_role.json b/.oca/oca-port/blacklist/base_user_role.json new file mode 100644 index 00000000..c9b46850 --- /dev/null +++ b/.oca/oca-port/blacklist/base_user_role.json @@ -0,0 +1,6 @@ +{ + "pull_requests": { + "OCA/server-backend#237": "(auto) Nothing to port from PR #237", + "OCA/server-backend#262": "(auto) Nothing to port from PR #262" + } +}