From 3b31270e07f2c324ce01d910f004edd540a1a646 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Tue, 25 Apr 2017 14:23:38 -0400 Subject: [PATCH] Fix Start Course vs Resume Course using Course Blocks. --- .../xmodule/modulestore/tests/django_utils.py | 1 - .../transformers/library_content.py | 31 +------ lms/djangoapps/course_blocks/utils.py | 32 +++++++ lms/djangoapps/courseware/courses.py | 25 ------ lms/djangoapps/courseware/views/views.py | 27 +++++- .../course-home-fragment.html | 20 +++-- .../course-outline-fragment.html | 6 +- .../tests/views/test_course_outline.py | 88 +++++++++++++++---- openedx/features/course_experience/utils.py | 78 ++++++++++++++++ .../course_experience/views/course_home.py | 53 +++++++++-- .../course_experience/views/course_outline.py | 42 ++------- 11 files changed, 277 insertions(+), 126 deletions(-) create mode 100644 lms/djangoapps/course_blocks/utils.py create mode 100644 openedx/features/course_experience/utils.py diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 870e342c6d..1a76094362 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -23,7 +23,6 @@ from xmodule.modulestore.django import modulestore, clear_existing_modulestores, from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK -from openedx.core.djangoapps.bookmarks.signals import trigger_update_xblocks_cache_task from openedx.core.djangolib.testing.utils import CacheIsolationMixin, CacheIsolationTestCase diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index 44f7958aea..9743fc2460 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -12,6 +12,8 @@ from xmodule.modulestore.django import modulestore from eventtracking import tracker from track import contexts +from ..utils import get_student_module_as_dict + class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransformer): """ @@ -78,12 +80,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo max_count = block_structure.get_xblock_field(block_key, 'max_count') # Retrieve "selected" json from LMS MySQL database. - module = self._get_student_module(usage_info.user, usage_info.course_key, block_key) - if module: - state_dict = json.loads(module.state) - else: - state_dict = {} - + state_dict = get_student_module_as_dict(usage_info.user, usage_info.course_key, block_key) for selected_block in state_dict.get('selected', []): # Add all selected entries for this user for this # library module to the selected list. @@ -135,28 +132,6 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo return [block_structure.create_removal_filter(check_child_removal)] - @classmethod - def _get_student_module(cls, user, course_key, block_key): - """ - Get the student module for the given user for the given block. - - Arguments: - user (User) - course_key (CourseLocator) - block_key (BlockUsageLocator) - - Returns: - StudentModule if exists, or None. - """ - try: - return StudentModule.objects.get( - student=user, - course_id=course_key, - module_state_key=block_key, - ) - except StudentModule.DoesNotExist: - return None - def _publish_events(self, block_structure, location, previous_count, max_count, block_keys, user_id): """ Helper method to publish events for analytics purposes diff --git a/lms/djangoapps/course_blocks/utils.py b/lms/djangoapps/course_blocks/utils.py new file mode 100644 index 0000000000..33097b7e00 --- /dev/null +++ b/lms/djangoapps/course_blocks/utils.py @@ -0,0 +1,32 @@ +""" +Common utilities for use along with the course blocks. +""" +import json +from courseware.models import StudentModule + + +def get_student_module_as_dict(user, course_key, block_key): + """ + Get the student module as a dict for the given user for the given block. + + Arguments: + user (User) + course_key (CourseLocator) + block_key (BlockUsageLocator) + + Returns: + StudentModule as a (possibly empty) dict. + """ + try: + student_module = StudentModule.objects.get( + student=user, + course_id=course_key, + module_state_key=block_key, + ) + except StudentModule.DoesNotExist: + student_module = None + + if student_module: + return json.loads(student_module.state) + else: + return {} diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index fb9a3dabeb..bfbae0f817 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -518,28 +518,3 @@ def get_current_child(xmodule, min_depth=None, requested_child=None): child = _get_default_child_module(children) return child - - -def get_last_accessed_courseware(course, request, user): - """ - Returns a tuple containing the courseware module (URL, id) that the user last accessed, - or (None, None) if it cannot be found. - """ - # TODO: convert this method to use the Course Blocks API - field_data_cache = FieldDataCache.cache_for_descriptor_descendents( - course.id, request.user, course, depth=2 - ) - course_module = get_module_for_descriptor( - user, request, course, field_data_cache, course.id, course=course - ) - chapter_module = get_current_child(course_module) - if chapter_module is not None: - section_module = get_current_child(chapter_module) - if section_module is not None: - url = reverse('courseware_section', kwargs={ - 'course_id': unicode(course.id), - 'chapter': chapter_module.url_name, - 'section': section_module.url_name - }) - return (url, section_module.url_name) - return (None, None) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 5e24a5466c..ad0ba491c2 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -66,7 +66,7 @@ from courseware.courses import ( get_course_by_id, get_course_overview_with_access, get_course_with_access, - get_last_accessed_courseware, + get_current_child, get_permission_for_course_about, get_studio_url, sort_by_announcement, @@ -97,7 +97,6 @@ from openedx.features.course_experience import ( UNIFIED_COURSE_VIEW_FLAG, ) from openedx.features.enterprise_support.api import data_sharing_consent_required -from shoppingcart.models import CourseRegistrationCode from shoppingcart.utils import is_shopping_cart_enabled from student.models import UserTestGroup, CourseEnrollment from survey.utils import must_answer_survey @@ -249,6 +248,28 @@ def course_info(request, course_id): Assumes the course_id is in a valid format. """ + def get_last_accessed_courseware(course, request, user): + """ + Returns the courseware module URL that the user last accessed, or None if it cannot be found. + """ + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=2 + ) + course_module = get_module_for_descriptor( + user, request, course, field_data_cache, course.id, course=course + ) + chapter_module = get_current_child(course_module) + if chapter_module is not None: + section_module = get_current_child(chapter_module) + if section_module is not None: + url = reverse('courseware_section', kwargs={ + 'course_id': unicode(course.id), + 'chapter': chapter_module.url_name, + 'section': section_module.url_name + }) + return url + return None + # If the unified course experience is enabled, redirect to the "Course" tab if waffle.flag_is_active(request, UNIFIED_COURSE_EXPERIENCE_FLAG): return redirect(reverse(course_home_url_name(request), args=[course_id])) @@ -343,7 +364,7 @@ def course_info(request, course_id): # Get the URL of the user's last position in order to display the 'where you were last' message context['last_accessed_courseware_url'] = None if SelfPacedConfiguration.current().enable_course_home_improvements: - context['last_accessed_courseware_url'], _ = get_last_accessed_courseware(course, request, user) + context['last_accessed_courseware_url'] = get_last_accessed_courseware(course, request, user) now = datetime.now(UTC()) effective_start = _adjust_start_date_for_beta_testers(user, course, course_key) diff --git a/openedx/features/course_experience/templates/course_experience/course-home-fragment.html b/openedx/features/course_experience/templates/course_experience/course-home-fragment.html index 553662c2aa..45ee9442cb 100644 --- a/openedx/features/course_experience/templates/course_experience/course-home-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-home-fragment.html @@ -50,17 +50,19 @@ from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG % endif
% if not waffle.flag_is_active(request, UNIFIED_COURSE_EXPERIENCE_FLAG): - + ${_("Bookmarks")} % endif - - % if has_visited_course: - ${_("Resume Course")} - % else: - ${_("Start Course")} - % endif - + % if resume_course_url: + + % if has_visited_course: + ${_("Resume Course")} + % else: + ${_("Start Course")} + % endif + + % endif
@@ -75,7 +77,7 @@ from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG

${_("Course Tools")}