From 19d4e96ed3e64d1f1313dc213de3f9a40f3b9684 Mon Sep 17 00:00:00 2001 From: Julian Palmerio Date: Tue, 21 Nov 2023 17:00:23 -0300 Subject: [PATCH] feat: remove org parameter from course_org helper functions --- .../core/djangoapps/course_roles/helpers.py | 12 ++--- .../course_roles/tests/test_helpers.py | 47 ++++++++----------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/openedx/core/djangoapps/course_roles/helpers.py b/openedx/core/djangoapps/course_roles/helpers.py index 89a4c839b3e6..efd89f9809b8 100644 --- a/openedx/core/djangoapps/course_roles/helpers.py +++ b/openedx/core/djangoapps/course_roles/helpers.py @@ -117,8 +117,7 @@ def user_has_permission_list_org( def user_has_permission_course_org( user: Union[User, AnonymousUser], permission: Union[CourseRolesPermission, str], - course_key: CourseKey, - organization_name: str = None + course_key: CourseKey ): """ Check if a user has a permission for all courses in an organization or for a specific course. @@ -129,18 +128,15 @@ def user_has_permission_course_org( return False if isinstance(permission, CourseRolesPermission): permission = permission.value.name - if organization_name is None: - organization_name = course_key.org return (user_has_permission_course(user, permission, course_key) or - user_has_permission_org(user, permission, organization_name) + user_has_permission_org(user, permission, course_key.org) ) def user_has_permission_list_course_org( user: Union[User, AnonymousUser], permissions: List[Union[CourseRolesPermission, str]], - course_key: CourseKey, - organization_name: str = None + course_key: CourseKey ): """ Check if a user has all of the given permissions for all courses in an organization or for a specific course. @@ -148,7 +144,7 @@ def user_has_permission_list_course_org( if not use_permission_checks(): return False return all( - user_has_permission_course_org(user, permission, course_key, organization_name) + user_has_permission_course_org(user, permission, course_key) for permission in permissions ) diff --git a/openedx/core/djangoapps/course_roles/tests/test_helpers.py b/openedx/core/djangoapps/course_roles/tests/test_helpers.py index cae950ef2506..07e86ae5c655 100644 --- a/openedx/core/djangoapps/course_roles/tests/test_helpers.py +++ b/openedx/core/djangoapps/course_roles/tests/test_helpers.py @@ -308,7 +308,7 @@ def test_user_has_permission_course_org_with_anonymous_user(self): Test that user_has_permission_course_org returns False when the user is anonymous """ assert not user_has_permission_course_org( - self.anonymous_user, self.permission_1.name, self.course_1_key, self.organization_1.name + self.anonymous_user, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_with_course_level_permission(self): @@ -320,7 +320,7 @@ def test_user_has_permission_course_org_with_course_level_permission(self): user=self.user_1, role=self.role_1, course_id=self.course_1_key, org=self.organization_1 ) assert user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_without_course_level_permission(self): @@ -329,7 +329,7 @@ def test_user_has_permission_course_org_without_course_level_permission(self): permission at the course level """ assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_with_organization_level_permission(self): @@ -339,25 +339,16 @@ def test_user_has_permission_course_org_with_organization_level_permission(self) """ UserRole.objects.create(user=self.user_1, role=self.role_1, org=self.organization_1) assert user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) - def test_user_has_permission_course_org_with_organization_level_permission_without_org_param(self): - """ - Test that user_has_permission_course_org returns True when the user has the correct permission at - the organization level - """ - UserRole.objects.create(user=self.user_1, role=self.role_1, org=self.organization_1) - assert user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key) - def test_user_has_permission_course_org_without_organization_level_permission(self): """ Test that user_has_permission_course_org returns False when the user does not have the correct permission at the organization level """ assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_with_instance_level_permission(self): @@ -367,7 +358,7 @@ def test_user_has_permission_course_org_with_instance_level_permission(self): """ UserRole.objects.create(user=self.user_1, role=self.role_1) assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_with_permission_in_another_course(self): @@ -379,7 +370,7 @@ def test_user_has_permission_course_org_with_permission_in_another_course(self): user=self.user_1, role=self.role_1, course_id=self.course_2.id, org=self.organization_1 ) assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_with_permission_in_another_organization(self): @@ -389,7 +380,7 @@ def test_user_has_permission_course_org_with_permission_in_another_organization( """ UserRole.objects.create(user=self.user_1, role=self.role_1, org=self.organization_2) assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_user_has_permission_course_org_with_permission_in_another_course_and_organization(self): @@ -401,7 +392,7 @@ def test_user_has_permission_course_org_with_permission_in_another_course_and_or user=self.user_1, role=self.role_1, course_id=self.course_2.id, org=self.organization_2 ) assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) def test_course_or_organization_list_permission_check_with_course_level_permission(self): @@ -414,7 +405,7 @@ def test_course_or_organization_list_permission_check_with_course_level_permissi ) test_permissions = [self.permission_1.name, self.permission_2.name] assert user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_without_course_level_permission(self): @@ -427,7 +418,7 @@ def test_course_or_organization_list_permission_check_without_course_level_permi ) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_with_organization_level_permission(self): @@ -438,7 +429,7 @@ def test_course_or_organization_list_permission_check_with_organization_level_pe UserRole.objects.create(user=self.user_1, role=self.role_2, org=self.organization_1) test_permissions = [self.permission_1.name, self.permission_2.name] assert user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_without_organization_level_permission(self): @@ -449,7 +440,7 @@ def test_course_or_organization_list_permission_check_without_organization_level UserRole.objects.create(user=self.user_1, role=self.role_1, org=self.organization_1) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_with_instance_level_permission(self): @@ -460,7 +451,7 @@ def test_course_or_organization_list_permission_check_with_instance_level_permis UserRole.objects.create(user=self.user_1, role=self.role_2) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_with_permission_in_another_course(self): @@ -473,7 +464,7 @@ def test_course_or_organization_list_permission_check_with_permission_in_another ) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_with_permission_in_another_organization(self): @@ -484,7 +475,7 @@ def test_course_or_organization_list_permission_check_with_permission_in_another UserRole.objects.create(user=self.user_1, role=self.role_2, org=self.organization_2) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) def test_course_or_organization_list_permission_check_with_permission_in_another_course_and_organization(self): @@ -497,7 +488,7 @@ def test_course_or_organization_list_permission_check_with_permission_in_another ) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key ) @override_waffle_flag(USE_PERMISSION_CHECKS_FLAG, active=False) @@ -549,7 +540,7 @@ def test_user_has_permission_course_org_with_waffle_flag_disabled(self): user=self.user_1, role=self.role_1, course_id=self.course_1_key, org=self.organization_1 ) assert not user_has_permission_course_org( - self.user_1, self.permission_1.name, self.course_1_key, self.organization_1.name + self.user_1, self.permission_1.name, self.course_1_key ) @override_waffle_flag(USE_PERMISSION_CHECKS_FLAG, active=False) @@ -563,7 +554,7 @@ def test_course_or_organization_list_permission_check_with_waffle_flag_disabled( ) test_permissions = [self.permission_1.name, self.permission_2.name] assert not user_has_permission_list_course_org( - self.user_1, test_permissions, self.course_1_key, self.organization_1.name + self.user_1, test_permissions, self.course_1_key )