Bypass calls to has_access() when listing courses

When listing courses for which a user has "staff" level access, bypass
calls to has_access() and use a hopefully more performant and simpler
access pattern that uses only a single ORM query and far less processing
per course.

Caution: this short-circuit implementation does not handle org-level
access grants.

lms/djangoapps/course_api/api.py

@@ -18,8 +18,10 @@ from lms.djangoapps.courseware.courses import (
     get_courses,
     get_permission_for_course_about
 )
+from opaque_keys.edx.django.models import CourseKeyField
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.lib.api.view_utils import LazySequence
+from student.models import CourseAccessRole
 from student.roles import GlobalStaff
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -170,19 +172,40 @@ def list_course_keys(request, username, role):
     """
     user = get_effective_user(request.user, username)
 
-    course_keys = CourseOverview.get_all_course_keys()
+    all_course_keys = CourseOverview.get_all_course_keys()
 
     # Global staff have access to all courses. Filter courses for non-global staff.
     if GlobalStaff().has_user(user):
-        return course_keys
+        return all_course_keys
 
-    return LazySequence(
-        (
-            course_key for course_key in course_keys
-            if has_access(user, role, course_key)
-        ),
-        est_len=len(course_keys)
-    )
+    if role == 'staff':
+        # This short-circuit implementation bypasses has_access() which we think is too slow for some users when
+        # evaluating staff-level course access for Insights.  Various tickets have context on this issue: CR-2487,
+        # TNL-7448, DESUPPORT-416, and probably more.
+        #
+        # This is a simplified implementation that does not consider org-level access grants (e.g. when course_id is
+        # blank).
+        filtered_course_keys = (
+            CourseAccessRole.objects.filter(
+                user=user,
+                # Having the instructor role implies staff access. This fact was reverse-engineered from unit tests.
+                role__in=['staff', 'instructor'],
+            )
+            .exclude(course_id=CourseKeyField.Empty)
+            .order_by('course_id')
+            .values_list('course_id', flat=True)
+            .distinct()
+        )
+    else:
+        # This is the original implementation which still covers the case where role = "instructor":
+        filtered_course_keys = LazySequence(
+            (
+                course_key for course_key in all_course_keys
+                if has_access(user, role, course_key)
+            ),
+            est_len=len(all_course_keys)
+        )
+    return filtered_course_keys
 
 
 def get_due_dates(request, course_key, user):

lms/djangoapps/course_api/tests/test_views.py

@@ -459,9 +459,15 @@ class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase):
         add_users(self.global_admin, CourseStaffRole(self.course.id), course_staff_user)
 
         # Create a second course, along with an instructor user for it.
-        alternate_course = self.create_course(org='test')
+        alternate_course1 = self.create_course(org='test1')
         course_instructor_user = self.create_user(username='course_instructor', is_staff=False)
-        add_users(self.global_admin, CourseInstructorRole(alternate_course.id), course_instructor_user)
+        add_users(self.global_admin, CourseInstructorRole(alternate_course1.id), course_instructor_user)
+
+        # Create a third course, along with an user that has both staff and instructor for it.
+        alternate_course2 = self.create_course(org='test2')
+        course_instructor_staff_user = self.create_user(username='course_instructor_staff', is_staff=False)
+        add_users(self.global_admin, CourseInstructorRole(alternate_course2.id), course_instructor_staff_user)
+        add_users(self.global_admin, CourseStaffRole(alternate_course2.id), course_instructor_staff_user)
 
         # Requesting the courses for which the course staff user is staff should return *only* the single course.
         self.setup_user(self.staff_user)
@@ -485,7 +491,7 @@ class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase):
             'role': 'instructor'
         })
         self.assertEqual(len(filtered_response.data['results']), 1)
-        self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course.org))
+        self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course1.org))
 
         # The course instructor user has the inferred course staff role on one course.
         self.setup_user(course_instructor_user)
@@ -494,7 +500,16 @@ class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase):
             'role': 'staff'
         })
         self.assertEqual(len(filtered_response.data['results']), 1)
-        self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course.org))
+        self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course1.org))
+
+        # The user with both instructor AND staff on a course has the inferred course staff role on that one course.
+        self.setup_user(course_instructor_staff_user)
+        filtered_response = self.verify_response(params={
+            'username': course_instructor_staff_user.username,
+            'role': 'staff'
+        })
+        self.assertEqual(len(filtered_response.data['results']), 1)
+        self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course2.org))
 
 
 class LazyPageNumberPaginationTestCase(TestCase):