Merge pull request #14546 from edx/ormsbee/cache_overviews_on_enrollments

Student Dashboard CourseOverviews with one query.

common/djangoapps/student/models.py

@@ -1442,6 +1442,31 @@ class CourseEnrollment(models.Model):
         return cls.objects.filter(user=user, is_active=1).select_related('user')
 
     @classmethod
+    def enrollments_for_user_with_overviews_preload(cls, user):  # pylint: disable=invalid-name
+        """
+        List of user's CourseEnrollments, CourseOverviews preloaded if possible.
+
+        We try to preload all CourseOverviews, which are usually lazily loaded
+        as the .course_overview property. This is to avoid making an extra
+        query for every enrollment when displaying something like the student
+        dashboard. If some of the CourseOverviews are not found, we make no
+        attempt to initialize them -- we just fall back to existing lazy-load
+        behavior. The goal is to optimize the most common case as simply as
+        possible, without changing any of the existing contracts.
+
+        The name of this method is long, but was the end result of hashing out a
+        number of alternatives, so pylint can stuff it (disable=invalid-name)
+        """
+        enrollments = list(cls.enrollments_for_user(user))
+        overviews = CourseOverview.get_from_ids_if_exists(
+            enrollment.course_id for enrollment in enrollments
+        )
+        for enrollment in enrollments:
+            enrollment._course_overview = overviews.get(enrollment.course_id)  # pylint: disable=protected-access
+
+        return enrollments
+
+    @classmethod
     def enrollment_status_hash_cache_key(cls, user):
         """ Returns the cache key for the cached enrollment status hash.
 

common/djangoapps/student/views.py

@@ -301,7 +301,7 @@ def get_course_enrollments(user, orgs_to_include, orgs_to_exclude):
         generator[CourseEnrollment]: a sequence of enrollments to be displayed
         on the user's dashboard.
     """
-    for enrollment in CourseEnrollment.enrollments_for_user(user):
+    for enrollment in CourseEnrollment.enrollments_for_user_with_overviews_preload(user):
 
         # If the course is missing or broken, log an error and skip it.
         course_overview = enrollment.course_overview

openedx/core/djangoapps/content/course_overviews/models.py

@@ -274,6 +274,27 @@ class CourseOverview(TimeStampedModel):
 
         return course_overview or cls.load_from_module_store(course_id)
 
+    @classmethod
+    def get_from_ids_if_exists(cls, course_ids):
+        """
+        Return a dict mapping course_ids to CourseOverviews, if they exist.
+
+        This method will *not* generate new CourseOverviews or delete outdated
+        ones. It exists only as a small optimization used when CourseOverviews
+        are known to exist, for common situations like the student dashboard.
+
+        Callers should assume that this list is incomplete and fall back to
+        get_from_id if they need to guarantee CourseOverview generation.
+        """
+        return {
+            overview.id: overview
+            for overview
+            in cls.objects.select_related('image_set').filter(
+                id__in=course_ids,
+                version__gte=cls.VERSION
+            )
+        }
+
     def clean_id(self, padding_char='='):
         """
         Returns a unique deterministic base32-encoded ID for the course.

openedx/core/djangoapps/content/course_overviews/tests.py

@@ -515,6 +515,35 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
                 "testing CourseOverview.get_all_courses with filter_={}".format(filter_),
             )
 
+    def test_get_from_ids_if_exists(self):
+        course_with_overview_1 = CourseFactory.create(emit_signals=True)
+        course_with_overview_2 = CourseFactory.create(emit_signals=True)
+        course_without_overview = CourseFactory.create(emit_signals=False)
+
+        courses = [course_with_overview_1, course_with_overview_2, course_without_overview]
+        course_ids_to_overviews = CourseOverview.get_from_ids_if_exists(
+            course.id for course in courses
+        )
+
+        # We should see the ones that have published CourseOverviews
+        # (when the signals were emitted), but not the one that didn't issue
+        # a publish signal.
+        self.assertEqual(len(course_ids_to_overviews), 2)
+        self.assertIn(course_with_overview_1.id, course_ids_to_overviews)
+        self.assertIn(course_with_overview_2.id, course_ids_to_overviews)
+        self.assertNotIn(course_without_overview.id, course_ids_to_overviews)
+
+        # But if we set a CourseOverview to be an old version, it shouldn't be
+        # returned by get_from_ids_if_exists()
+        overview_2 = course_ids_to_overviews[course_with_overview_2.id]
+        overview_2.version = CourseOverview.VERSION - 1
+        overview_2.save()
+        course_ids_to_overviews = CourseOverview.get_from_ids_if_exists(
+            course.id for course in courses
+        )
+        self.assertEqual(len(course_ids_to_overviews), 1)
+        self.assertIn(course_with_overview_1.id, course_ids_to_overviews)
+
 
 @attr(shard=3)
 @ddt.ddt