diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 06abbe7706..85c2b6def0 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -23,8 +23,9 @@ VERIFY_STATUS_MISSED_DEADLINE = "verify_missed_deadline" VERIFY_STATUS_NEED_TO_REVERIFY = "verify_need_to_reverify" -def check_verify_status_by_course(user, course_enrollment_pairs, all_course_modes): - """Determine the per-course verification statuses for a given user. +def check_verify_status_by_course(user, course_enrollments, all_course_modes): + """ + Determine the per-course verification statuses for a given user. The possible statuses are: * VERIFY_STATUS_NEED_TO_VERIFY: The student has not yet submitted photos for verification. @@ -46,8 +47,7 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode Arguments: user (User): The currently logged-in user. - course_enrollment_pairs (list): The courses the user is enrolled in. - The list should contain tuples of `(Course, CourseEnrollment)`. + course_enrollments (list[CourseEnrollment]): The courses the user is enrolled in. all_course_modes (list): List of all course modes for the student's enrolled courses, including modes that have expired. @@ -75,15 +75,15 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode recent_verification_datetime = None - for course, enrollment in course_enrollment_pairs: + for enrollment in course_enrollments: # Get the verified mode (if any) for this course # We pass in the course modes we have already loaded to avoid # another database hit, as well as to ensure that expired # course modes are included in the search. verified_mode = CourseMode.verified_mode_for_course( - course.id, - modes=all_course_modes[course.id] + enrollment.course_id, + modes=all_course_modes[enrollment.course_id] ) # If no verified mode has ever been offered, or the user hasn't enrolled @@ -156,7 +156,7 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode if deadline is not None and deadline > now: days_until_deadline = (deadline - now).days - status_by_course[course.id] = { + status_by_course[enrollment.course_id] = { 'status': status, 'days_until_deadline': days_until_deadline } diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 0e9f5566fb..23abdd071c 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -850,6 +850,13 @@ class CourseEnrollment(models.Model): unique_together = (('user', 'course_id'),) ordering = ('user', 'course_id') + def __init__(self, *args, **kwargs): + super(CourseEnrollment, self).__init__(*args, **kwargs) + + # Private variable for storing course_overview to minimize calls to the database. + # When the property .course_overview is accessed for the first time, this variable will be set. + self._course_overview = None + def __unicode__(self): return ( "[CourseEnrollment] {}: {} ({}); active: ({})" @@ -1318,10 +1325,21 @@ class CourseEnrollment(models.Model): @property def course_overview(self): """ - Return a CourseOverview of this enrollment's course. - """ - from openedx.core.djangoapps.content.course_overviews.models import CourseOverview - return CourseOverview.get_from_id(self.course_id) + Returns a CourseOverview of the course to which this enrollment refers. + Returns None if an error occurred while trying to load the course. + + Note: + If the course is re-published within the lifetime of this + CourseEnrollment object, then the value of this property will + become stale. + """ + if not self._course_overview: + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + try: + self._course_overview = CourseOverview.get_from_id(self.course_id) + except (CourseOverview.DoesNotExist, IOError): + self._course_overview = None + return self._course_overview def is_verified_enrollment(self): """ diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 0e6bdd7299..cdf75a342b 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -14,7 +14,7 @@ from xmodule.modulestore.django import modulestore from xmodule.error_module import ErrorDescriptor from django.test.client import Client from student.models import CourseEnrollment -from student.views import get_course_enrollment_pairs +from student.views import get_course_enrollments from util.milestones_helpers import ( get_pre_requisite_courses_not_completed, set_prerequisite_courses, @@ -73,13 +73,13 @@ class TestCourseListing(ModuleStoreTestCase): self._create_course_with_access_groups(course_location) # get dashboard - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 1) - self.assertEqual(courses_list[0][0].id, course_location) + self.assertEqual(courses_list[0].course_id, course_location) CourseEnrollment.unenroll(self.student, course_location) # get dashboard - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 0) def test_errored_course_regular_access(self): @@ -95,7 +95,7 @@ class TestCourseListing(ModuleStoreTestCase): self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor) # get courses through iterating all courses - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(courses_list, []) def test_course_listing_errored_deleted_courses(self): @@ -112,9 +112,9 @@ class TestCourseListing(ModuleStoreTestCase): self._create_course_with_access_groups(course_location, default_store=ModuleStoreEnum.Type.mongo) mongo_store.delete_course(course_location, ModuleStoreEnum.UserID.test) - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 1, courses_list) - self.assertEqual(courses_list[0][0].id, good_location) + self.assertEqual(courses_list[0].course_id, good_location) @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) def test_course_listing_has_pre_requisite_courses(self): @@ -142,9 +142,11 @@ class TestCourseListing(ModuleStoreTestCase): set_prerequisite_courses(course_location, pre_requisite_courses) # get dashboard - course_enrollment_pairs = list(get_course_enrollment_pairs(self.student, None, [])) - courses_having_prerequisites = frozenset(course.id for course, _enrollment in course_enrollment_pairs - if course.pre_requisite_courses) + course_enrollments = list(get_course_enrollments(self.student, None, [])) + courses_having_prerequisites = frozenset( + enrollment.course_id for enrollment in course_enrollments + if enrollment.course_overview.pre_requisite_courses + ) courses_requirements_not_met = get_pre_requisite_courses_not_completed( self.student, courses_having_prerequisites diff --git a/common/djangoapps/student/tests/test_recent_enrollments.py b/common/djangoapps/student/tests/test_recent_enrollments.py index 618c378477..07516af01a 100644 --- a/common/djangoapps/student/tests/test_recent_enrollments.py +++ b/common/djangoapps/student/tests/test_recent_enrollments.py @@ -15,7 +15,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from course_modes.tests.factories import CourseModeFactory from student.models import CourseEnrollment, DashboardConfiguration -from student.views import get_course_enrollment_pairs, _get_recently_enrolled_courses +from student.views import get_course_enrollments, _get_recently_enrolled_courses # pylint: disable=protected-access @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -67,7 +67,7 @@ class TestRecentEnrollments(ModuleStoreTestCase): self._configure_message_timeout(60) # get courses through iterating all courses - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 2) recent_course_list = _get_recently_enrolled_courses(courses_list) @@ -78,7 +78,7 @@ class TestRecentEnrollments(ModuleStoreTestCase): Tests that the recent enrollment list is empty if configured to zero seconds. """ self._configure_message_timeout(0) - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 2) recent_course_list = _get_recently_enrolled_courses(courses_list) @@ -106,16 +106,16 @@ class TestRecentEnrollments(ModuleStoreTestCase): enrollment.save() courses.append(course) - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) + courses_list = list(get_course_enrollments(self.student, None, [])) self.assertEqual(len(courses_list), 6) recent_course_list = _get_recently_enrolled_courses(courses_list) self.assertEqual(len(recent_course_list), 5) - self.assertEqual(recent_course_list[1][0], courses[0]) - self.assertEqual(recent_course_list[2][0], courses[1]) - self.assertEqual(recent_course_list[3][0], courses[2]) - self.assertEqual(recent_course_list[4][0], courses[3]) + self.assertEqual(recent_course_list[1].course, courses[0]) + self.assertEqual(recent_course_list[2].course, courses[1]) + self.assertEqual(recent_course_list[3].course, courses[2]) + self.assertEqual(recent_course_list[4].course, courses[3]) def test_dashboard_rendering(self): """ diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 82f4900907..e1575da5e9 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -1,9 +1,6 @@ # -*- coding: utf-8 -*- """ -This file demonstrates writing tests using the unittest module. These will pass -when you run "manage.py test". - -Replace this with more appropriate tests for your application. +Miscellaneous tests for the student app. """ from datetime import datetime, timedelta import logging @@ -28,8 +25,8 @@ from student.views import (process_survey_link, _cert_info, from student.tests.factories import UserFactory, CourseModeFactory from util.testing import EventTestMixin from util.model_utils import USER_SETTINGS_CHANGED_EVENT_NAME -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, ModuleStoreEnum # These imports refer to lms djangoapps. # Their testcases are only run under lms. @@ -193,6 +190,7 @@ class CourseEndingTest(TestCase): self.assertIsNone(_cert_info(user, course2, cert_status, course_mode)) +@ddt.ddt class DashboardTest(ModuleStoreTestCase): """ Tests for dashboard utility functions @@ -487,6 +485,48 @@ class DashboardTest(ModuleStoreTestCase): ) self.assertContains(response, expected_url) + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + @ddt.data((ModuleStoreEnum.Type.mongo, 1), (ModuleStoreEnum.Type.split, 3)) + @ddt.unpack + def test_dashboard_metadata_caching(self, modulestore_type, expected_mongo_calls): + """ + Check that the student dashboard makes use of course metadata caching. + + The first time the student dashboard displays a specific course, it will + make a call to the module store. After that first request, though, the + course's metadata should be cached as a CourseOverview. + + Arguments: + modulestore_type (ModuleStoreEnum.Type): Type of modulestore to create + test course in. + expected_mongo_calls (int >=0): Number of MongoDB queries expected for + a single call to the module store. + + Note to future developers: + If you break this test so that the "check_mongo_calls(0)" fails, + please do NOT change it to "check_mongo_calls(n>1)". Instead, change + your code to not load courses from the module store. This may + involve adding fields to CourseOverview so that loading a full + CourseDescriptor isn't necessary. + """ + # Create a course, log in the user, and enroll them in the course. + test_course = CourseFactory.create(default_store=modulestore_type) + self.client.login(username="jack", password="test") + CourseEnrollment.enroll(self.user, test_course.id) + + # The first request will result in a modulestore query. + with check_mongo_calls(expected_mongo_calls): + response_1 = self.client.get(reverse('dashboard')) + self.assertEquals(response_1.status_code, 200) + + # Subsequent requests will only result in SQL queries to load the + # CourseOverview object that has been created. + with check_mongo_calls(0): + response_2 = self.client.get(reverse('dashboard')) + self.assertEquals(response_2.status_code, 200) + response_3 = self.client.get(reverse('dashboard')) + self.assertEquals(response_3.status_code, 200) + class UserSettingsEventTestMixin(EventTestMixin): """ diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 5a4e3aafb3..6a827fcd99 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -4,7 +4,6 @@ Student Views import datetime import logging import uuid -import time import json import warnings from datetime import timedelta @@ -115,7 +114,6 @@ from student.helpers import ( ) from student.cookies import set_logged_in_cookies, delete_logged_in_cookies from student.models import anonymous_id_for_user -from xmodule.error_module import ErrorDescriptor from shoppingcart.models import DonationConfiguration, CourseRegistrationCode from embargo import api as embargo_api @@ -185,33 +183,42 @@ def process_survey_link(survey_link, user): return survey_link.format(UNIQUE_ID=unique_id_for_user(user)) -def cert_info(user, course, course_mode): +def cert_info(user, course_overview, course_mode): """ Get the certificate info needed to render the dashboard section for the given - student and course. Returns a dictionary with keys: + student and course. - 'status': one of 'generating', 'ready', 'notpassing', 'processing', 'restricted' - 'show_download_url': bool - 'download_url': url, only present if show_download_url is True - 'show_disabled_download_button': bool -- true if state is 'generating' - 'show_survey_button': bool - 'survey_url': url, only if show_survey_button is True - 'grade': if status is not 'processing' + Arguments: + user (User): A user. + course_overview (CourseOverview): A course. + course_mode (str): The enrollment mode (honor, verified, audit, etc.) + + Returns: + dict: A dictionary with keys: + 'status': one of 'generating', 'ready', 'notpassing', 'processing', 'restricted' + 'show_download_url': bool + 'download_url': url, only present if show_download_url is True + 'show_disabled_download_button': bool -- true if state is 'generating' + 'show_survey_button': bool + 'survey_url': url, only if show_survey_button is True + 'grade': if status is not 'processing' """ - if not course.may_certify(): + if not course_overview.may_certify(): return {} - - return _cert_info(user, course, certificate_status_for_student(user, course.id), course_mode) + return _cert_info( + user, + course_overview, + certificate_status_for_student(user, course_overview.id), + course_mode + ) -def reverification_info(course_enrollment_pairs, user, statuses): +def reverification_info(statuses): """ Returns reverification-related information for *all* of user's enrollments whose - reverification status is in status_list + reverification status is in statuses. Args: - course_enrollment_pairs (list): list of (course, enrollment) tuples - user (User): the user whose information we want statuses (list): a list of reverification statuses we want information for example: ["must_reverify", "denied"] @@ -229,39 +236,56 @@ def reverification_info(course_enrollment_pairs, user, statuses): return reverifications -def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set): +def get_course_enrollments(user, org_to_include, orgs_to_exclude): """ - Get the relevant set of (Course, CourseEnrollment) pairs to be displayed on - a student's dashboard. + Given a user, return a filtered set of his or her course enrollments. + + Arguments: + user (User): the user in question. + org_to_include (str): for use in Microsites. If not None, ONLY courses + of this org will be returned. + orgs_to_exclude (list[str]): If org_to_include is not None, this + argument is ignored. Else, courses of this org will be excluded. + + Returns: + generator[CourseEnrollment]: a sequence of enrollments to be displayed + on the user's dashboard. """ for enrollment in CourseEnrollment.enrollments_for_user(user): - store = modulestore() - with store.bulk_operations(enrollment.course_id): - course = store.get_course(enrollment.course_id) - if course and not isinstance(course, ErrorDescriptor): - # if we are in a Microsite, then filter out anything that is not - # attributed (by ORG) to that Microsite - if course_org_filter and course_org_filter != course.location.org: - continue - # Conversely, if we are not in a Microsite, then let's filter out any enrollments - # with courses attributed (by ORG) to Microsites - elif course.location.org in org_filter_out_set: - continue + # If the course is missing or broken, log an error and skip it. + course_overview = enrollment.course_overview + if not course_overview: + log.error( + "User %s enrolled in broken or non-existent course %s", + user.username, + enrollment.course_id + ) + continue - yield (course, enrollment) - else: - log.error( - u"User %s enrolled in %s course %s", - user.username, - "broken" if course else "non-existent", - enrollment.course_id - ) + # If we are in a Microsite, then filter out anything that is not + # attributed (by ORG) to that Microsite. + if org_to_include and course_overview.location.org != org_to_include: + continue + + # Conversely, if we are not in a Microsite, then filter out any enrollments + # with courses attributed (by ORG) to Microsites. + elif course_overview.location.org in orgs_to_exclude: + continue + + # Else, include the enrollment. + else: + yield enrollment -def _cert_info(user, course, cert_status, course_mode): +def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disable=unused-argument """ Implements the logic for cert_info -- split out for testing. + + Arguments: + user (User): A user. + course_overview (CourseOverview): A course. + course_mode (str): The enrollment mode (honor, verified, audit, etc.) """ # simplify the status for the template using this lookup table template_state = { @@ -285,7 +309,7 @@ def _cert_info(user, course, cert_status, course_mode): is_hidden_status = cert_status['status'] in ('unavailable', 'processing', 'generating', 'notpassing') - if course.certificates_display_behavior == 'early_no_info' and is_hidden_status: + if course_overview.certificates_display_behavior == 'early_no_info' and is_hidden_status: return None status = template_state.get(cert_status['status'], default_status) @@ -299,20 +323,20 @@ def _cert_info(user, course, cert_status, course_mode): } if (status in ('generating', 'ready', 'notpassing', 'restricted') and - course.end_of_course_survey_url is not None): + course_overview.end_of_course_survey_url is not None): status_dict.update({ 'show_survey_button': True, - 'survey_url': process_survey_link(course.end_of_course_survey_url, user)}) + 'survey_url': process_survey_link(course_overview.end_of_course_survey_url, user)}) else: status_dict['show_survey_button'] = False if status == 'ready': # showing the certificate web view button if certificate is ready state and feature flags are enabled. - if has_html_certificates_enabled(course.id, course): - if get_active_web_certificate(course) is not None: + if has_html_certificates_enabled(course_overview.id, course_overview): + if course_overview.has_any_active_web_certificate: certificate_url = get_certificate_url( user_id=user.id, - course_id=unicode(course.id) + course_id=unicode(course_overview.id), ) status_dict.update({ 'show_cert_web_view': True, @@ -325,7 +349,7 @@ def _cert_info(user, course, cert_status, course_mode): log.warning( u"User %s has a downloadable cert for %s, but no download url", user.username, - course.id + course_overview.id ) return default_info else: @@ -337,8 +361,8 @@ def _cert_info(user, course, cert_status, course_mode): linkedin_config = LinkedInAddToProfileConfiguration.current() if linkedin_config.enabled: status_dict['linked_in_url'] = linkedin_config.add_to_profile_url( - course.id, - course.display_name, + course_overview.id, + course_overview.display_name, cert_status.get('mode'), cert_status['download_url'] ) @@ -506,13 +530,13 @@ def dashboard(request): # Build our (course, enrollment) list for the user, but ignore any courses that no # longer exist (because the course IDs have changed). Still, we don't delete those # enrollments, because it could have been a data push snafu. - course_enrollment_pairs = list(get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set)) + course_enrollments = list(get_course_enrollments(user, course_org_filter, org_filter_out_set)) # sort the enrollment pairs by the enrollment date - course_enrollment_pairs.sort(key=lambda x: x[1].created, reverse=True) + course_enrollments.sort(key=lambda x: x.created, reverse=True) # Retrieve the course modes for each course - enrolled_course_ids = [course.id for course, __ in course_enrollment_pairs] + enrolled_course_ids = [enrollment.course_id for enrollment in course_enrollments] all_course_modes, unexpired_course_modes = CourseMode.all_and_unexpired_modes_for_courses(enrolled_course_ids) course_modes_by_course = { course_id: { @@ -525,14 +549,9 @@ def dashboard(request): # Check to see if the student has recently enrolled in a course. # If so, display a notification message confirming the enrollment. enrollment_message = _create_recent_enrollment_message( - course_enrollment_pairs, course_modes_by_course + course_enrollments, course_modes_by_course ) - # Retrieve the course modes for each course - enrolled_courses_dict = {} - for course, __ in course_enrollment_pairs: - enrolled_courses_dict[unicode(course.id)] = course - course_optouts = Optout.objects.filter(user=user).values_list('course_id', flat=True) message = "" @@ -551,20 +570,20 @@ def dashboard(request): errored_courses = modulestore().get_errored_courses() show_courseware_links_for = frozenset( - course.id for course, _enrollment in course_enrollment_pairs - if has_access(request.user, 'load', course) - and has_access(request.user, 'view_courseware_with_prerequisites', course) + enrollment.course_id for enrollment in course_enrollments + if has_access(request.user, 'load', enrollment.course_overview) + and has_access(request.user, 'view_courseware_with_prerequisites', enrollment.course_overview) ) # Construct a dictionary of course mode information # used to render the course list. We re-use the course modes dict # we loaded earlier to avoid hitting the database. course_mode_info = { - course.id: complete_course_mode_info( - course.id, enrollment, - modes=course_modes_by_course[course.id] + enrollment.course_id: complete_course_mode_info( + enrollment.course_id, enrollment, + modes=course_modes_by_course[enrollment.course_id] ) - for course, enrollment in course_enrollment_pairs + for enrollment in course_enrollments } # Determine the per-course verification status @@ -583,20 +602,20 @@ def dashboard(request): # there is no verification messaging to display. verify_status_by_course = check_verify_status_by_course( user, - course_enrollment_pairs, + course_enrollments, all_course_modes ) cert_statuses = { - course.id: cert_info(request.user, course, _enrollment.mode) - for course, _enrollment in course_enrollment_pairs + enrollment.course_id: cert_info(request.user, enrollment.course_overview, enrollment.mode) + for enrollment in course_enrollments } # only show email settings for Mongo course and when bulk email is turned on show_email_settings_for = frozenset( - course.id for course, _enrollment in course_enrollment_pairs if ( + enrollment.course_id for enrollment in course_enrollments if ( settings.FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and - modulestore().get_modulestore_type(course.id) != ModuleStoreEnum.Type.xml and - CourseAuthorization.instructor_email_enabled(course.id) + modulestore().get_modulestore_type(enrollment.course_id) != ModuleStoreEnum.Type.xml and + CourseAuthorization.instructor_email_enabled(enrollment.course_id) ) ) @@ -606,16 +625,29 @@ def dashboard(request): # Gets data for midcourse reverifications, if any are necessary or have failed statuses = ["approved", "denied", "pending", "must_reverify"] - reverifications = reverification_info(course_enrollment_pairs, user, statuses) + reverifications = reverification_info(statuses) - show_refund_option_for = frozenset(course.id for course, _enrollment in course_enrollment_pairs - if _enrollment.refundable()) + show_refund_option_for = frozenset( + enrollment.course_id for enrollment in course_enrollments + if enrollment.refundable() + ) - block_courses = frozenset(course.id for course, enrollment in course_enrollment_pairs - if is_course_blocked(request, CourseRegistrationCode.objects.filter(course_id=course.id, registrationcoderedemption__redeemed_by=request.user), course.id)) + block_courses = frozenset( + enrollment.course_id for enrollment in course_enrollments + if is_course_blocked( + request, + CourseRegistrationCode.objects.filter( + course_id=enrollment.course_id, + registrationcoderedemption__redeemed_by=request.user + ), + enrollment.course_id + ) + ) - enrolled_courses_either_paid = frozenset(course.id for course, _enrollment in course_enrollment_pairs - if _enrollment.is_paid_course()) + enrolled_courses_either_paid = frozenset( + enrollment.course_id for enrollment in course_enrollments + if enrollment.is_paid_course() + ) # If there are *any* denied reverifications that have not been toggled off, # we'll display the banner @@ -625,8 +657,10 @@ def dashboard(request): order_history_list = order_history(user, course_org_filter=course_org_filter, org_filter_out_set=org_filter_out_set) # get list of courses having pre-requisites yet to be completed - courses_having_prerequisites = frozenset(course.id for course, _enrollment in course_enrollment_pairs - if course.pre_requisite_courses) + courses_having_prerequisites = frozenset( + enrollment.course_id for enrollment in course_enrollments + if enrollment.course_overview.pre_requisite_courses + ) courses_requirements_not_met = get_pre_requisite_courses_not_completed(user, courses_having_prerequisites) ccx_membership_triplets = [] @@ -638,7 +672,7 @@ def dashboard(request): context = { 'enrollment_message': enrollment_message, - 'course_enrollment_pairs': course_enrollment_pairs, + 'course_enrollments': course_enrollments, 'course_optouts': course_optouts, 'message': message, 'staff_access': staff_access, @@ -646,7 +680,7 @@ def dashboard(request): 'show_courseware_links_for': show_courseware_links_for, 'all_course_modes': course_mode_info, 'cert_statuses': cert_statuses, - 'credit_statuses': _credit_statuses(user, course_enrollment_pairs), + 'credit_statuses': _credit_statuses(user, course_enrollments), 'show_email_settings_for': show_email_settings_for, 'reverifications': reverifications, 'verification_status': verification_status, @@ -669,13 +703,15 @@ def dashboard(request): return render_to_response('dashboard.html', context) -def _create_recent_enrollment_message(course_enrollment_pairs, course_modes): - """Builds a recent course enrollment message +def _create_recent_enrollment_message(course_enrollments, course_modes): # pylint: disable=invalid-name + """ + Builds a recent course enrollment message. - Constructs a new message template based on any recent course enrollments for the student. + Constructs a new message template based on any recent course enrollments + for the student. Args: - course_enrollment_pairs (list): A list of tuples containing courses, and the associated enrollment information. + course_enrollments (list[CourseEnrollment]): a list of course enrollments. course_modes (dict): Mapping of course ID's to course mode dictionaries. Returns: @@ -683,16 +719,16 @@ def _create_recent_enrollment_message(course_enrollment_pairs, course_modes): None if there are no recently enrolled courses. """ - recently_enrolled_courses = _get_recently_enrolled_courses(course_enrollment_pairs) + recently_enrolled_courses = _get_recently_enrolled_courses(course_enrollments) if recently_enrolled_courses: messages = [ { - "course_id": course.id, - "course_name": course.display_name, - "allow_donation": _allow_donation(course_modes, course.id, enrollment) + "course_id": enrollment.course_overview.id, + "course_name": enrollment.course_overview.display_name, + "allow_donation": _allow_donation(course_modes, enrollment.course_overview.id, enrollment) } - for course, enrollment in recently_enrolled_courses + for enrollment in recently_enrolled_courses ] platform_name = microsite.get_value('platform_name', settings.PLATFORM_NAME) @@ -703,22 +739,20 @@ def _create_recent_enrollment_message(course_enrollment_pairs, course_modes): ) -def _get_recently_enrolled_courses(course_enrollment_pairs): - """Checks to see if the student has recently enrolled in courses. - - Checks to see if any of the enrollments in the course_enrollment_pairs have been recently created and activated. +def _get_recently_enrolled_courses(course_enrollments): + """ + Given a list of enrollments, filter out all but recent enrollments. Args: - course_enrollment_pairs (list): A list of tuples containing courses, and the associated enrollment information. + course_enrollments (list[CourseEnrollment]): A list of course enrollments. Returns: - A list of courses - + list[CourseEnrollment]: A list of recent course enrollments. """ seconds = DashboardConfiguration.current().recent_enrollment_time_delta time_delta = (datetime.datetime.now(UTC) - datetime.timedelta(seconds=seconds)) return [ - (course, enrollment) for course, enrollment in course_enrollment_pairs + enrollment for enrollment in course_enrollments # If the enrollment has no created date, we are explicitly excluding the course # from the list of recent enrollments. if enrollment.is_active and enrollment.created > time_delta @@ -752,7 +786,7 @@ def _update_email_opt_in(request, org): preferences_api.update_email_opt_in(request.user, org, email_opt_in_boolean) -def _credit_statuses(user, course_enrollment_pairs): +def _credit_statuses(user, course_enrollments): """ Retrieve the status for credit courses. @@ -768,7 +802,8 @@ def _credit_statuses(user, course_enrollment_pairs): Arguments: user (User): The currently logged-in user. - course_enrollment_pairs (list): List of (Course, CourseEnrollment) tuples. + course_enrollments (list[CourseEnrollment]): List of enrollments for the + user. Returns: dict @@ -785,7 +820,7 @@ def _credit_statuses(user, course_enrollment_pairs): so the user should contact the support team. Example: - >>> _credit_statuses(user, course_enrollment_pairs) + >>> _credit_statuses(user, course_enrollments) { CourseKey.from_string("edX/DemoX/Demo_Course"): { "course_key": "edX/DemoX/Demo_Course", @@ -812,8 +847,8 @@ def _credit_statuses(user, course_enrollment_pairs): } credit_enrollments = { - course.id: enrollment - for course, enrollment in course_enrollment_pairs + enrollment.course_id: enrollment + for enrollment in course_enrollments if enrollment.mode == "credit" } diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 6487f407fa..3691ac4d45 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -12,6 +12,7 @@ from django.core.urlresolvers import reverse from eventtracking import tracker from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from xmodule.modulestore.django import modulestore from certificates.models import ( @@ -216,13 +217,18 @@ def generate_example_certificates(course_key): def has_html_certificates_enabled(course_key, course=None): """ - It determines if course has html certificates enabled + Determine if a course has html certificates enabled. + + Arguments: + course_key (CourseKey|str): A course key or a string representation + of one. + course (CourseDescriptor|CourseOverview): A course. """ html_certificates_enabled = False try: if not isinstance(course_key, CourseKey): course_key = CourseKey.from_string(course_key) - course = course if course else modulestore().get_course(course_key, depth=0) + course = course if course else CourseOverview.get_from_id(course_key) if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False) and course.cert_html_view_enabled: html_certificates_enabled = True except: # pylint: disable=bare-except diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 43e9b9b791..20f7db7fcf 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -152,6 +152,16 @@ def find_file(filesystem, dirs, filename): raise ResourceNotFoundError(u"Could not find {0}".format(filename)) +def get_course_university_about_section(course): # pylint: disable=invalid-name + """ + Returns a snippet of HTML displaying the course's university. + + Arguments: + course (CourseDescriptor|CourseOverview): A course. + """ + return course.display_org_with_default + + def get_course_about_section(course, section_key): """ This returns the snippet of html to be rendered on the course about page, @@ -227,7 +237,7 @@ def get_course_about_section(course, section_key): elif section_key == "title": return course.display_name_with_default elif section_key == "university": - return course.display_org_with_default + return get_course_university_about_section(course) elif section_key == "number": return course.display_number_with_default diff --git a/lms/templates/ccx/_dashboard_ccx_listing.html b/lms/templates/ccx/_dashboard_ccx_listing.html index 5856238498..c9d1c36f4b 100644 --- a/lms/templates/ccx/_dashboard_ccx_listing.html +++ b/lms/templates/ccx/_dashboard_ccx_listing.html @@ -1,4 +1,4 @@ -<%page args="ccx, membership, course, show_courseware_link, is_course_blocked" /> +<%page args="ccx, membership, course_overview, show_courseware_link, is_course_blocked" /> <%! from django.utils.translation import ugettext as _ @@ -7,7 +7,7 @@ from courseware.courses import course_image_url, get_course_about_section from ccx_keys.locator import CCXLocator %> <% - ccx_target = reverse('info', args=[CCXLocator.from_course_locator(course.id, ccx.id)]) + ccx_target = reverse('info', args=[CCXLocator.from_course_locator(course_overview.id, ccx.id)]) %>
  • @@ -16,16 +16,16 @@ from ccx_keys.locator import CCXLocator % if show_courseware_link: % if not is_course_blocked: - ${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h} + ${_('{course_number} {ccx_name} Cover Image').format(course_number=course_overview.number, ccx_name=ccx.display_name) |h} % else: - ${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h} + ${_('{course_number} {ccx_name} Cover Image').format(course_number=course_overview.number, ccx_name=ccx.display_name) |h} % endif % else: - ${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h} + ${_('{course_number} {ccx_name} Cover Image').format(course_number=course_overview.number, ccx_name=ccx.display_name) |h} % endif @@ -43,7 +43,7 @@ from ccx_keys.locator import CCXLocator
    ${get_course_about_section(course, 'university')} - - ${course.display_number_with_default | h} + ${course_overview.display_number_with_default | h} % if ccx.has_ended(): ${_("Ended - {end_date}").format(end_date=ccx.end_datetime_text("SHORT_DATE"))} diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 231ffc6575..8d3fb0ad43 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -70,28 +70,28 @@ from django.core.urlresolvers import reverse - % if len(course_enrollment_pairs) > 0: + % if len(course_enrollments) > 0:
    - ${_('Verify Now')} + ${_('Verify Now')}
    % elif verification_status['status'] == VERIFY_STATUS_SUBMITTED:

    ${_('You have already verified your ID!')}

    @@ -329,7 +329,7 @@ from student.helpers import (