Prefetch course modes used in has_access calls in course api

This commit is contained in:
Calen Pennington
2019-01-22 22:19:19 -05:00
parent a842921e1f
commit 0d273e9506
7 changed files with 77 additions and 20 deletions

View File

@@ -312,7 +312,7 @@ class CourseMode(models.Model):
@classmethod
@request_cached(CACHE_NAMESPACE)
def modes_for_course(cls, course_id, include_expired=False, only_selectable=True):
def modes_for_course(cls, course_id=None, include_expired=False, only_selectable=True, course=None):
"""
Returns a list of the non-expired modes for a given course id
@@ -330,11 +330,25 @@ class CourseMode(models.Model):
aren't available to users until they complete the course, so
they are hidden in track selection.)
course (CourseOverview): The course to select course modes from.
Returns:
list of `Mode` tuples
"""
found_course_modes = cls.objects.filter(course_id=course_id)
if course_id is None and course is None:
raise ValueError("One of course_id or course must not be None.")
if course is not None and not isinstance(course, CourseOverview):
# CourseModules don't have the data needed to pull related modes,
# so we'll fall back on course_id-based lookup instead
course_id = course.id
course = None
if course_id is not None:
found_course_modes = cls.objects.filter(course_id=course_id)
else:
found_course_modes = course.modes
# Filter out expired course modes if include_expired is not set
if not include_expired:
@@ -347,7 +361,10 @@ class CourseMode(models.Model):
# we exclude them from the list if we're only looking for selectable modes
# (e.g. on the track selection page or in the payment/verification flows).
if only_selectable:
found_course_modes = found_course_modes.exclude(mode_slug__in=cls.CREDIT_MODES)
if course is not None and hasattr(course, 'selectable_modes'):
found_course_modes = course.selectable_modes
else:
found_course_modes = found_course_modes.exclude(mode_slug__in=cls.CREDIT_MODES)
modes = ([mode.to_tuple() for mode in found_course_modes])
if not modes:
@@ -356,7 +373,7 @@ class CourseMode(models.Model):
return modes
@classmethod
def modes_for_course_dict(cls, course_id, modes=None, **kwargs):
def modes_for_course_dict(cls, course_id=None, modes=None, **kwargs):
"""Returns the non-expired modes for a particular course.
Arguments:
@@ -418,7 +435,7 @@ class CourseMode(models.Model):
return None
@classmethod
def verified_mode_for_course(cls, course_id, modes=None, include_expired=False):
def verified_mode_for_course(cls, course_id=None, modes=None, include_expired=False, course=None):
"""Find a verified mode for a particular course.
Since we have multiple modes that can go through the verify flow,
@@ -439,7 +456,12 @@ class CourseMode(models.Model):
Mode or None
"""
modes_dict = cls.modes_for_course_dict(course_id, modes=modes, include_expired=include_expired)
modes_dict = cls.modes_for_course_dict(
course_id=course_id,
modes=modes,
include_expired=include_expired,
course=course
)
verified_mode = modes_dict.get('verified', None)
professional_mode = modes_dict.get('professional', None)
# we prefer professional over verify
@@ -703,7 +725,7 @@ class CourseMode(models.Model):
Takes a mode model and turns it into a model named tuple.
Returns:
A 'Model' namedtuple with all the same attributes as the model.
A 'Mode' namedtuple with all the same attributes as the model.
"""
return Mode(

View File

@@ -392,7 +392,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
self.setup_user(self.audit_user)
# These query counts were found empirically
query_counts = [122, 140, 170, 200, 230, 260, 290, 320, 350, 380, 327]
query_counts = [93, 81, 81, 81, 81, 81, 81, 81, 81, 81, 25]
ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])])
self.clear_caches()

View File

@@ -23,7 +23,9 @@ from courseware.date_summary import (
from courseware.masquerade import check_content_start_date_for_masquerade_user
from courseware.model_data import FieldDataCache
from courseware.module_render import get_module
from course_modes.models import CourseMode
from django.conf import settings
from django.db.models import Prefetch
from django.urls import reverse
from django.http import Http404, QueryDict
from enrollment.api import get_course_enrollment_details
@@ -454,7 +456,16 @@ def get_courses(user, org=None, filter_=None):
"""
Return a LazySequence of courses available, optionally filtered by org code (case-insensitive).
"""
courses = branding.get_visible_courses(org=org, filter_=filter_)
courses = branding.get_visible_courses(
org=org,
filter_=filter_,
).prefetch_related(
Prefetch(
'modes',
queryset=CourseMode.objects.exclude(mode_slug__in=CourseMode.CREDIT_MODES),
to_attr='selectable_modes',
)
)
permission_name = configuration_helpers.get_value(
'COURSE_CATALOG_VISIBILITY_PERMISSION',

View File

@@ -66,7 +66,9 @@ def get_user_course_expiration_date(user, course):
access_duration = MIN_DURATION
if not CourseMode.verified_mode_for_course(course.id, include_expired=True):
verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True)
if not verified_mode:
return None
enrollment = CourseEnrollment.get_enrollment(user, course.id)

View File

@@ -63,7 +63,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
def test_enrollment_mode(self):
"""Tests that verified enrollments do not have an expiration"""
CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED)
result = get_user_course_expiration_date(self.user, self.course)
result = get_user_course_expiration_date(self.user, CourseOverview.get_from_id(self.course.id))
self.assertEqual(result, None)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -94,7 +94,10 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
self.course.self_paced = True
mock_get_course_run_details.return_value = {'weeks_to_complete': weeks_to_complete}
enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT)
result = get_user_course_expiration_date(self.user, self.course)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(self.course.id),
)
self.assertEqual(result, enrollment.created + access_duration)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -109,11 +112,17 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
start_date = now() - timedelta(weeks=10)
past_course = CourseFactory(start=start_date)
enrollment = CourseEnrollment.enroll(self.user, past_course.id, CourseMode.AUDIT)
result = get_user_course_expiration_date(self.user, past_course)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(past_course.id),
)
self.assertEqual(result, None)
add_course_mode(past_course, upgrade_deadline_expired=False)
result = get_user_course_expiration_date(self.user, past_course)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(past_course.id),
)
content_availability_date = enrollment.created
self.assertEqual(result, content_availability_date + access_duration)
@@ -121,12 +130,18 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
start_date = now() + timedelta(weeks=10)
future_course = CourseFactory(start=start_date)
enrollment = CourseEnrollment.enroll(self.user, future_course.id, CourseMode.AUDIT)
result = get_user_course_expiration_date(self.user, future_course)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(future_course.id),
)
self.assertEqual(result, None)
add_course_mode(future_course, upgrade_deadline_expired=False)
result = get_user_course_expiration_date(self.user, future_course)
content_availability_date = start_date
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(future_course.id),
)
content_availability_date = start_date.replace(microsecond=0)
self.assertEqual(result, content_availability_date + access_duration)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -141,7 +156,10 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
course = CourseFactory(start=start_date)
enrollment = CourseEnrollment.enroll(self.user, course.id, CourseMode.AUDIT)
add_course_mode(course, upgrade_deadline_expired=True)
result = get_user_course_expiration_date(self.user, course)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(course.id),
)
content_availability_date = enrollment.created
self.assertEqual(result, content_availability_date + access_duration)

View File

@@ -204,7 +204,7 @@ class TestCourseHomePage(CourseHomePageTestCase):
# Fetch the view and verify the query counts
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(87, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(89, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_home_url(self.course)
self.client.get(url)

View File

@@ -25,6 +25,7 @@ from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.courseware.views.views import CourseTabView
from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
from openedx.core.djangoapps.util.maintenance_banner import add_maintenance_banner
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.features.course_experience.course_tools import CourseToolsPluginManager
from openedx.features.course_duration_limits.access import generate_course_expired_fragment
from student.models import CourseEnrollment
@@ -153,7 +154,10 @@ class CourseHomeFragmentView(EdxFragmentView):
course_sock_fragment = CourseSockFragmentView().render_to_fragment(request, course=course, **kwargs)
has_visited_course, resume_course_url = self._get_resume_course_info(request, course_id)
handouts_html = self._get_course_handouts(request, course)
course_expiration_fragment = generate_course_expired_fragment(request.user, course)
course_expiration_fragment = generate_course_expired_fragment(
request.user,
CourseOverview.get_from_id(course.id)
)
elif allow_public_outline or allow_public:
outline_fragment = CourseOutlineFragmentView().render_to_fragment(
request, course_id=course_id, user_is_enrolled=False, **kwargs