From 2f581448dafe90284d2e1c7e503b4b6b9a8182ab Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 26 Apr 2016 20:56:41 -0400 Subject: [PATCH 1/2] Refactor Courseware Index MA-2189 --- common/djangoapps/student/tests/tests.py | 7 +- lms/djangoapps/courseware/entrance_exams.py | 10 +- lms/djangoapps/courseware/exceptions.py | 12 + lms/djangoapps/courseware/index.py | 541 ++++++++++++++++++ lms/djangoapps/courseware/module_render.py | 19 +- .../courseware/tests/test_entrance_exam.py | 28 +- .../courseware/tests/test_module_render.py | 36 +- .../courseware/tests/test_navigation.py | 61 +- lms/djangoapps/courseware/tests/test_views.py | 38 +- lms/djangoapps/courseware/views.py | 462 +-------------- lms/djangoapps/mobile_api/users/views.py | 3 +- lms/urls.py | 9 +- 12 files changed, 681 insertions(+), 545 deletions(-) create mode 100644 lms/djangoapps/courseware/exceptions.py create mode 100644 lms/djangoapps/courseware/index.py diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index a4a5ca5fcc..319359fd96 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -298,7 +298,7 @@ class DashboardTest(ModuleStoreTestCase): self.assertIsNone(course_mode_info['days_for_upsell']) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - @patch('courseware.views.log.warning') + @patch('courseware.index.log.warning') @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) def test_blocked_course_scenario(self, log_warning): @@ -349,7 +349,10 @@ class DashboardTest(ModuleStoreTestCase): # Direct link to course redirect to user dashboard self.client.get(reverse('courseware', kwargs={"course_id": self.course.id.to_deprecated_string()})) log_warning.assert_called_with( - u'User %s cannot access the course %s because payment has not yet been received', self.user, self.course.id.to_deprecated_string()) + u'User %s cannot access the course %s because payment has not yet been received', + self.user, + unicode(self.course.id), + ) # Now re-validating the invoice invoice = shoppingcart.models.Invoice.objects.get(id=sale_invoice_1.id) diff --git a/lms/djangoapps/courseware/entrance_exams.py b/lms/djangoapps/courseware/entrance_exams.py index 82e3861e3d..3968229fd1 100644 --- a/lms/djangoapps/courseware/entrance_exams.py +++ b/lms/djangoapps/courseware/entrance_exams.py @@ -25,7 +25,7 @@ def course_has_entrance_exam(course): return True -def user_can_skip_entrance_exam(request, user, course): +def user_can_skip_entrance_exam(user, course): """ Checks all of the various override conditions for a user to skip an entrance exam Begin by short-circuiting if the course does not have an entrance exam @@ -38,7 +38,7 @@ def user_can_skip_entrance_exam(request, user, course): return True if EntranceExamConfiguration.user_can_skip_entrance_exam(user, course.id): return True - if not get_entrance_exam_content(request, course): + if not get_entrance_exam_content(user, course): return True return False @@ -66,7 +66,7 @@ def user_must_complete_entrance_exam(request, user, course): whether or not the user is allowed to clear the Entrance Exam gate and access the rest of the course. """ # First, let's see if the user is allowed to skip - if user_can_skip_entrance_exam(request, user, course): + if user_can_skip_entrance_exam(user, course): return False # If they can't actually skip the exam, we'll need to see if they've already passed it if user_has_passed_entrance_exam(request, course): @@ -157,11 +157,11 @@ def get_entrance_exam_score(request, course): return _calculate_entrance_exam_score(request.user, course, exam_modules) -def get_entrance_exam_content(request, course): +def get_entrance_exam_content(user, course): """ Get the entrance exam content information (ie, chapter module) """ - required_content = get_required_content(course, request.user) + required_content = get_required_content(course, user) exam_module = None for content in required_content: diff --git a/lms/djangoapps/courseware/exceptions.py b/lms/djangoapps/courseware/exceptions.py new file mode 100644 index 0000000000..d7b38f2dec --- /dev/null +++ b/lms/djangoapps/courseware/exceptions.py @@ -0,0 +1,12 @@ +""" +Exception classes used in lms/courseware. +""" + + +class Redirect(Exception): + """ + Exception class that requires redirecting to a URL. + """ + def __init__(self, url): + super(Redirect, self).__init__() + self.url = url diff --git a/lms/djangoapps/courseware/index.py b/lms/djangoapps/courseware/index.py new file mode 100644 index 0000000000..ad1efd1713 --- /dev/null +++ b/lms/djangoapps/courseware/index.py @@ -0,0 +1,541 @@ +""" +View for Courseware Index +""" +# pylint: disable=attribute-defined-outside-init +from datetime import datetime +from django.conf import settings +from django.contrib.auth.decorators import login_required +from django.contrib.auth.models import User +from django.core.context_processors import csrf +from django.core.urlresolvers import reverse +from django.http import Http404 +from django.utils.decorators import method_decorator +from django.utils.timezone import UTC +from django.views.decorators.cache import cache_control +from django.views.decorators.csrf import ensure_csrf_cookie +from django.views.generic import View +from django.shortcuts import redirect +from edxmako.shortcuts import render_to_response, render_to_string +import logging +import newrelic.agent +import urllib + +from lang_pref import LANGUAGE_KEY +from xblock.fragment import Fragment +from opaque_keys.edx.keys import CourseKey +from openedx.core.lib.gating import api as gating_api +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference +from shoppingcart.models import CourseRegistrationCode +from student.views import is_course_blocked +from util.views import ensure_valid_course_key +from xmodule.modulestore.django import modulestore +from xmodule.x_module import STUDENT_VIEW +from survey.utils import must_answer_survey + +from .access import has_access, _adjust_start_date_for_beta_testers +from .access_utils import in_preview_mode +from .courses import get_studio_url, get_course_with_access +from .entrance_exams import ( + course_has_entrance_exam, + get_entrance_exam_content, + get_entrance_exam_score, + user_has_passed_entrance_exam, + user_must_complete_entrance_exam, +) +from .exceptions import Redirect +from .masquerade import setup_masquerade +from .model_data import FieldDataCache +from .module_render import toc_for_course, get_module_for_descriptor +from .views import get_current_child, registered_for_course + + +log = logging.getLogger("edx.courseware.index") +TEMPLATE_IMPORTS = {'urllib': urllib} +CONTENT_DEPTH = 2 + + +class CoursewareIndex(View): + """ + View class for the Courseware page. + """ + @method_decorator(login_required) + @method_decorator(ensure_csrf_cookie) + @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True)) + @method_decorator(ensure_valid_course_key) + def get(self, request, course_id, chapter=None, section=None, position=None): + """ + Displays courseware accordion and associated content. If course, chapter, + and section are all specified, renders the page, or returns an error if they + are invalid. + + If section is not specified, displays the accordion opened to the right + chapter. + + If neither chapter or section are specified, displays the user's most + recent chapter, or the first chapter if this is the user's first visit. + + Arguments: + request: HTTP request + course_id (unicode): course id + chapter (unicode): chapter url_name + section (unicode): section url_name + position (unicode): position in module, eg of module + """ + self.course_key = CourseKey.from_string(course_id) + self.request = request + self.original_chapter_url_name = chapter + self.original_section_url_name = section + self.chapter_url_name = chapter + self.section_url_name = section + self.position = position + self.chapter, self.section = None, None + + try: + self._init_new_relic() + self._verify_position() + with modulestore().bulk_operations(self.course_key): + self.course = get_course_with_access(request.user, 'load', self.course_key, depth=CONTENT_DEPTH) + self.is_staff = has_access(request.user, 'staff', self.course) + self._setup_masquerade_for_effective_user() + return self._get() + except Redirect as redirect_error: + return redirect(redirect_error.url) + except UnicodeEncodeError: + raise Http404("URL contains Unicode characters") + except Http404: + # let it propagate + raise + except Exception: # pylint: disable=broad-except + return self._handle_unexpected_error() + + def _setup_masquerade_for_effective_user(self): + """ + Setup the masquerade information to allow the request to + be processed for the requested effective user. + """ + self.real_user = self.request.user + self.masquerade, self.effective_user = setup_masquerade( + self.request, + self.course_key, + self.is_staff, + reset_masquerade_data=True + ) + # Set the user in the request to the effective user. + self.request.user = self.effective_user + + def _get(self): + """ + Render the index page. + """ + self._redirect_if_needed_to_access_course() + self._prefetch_and_bind_course() + + if self.course.has_children_at_depth(CONTENT_DEPTH): + self._reset_section_to_exam_if_required() + self.chapter = self._find_chapter() + self.section = self._find_section() + + if self.chapter and self.section: + self._redirect_if_not_requested_section() + self._verify_section_not_gated() + self._save_positions() + self._prefetch_and_bind_section() + + return render_to_response('courseware/courseware.html', self._create_courseware_context()) + + def _redirect_if_not_requested_section(self): + """ + If the resulting section and chapter are different from what was initially + requested, redirect back to the index page, but with an updated URL that includes + the correct section and chapter values. We do this so that our analytics events + and error logs have the appropriate URLs. + """ + if ( + self.chapter.url_name != self.original_chapter_url_name or + (self.original_section_url_name and self.section.url_name != self.original_section_url_name) + ): + raise Redirect( + reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(self.course_key), + 'chapter': self.chapter.url_name, + 'section': self.section.url_name, + }, + ) + ) + + def _init_new_relic(self): + """ + Initialize metrics for New Relic so we can slice data in New Relic Insights + """ + newrelic.agent.add_custom_parameter('course_id', unicode(self.course_key)) + newrelic.agent.add_custom_parameter('org', unicode(self.course_key.org)) + + def _verify_position(self): + """ + Verify that the given position is in fact an int. + """ + if self.position is not None: + try: + int(self.position) + except ValueError: + raise Http404(u"Position {} is not an integer!".format(self.position)) + + def _redirect_if_needed_to_access_course(self): + """ + Verifies that the user can enter the course. + """ + self._redirect_if_needed_to_pay_for_course() + self._redirect_if_needed_to_register_for_course() + self._redirect_if_needed_for_course_prerequisites() + self._redirect_if_needed_for_course_survey() + + def _redirect_if_needed_to_pay_for_course(self): + """ + Redirect to dashboard if the course is blocked due to non-payment. + """ + self.real_user = User.objects.prefetch_related("groups").get(id=self.real_user.id) + redeemed_registration_codes = CourseRegistrationCode.objects.filter( + course_id=self.course_key, + registrationcoderedemption__redeemed_by=self.real_user + ) + if is_course_blocked(self.request, redeemed_registration_codes, self.course_key): + # registration codes may be generated via Bulk Purchase Scenario + # we have to check only for the invoice generated registration codes + # that their invoice is valid or not + log.warning( + u'User %s cannot access the course %s because payment has not yet been received', + self.real_user, + unicode(self.course_key), + ) + raise Redirect(reverse('dashboard')) + + def _redirect_if_needed_to_register_for_course(self): + """ + Verify that the user is registered in the course. + """ + if not registered_for_course(self.course, self.effective_user): + log.debug( + u'User %s tried to view course %s but is not enrolled', + self.effective_user, + unicode(self.course.id) + ) + raise Redirect(reverse('about_course', args=[unicode(self.course.id)])) + + def _redirect_if_needed_for_course_prerequisites(self): + """ + See if all pre-requisites (as per the milestones app feature) have been + fulfilled. Note that if the pre-requisite feature flag has been turned off + (default) then this check will always pass. + """ + if not has_access(self.effective_user, 'view_courseware_with_prerequisites', self.course): + # Prerequisites have not been fulfilled. + # Therefore redirect to the Dashboard. + log.info( + u'User %d tried to view course %s ' + u'without fulfilling prerequisites', + self.effective_user.id, unicode(self.course.id)) + raise Redirect(reverse('dashboard')) + + def _redirect_if_needed_for_course_survey(self): + """ + Check to see if there is a required survey that must be taken before + the user can access the course. + """ + if must_answer_survey(self.course, self.effective_user): + raise Redirect(reverse('course_survey', args=[unicode(self.course.id)])) + + def _reset_section_to_exam_if_required(self): + """ + Check to see if an Entrance Exam is required for the user. + """ + if ( + course_has_entrance_exam(self.course) and + user_must_complete_entrance_exam(self.request, self.effective_user, self.course) + ): + exam_chapter = get_entrance_exam_content(self.effective_user, self.course) + if exam_chapter and exam_chapter.get_children(): + exam_section = exam_chapter.get_children()[0] + if exam_section: + self.chapter_url_name = exam_chapter.url_name + self.section_url_name = exam_section.url_name + + def _verify_section_not_gated(self): + """ + Verify whether the section is gated and accessible to the user. + """ + gated_content = gating_api.get_gated_content(self.course, self.effective_user) + if gated_content: + if unicode(self.section.location) in gated_content: + raise Http404 + + def _get_language_preference(self): + """ + Returns the preferred language for the actual user making the request. + """ + language_preference = get_user_preference(self.real_user, LANGUAGE_KEY) + if not language_preference: + language_preference = settings.LANGUAGE_CODE + return language_preference + + def _is_masquerading_as_student(self): + """ + Returns whether the current request is masquerading as a student. + """ + return self.masquerade and self.masquerade.role == 'student' + + def _find_block(self, parent, url_name, block_type, min_depth=None): + """ + Finds the block in the parent with the specified url_name. + If not found, calls get_current_child on the parent. + """ + child = None + if url_name: + child = parent.get_child_by(lambda m: m.location.name == url_name) + if not child: + # User may be trying to access a child that isn't live yet + if not self._is_masquerading_as_student(): + raise Http404('No {block_type} found with name {url_name}'.format( + block_type=block_type, + url_name=url_name, + )) + elif min_depth and not child.has_children_at_depth(min_depth - 1): + child = None + if not child: + child = get_current_child(parent, min_depth=min_depth, requested_child=self.request.GET.get("child")) + return child + + def _find_chapter(self): + """ + Finds the requested chapter. + """ + return self._find_block(self.course, self.chapter_url_name, 'chapter', CONTENT_DEPTH - 1) + + def _find_section(self): + """ + Finds the requested section. + """ + if self.chapter: + return self._find_block(self.chapter, self.section_url_name, 'section') + + def _prefetch_and_bind_course(self): + """ + Prefetches all descendant data for the requested section and + sets up the runtime, which binds the request user to the section. + """ + self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + self.course_key, self.effective_user, self.course, depth=CONTENT_DEPTH, + ) + + self.course = get_module_for_descriptor( + self.effective_user, + self.request, + self.course, + self.field_data_cache, + self.course_key, + course=self.course, + ) + + def _prefetch_and_bind_section(self): + """ + Prefetches all descendant data for the requested section and + sets up the runtime, which binds the request user to the section. + """ + # Pre-fetch all descendant data + self.section = modulestore().get_item(self.section.location, depth=None) + self.field_data_cache.add_descriptor_descendents(self.section, depth=None) + + # Bind section to user + self.section = get_module_for_descriptor( + self.effective_user, + self.request, + self.section, + self.field_data_cache, + self.course_key, + self.position, + course=self.course, + ) + + def _save_positions(self): + """ + Save where we are in the course and chapter. + """ + save_child_position(self.course, self.chapter_url_name) + save_child_position(self.chapter, self.section_url_name) + + def _create_courseware_context(self): + """ + Returns and creates the rendering context for the courseware. + Also returns the table of contents for the courseware. + """ + courseware_context = { + 'csrf': csrf(self.request)['csrf_token'], + 'COURSE_TITLE': self.course.display_name_with_default_escaped, + 'course': self.course, + 'init': '', + 'fragment': Fragment(), + 'staff_access': self.is_staff, + 'studio_url': get_studio_url(self.course, 'course'), + 'masquerade': self.masquerade, + 'xqa_server': settings.FEATURES.get('XQA_SERVER', "http://your_xqa_server.com"), + 'bookmarks_api_url': reverse('bookmarks'), + 'language_preference': self._get_language_preference(), + 'disable_optimizely': True, + } + table_of_contents = toc_for_course( + self.effective_user, + self.request, + self.course, + self.chapter_url_name, + self.section_url_name, + self.field_data_cache, + ) + courseware_context['accordion'] = render_accordion(self.request, self.course, table_of_contents['chapters']) + + # entrance exam data + if course_has_entrance_exam(self.course): + if getattr(self.chapter, 'is_entrance_exam', False): + courseware_context['entrance_exam_current_score'] = get_entrance_exam_score(self.request, self.course) + courseware_context['entrance_exam_passed'] = user_has_passed_entrance_exam(self.request, self.course) + + # staff masquerading data + now = datetime.now(UTC()) + effective_start = _adjust_start_date_for_beta_testers(self.effective_user, self.course, self.course_key) + if not in_preview_mode() and self.is_staff and now < effective_start: + # Disable student view button if user is staff and + # course is not yet visible to students. + courseware_context['disable_student_access'] = True + + if self.section: + # chromeless data + if self.section.chrome: + chrome = [s.strip() for s in self.section.chrome.lower().split(",")] + if 'accordion' not in chrome: + courseware_context['disable_accordion'] = True + if 'tabs' not in chrome: + courseware_context['disable_tabs'] = True + + # default tab + if self.section.default_tab: + courseware_context['default_tab'] = self.section.default_tab + + # section data + courseware_context['section_title'] = self.section.display_name_with_default_escaped + section_context = self._create_section_context( + table_of_contents['previous_of_active_section'], + table_of_contents['next_of_active_section'], + ) + courseware_context['fragment'] = self.section.render(STUDENT_VIEW, section_context) + + return courseware_context + + def _create_section_context(self, previous_of_active_section, next_of_active_section): + """ + Returns and creates the rendering context for the section. + """ + def _compute_section_url(section_info, requested_child): + """ + Returns the section URL for the given section_info with the given child parameter. + """ + return "{url}?child={requested_child}".format( + url=reverse( + 'courseware_section', + args=[unicode(self.course.id), section_info['chapter_url_name'], section_info['url_name']], + ), + requested_child=requested_child, + ) + + section_context = { + 'activate_block_id': self.request.GET.get('activate_block_id'), + 'requested_child': self.request.GET.get("child"), + } + if previous_of_active_section: + section_context['prev_url'] = _compute_section_url(previous_of_active_section, 'last') + if next_of_active_section: + section_context['next_url'] = _compute_section_url(next_of_active_section, 'first') + return section_context + + def _handle_unexpected_error(self): + """ + Handle unexpected exceptions raised by View. + """ + # In production, don't want to let a 500 out for any reason + if settings.DEBUG: + raise + log.exception( + u"Error in index view: user=%s, effective_user=%s, course=%s, chapter=%s section=%s position=%s", + self.real_user, + self.effective_user, + unicode(self.course_key), + self.chapter_url_name, + self.section_url_name, + self.position, + ) + try: + return render_to_response('courseware/courseware-error.html', { + 'staff_access': self.is_staff, + 'course': self.course + }) + except: + # Let the exception propagate, relying on global config to + # at least return a nice error message + log.exception("Error while rendering courseware-error page") + raise + + +def render_accordion(request, course, table_of_contents): + """ + Returns the HTML that renders the navigation for the given course. + Expects the table_of_contents to have data on each chapter and section, + including which ones are active. + """ + context = dict( + [ + ('toc', table_of_contents), + ('course_id', unicode(course.id)), + ('csrf', csrf(request)['csrf_token']), + ('due_date_display_format', course.due_date_display_format), + ] + TEMPLATE_IMPORTS.items() + ) + return render_to_string('courseware/accordion.html', context) + + +def save_child_position(seq_module, child_name): + """ + child_name: url_name of the child + """ + for position, child in enumerate(seq_module.get_display_items(), start=1): + if child.location.name == child_name: + # Only save if position changed + if position != seq_module.position: + seq_module.position = position + # Save this new position to the underlying KeyValueStore + seq_module.save() + + +def save_positions_recursively_up(user, request, field_data_cache, xmodule, course=None): + """ + Recurses up the course tree starting from a leaf + Saving the position property based on the previous node as it goes + """ + current_module = xmodule + + while current_module: + parent_location = modulestore().get_parent_location(current_module.location) + parent = None + if parent_location: + parent_descriptor = modulestore().get_item(parent_location) + parent = get_module_for_descriptor( + user, + request, + parent_descriptor, + field_data_cache, + current_module.location.course_key, + course=course + ) + + if parent and hasattr(parent, 'position'): + save_child_position(parent, current_module.location.name) + + current_module = parent diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 958f74cdd2..f7f5fbf442 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -123,13 +123,20 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ Create a table of contents from the module store Return format: - [ {'display_name': name, 'url_name': url_name, - 'sections': SECTIONS, 'active': bool}, ... ] + { 'chapters': [ + {'display_name': name, 'url_name': url_name, 'sections': SECTIONS, 'active': bool}, + ], + 'previous_of_active_section': {..}, + 'next_of_active_section': {..} + } where SECTIONS is a list [ {'display_name': name, 'url_name': url_name, 'format': format, 'due': due, 'active' : bool, 'graded': bool}, ...] + where previous_of_active_section and next_of_active_section have information on the + next/previous sections of the active section. + active is set for the section and chapter corresponding to the passed parameters, which are expected to be url_names of the chapter+section. Everything else comes from the xml, or defaults to "". @@ -139,7 +146,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ NOTE: assumes that if we got this far, user has access to course. Returns None if this is not the case. - field_data_cache must include data from the course module and 2 levels of its descendents + field_data_cache must include data from the course module and 2 levels of its descendants ''' with modulestore().bulk_operations(course.id): @@ -221,7 +228,11 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ 'sections': sections, 'active': chapter.url_name == active_chapter }) - return toc_chapters, previous_of_active_section, next_of_active_section + return { + 'chapters': toc_chapters, + 'previous_of_active_section': previous_of_active_section, + 'next_of_active_section': next_of_active_section, + } def _add_timed_exam_info(user, course, section, section_context): diff --git a/lms/djangoapps/courseware/tests/test_entrance_exam.py b/lms/djangoapps/courseware/tests/test_entrance_exam.py index 8e33be6ada..8d05bcc606 100644 --- a/lms/djangoapps/courseware/tests/test_entrance_exam.py +++ b/lms/djangoapps/courseware/tests/test_entrance_exam.py @@ -62,7 +62,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest parent=self.course, display_name='Overview' ) - ItemFactory.create( + self.welcome = ItemFactory.create( parent=self.chapter, display_name='Welcome' ) @@ -250,7 +250,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest kwargs={ 'course_id': unicode(self.course.id), 'chapter': self.chapter.location.name, - 'section': self.chapter_subsection.location.name + 'section': self.welcome.location.name }) resp = self.client.get(url) self.assertRedirects(resp, expected_url, status_code=302, target_status_code=200) @@ -278,14 +278,14 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest """ test get entrance exam content method """ - exam_chapter = get_entrance_exam_content(self.request, self.course) + exam_chapter = get_entrance_exam_content(self.request.user, self.course) self.assertEqual(exam_chapter.url_name, self.entrance_exam.url_name) self.assertFalse(user_has_passed_entrance_exam(self.request, self.course)) answer_entrance_exam_problem(self.course, self.request, self.problem_1) answer_entrance_exam_problem(self.course, self.request, self.problem_2) - exam_chapter = get_entrance_exam_content(self.request, self.course) + exam_chapter = get_entrance_exam_content(self.request.user, self.course) self.assertEqual(exam_chapter, None) self.assertTrue(user_has_passed_entrance_exam(self.request, self.course)) @@ -314,7 +314,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest kwargs={ 'course_id': unicode(self.course.id), 'chapter': self.entrance_exam.location.name, - 'section': self.exam_1.location.name + 'section': self.exam_1.location.name, } ) resp = self.client.get(url) @@ -457,11 +457,13 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest kwargs={'course_id': unicode(self.course.id), 'chapter': self.chapter.url_name} ) response = self.client.get(url) - redirect_url = reverse('courseware', args=[unicode(self.course.id)]) - self.assertRedirects(response, redirect_url, status_code=302, target_status_code=302) - response = self.client.get(redirect_url) - exam_url = response.get('Location') - self.assertRedirects(response, exam_url) + expected_url = reverse('courseware_section', + kwargs={ + 'course_id': unicode(self.course.id), + 'chapter': self.entrance_exam.location.name, + 'section': self.exam_1.location.name + }) + self.assertRedirects(response, expected_url, status_code=302, target_status_code=200) @patch('courseware.entrance_exams.user_has_passed_entrance_exam', Mock(return_value=False)) def test_courseinfo_page_access_without_passing_entrance_exam(self): @@ -516,7 +518,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest """ Test can_skip_entrance_exam method with anonymous user """ - self.assertFalse(user_can_skip_entrance_exam(self.request, self.anonymous_user, self.course)) + self.assertFalse(user_can_skip_entrance_exam(self.anonymous_user, self.course)) def test_has_passed_entrance_exam_with_anonymous_user(self): """ @@ -583,7 +585,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest self.request.user, self.entrance_exam ) - toc, __, __ = toc_for_course( + toc = toc_for_course( self.request.user, self.request, self.course, @@ -591,7 +593,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest self.exam_1.url_name, self.field_data_cache ) - return toc + return toc['chapters'] def answer_entrance_exam_problem(course, request, problem, user=None): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 214c0e4dd1..8cb1b81b26 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -668,13 +668,13 @@ class TestTOC(ModuleStoreTestCase): course = self.store.get_course(self.toy_course.id, depth=2) with check_mongo_calls(toc_finds): - actual, prev_sequential, next_sequential = render.toc_for_course( + actual = render.toc_for_course( self.request.user, self.request, course, self.chapter, None, self.field_data_cache ) for toc_section in expected: - self.assertIn(toc_section, actual) - self.assertIsNone(prev_sequential) - self.assertIsNone(next_sequential) + self.assertIn(toc_section, actual['chapters']) + self.assertIsNone(actual['previous_of_active_section']) + self.assertIsNone(actual['next_of_active_section']) # Mongo makes 3 queries to load the course to depth 2: # - 1 for the course @@ -709,13 +709,13 @@ class TestTOC(ModuleStoreTestCase): 'url_name': 'secret:magic', 'display_name': 'secret:magic', 'display_id': 'secretmagic'}]) with check_mongo_calls(toc_finds): - actual, prev_sequential, next_sequential = render.toc_for_course( + actual = render.toc_for_course( self.request.user, self.request, self.toy_course, self.chapter, section, self.field_data_cache ) for toc_section in expected: - self.assertIn(toc_section, actual) - self.assertEquals(prev_sequential['url_name'], 'Toy_Videos') - self.assertEquals(next_sequential['url_name'], 'video_123456789012') + self.assertIn(toc_section, actual['chapters']) + self.assertEquals(actual['previous_of_active_section']['url_name'], 'Toy_Videos') + self.assertEquals(actual['next_of_active_section']['url_name'], 'video_123456789012') @attr('shard_1') @@ -856,7 +856,7 @@ class TestProctoringRendering(SharedModuleStoreTestCase): """ self._setup_test_data(enrollment_mode, is_practice_exam, attempt_status) - actual, prev_sequential, next_sequential = render.toc_for_course( + actual = render.toc_for_course( self.request.user, self.request, self.toy_course, @@ -864,15 +864,15 @@ class TestProctoringRendering(SharedModuleStoreTestCase): 'Toy_Videos', self.field_data_cache ) - section_actual = self._find_section(actual, 'Overview', 'Toy_Videos') + section_actual = self._find_section(actual['chapters'], 'Overview', 'Toy_Videos') if expected: self.assertIn(expected, [section_actual['proctoring']]) else: # we expect there not to be a 'proctoring' key in the dict self.assertNotIn('proctoring', section_actual) - self.assertIsNone(prev_sequential) - self.assertEquals(next_sequential['url_name'], u"Welcome") + self.assertIsNone(actual['previous_of_active_section']) + self.assertEquals(actual['next_of_active_section']['url_name'], u"Welcome") @ddt.data( ( @@ -1114,7 +1114,7 @@ class TestGatedSubsectionRendering(SharedModuleStoreTestCase, MilestonesTestCase """ Test generation of TOC for a course with a gated subsection """ - actual, prev_sequential, next_sequential = render.toc_for_course( + actual = render.toc_for_course( self.request.user, self.request, self.course, @@ -1122,11 +1122,11 @@ class TestGatedSubsectionRendering(SharedModuleStoreTestCase, MilestonesTestCase self.open_seq.display_name, self.field_data_cache ) - self.assertIsNotNone(self._find_sequential(actual, 'Chapter', 'Open_Sequential')) - self.assertIsNone(self._find_sequential(actual, 'Chapter', 'Gated_Sequential')) - self.assertIsNone(self._find_sequential(actual, 'Non-existant_Chapter', 'Non-existant_Sequential')) - self.assertIsNone(prev_sequential) - self.assertIsNone(next_sequential) + self.assertIsNotNone(self._find_sequential(actual['chapters'], 'Chapter', 'Open_Sequential')) + self.assertIsNone(self._find_sequential(actual['chapters'], 'Chapter', 'Gated_Sequential')) + self.assertIsNone(self._find_sequential(actual['chapters'], 'Non-existent_Chapter', 'Non-existent_Sequential')) + self.assertIsNone(actual['previous_of_active_section']) + self.assertIsNone(actual['next_of_active_section']) @attr('shard_1') diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py index 407d0d6413..f61ebf09a6 100644 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ b/lms/djangoapps/courseware/tests/test_navigation.py @@ -44,7 +44,7 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): cls.section9 = ItemFactory.create(parent=cls.chapter9, display_name='factory_section') cls.unit0 = ItemFactory.create(parent=cls.section0, - display_name='New Unit') + display_name='New Unit 0') cls.chapterchrome = ItemFactory.create(parent=cls.course, display_name='Chrome') @@ -119,6 +119,7 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): 'section': displayname, })) self.assertEquals('course-tabs' in response.content, tabs) + self.assertEquals('course-navigation' in response.content, accordion) self.assertTabInactive('progress', response) self.assertTabActive('courseware', response) @@ -165,7 +166,6 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): resp = self.client.get(reverse('courseware', kwargs={'course_id': self.course.id.to_deprecated_string()})) - self.assertRedirects(resp, reverse( 'courseware_section', kwargs={'course_id': self.course.id.to_deprecated_string(), 'chapter': 'Overview', @@ -174,30 +174,26 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): def test_redirects_second_time(self): """ Verify the accordion remembers we've already visited the Welcome section - and redirects correpondingly. + and redirects correspondingly. """ email, password = self.STUDENT_INFO[0] self.login(email, password) self.enroll(self.course, True) self.enroll(self.test_course, True) - self.client.get(reverse('courseware_section', kwargs={ - 'course_id': self.course.id.to_deprecated_string(), - 'chapter': 'Overview', - 'section': 'Welcome', - })) - - resp = self.client.get(reverse('courseware', - kwargs={'course_id': self.course.id.to_deprecated_string()})) - - redirect_url = reverse( - 'courseware_chapter', + section_url = reverse( + 'courseware_section', kwargs={ 'course_id': self.course.id.to_deprecated_string(), - 'chapter': 'Overview' - } + 'chapter': 'Overview', + 'section': 'Welcome', + }, ) - self.assertRedirects(resp, redirect_url) + self.client.get(section_url) + resp = self.client.get( + reverse('courseware', kwargs={'course_id': self.course.id.to_deprecated_string()}), + ) + self.assertRedirects(resp, section_url) def test_accordion_state(self): """ @@ -209,15 +205,15 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.enroll(self.test_course, True) # Now we directly navigate to a section in a chapter other than 'Overview'. - url = reverse( + section_url = reverse( 'courseware_section', kwargs={ 'course_id': self.course.id.to_deprecated_string(), 'chapter': 'factory_chapter', - 'section': 'factory_section' + 'section': 'factory_section', } ) - self.assert_request_status_code(200, url) + self.assert_request_status_code(200, section_url) # And now hitting the courseware tab should redirect to 'factory_chapter' url = reverse( @@ -225,15 +221,7 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): kwargs={'course_id': self.course.id.to_deprecated_string()} ) resp = self.client.get(url) - - redirect_url = reverse( - 'courseware_chapter', - kwargs={ - 'course_id': self.course.id.to_deprecated_string(), - 'chapter': 'factory_chapter', - } - ) - self.assertRedirects(resp, redirect_url) + self.assertRedirects(resp, section_url) def test_incomplete_course(self): email = self.staff_user.email @@ -247,7 +235,8 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): 'courseware', kwargs={'course_id': test_course_id} ) - self.assert_request_status_code(200, url) + response = self.assert_request_status_code(200, url) + self.assertIn("No content has been added to this course", response.content) section = ItemFactory.create( parent_location=self.test_course.location, @@ -257,21 +246,25 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase): 'courseware', kwargs={'course_id': test_course_id} ) - self.assert_request_status_code(200, url) + response = self.assert_request_status_code(200, url) + self.assertNotIn("No content has been added to this course", response.content) + self.assertIn("New Section", response.content) subsection = ItemFactory.create( parent_location=section.location, - display_name='New Subsection' + display_name='New Subsection', ) url = reverse( 'courseware', kwargs={'course_id': test_course_id} ) - self.assert_request_status_code(200, url) + response = self.assert_request_status_code(200, url) + self.assertIn("New Subsection", response.content) + self.assertNotIn("sequence-nav", response.content) ItemFactory.create( parent_location=subsection.location, - display_name='New Unit' + display_name='New Unit', ) url = reverse( 'courseware', diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 3d0004cd1a..f5e47ce418 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -34,6 +34,7 @@ from certificates.tests.factories import GeneratedCertificateFactory from commerce.models import CommerceConfiguration from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory +from courseware.index import render_accordion, CoursewareIndex from courseware.model_data import set_score from courseware.module_render import toc_for_course from courseware.testutils import RenderXBlockTestMixin @@ -255,7 +256,7 @@ class ViewsTestCase(ModuleStoreTestCase): self._verify_index_response(expected_response_code=404, chapter_name='non-existent') def test_index_nonexistent_chapter_masquerade(self): - with patch('courseware.views.setup_masquerade') as patch_masquerade: + with patch('courseware.index.setup_masquerade') as patch_masquerade: masquerade = MagicMock(role='student') patch_masquerade.return_value = (masquerade, self.user) self._verify_index_response(expected_response_code=302, chapter_name='non-existent') @@ -264,7 +265,7 @@ class ViewsTestCase(ModuleStoreTestCase): self._verify_index_response(expected_response_code=404, section_name='non-existent') def test_index_nonexistent_section_masquerade(self): - with patch('courseware.views.setup_masquerade') as patch_masquerade: + with patch('courseware.index.setup_masquerade') as patch_masquerade: masquerade = MagicMock(role='student') patch_masquerade.return_value = (masquerade, self.user) self._verify_index_response(expected_response_code=302, section_name='non-existent') @@ -416,14 +417,6 @@ class ViewsTestCase(ModuleStoreTestCase): get_redirect_url(self.course_key, self.section.location), ) - def test_redirect_to_course_position(self): - mock_module = MagicMock() - mock_module.descriptor.id = 'Underwater Basketweaving' - mock_module.position = 3 - mock_module.get_display_items.return_value = [] - self.assertRaises(Http404, views.redirect_to_course_position, - mock_module, views.CONTENT_DEPTH) - def test_invalid_course_id(self): response = self.client.get('/courses/MITx/3.091X/') self.assertEqual(response.status_code, 404) @@ -462,15 +455,6 @@ class ViewsTestCase(ModuleStoreTestCase): response = self.client.get(request_url) self.assertEqual(response.status_code, 404) - def test_registered_for_course(self): - self.assertFalse(views.registered_for_course('Basketweaving', None)) - mock_user = MagicMock() - mock_user.is_authenticated.return_value = False - self.assertFalse(views.registered_for_course('dummy', mock_user)) - mock_course = MagicMock() - mock_course.id = self.course_key - self.assertTrue(views.registered_for_course(mock_course, self.user)) - @override_settings(PAID_COURSE_REGISTRATION_CURRENCY=["USD", "$"]) def test_get_cosmetic_display_price(self): """ @@ -917,10 +901,10 @@ class TestAccordionDueDate(BaseDueDateTests): def get_text(self, course): """ Returns the HTML for the accordion """ - table_of_contents, __, __ = toc_for_course( + table_of_contents = toc_for_course( self.request.user, self.request, course, unicode(course.get_children()[0].scope_ids.usage_id), None, None ) - return views.render_accordion(self.request, course, table_of_contents) + return render_accordion(self.request, course, table_of_contents['chapters']) @attr('shard_1') @@ -1497,7 +1481,9 @@ class TestIndexView(ModuleStoreTestCase): mako_middleware_process_request(request) # Trigger the assertions embedded in the ViewCheckerBlocks - response = views.index(request, unicode(course.id), chapter=chapter.url_name, section=section.url_name) + response = CoursewareIndex.as_view()( + request, unicode(course.id), chapter=chapter.url_name, section=section.url_name + ) self.assertEquals(response.content.count("ViewCheckerPassed"), 3) @XBlock.register_temp_plugin(ActivateIDCheckerBlock, 'id_checker') @@ -1525,7 +1511,9 @@ class TestIndexView(ModuleStoreTestCase): request.user = user mako_middleware_process_request(request) - response = views.index(request, unicode(course.id), chapter=chapter.url_name, section=section.url_name) + response = CoursewareIndex.as_view()( + request, unicode(course.id), chapter=chapter.url_name, section=section.url_name + ) self.assertIn("Activate Block ID: test_block_id", response.content) @@ -1546,7 +1534,9 @@ class TestIndexViewWithGating(ModuleStoreTestCase, MilestonesTestCaseMixin): self.store.update_item(self.course, 0) self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter") self.open_seq = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Open Sequential") + ItemFactory.create(parent=self.open_seq, category='problem', display_name="Problem 1") self.gated_seq = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Gated Sequential") + ItemFactory.create(parent=self.gated_seq, category='problem', display_name="Problem 2") gating_api.add_prerequisite(self.course.id, self.open_seq.location) gating_api.set_required_content(self.course.id, self.gated_seq.location, self.open_seq.location, 100) @@ -1570,7 +1560,7 @@ class TestIndexViewWithGating(ModuleStoreTestCase, MilestonesTestCaseMixin): mako_middleware_process_request(request) with self.assertRaises(Http404): - __ = views.index( + CoursewareIndex.as_view()( request, unicode(self.course.id), chapter=self.chapter.url_name, diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 6a9418e570..4e02aab0b1 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -9,11 +9,9 @@ from collections import OrderedDict from datetime import datetime import analytics -import newrelic.agent from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User, AnonymousUser -from django.core.context_processors import csrf from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.db import transaction @@ -32,13 +30,11 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from rest_framework import status -from xblock.fragment import Fragment import shoppingcart import survey.utils import survey.views from certificates import api as certs_api -from openedx.core.lib.gating import api as gating_api from commerce.utils import EcommerceService from course_modes.models import CourseMode from courseware import grades @@ -66,7 +62,6 @@ from edxmako.shortcuts import render_to_response, render_to_string, marketing_li from instructor.enrollment import uses_shib from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.coursetalk.helpers import inject_coursetalk_keys_into_context -from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from openedx.core.djangoapps.credit.api import ( get_credit_requirement_status, is_user_eligible_for_credit, @@ -77,7 +72,6 @@ from shoppingcart.models import CourseRegistrationCode from shoppingcart.utils import is_shopping_cart_enabled from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from student.models import UserTestGroup, CourseEnrollment -from student.views import is_course_blocked from util.cache import cache, cache_if_anonymous from util.date_utils import strftime_localized from util.db import outer_atomic @@ -89,22 +83,13 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from xmodule.tabs import CourseTabList from xmodule.x_module import STUDENT_VIEW from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException -from .entrance_exams import ( - course_has_entrance_exam, - get_entrance_exam_content, - get_entrance_exam_score, - user_must_complete_entrance_exam, - user_has_passed_entrance_exam -) -from .module_render import toc_for_course, get_module_for_descriptor, get_module, get_module_by_usage_id +from .entrance_exams import user_must_complete_entrance_exam +from .module_render import get_module_for_descriptor, get_module, get_module_by_usage_id -from lang_pref import LANGUAGE_KEY log = logging.getLogger("edx.courseware") -template_imports = {'urllib': urllib} -CONTENT_DEPTH = 2 # Only display the requirements on learner dashboard for # credit and verified modes. REQUIREMENTS_DISPLAY_MODES = CourseMode.CREDIT_MODES + [CourseMode.VERIFIED] @@ -158,31 +143,11 @@ def courses(request): ) -def render_accordion(request, course, toc): - """ - Draws navigation bar. Takes current position in accordion as - parameter. - - If chapter and section are '' or None, renders a default accordion. - - course, chapter, and section are the url_names. - - Returns the html string - """ - context = dict([ - ('toc', toc), - ('course_id', course.id.to_deprecated_string()), - ('csrf', csrf(request)['csrf_token']), - ('due_date_display_format', course.due_date_display_format) - ] + template_imports.items()) - return render_to_string('courseware/accordion.html', context) - - def get_current_child(xmodule, min_depth=None, requested_child=None): """ Get the xmodule.position's display item of an xmodule that has a position and children. If xmodule has no position or is out of bounds, return the first - child with children extending down to content_depth. + child with children of min_depth. For example, if chapter_one has no position set, with two child sections, section-A having no children and section-B having a discussion unit, @@ -205,414 +170,31 @@ def get_current_child(xmodule, min_depth=None, requested_child=None): def _get_default_child_module(child_modules): """Returns the first child of xmodule, subject to min_depth.""" - if not child_modules: - default_child = None - elif not min_depth > 0: - default_child = _get_child(child_modules) + if min_depth <= 0: + return _get_child(child_modules) else: - content_children = [child for child in child_modules if - child.has_children_at_depth(min_depth - 1) and child.get_display_items()] - default_child = _get_child(content_children) if content_children else None + content_children = [ + child for child in child_modules + if child.has_children_at_depth(min_depth - 1) and child.get_display_items() + ] + return _get_child(content_children) if content_children else None - return default_child + child = None + if hasattr(xmodule, 'position'): + children = xmodule.get_display_items() + if len(children) > 0: + if xmodule.position is not None and not requested_child: + pos = xmodule.position - 1 # position is 1-indexed + if 0 <= pos < len(children): + child = children[pos] + if min_depth > 0 and not child.has_children_at_depth(min_depth - 1): + child = None + if child is None: + child = _get_default_child_module(children) - if not hasattr(xmodule, 'position'): - return None - - if xmodule.position is None or requested_child: - return _get_default_child_module(xmodule.get_display_items()) - else: - # position is 1-indexed. - pos = xmodule.position - 1 - - children = xmodule.get_display_items() - if 0 <= pos < len(children): - child = children[pos] - elif len(children) > 0: - # module has a set position, but the position is out of range. - # return default child. - child = _get_default_child_module(children) - else: - child = None return child -def redirect_to_course_position(course_module, content_depth): - """ - Return a redirect to the user's current place in the course. - - If this is the user's first time, redirects to COURSE/CHAPTER/SECTION. - If this isn't the users's first time, redirects to COURSE/CHAPTER, - and the view will find the current section and display a message - about reusing the stored position. - - If there is no current position in the course or chapter, then selects - the first child. - - """ - urlargs = {'course_id': course_module.id.to_deprecated_string()} - chapter = get_current_child(course_module, min_depth=content_depth) - if chapter is None: - # oops. Something bad has happened. - raise Http404("No chapter found when loading current position in course") - - urlargs['chapter'] = chapter.url_name - if course_module.position is not None: - return redirect(reverse('courseware_chapter', kwargs=urlargs)) - - # Relying on default of returning first child - section = get_current_child(chapter, min_depth=content_depth - 1) - if section is None: - raise Http404("No section found when loading current position in course") - - urlargs['section'] = section.url_name - return redirect(reverse('courseware_section', kwargs=urlargs)) - - -def save_child_position(seq_module, child_name): - """ - child_name: url_name of the child - """ - for position, c in enumerate(seq_module.get_display_items(), start=1): - if c.location.name == child_name: - # Only save if position changed - if position != seq_module.position: - seq_module.position = position - # Save this new position to the underlying KeyValueStore - seq_module.save() - - -def save_positions_recursively_up(user, request, field_data_cache, xmodule, course=None): - """ - Recurses up the course tree starting from a leaf - Saving the position property based on the previous node as it goes - """ - current_module = xmodule - - while current_module: - parent_location = modulestore().get_parent_location(current_module.location) - parent = None - if parent_location: - parent_descriptor = modulestore().get_item(parent_location) - parent = get_module_for_descriptor( - user, - request, - parent_descriptor, - field_data_cache, - current_module.location.course_key, - course=course - ) - - if parent and hasattr(parent, 'position'): - save_child_position(parent, current_module.location.name) - - current_module = parent - - -@transaction.non_atomic_requests -@login_required -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@ensure_valid_course_key -@outer_atomic(read_committed=True) -def index(request, course_id, chapter=None, section=None, - position=None): - """ - Displays courseware accordion and associated content. If course, chapter, - and section are all specified, renders the page, or returns an error if they - are invalid. - - If section is not specified, displays the accordion opened to the right chapter. - - If neither chapter or section are specified, redirects to user's most recent - chapter, or the first chapter if this is the user's first visit. - - Arguments: - - - request : HTTP request - - course_id : course id (str: ORG/course/URL_NAME) - - chapter : chapter url_name (str) - - section : section url_name (str) - - position : position in module, eg of module (str) - - Returns: - - - HTTPresponse - """ - - course_key = CourseKey.from_string(course_id) - - # Gather metrics for New Relic so we can slice data in New Relic Insights - newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) - newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) - - user = User.objects.prefetch_related("groups").get(id=request.user.id) - - redeemed_registration_codes = CourseRegistrationCode.objects.filter( - course_id=course_key, - registrationcoderedemption__redeemed_by=request.user - ) - - # Redirect to dashboard if the course is blocked due to non-payment. - if is_course_blocked(request, redeemed_registration_codes, course_key): - # registration codes may be generated via Bulk Purchase Scenario - # we have to check only for the invoice generated registration codes - # that their invoice is valid or not - log.warning( - u'User %s cannot access the course %s because payment has not yet been received', - user, - course_key.to_deprecated_string() - ) - return redirect(reverse('dashboard')) - - request.user = user # keep just one instance of User - with modulestore().bulk_operations(course_key): - return _index_bulk_op(request, course_key, chapter, section, position) - - -# pylint: disable=too-many-statements -def _index_bulk_op(request, course_key, chapter, section, position): - """ - Render the index page for the specified course. - """ - # Verify that position a string is in fact an int - if position is not None: - try: - int(position) - except ValueError: - raise Http404(u"Position {} is not an integer!".format(position)) - - course = get_course_with_access(request.user, 'load', course_key, depth=2) - staff_access = has_access(request.user, 'staff', course) - masquerade, user = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True) - - registered = registered_for_course(course, user) - if not registered: - # TODO (vshnayder): do course instructors need to be registered to see course? - log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.to_deprecated_string()) - return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) - - # see if all pre-requisites (as per the milestones app feature) have been fulfilled - # Note that if the pre-requisite feature flag has been turned off (default) then this check will - # always pass - if not has_access(user, 'view_courseware_with_prerequisites', course): - # prerequisites have not been fulfilled therefore redirect to the Dashboard - log.info( - u'User %d tried to view course %s ' - u'without fulfilling prerequisites', - user.id, unicode(course.id)) - return redirect(reverse('dashboard')) - - # Entrance Exam Check - # If the course has an entrance exam and the requested chapter is NOT the entrance exam, and - # the user hasn't yet met the criteria to bypass the entrance exam, redirect them to the exam. - if chapter and course_has_entrance_exam(course): - chapter_descriptor = course.get_child_by(lambda m: m.location.name == chapter) - if chapter_descriptor and not getattr(chapter_descriptor, 'is_entrance_exam', False) \ - and user_must_complete_entrance_exam(request, user, course): - log.info(u'User %d tried to view course %s without passing entrance exam', user.id, unicode(course.id)) - return redirect(reverse('courseware', args=[unicode(course.id)])) - - # Gated Content Check - gated_content = gating_api.get_gated_content(course, user) - if section and gated_content: - for usage_key in gated_content: - if section in usage_key: - raise Http404 - - # check to see if there is a required survey that must be taken before - # the user can access the course. - if survey.utils.must_answer_survey(course, user): - return redirect(reverse('course_survey', args=[unicode(course.id)])) - - bookmarks_api_url = reverse('bookmarks') - - try: - field_data_cache = FieldDataCache.cache_for_descriptor_descendents( - course_key, user, course, depth=2) - - studio_url = get_studio_url(course, 'course') - - language_preference = get_user_preference(request.user, LANGUAGE_KEY) - if not language_preference: - language_preference = settings.LANGUAGE_CODE - - context = { - 'csrf': csrf(request)['csrf_token'], - 'COURSE_TITLE': course.display_name_with_default_escaped, - 'course': course, - 'init': '', - 'fragment': Fragment(), - 'staff_access': staff_access, - 'studio_url': studio_url, - 'masquerade': masquerade, - 'xqa_server': settings.FEATURES.get('XQA_SERVER', "http://your_xqa_server.com"), - 'bookmarks_api_url': bookmarks_api_url, - 'language_preference': language_preference, - 'disable_optimizely': True, - } - table_of_contents, __, __ = toc_for_course(user, request, course, chapter, section, field_data_cache) - context['accordion'] = render_accordion(request, course, table_of_contents) - - now = datetime.now(UTC()) - effective_start = _adjust_start_date_for_beta_testers(user, course, course_key) - if not in_preview_mode() and staff_access and now < effective_start: - # Disable student view button if user is staff and - # course is not yet visible to students. - context['disable_student_access'] = True - - has_content = course.has_children_at_depth(CONTENT_DEPTH) - if not has_content: - # Show empty courseware for a course with no units - return render_to_response('courseware/courseware.html', context) - elif chapter is None: - # Check first to see if we should instead redirect the user to an Entrance Exam - if course_has_entrance_exam(course): - exam_chapter = get_entrance_exam_content(request, course) - if exam_chapter: - if exam_chapter.get_children(): - exam_section = exam_chapter.get_children()[0] - if exam_section: - return redirect('courseware_section', - course_id=unicode(course_key), - chapter=exam_chapter.url_name, - section=exam_section.url_name) - - # passing CONTENT_DEPTH avoids returning 404 for a course with an - # empty first section and a second section with content - return redirect_to_course_position(course, CONTENT_DEPTH) - - chapter_descriptor = course.get_child_by(lambda m: m.location.name == chapter) - if chapter_descriptor is not None: - save_child_position(course, chapter) - else: - # User may be trying to access a chapter that isn't live yet - if masquerade and masquerade.role == 'student': # if staff is masquerading as student be kinder, don't 404 - log.debug('staff masquerading as student: no chapter %s', chapter) - return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) - raise Http404('No chapter descriptor found with name {}'.format(chapter)) - - if course_has_entrance_exam(course): - # Message should not appear outside the context of entrance exam subsection. - # if section is none then we don't need to show message on welcome back screen also. - if getattr(chapter_descriptor, 'is_entrance_exam', False) and section is not None: - context['entrance_exam_current_score'] = get_entrance_exam_score(request, course) - context['entrance_exam_passed'] = user_has_passed_entrance_exam(request, course) - - if section is None: - section_descriptor = get_current_child(chapter_descriptor, requested_child=request.GET.get("child")) - if section_descriptor: - section = section_descriptor.url_name - else: - # Something went wrong -- perhaps this chapter has no sections visible to the user. - # Clearing out the last-visited state and showing "first-time" view by redirecting - # to courseware. - course.position = None - course.save() - return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) - else: - section_descriptor = chapter_descriptor.get_child_by(lambda m: m.location.name == section) - - if section_descriptor is None: - # Specifically asked-for section doesn't exist - if masquerade and masquerade.role == 'student': # don't 404 if staff is masquerading as student - log.debug('staff masquerading as student: no section %s', section) - return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) - raise Http404 - - # Allow chromeless operation - if section_descriptor.chrome: - chrome = [s.strip() for s in section_descriptor.chrome.lower().split(",")] - if 'accordion' not in chrome: - context['disable_accordion'] = True - if 'tabs' not in chrome: - context['disable_tabs'] = True - - if section_descriptor.default_tab: - context['default_tab'] = section_descriptor.default_tab - - # cdodge: this looks silly, but let's refetch the section_descriptor with depth=None - # which will prefetch the children more efficiently than doing a recursive load - section_descriptor = modulestore().get_item(section_descriptor.location, depth=None) - - # Load all descendants of the section, because we're going to display its - # html, which in general will need all of its children - field_data_cache.add_descriptor_descendents( - section_descriptor, depth=None - ) - - section_module = get_module_for_descriptor( - user, - request, - section_descriptor, - field_data_cache, - course_key, - position, - course=course - ) - - # Save where we are in the chapter. - save_child_position(chapter_descriptor, section) - - table_of_contents, prev_section_info, next_section_info = toc_for_course( - user, request, course, chapter, section, field_data_cache - ) - context['accordion'] = render_accordion(request, course, table_of_contents) - - def _compute_section_url(section_info, requested_child): - """ - Returns the section URL for the given section_info with the given child parameter. - """ - return "{url}?child={requested_child}".format( - url=reverse( - 'courseware_section', - args=[unicode(course.id), section_info['chapter_url_name'], section_info['url_name']], - ), - requested_child=requested_child, - ) - - section_render_context = { - 'activate_block_id': request.GET.get('activate_block_id'), - 'requested_child': request.GET.get("child"), - 'prev_url': _compute_section_url(prev_section_info, 'last') if prev_section_info else None, - 'next_url': _compute_section_url(next_section_info, 'first') if next_section_info else None, - } - context['fragment'] = section_module.render(STUDENT_VIEW, section_render_context) - context['section_title'] = section_descriptor.display_name_with_default_escaped - result = render_to_response('courseware/courseware.html', context) - except Exception as e: - - # Doesn't bar Unicode characters from URL, but if Unicode characters do - # cause an error it is a graceful failure. - if isinstance(e, UnicodeEncodeError): - raise Http404("URL contains Unicode characters") - - if isinstance(e, Http404): - # let it propagate - raise - - # In production, don't want to let a 500 out for any reason - if settings.DEBUG: - raise - else: - log.exception( - u"Error in index view: user=%s, effective_user=%s, course=%s, chapter=%s section=%s position=%s", - request.user, user, course, chapter, section, position - ) - try: - result = render_to_response('courseware/courseware-error.html', { - 'staff_access': staff_access, - 'course': course - }) - except: - # Let the exception propagate, relying on global config to at - # at least return a nice error message - log.exception("Error while rendering courseware-error page") - raise - - return result - - @ensure_csrf_cookie @ensure_valid_course_key def jump_to_id(request, course_id, module_id): diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 6bf37f83de..f8f37b3157 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -15,7 +15,8 @@ from opaque_keys import InvalidKeyError from courseware.access import is_mobile_available_for_user from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor -from courseware.views import get_current_child, save_positions_recursively_up +from courseware.index import save_positions_recursively_up +from courseware.views import get_current_child from student.models import CourseEnrollment, User from xblock.fields import Scope diff --git a/lms/urls.py b/lms/urls.py index f3db4952ce..4db872023d 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -12,6 +12,7 @@ from microsite_configuration import microsite import auth_exchange.views from config_models.views import ConfigurationModelCurrentAPIView +from courseware.index import CoursewareIndex from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration @@ -462,28 +463,28 @@ urlpatterns += ( r'^courses/{}/courseware/?$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.index', + CoursewareIndex.as_view(), name='courseware', ), url( r'^courses/{}/courseware/(?P[^/]*)/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.index', + CoursewareIndex.as_view(), name='courseware_chapter', ), url( r'^courses/{}/courseware/(?P[^/]*)/(?P
[^/]*)/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.index', + CoursewareIndex.as_view(), name='courseware_section', ), url( r'^courses/{}/courseware/(?P[^/]*)/(?P
[^/]*)/(?P[^/]*)/?$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.index', + CoursewareIndex.as_view(), name='courseware_position', ), From c6954902e3d189bec4cba3a9726646889580f6fd Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 2 May 2016 15:13:30 -0400 Subject: [PATCH 2/2] Create courseware/views folder --- common/djangoapps/student/tests/tests.py | 2 +- common/lib/xmodule/xmodule/lti_module.py | 2 +- docs/en_us/internal/overview.md | 2 +- docs/en_us/platform_api/source/conf.py | 2 +- lms/djangoapps/branding/tests/test_page.py | 8 ++--- lms/djangoapps/branding/views.py | 6 ++-- .../tests/test_field_override_performance.py | 2 +- lms/djangoapps/ccx/tests/test_views.py | 4 +-- .../course_api/blocks/serializers.py | 2 +- lms/djangoapps/courseware/middleware.py | 2 +- .../courseware/tests/test_access.py | 2 +- .../courseware/tests/test_lti_integration.py | 2 +- lms/djangoapps/courseware/tests/test_tabs.py | 5 ++- lms/djangoapps/courseware/tests/test_views.py | 12 +++---- lms/djangoapps/courseware/views/__init__.py | 0 .../courseware/{ => views}/index.py | 26 +++++++------- .../courseware/{ => views}/views.py | 9 +++-- lms/djangoapps/edxnotes/helpers.py | 2 +- lms/djangoapps/lti_provider/views.py | 2 +- lms/djangoapps/mobile_api/users/views.py | 4 +-- lms/urls.py | 36 +++++++++---------- 21 files changed, 65 insertions(+), 67 deletions(-) create mode 100644 lms/djangoapps/courseware/views/__init__.py rename lms/djangoapps/courseware/{ => views}/index.py (96%) rename lms/djangoapps/courseware/{ => views}/views.py (99%) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 319359fd96..38e5084196 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -298,7 +298,7 @@ class DashboardTest(ModuleStoreTestCase): self.assertIsNone(course_mode_info['days_for_upsell']) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - @patch('courseware.index.log.warning') + @patch('courseware.views.index.log.warning') @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) def test_blocked_course_scenario(self, log_warning): diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 84b1e71640..ed2b05a7d9 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -43,7 +43,7 @@ What is supported: (http://www.imsglobal.org/lti/ltiv2p0/uml/purl.imsglobal.org/vocab/lis/v2/outcomes/Result/service.html) a.) Discovery of all such LTI http endpoints for a course. External tools GET from this discovery endpoint and receive URLs for interacting with individual grading units. - (see lms/djangoapps/courseware/views.py:get_course_lti_endpoints) + (see lms/djangoapps/courseware/views/views.py:get_course_lti_endpoints) b.) GET, PUT and DELETE in LTI Result JSON binding (http://www.imsglobal.org/lti/ltiv2p0/mediatype/application/vnd/ims/lis/v2/result+json/index.html) for a provider to synchronize grades into edx-platform. Reading, Setting, and Deleteing diff --git a/docs/en_us/internal/overview.md b/docs/en_us/internal/overview.md index 9d5d729ca3..fad76ba30b 100644 --- a/docs/en_us/internal/overview.md +++ b/docs/en_us/internal/overview.md @@ -91,7 +91,7 @@ The LMS is a django site, with root in `lms/`. It runs in many different enviro - `lms/djangoapps/courseware/models.py` - Core rendering path: - - `lms/urls.py` points to `courseware.views.index`, which gets module info from the course xml file, pulls list of `StudentModule` objects for this user (to avoid multiple db hits). + - `lms/urls.py` points to `courseware.views.views.index`, which gets module info from the course xml file, pulls list of `StudentModule` objects for this user (to avoid multiple db hits). - Calls `render_accordion` to render the "accordion"--the display of the course structure. diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index 5cacb96b19..af7c8e42ef 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -44,7 +44,7 @@ MOCK_MODULES = [ 'courseware.access', 'courseware.model_data', 'courseware.module_render', - 'courseware.views', + 'courseware.views.views', 'util.request', 'eventtracking', 'xmodule', diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index 16fcd87c0c..af789bf88e 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -196,7 +196,7 @@ class IndexPageCourseCardsSortingTests(ModuleStoreTestCase): self.factory = RequestFactory() @patch('student.views.render_to_response', RENDER_MOCK) - @patch('courseware.views.render_to_response', RENDER_MOCK) + @patch('courseware.views.views.render_to_response', RENDER_MOCK) @patch.dict('django.conf.settings.FEATURES', {'ENABLE_COURSE_DISCOVERY': False}) def test_course_discovery_off(self): """ @@ -220,7 +220,7 @@ class IndexPageCourseCardsSortingTests(ModuleStoreTestCase): self.assertIn('
.*)$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.jump_to', + 'courseware.views.views.jump_to', name='jump_to', ), url( r'^courses/{}/jump_to_id/(?P.*)$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.jump_to_id', + 'courseware.views.views.jump_to_id', name='jump_to_id', ), @@ -318,7 +318,7 @@ urlpatterns += ( # Note: This is not an API. Compare this with the xblock_view API above. url( r'^xblock/{usage_key_string}$'.format(usage_key_string=settings.USAGE_KEY_PATTERN), - 'courseware.views.render_xblock', + 'courseware.views.views.render_xblock', name='render_xblock', ), @@ -362,7 +362,7 @@ urlpatterns += ( r'^courses/{}/about$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.course_about', + 'courseware.views.views.course_about', name='about_course', ), @@ -371,14 +371,14 @@ urlpatterns += ( r'^courses/{}/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.course_info', + 'courseware.views.views.course_info', name='course_root', ), url( r'^courses/{}/info$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.course_info', + 'courseware.views.views.course_info', name='info', ), # TODO arjun remove when custom tabs in place, see courseware/courses.py @@ -386,7 +386,7 @@ urlpatterns += ( r'^courses/{}/syllabus$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.syllabus', + 'courseware.views.views.syllabus', name='syllabus', ), @@ -395,7 +395,7 @@ urlpatterns += ( r'^courses/{}/survey$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.course_survey', + 'courseware.views.views.course_survey', name='course_survey', ), @@ -492,7 +492,7 @@ urlpatterns += ( r'^courses/{}/progress$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.progress', + 'courseware.views.views.progress', name='progress', ), # Takes optional student_id for instructor use--shows profile as that student sees it. @@ -500,7 +500,7 @@ urlpatterns += ( r'^courses/{}/progress/(?P[^/]*)/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.progress', + 'courseware.views.views.progress', name='student_progress', ), @@ -632,7 +632,7 @@ urlpatterns += ( r'^courses/{}/lti_rest_endpoints/'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.get_course_lti_endpoints', + 'courseware.views.views.get_course_lti_endpoints', name='lti_rest_endpoints', ), @@ -697,7 +697,7 @@ urlpatterns += ( r'^courses/{}/generate_user_cert'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.generate_user_cert', + 'courseware.views.views.generate_user_cert', name='generate_user_cert', ), ) @@ -750,7 +750,7 @@ urlpatterns += ( r'^courses/{}/(?P[^/]+)/$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.static_tab', + 'courseware.views.views.static_tab', name='static_tab', ), ) @@ -761,7 +761,7 @@ if settings.FEATURES.get('ENABLE_STUDENT_HISTORY_VIEW'): r'^courses/{}/submission_history/(?P[^/]*)/(?P.*?)$'.format( settings.COURSE_ID_PATTERN ), - 'courseware.views.submission_history', + 'courseware.views.views.submission_history', name='submission_history', ), ) @@ -994,17 +994,17 @@ if settings.FEATURES.get('ENABLE_FINANCIAL_ASSISTANCE_FORM'): urlpatterns += ( url( r'^financial-assistance/$', - 'courseware.views.financial_assistance', + 'courseware.views.views.financial_assistance', name='financial_assistance' ), url( r'^financial-assistance/apply/$', - 'courseware.views.financial_assistance_form', + 'courseware.views.views.financial_assistance_form', name='financial_assistance_form' ), url( r'^financial-assistance/submit/$', - 'courseware.views.financial_assistance_request', + 'courseware.views.views.financial_assistance_request', name='submit_financial_assistance_request' ) )