diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 2552fbedad..228067a67c 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1441,6 +1441,31 @@ class CourseEnrollment(models.Model): def enrollments_for_user(cls, user): 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. diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 5f47eb11c7..dd24d47d9b 100644 --- a/common/djangoapps/student/views.py +++ b/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 diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index ccd4698db8..346afd26ec 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/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. diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py index a8dff4fe9f..6fcfb45336 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests.py +++ b/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