fix: apply feedback related to test and types

Author: MaferMazu

cms/djangoapps/contentstore/tests/test_course_listing.py

@@ -21,7 +21,6 @@
     _accessible_courses_iter_for_tests,
     _accessible_courses_list_from_groups,
     _accessible_courses_summary_iter,
-    _get_authz_accessible_courses_list,
     get_courses_accessible_to_user,
 )
 from common.djangoapps.course_action_state.models import CourseRerunState
@@ -718,57 +717,118 @@ def test_superuser_gets_all_courses(self):
 
             self.assertEqual(result_ids, expected_ids)
 
-    def test_get_authz_accessible_courses_list_with_org_scope(self):
+    def test_course_listing_with_org_scope(self):
         """
-        When the scope is an OrgCourseOverviewGlobData, all course keys
-        returned by `_get_course_keys_for_org_scope` should be included in
-        the resulting authz keys when the authz toggle is enabled.
+        Verify that assigning a course role like course_staff with an org-wide scope
+        (`course-v1:Org1+*`) grants access to all courses in that org when
+        the AuthZ course authoring toggle is enabled.
         """
-        authz_keys, _, authz_courses, _ = self._create_courses()
+        _, _, authz_courses, legacy_courses = self._create_courses()
         org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*')
+        assign_role_to_user_in_scope(
+            self.authorized_user.username,
+            COURSE_STAFF.external_key,
+            org_scope.external_key,
+        )
 
         request = self._make_request(self.authorized_user)
 
-        with patch(
-            "cms.djangoapps.contentstore.views.course.get_scopes_for_user_and_permission",
-            return_value=[org_scope],
-        ), patch(
-            "cms.djangoapps.contentstore.views.course._get_course_keys_for_org_scope",
-            return_value=authz_keys,
-        ) as mock_get_keys, patch.object(
+        with patch.object(
             core_toggles.AUTHZ_COURSE_AUTHORING_FLAG,
             "is_enabled",
             return_value=True,
         ):
-            result = _get_authz_accessible_courses_list(request)
+            courses, _ = get_courses_accessible_to_user(request)
 
-            mock_get_keys.assert_called_once_with(org_scope)
-            self.assertEqual(result, set(authz_keys))
+            result_ids = {c.id for c in courses}
+
+            expected_ids = {
+                *(c.id for c in authz_courses),
+                *(c.id for c in legacy_courses),
+            }
+
+            self.assertEqual(result_ids, expected_ids)
 
-    def test_get_authz_accessible_courses_list_org_scope_respects_toggle(self):
+    def test_course_listing_with_org_scope_with_toggle(self):
         """
         If the authz toggle is enabled only for a subset of org courses, only
-        those course keys should appear in the resulting authz keys.
+        those course keys should appear in the resulting course list.
         """
-        authz_keys, _, authz_courses, _ = self._create_courses()
+        authz_keys, _, _, _ = self._create_courses()
         # enable only the first and third course keys
         enabled_keys = {str(authz_keys[0]), str(authz_keys[2])}
         org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*')
+        assign_role_to_user_in_scope(
+            self.authorized_user.username,
+            COURSE_STAFF.external_key,
+            org_scope.external_key,
+        )
 
         request = self._make_request(self.authorized_user)
 
-        with patch(
-            "cms.djangoapps.contentstore.views.course.get_scopes_for_user_and_permission",
-            return_value=[org_scope],
-        ), patch(
-            "cms.djangoapps.contentstore.views.course._get_course_keys_for_org_scope",
-            return_value=authz_keys,
-        ), patch.object(
+        with patch.object(
             core_toggles.AUTHZ_COURSE_AUTHORING_FLAG,
             "is_enabled",
             side_effect=self._mock_authz_toggle(enabled_keys),
         ):
-            result = _get_authz_accessible_courses_list(request)
+            courses, _ = get_courses_accessible_to_user(request)
+
+            result_ids = {c.id for c in courses}
 
             expected = {authz_keys[0], authz_keys[2]}
-            self.assertEqual(result, expected)
+            self.assertEqual(result_ids, expected)
+
+    def test_course_listing_with_org_scope_without_courses(self):
+        """
+        When the scope is an OrgCourseOverviewGlobData for an org that has no
+        courses, `get_courses_accessible_to_user` should return an empty
+        list.
+        """
+        org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*')
+        assign_role_to_user_in_scope(
+            self.authorized_user.username,
+            COURSE_STAFF.external_key,
+            org_scope.external_key,
+        )
+
+        request = self._make_request(self.authorized_user)
+
+        with patch.object(
+            core_toggles.AUTHZ_COURSE_AUTHORING_FLAG,
+            "is_enabled",
+            return_value=True,
+        ):
+            courses, _ = get_courses_accessible_to_user(request)
+
+            self.assertEqual(courses, [])
+
+    def test_course_listing_with_org_scope_fetched_once(self):
+        """
+        Verify that course overviews are fetched once with all authorized orgs.
+        """
+        org_scope1 = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*')
+        org_scope2 = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*')
+        assign_role_to_user_in_scope(
+            self.authorized_user.username,
+            COURSE_STAFF.external_key,
+            org_scope1.external_key,
+        )
+        assign_role_to_user_in_scope(
+            self.authorized_user.username,
+            COURSE_STAFF.external_key,
+            org_scope2.external_key,
+        )
+
+        request = self._make_request(self.authorized_user)
+
+        with patch.object(
+            core_toggles.AUTHZ_COURSE_AUTHORING_FLAG,
+            "is_enabled",
+            return_value=True,
+        ), patch.object(
+            CourseOverview,
+            "get_all_courses",
+        ) as mock_get_all_courses:
+            courses, _ = get_courses_accessible_to_user(request)
+
+        mock_get_all_courses.assert_called_once_with(orgs={"Org1", "Org2"})

cms/djangoapps/contentstore/views/course.py

@@ -817,12 +817,12 @@ def filter_course(course):
     return filter(filter_course, filtered_courses)
 
 
-def _get_course_keys_for_org_scope(access):
+def _get_course_keys_for_org_scope(org_keys: set[str]):
     """
-    Function to convert an OrgCourseOverviewGlobData scope
-    into specific course keys.
+    Convert a set of organization keys into specific course keys.
     """
-    return CourseOverview.get_all_courses(orgs=[access.org]).values_list('id', flat=True)
+
+    return CourseOverview.get_all_courses(orgs=org_keys).values_list('id', flat=True)
 
 
 def _get_authz_accessible_courses_list(request):
@@ -836,18 +836,21 @@ def _get_authz_accessible_courses_list(request):
         COURSES_VIEW_COURSE.identifier
     )
 
-    authz_keys = set()
+    course_keys = set()
+    org_keys = set()
     for access in authz_scopes:
         if isinstance(access, CourseOverviewData) and access.course_key:
             if core_toggles.enable_authz_course_authoring(access.course_key):
-                authz_keys.add(access.course_key)
+                course_keys.add(access.course_key)
         elif isinstance(access, OrgCourseOverviewGlobData) and access.org:
-            authz_keys.update(
-                key for key in _get_course_keys_for_org_scope(access)
-                if core_toggles.enable_authz_course_authoring(key)
-            )
+            org_keys.add(access.org)
+    if org_keys:
+        course_keys.update(
+            key for key in _get_course_keys_for_org_scope(org_keys)
+            if core_toggles.enable_authz_course_authoring(key)
+        )
 
-    return authz_keys
+    return course_keys
 
 
 def _get_legacy_accessible_courses_list(request):