From 8cbf99aca4274d51c57486a67b63015e3b488d2b Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 25 Feb 2016 11:05:41 -0500 Subject: [PATCH] Activate Next and Previous Buttons across sections MA-2152 MA-2153 --- .../xmodule/js/src/sequence/display.coffee | 53 ++++-- common/lib/xmodule/xmodule/seq_module.py | 117 +++++++++++-- .../xmodule/xmodule/tests/test_sequence.py | 163 ++++++++++++++++++ .../lib/xmodule/xmodule/tests/xml/__init__.py | 3 +- .../xmodule/xmodule/tests/xml/factories.py | 8 +- .../test/acceptance/pages/lms/course_nav.py | 19 +- .../test/acceptance/pages/lms/courseware.py | 75 ++++++-- .../tests/lms/test_lms_courseware.py | 73 +++++++- .../acceptance/tests/lms/test_lms_edxnotes.py | 20 +-- .../tests/studio/test_studio_outline.py | 4 +- .../tests/video/test_video_module.py | 8 +- lms/djangoapps/courseware/tests/test_views.py | 123 ++++++++++++- lms/djangoapps/courseware/url_helpers.py | 6 +- lms/djangoapps/courseware/views.py | 157 +++++++++-------- lms/envs/bok_choy.py | 3 + lms/envs/common.py | 5 +- lms/templates/seq_module.html | 2 +- 17 files changed, 680 insertions(+), 159 deletions(-) create mode 100644 common/lib/xmodule/xmodule/tests/test_sequence.py diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee index 9c098efbcc..cf14d9644f 100644 --- a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee @@ -8,6 +8,8 @@ class @Sequence @num_contents = @contents.length @id = @el.data('id') @ajaxUrl = @el.data('ajax-url') + @nextUrl = @el.data('next-url') + @prevUrl = @el.data('prev-url') @base_page_title = " | " + document.title @initProgress() @bind() @@ -73,23 +75,35 @@ class @Sequence when 'in_progress' then element.addClass('progress-some') when 'done' then element.addClass('progress-done') + enableButton: (button_class, button_action) -> + @$(button_class).removeClass('disabled').removeAttr('disabled').click(button_action) + + disableButton: (button_class) -> + @$(button_class).addClass('disabled').attr('disabled', true) + + setButtonLabel: (button_class, button_label) -> + @$(button_class + ' .sr').html(button_label) + + updateButtonState: (button_class, button_action, action_label_prefix, is_at_boundary, boundary_url) -> + if is_at_boundary and boundary_url == 'None' + @disableButton(button_class) + else + button_label = action_label_prefix + (if is_at_boundary then ' Section' else ' Unit') + @setButtonLabel(button_class, button_label) + @enableButton(button_class, button_action) + toggleArrows: => @$('.sequence-nav-button').unbind('click') - if @contents.length == 0 ## There are no modules to display, and therefore no nav to build. - @$('.sequence-nav-button.button-previous').addClass('disabled').attr('disabled', true) - @$('.sequence-nav-button.button-next').addClass('disabled').attr('disabled', true) - return + # previous button + first_tab = @position == 1 + previous_button_class = '.sequence-nav-button.button-previous' + @updateButtonState(previous_button_class, @previous, 'Previous', first_tab, @prevUrl) - if @position == 1 ## 1 != 0 here. 1 is the first item in the sequence nav. - @$('.sequence-nav-button.button-previous').addClass('disabled').attr('disabled', true) - else - @$('.sequence-nav-button.button-previous').removeClass('disabled').removeAttr('disabled').click(@previous) - - if @position == @contents.length ## If the final position on the nav matches the total contents. - @$('.sequence-nav-button.button-next').addClass('disabled').attr('disabled', true) - else - @$('.sequence-nav-button.button-next').removeClass('disabled').removeAttr('disabled').click(@next) + # next button + last_tab = @position >= @contents.length # use inequality in case contents.length is 0 and position is 1. + next_button_class = '.sequence-nav-button.button-next' + @updateButtonState(next_button_class, @next, 'Next', last_tab, @nextUrl) render: (new_position) -> if @position != new_position @@ -164,10 +178,15 @@ class @Sequence new: new_position id: @id - # If the bottom nav is used, scroll to the top of the page on change. - if $(event.target).closest('nav[class="sequence-bottom"]').length > 0 - $.scrollTo 0, 150 - @render new_position + if (direction == "seq_next") and (@position == @contents.length) + window.location.href = @nextUrl + else if (direction == "seq_prev") and (@position == 1) + window.location.href = @prevUrl + else + # If the bottom nav is used, scroll to the top of the page on change. + if $(event.target).closest('nav[class="sequence-bottom"]').length > 0 + $.scrollTo 0, 150 + @render new_position link_for: (position) -> @$("#sequence-list a[data-element=#{position}]") diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 2e9c561fcc..9548bfa57d 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -141,16 +141,8 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): # If position is specified in system, then use that instead. position = getattr(self.system, 'position', None) if position is not None: - try: - self.position = int(self.system.position) - except (ValueError, TypeError): - # Check for https://openedx.atlassian.net/browse/LMS-6496 - warnings.warn( - "Sequential position cannot be converted to an integer: {pos!r}".format( - pos=self.system.position, - ), - RuntimeWarning, - ) + assert isinstance(position, int) + self.position = self.system.position def get_progress(self): ''' Return the total progress, adding total done and total available. @@ -177,9 +169,16 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): raise NotFoundError('Unexpected dispatch type') def student_view(self, context): + display_items = self.get_display_items() + # If we're rendering this sequence, but no position is set yet, + # or exceeds the length of the displayable items, # default the position to the first element - if self.position is None: + if context.get('requested_child') == 'first': + self.position = 1 + elif context.get('requested_child') == 'last': + self.position = len(display_items) or None + elif self.position is None or self.position > len(display_items): self.position = 1 ## Returns a set of all types of all sub-children @@ -211,7 +210,6 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): fragment.add_content(view_html) return fragment - display_items = self.get_display_items() for child in display_items: is_bookmarked = bookmarks_service.is_bookmarked(usage_key=child.scope_ids.usage_id) context["bookmarked"] = is_bookmarked @@ -245,6 +243,16 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): 'position': self.position, 'tag': self.location.category, 'ajax_url': self.system.ajax_url, + 'next_url': _compute_next_url( + self.location, + parent_module, + context.get('redirect_url_func'), + ), + 'prev_url': _compute_previous_url( + self.location, + parent_module, + context.get('redirect_url_func'), + ), } fragment.add_content(self.system.render_template("seq_module.html", params)) @@ -453,3 +461,88 @@ class SequenceDescriptor(SequenceFields, ProctoringFields, MakoModuleDescriptor, xblock_body["content_type"] = "Sequence" return xblock_body + + +def _compute_next_url(block_location, parent_block, redirect_url_func): + """ + Returns the url for the next block after the given block. + """ + def get_next_block_location(parent_block, index_in_parent): + """ + Returns the next block in the parent_block after the block with the given + index_in_parent. + """ + if index_in_parent + 1 < len(parent_block.children): + return parent_block.children[index_in_parent + 1] + else: + return None + + return _compute_next_or_prev_url( + block_location, + parent_block, + redirect_url_func, + get_next_block_location, + 'first', + ) + + +def _compute_previous_url(block_location, parent_block, redirect_url_func): + """ + Returns the url for the previous block after the given block. + """ + def get_previous_block_location(parent_block, index_in_parent): + """ + Returns the previous block in the parent_block before the block with the given + index_in_parent. + """ + return parent_block.children[index_in_parent - 1] if index_in_parent else None + + return _compute_next_or_prev_url( + block_location, + parent_block, + redirect_url_func, + get_previous_block_location, + 'last', + ) + + +def _compute_next_or_prev_url( + block_location, + parent_block, + redirect_url_func, + get_next_or_prev_block, + redirect_url_child_param, +): + """ + Returns the url for the next or previous block from the given block. + + Arguments: + block_location: Location of the block that is being navigated. + parent_block: Parent block of the given block. + redirect_url_func: Function that computes a redirect URL directly to + a block, given the block's location. + get_next_or_prev_block: Function that returns the next or previous + block in the parent, or None if doesn't exist. + redirect_url_child_param: Value to pass for the child parameter to the + redirect_url_func. + """ + if redirect_url_func: + index_in_parent = parent_block.children.index(block_location) + next_or_prev_block_location = get_next_or_prev_block(parent_block, index_in_parent) + if next_or_prev_block_location: + return redirect_url_func( + block_location.course_key, + next_or_prev_block_location, + child=redirect_url_child_param, + ) + else: + grandparent = parent_block.get_parent() + if grandparent: + return _compute_next_or_prev_url( + parent_block.location, + grandparent, + redirect_url_func, + get_next_or_prev_block, + redirect_url_child_param, + ) + return None diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py new file mode 100644 index 0000000000..9e4e1aba36 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -0,0 +1,163 @@ +""" +Tests for sequence module. +""" +# pylint: disable=no-member +from mock import Mock +from xblock.reference.user_service import XBlockUser, UserService +from xmodule.tests import get_test_system +from xmodule.tests.xml import XModuleXmlImportTest +from xmodule.tests.xml import factories as xml +from xmodule.x_module import STUDENT_VIEW +from xmodule.seq_module import _compute_next_url, _compute_previous_url, SequenceModule + + +class StubUserService(UserService): + """ + Stub UserService for testing the sequence module. + """ + def get_current_user(self): + """ + Implements abstract method for getting the current user. + """ + user = XBlockUser() + user.opt_attrs['edx-platform.username'] = 'test user' + return user + + +class SequenceBlockTestCase(XModuleXmlImportTest): + """ + Tests for the Sequence Module. + """ + @classmethod + def setUpClass(cls): + super(SequenceBlockTestCase, cls).setUpClass() + + course_xml = cls._set_up_course_xml() + cls.course = cls.process_xml(course_xml) + cls._set_up_module_system(cls.course) + + for chapter_index in range(len(cls.course.get_children())): + chapter = cls._set_up_block(cls.course, chapter_index) + setattr(cls, 'chapter_{}'.format(chapter_index + 1), chapter) + + for sequence_index in range(len(chapter.get_children())): + sequence = cls._set_up_block(chapter, sequence_index) + setattr(cls, 'sequence_{}_{}'.format(chapter_index + 1, sequence_index + 1), sequence) + + @classmethod + def _set_up_course_xml(cls): + """ + Sets up and returns XML course structure. + """ + course = xml.CourseFactory.build() + + chapter_1 = xml.ChapterFactory.build(parent=course) # has 2 child sequences + xml.ChapterFactory.build(parent=course) # has 0 child sequences + chapter_3 = xml.ChapterFactory.build(parent=course) # has 1 child sequence + chapter_4 = xml.ChapterFactory.build(parent=course) # has 2 child sequences + + xml.SequenceFactory.build(parent=chapter_1) + xml.SequenceFactory.build(parent=chapter_1) + sequence_3_1 = xml.SequenceFactory.build(parent=chapter_3) # has 3 verticals + xml.SequenceFactory.build(parent=chapter_4) + xml.SequenceFactory.build(parent=chapter_4) + + for _ in range(3): + xml.VerticalFactory.build(parent=sequence_3_1) + + return course + + @classmethod + def _set_up_block(cls, parent, index_in_parent): + """ + Sets up the stub sequence module for testing. + """ + block = parent.get_children()[index_in_parent] + + cls._set_up_module_system(block) + + block.xmodule_runtime._services['bookmarks'] = Mock() # pylint: disable=protected-access + block.xmodule_runtime._services['user'] = StubUserService() # pylint: disable=protected-access + block.xmodule_runtime.xmodule_instance = getattr(block, '_xmodule', None) # pylint: disable=protected-access + block.parent = parent.location + return block + + @classmethod + def _set_up_module_system(cls, block): + """ + Sets up the test module system for the given block. + """ + module_system = get_test_system() + module_system.descriptor_runtime = block._runtime # pylint: disable=protected-access + block.xmodule_runtime = module_system + + def test_student_view_init(self): + seq_module = SequenceModule(runtime=Mock(position=2), descriptor=Mock(), scope_ids=Mock()) + self.assertEquals(seq_module.position, 2) # matches position set in the runtime + + def test_render_student_view(self): + html = self._get_rendered_student_view(self.sequence_3_1, requested_child=None) + self._assert_view_at_position(html, expected_position=1) + self.assertIn(unicode(self.sequence_3_1.location), html) + self.assertIn("'next_url': u'{}'".format(unicode(self.chapter_4.location)), html) + self.assertIn("'prev_url': u'{}'".format(unicode(self.chapter_2.location)), html) + + def test_student_view_first_child(self): + html = self._get_rendered_student_view(self.sequence_3_1, requested_child='first') + self._assert_view_at_position(html, expected_position=1) + + def test_student_view_last_child(self): + html = self._get_rendered_student_view(self.sequence_3_1, requested_child='last') + self._assert_view_at_position(html, expected_position=3) + + def _get_rendered_student_view(self, sequence, requested_child): + """ + Returns the rendered student view for the given sequence and the + requested_child parameter. + """ + return sequence.xmodule_runtime.render( + sequence, + STUDENT_VIEW, + { + 'redirect_url_func': lambda course_key, block_location, child: unicode(block_location), + 'requested_child': requested_child, + }, + ).content + + def _assert_view_at_position(self, rendered_html, expected_position): + """ + Verifies that the rendered view contains the expected position. + """ + self.assertIn("'position': {}".format(expected_position), rendered_html) + + def test_compute_next_url(self): + + for sequence, parent, expected_next_sequence_location in [ + (self.sequence_1_1, self.chapter_1, self.sequence_1_2.location), + (self.sequence_1_2, self.chapter_1, self.chapter_2.location), + (self.sequence_3_1, self.chapter_3, self.chapter_4.location), + (self.sequence_4_1, self.chapter_4, self.sequence_4_2.location), + (self.sequence_4_2, self.chapter_4, None), + ]: + actual_next_sequence_location = _compute_next_url( + sequence.location, + parent, + lambda course_key, block_location, child: block_location, + ) + self.assertEquals(actual_next_sequence_location, expected_next_sequence_location) + + def test_compute_previous_url(self): + + for sequence, parent, expected_prev_sequence_location in [ + (self.sequence_1_1, self.chapter_1, None), + (self.sequence_1_2, self.chapter_1, self.sequence_1_1.location), + (self.sequence_3_1, self.chapter_3, self.chapter_2.location), + (self.sequence_4_1, self.chapter_4, self.chapter_3.location), + (self.sequence_4_2, self.chapter_4, self.sequence_4_1.location), + ]: + actual_next_sequence_location = _compute_previous_url( + sequence.location, + parent, + lambda course_key, block_location, child: block_location, + ) + self.assertEquals(actual_next_sequence_location, expected_prev_sequence_location) diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py index 3beeebf271..2c777c69c6 100644 --- a/common/lib/xmodule/xmodule/tests/xml/__init__.py +++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py @@ -57,7 +57,8 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable class XModuleXmlImportTest(TestCase): """Base class for tests that use basic XML parsing""" - def process_xml(self, xml_import_data): + @classmethod + def process_xml(cls, xml_import_data): """Use the `xml_import_data` to import an :class:`XBlock` from XML.""" system = InMemorySystem(xml_import_data) return system.process_xml(xml_import_data.xml_string) diff --git a/common/lib/xmodule/xmodule/tests/xml/factories.py b/common/lib/xmodule/xmodule/tests/xml/factories.py index b131a65c76..903f2d925e 100644 --- a/common/lib/xmodule/xmodule/tests/xml/factories.py +++ b/common/lib/xmodule/xmodule/tests/xml/factories.py @@ -8,6 +8,7 @@ from fs.memoryfs import MemoryFS from factory import Factory, lazy_attribute, post_generation, Sequence from lxml import etree +from xblock.mixins import HierarchyMixin from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.x_module import XModuleMixin from xmodule.modulestore import only_xmodules @@ -68,7 +69,7 @@ class XmlImportFactory(Factory): model = XmlImportData filesystem = MemoryFS() - xblock_mixins = (InheritanceMixin, XModuleMixin) + xblock_mixins = (InheritanceMixin, XModuleMixin, HierarchyMixin) xblock_select = only_xmodules url_name = Sequence(str) attribs = {} @@ -142,6 +143,11 @@ class CourseFactory(XmlImportFactory): static_asset_path = 'xml_test_course' +class ChapterFactory(XmlImportFactory): + """Factory for nodes""" + tag = 'chapter' + + class SequenceFactory(XmlImportFactory): """Factory for nodes""" tag = 'sequential' diff --git a/common/test/acceptance/pages/lms/course_nav.py b/common/test/acceptance/pages/lms/course_nav.py index bc68b55077..980772440a 100644 --- a/common/test/acceptance/pages/lms/course_nav.py +++ b/common/test/acceptance/pages/lms/course_nav.py @@ -3,7 +3,7 @@ Course navigation page object """ import re -from bok_choy.page_object import PageObject +from bok_choy.page_object import PageObject, unguarded from bok_choy.promise import EmptyPromise @@ -165,10 +165,11 @@ class CourseNavPage(PageObject): """ desc = "currently at section '{0}' and subsection '{1}'".format(section_title, subsection_title) return EmptyPromise( - lambda: self._is_on_section(section_title, subsection_title), desc + lambda: self.is_on_section(section_title, subsection_title), desc ) - def _is_on_section(self, section_title, subsection_title): + @unguarded + def is_on_section(self, section_title, subsection_title): """ Return a boolean indicating whether the user is on the section and subsection with the specified titles. @@ -203,13 +204,9 @@ class CourseNavPage(PageObject): """ return self.REMOVE_SPAN_TAG_RE.search(element.get_attribute('innerHTML')).groups()[0].strip() - def go_to_sequential_position(self, sequential_position): + @property + def active_subsection_url(self): """ - Within a section/subsection navigate to the sequential position specified by `sequential_position`. - - Arguments: - sequential_position (int): position in sequential bar - + return the url of the active subsection in the left nav """ - sequential_position_css = '#tab_{0}'.format(sequential_position - 1) - self.q(css=sequential_position_css).first.click() + return self.q(css='.chapter-content-container .menu-item.active a').attrs('href')[0] diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index b87b3c194e..62ddca6e33 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -20,6 +20,13 @@ class CoursewarePage(CoursePage): def is_browser_on_page(self): return self.q(css='body.courseware').present + @property + def chapter_count_in_navigation(self): + """ + Returns count of chapters available on LHS navigation. + """ + return len(self.q(css='nav.course-navigation a.chapter')) + @property def num_sections(self): """ @@ -101,11 +108,66 @@ class CoursewarePage(CoursePage): return element.text[0] return None - def get_active_subsection_url(self): + def go_to_sequential_position(self, sequential_position): """ - return the url of the active subsection in the left nav + Within a section/subsection navigate to the sequential position specified by `sequential_position`. + + Arguments: + sequential_position (int): position in sequential bar """ - return self.q(css='.chapter-content-container .menu-item.active a').attrs('href')[0] + sequential_position_css = '#sequence-list #tab_{0}'.format(sequential_position - 1) + self.q(css=sequential_position_css).first.click() + + @property + def sequential_position(self): + """ + Returns the position of the active tab in the sequence. + """ + tab_id = self._active_sequence_tab.attrs('id')[0] + return int(tab_id.split('_')[1]) + + @property + def _active_sequence_tab(self): # pylint: disable=missing-docstring + return self.q(css='#sequence-list .nav-item.active') + + @property + def is_next_button_enabled(self): # pylint: disable=missing-docstring + return not self.q(css='.sequence-nav > .sequence-nav-button.button-next.disabled').is_present() + + @property + def is_previous_button_enabled(self): # pylint: disable=missing-docstring + return not self.q(css='.sequence-nav > .sequence-nav-button.button-previous.disabled').is_present() + + def click_next_button_on_top(self): # pylint: disable=missing-docstring + self._click_navigation_button('sequence-nav', 'button-next') + + def click_next_button_on_bottom(self): # pylint: disable=missing-docstring + self._click_navigation_button('sequence-bottom', 'button-next') + + def click_previous_button_on_top(self): # pylint: disable=missing-docstring + self._click_navigation_button('sequence-nav', 'button-previous') + + def click_previous_button_on_bottom(self): # pylint: disable=missing-docstring + self._click_navigation_button('sequence-bottom', 'button-previous') + + def _click_navigation_button(self, top_or_bottom_class, next_or_previous_class): + """ + Clicks the navigation button, given the respective CSS classes. + """ + previous_tab_id = self._active_sequence_tab.attrs('data-id')[0] + + def is_at_new_tab_id(): + """ + Returns whether the active tab has changed. It is defensive + against the case where the page is still being loaded. + """ + active_tab = self._active_sequence_tab + return active_tab and previous_tab_id != active_tab.attrs('data-id')[0] + + self.q( + css='.{} > .sequence-nav-button.{}'.format(top_or_bottom_class, next_or_previous_class) + ).first.click() + EmptyPromise(is_at_new_tab_id, "Button navigation fulfilled").fulfill() @property def can_start_proctored_exam(self): @@ -160,13 +222,6 @@ class CoursewarePage(CoursePage): return self.entrance_exam_message_selector.is_present() \ and "You have passed the entrance exam" in self.entrance_exam_message_selector.text[0] - @property - def chapter_count_in_navigation(self): - """ - Returns count of chapters available on LHS navigation. - """ - return len(self.q(css='nav.course-navigation a.chapter')) - @property def is_timer_bar_present(self): """ diff --git a/common/test/acceptance/tests/lms/test_lms_courseware.py b/common/test/acceptance/tests/lms/test_lms_courseware.py index c6391212be..dd8beea9bc 100644 --- a/common/test/acceptance/tests/lms/test_lms_courseware.py +++ b/common/test/acceptance/tests/lms/test_lms_courseware.py @@ -2,7 +2,8 @@ """ End-to-end tests for the LMS. """ -import time +from nose.plugins.attrib import attr +from unittest import skip from ..helpers import UniqueCourseTest from ...pages.studio.auto_auth import AutoAuthPage @@ -420,13 +421,20 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest): course_fix.add_children( XBlockFixtureDesc('chapter', 'Test Section 1').add_children( - XBlockFixtureDesc('sequential', 'Test Subsection 1').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection 1,1').add_children( XBlockFixtureDesc('problem', 'Test Problem 1', data='problem 1 dummy body'), XBlockFixtureDesc('html', 'html 1', data="html 1 dummy body"), XBlockFixtureDesc('problem', 'Test Problem 2', data="problem 2 dummy body"), XBlockFixtureDesc('html', 'html 2', data="html 2 dummy body"), ), - XBlockFixtureDesc('sequential', 'Test Subsection 2'), + XBlockFixtureDesc('sequential', 'Test Subsection 1,2').add_children( + XBlockFixtureDesc('problem', 'Test Problem 3', data='problem 3 dummy body'), + ), + ), + XBlockFixtureDesc('chapter', 'Test Section 2').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection 2,1').add_children( + XBlockFixtureDesc('problem', 'Test Problem 4', data='problem 4 dummy body'), + ), ), ).install() @@ -436,10 +444,53 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest): self.courseware_page.visit() self.course_nav = CourseNavPage(self.browser) + def test_navigation_buttons(self): + # start in first section + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,1', 0, next_enabled=True, prev_enabled=False) + + # next takes us to next tab in sequential + self.courseware_page.click_next_button_on_top() + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,1', 1, next_enabled=True, prev_enabled=True) + + # go to last sequential position + self.courseware_page.go_to_sequential_position(4) + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,1', 3, next_enabled=True, prev_enabled=True) + + # next takes us to next sequential + self.courseware_page.click_next_button_on_bottom() + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,2', 0, next_enabled=True, prev_enabled=True) + + # next takes us to next chapter + self.courseware_page.click_next_button_on_top() + self.assert_navigation_state('Test Section 2', 'Test Subsection 2,1', 0, next_enabled=False, prev_enabled=True) + + # previous takes us to previous chapter + self.courseware_page.click_previous_button_on_top() + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,2', 0, next_enabled=True, prev_enabled=True) + + # previous takes us to last tab in previous sequential + self.courseware_page.click_previous_button_on_bottom() + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,1', 3, next_enabled=True, prev_enabled=True) + + # previous takes us to previous tab in sequential + self.courseware_page.click_previous_button_on_bottom() + self.assert_navigation_state('Test Section 1', 'Test Subsection 1,1', 2, next_enabled=True, prev_enabled=True) + + def assert_navigation_state( + self, section_title, subsection_title, subsection_position, next_enabled, prev_enabled + ): + """ + Verifies that the navigation state is as expected. + """ + self.assertTrue(self.course_nav.is_on_section(section_title, subsection_title)) + self.assertEquals(self.courseware_page.sequential_position, subsection_position) + self.assertEquals(self.courseware_page.is_next_button_enabled, next_enabled) + self.assertEquals(self.courseware_page.is_previous_button_enabled, prev_enabled) + def test_tab_position(self): # test that using the position in the url direct to correct tab in courseware - self.course_nav.go_to_section('Test Section 1', 'Test Subsection 1') - subsection_url = self.courseware_page.get_active_subsection_url() + self.course_nav.go_to_section('Test Section 1', 'Test Subsection 1,1') + subsection_url = self.course_nav.active_subsection_url url_part_list = subsection_url.split('/') self.assertEqual(len(url_part_list), 9) @@ -481,3 +532,15 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest): position=4 ).visit() self.assertIn('html 2 dummy body', html2_page.get_selected_tab_content()) + + @skip('Fix a11y test errors.') + @attr('a11y') + def test_courseware_a11y(self): + """ + Run accessibility audit for the problem type. + """ + self.course_nav.go_to_section('Test Section 1', 'Test Subsection 1,1') + # Set the scope to the sequence navigation + self.courseware_page.a11y_audit.config.set_scope( + include=['div.sequence-nav']) + self.courseware_page.a11y_audit.check_for_accessibility_errors() diff --git a/common/test/acceptance/tests/lms/test_lms_edxnotes.py b/common/test/acceptance/tests/lms/test_lms_edxnotes.py index fe4314c7ba..b10c3d694c 100644 --- a/common/test/acceptance/tests/lms/test_lms_edxnotes.py +++ b/common/test/acceptance/tests/lms/test_lms_edxnotes.py @@ -194,14 +194,14 @@ class EdxNotesDefaultInteractionsTest(EdxNotesTestMixin): self.create_notes(components) self.assert_text_in_notes(self.note_unit_page.notes) - self.course_nav.go_to_sequential_position(2) + self.courseware_page.go_to_sequential_position(2) components = self.note_unit_page.components self.create_notes(components) components = self.note_unit_page.refresh() self.assert_text_in_notes(self.note_unit_page.notes) - self.course_nav.go_to_sequential_position(1) + self.courseware_page.go_to_sequential_position(1) components = self.note_unit_page.components self.assert_text_in_notes(self.note_unit_page.notes) @@ -227,7 +227,7 @@ class EdxNotesDefaultInteractionsTest(EdxNotesTestMixin): self.edit_notes(components) self.assert_text_in_notes(self.note_unit_page.notes) - self.course_nav.go_to_sequential_position(2) + self.courseware_page.go_to_sequential_position(2) components = self.note_unit_page.components self.edit_notes(components) self.assert_text_in_notes(self.note_unit_page.notes) @@ -235,7 +235,7 @@ class EdxNotesDefaultInteractionsTest(EdxNotesTestMixin): components = self.note_unit_page.refresh() self.assert_text_in_notes(self.note_unit_page.notes) - self.course_nav.go_to_sequential_position(1) + self.courseware_page.go_to_sequential_position(1) components = self.note_unit_page.components self.assert_text_in_notes(self.note_unit_page.notes) @@ -261,7 +261,7 @@ class EdxNotesDefaultInteractionsTest(EdxNotesTestMixin): self.remove_notes(components) self.assert_notes_are_removed(components) - self.course_nav.go_to_sequential_position(2) + self.courseware_page.go_to_sequential_position(2) components = self.note_unit_page.components self.remove_notes(components) self.assert_notes_are_removed(components) @@ -269,7 +269,7 @@ class EdxNotesDefaultInteractionsTest(EdxNotesTestMixin): components = self.note_unit_page.refresh() self.assert_notes_are_removed(components) - self.course_nav.go_to_sequential_position(1) + self.courseware_page.go_to_sequential_position(1) components = self.note_unit_page.components self.assert_notes_are_removed(components) @@ -1106,10 +1106,10 @@ class EdxNotesPageTest(EventsTestMixin, EdxNotesTestMixin): self.assertTrue(note.is_visible) note = self.note_unit_page.notes[1] self.assertFalse(note.is_visible) - self.course_nav.go_to_sequential_position(2) + self.courseware_page.go_to_sequential_position(2) note = self.note_unit_page.notes[0] self.assertFalse(note.is_visible) - self.course_nav.go_to_sequential_position(1) + self.courseware_page.go_to_sequential_position(1) note = self.note_unit_page.notes[0] self.assertFalse(note.is_visible) @@ -1494,7 +1494,7 @@ class EdxNotesToggleNotesTest(EdxNotesTestMixin): # Disable all notes self.note_unit_page.toggle_visibility() self.assertEqual(len(self.note_unit_page.notes), 0) - self.course_nav.go_to_sequential_position(2) + self.courseware_page.go_to_sequential_position(2) self.assertEqual(len(self.note_unit_page.notes), 0) self.course_nav.go_to_section(u"Test Section 1", u"Test Subsection 2") self.assertEqual(len(self.note_unit_page.notes), 0) @@ -1520,7 +1520,7 @@ class EdxNotesToggleNotesTest(EdxNotesTestMixin): # the page. self.note_unit_page.toggle_visibility() self.assertGreater(len(self.note_unit_page.notes), 0) - self.course_nav.go_to_sequential_position(2) + self.courseware_page.go_to_sequential_position(2) self.assertGreater(len(self.note_unit_page.notes), 0) self.course_nav.go_to_section(u"Test Section 1", u"Test Subsection 2") self.assertGreater(len(self.note_unit_page.notes), 0) diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index a72e0a0a4c..d7cd119d69 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -1546,7 +1546,7 @@ class PublishSectionTest(CourseOutlineTest): self.assertTrue(section.publish_action) self.courseware.visit() self.assertEqual(1, self.courseware.num_xblock_components) - self.course_nav.go_to_sequential_position(2) + self.courseware.go_to_sequential_position(2) self.assertEqual(1, self.courseware.num_xblock_components) def test_section_publishing(self): @@ -1571,7 +1571,7 @@ class PublishSectionTest(CourseOutlineTest): self.assertFalse(unit.publish_action) self.courseware.visit() self.assertEqual(1, self.courseware.num_xblock_components) - self.course_nav.go_to_sequential_position(2) + self.courseware.go_to_sequential_position(2) self.assertEqual(1, self.courseware.num_xblock_components) self.course_nav.go_to_section(SECTION_NAME, 'Test Subsection 2') self.assertEqual(1, self.courseware.num_xblock_components) diff --git a/common/test/acceptance/tests/video/test_video_module.py b/common/test/acceptance/tests/video/test_video_module.py index 90d89a482a..1e6a7418c7 100644 --- a/common/test/acceptance/tests/video/test_video_module.py +++ b/common/test/acceptance/tests/video/test_video_module.py @@ -11,6 +11,7 @@ from unittest import skipIf, skip from ..helpers import UniqueCourseTest, is_youtube_available, YouTubeStubConfig from ...pages.lms.video.video import VideoPage from ...pages.lms.tab_nav import TabNavPage +from ...pages.lms.courseware import CoursewarePage from ...pages.lms.course_nav import CourseNavPage from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.course_info import CourseInfoPage @@ -49,6 +50,7 @@ class VideoBaseTest(UniqueCourseTest): self.video = VideoPage(self.browser) self.tab_nav = TabNavPage(self.browser) self.course_nav = CourseNavPage(self.browser) + self.courseware = CoursewarePage(self.browser, self.course_id) self.course_info_page = CourseInfoPage(self.browser, self.course_id) self.auth_page = AutoAuthPage(self.browser, course_id=self.course_id) @@ -190,7 +192,7 @@ class VideoBaseTest(UniqueCourseTest): """ Navigate to sequential specified by `video_display_name` """ - self.course_nav.go_to_sequential_position(position) + self.courseware.go_to_sequential_position(position) self.video.wait_for_video_player_render() @@ -916,13 +918,13 @@ class YouTubeVideoTest(VideoBaseTest): execute_video_steps(['A']) # go to second sequential position - self.course_nav.go_to_sequential_position(2) + self.courseware.go_to_sequential_position(2) execute_video_steps(tab2_video_names) # go back to first sequential position # we are again playing tab 1 videos to ensure that switching didn't broke some video functionality. - self.course_nav.go_to_sequential_position(1) + self.courseware.go_to_sequential_position(1) execute_video_steps(tab1_video_names) self.video.browser.refresh() diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 94f6b2d326..03cc538de7 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -37,6 +37,7 @@ from course_modes.tests.factories import CourseModeFactory from courseware.model_data import set_score from courseware.testutils import RenderXBlockTestMixin from courseware.tests.factories import StudentModuleFactory +from courseware.url_helpers import get_redirect_url from courseware.user_state_client import DjangoXBlockUserStateClient from edxmako.tests import mako_middleware_process_request from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error @@ -192,9 +193,25 @@ class ViewsTestCase(ModuleStoreTestCase): super(ViewsTestCase, self).setUp() self.course = CourseFactory.create(display_name=u'teꜱᴛ course') self.chapter = ItemFactory.create(category='chapter', parent_location=self.course.location) - self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location, due=datetime(2013, 9, 18, 11, 30, 00)) + self.section = ItemFactory.create( + category='sequential', + parent_location=self.chapter.location, + due=datetime(2013, 9, 18, 11, 30, 00), + ) self.vertical = ItemFactory.create(category='vertical', parent_location=self.section.location) - self.component = ItemFactory.create(category='problem', parent_location=self.vertical.location) + self.component = ItemFactory.create( + category='problem', + parent_location=self.vertical.location, + display_name='Problem 1', + ) + + self.section2 = ItemFactory.create(category='sequential', parent_location=self.chapter.location) + self.vertical2 = ItemFactory.create(category='vertical', parent_location=self.section2.location) + ItemFactory.create( + category='problem', + parent_location=self.vertical2.location, + display_name='Problem 2', + ) self.course_key = self.course.id self.password = '123456' @@ -210,6 +227,74 @@ class ViewsTestCase(ModuleStoreTestCase): self.org = u"ꜱᴛᴀʀᴋ ɪɴᴅᴜꜱᴛʀɪᴇꜱ" self.org_html = "

'+Stark/Industries+'

" + def test_index_success(self): + response = self._verify_index_response() + self.assertIn('Problem 2', response.content) + + # re-access to the main course page redirects to last accessed view. + url = reverse('courseware', kwargs={'course_id': unicode(self.course_key)}) + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + response = self.client.get(response.url) # pylint: disable=no-member + self.assertNotIn('Problem 1', response.content) + self.assertIn('Problem 2', response.content) + + def test_index_nonexistent_chapter(self): + 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: + masquerade = MagicMock(role='student') + patch_masquerade.return_value = (masquerade, self.user) + self._verify_index_response(expected_response_code=302, chapter_name='non-existent') + + def test_index_nonexistent_section(self): + 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: + masquerade = MagicMock(role='student') + patch_masquerade.return_value = (masquerade, self.user) + self._verify_index_response(expected_response_code=302, section_name='non-existent') + + def _verify_index_response(self, expected_response_code=200, chapter_name=None, section_name=None): + """ + Verifies the response when the courseware index page is accessed with + the given chapter and section names. + """ + self.client.login(username=self.user.username, password=self.password) + url = reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(self.course_key), + 'chapter': unicode(self.chapter.location.name) if chapter_name is None else chapter_name, + 'section': unicode(self.section2.location.name) if section_name is None else section_name, + } + ) + response = self.client.get(url) + self.assertEqual(response.status_code, expected_response_code) + return response + + def test_index_no_visible_section_in_chapter(self): + self.client.login(username=self.user.username, password=self.password) + + # reload the chapter from the store so its children information is updated + self.chapter = self.store.get_item(self.chapter.location) + + # disable the visibility of the sections in the chapter + for section in self.chapter.get_children(): + section.visible_to_staff_only = True + self.store.update_item(section, ModuleStoreEnum.UserID.test) + + url = reverse( + 'courseware_chapter', + kwargs={'course_id': unicode(self.course.id), 'chapter': unicode(self.chapter.location.name)}, + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotIn('Problem 1', response.content) + self.assertNotIn('Problem 2', response.content) + @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) def test_course_about_in_cart(self): @@ -299,15 +384,37 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertEqual(views.user_groups(mock_user), []) def test_get_current_child(self): - self.assertIsNone(views.get_current_child(MagicMock())) mock_xmodule = MagicMock() + self.assertIsNone(views.get_current_child(mock_xmodule)) + mock_xmodule.position = -1 - mock_xmodule.get_display_items.return_value = ['one', 'two'] + mock_xmodule.get_display_items.return_value = ['one', 'two', 'three'] self.assertEqual(views.get_current_child(mock_xmodule), 'one') - mock_xmodule_2 = MagicMock() - mock_xmodule_2.position = 3 - mock_xmodule_2.get_display_items.return_value = [] - self.assertIsNone(views.get_current_child(mock_xmodule_2)) + + mock_xmodule.position = 2 + self.assertEqual(views.get_current_child(mock_xmodule), 'two') + self.assertEqual(views.get_current_child(mock_xmodule, requested_child='first'), 'one') + self.assertEqual(views.get_current_child(mock_xmodule, requested_child='last'), 'three') + + mock_xmodule.position = 3 + mock_xmodule.get_display_items.return_value = [] + self.assertIsNone(views.get_current_child(mock_xmodule)) + + def test_get_redirect_url(self): + self.assertIn( + 'activate_block_id', + get_redirect_url(self.course_key, self.section.location), + ) + + self.assertIn( + 'child=first', + get_redirect_url(self.course_key, self.section.location, child='first'), + ) + + self.assertIn( + 'child=last', + get_redirect_url(self.course_key, self.section.location, child='last'), + ) def test_redirect_to_course_position(self): mock_module = MagicMock() diff --git a/lms/djangoapps/courseware/url_helpers.py b/lms/djangoapps/courseware/url_helpers.py index 62ba41e42e..28355d0d34 100644 --- a/lms/djangoapps/courseware/url_helpers.py +++ b/lms/djangoapps/courseware/url_helpers.py @@ -7,12 +7,13 @@ from xmodule.modulestore.django import modulestore from django.core.urlresolvers import reverse -def get_redirect_url(course_key, usage_key): +def get_redirect_url(course_key, usage_key, child=None): """ Returns the redirect url back to courseware Args: course_id(str): Course Id string location(str): The location id of course component + child(str): Optional child parameter to pass to the URL Raises: ItemNotFoundError if no data at the location or NoPathToItem if location not in any class @@ -50,4 +51,7 @@ def get_redirect_url(course_key, usage_key): redirect_url += "?{}".format(urlencode({'activate_block_id': unicode(final_target_id)})) + if child: + redirect_url += "&child={}".format(child) + return redirect_url diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 8eae511794..a6ec17b38d 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -181,7 +181,7 @@ def render_accordion(user, request, course, chapter, section, field_data_cache): return render_to_string('courseware/accordion.html', context) -def get_current_child(xmodule, min_depth=None): +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 @@ -193,23 +193,36 @@ def get_current_child(xmodule, min_depth=None): Returns None only if there are no children at all. """ + def _get_child(children): + """ + Returns either the first or last child based on the value of + the requested_child parameter. If requested_child is None, + returns the first child. + """ + if requested_child == 'first': + return children[0] + elif requested_child == 'last': + return children[-1] + else: + return children[0] + 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 = child_modules[0] + default_child = _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 = content_children[0] if content_children else None + default_child = _get_child(content_children) if content_children else None return default_child if not hasattr(xmodule, 'position'): return None - if xmodule.position is None: + if xmodule.position is None or requested_child: return _get_default_child_module(xmodule.get_display_items()) else: # position is 1-indexed. @@ -421,14 +434,6 @@ def _index_bulk_op(request, course_key, chapter, section, position): field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course_key, user, course, depth=2) - course_module = get_module_for_descriptor( - user, request, course, field_data_cache, course_key, course=course - ) - if course_module is None: - log.warning(u'If you see this, something went wrong: if we got this' - u' far, should have gotten a course module for this user') - return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) - studio_url = get_studio_url(course, 'course') language_preference = get_user_preference(request.user, LANGUAGE_KEY) @@ -478,89 +483,89 @@ def _index_bulk_op(request, course_key, chapter, section, position): # 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_module, CONTENT_DEPTH) + 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_module, chapter) + save_child_position(course, chapter) else: - raise Http404('No chapter descriptor found with name {}'.format(chapter)) - - chapter_module = course_module.get_child_by(lambda m: m.location.name == chapter) - if chapter_module is None: # 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 + 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_module, 'is_entrance_exam', False) and section is not None: + 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 not None: - 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 - ) - - if section_module is None: - # User may be trying to be clever and access something - # they don't have access to. - raise Http404 - - # Save where we are in the chapter. - save_child_position(chapter_module, section) - section_render_context = {'activate_block_id': request.GET.get('activate_block_id')} - context['fragment'] = section_module.render(STUDENT_VIEW, section_render_context) - context['section_title'] = section_descriptor.display_name_with_default_escaped - else: - prev_section = get_current_child(chapter_module) - if prev_section is None: + 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_module.position = None - course_module.save() + 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) + section_render_context = { + 'activate_block_id': request.GET.get('activate_block_id'), + 'redirect_url_func': ( + get_redirect_url if settings.FEATURES.get('ENABLE_NEXT_BUTTON_ACROSS_SECTIONS') else None + ), + 'requested_child': request.GET.get("child"), + } + context['accordion'] = render_accordion(user, request, course, chapter, section, field_data_cache) + 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: diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index d461d4bbf3..a191b271f2 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -162,6 +162,9 @@ FEATURES['ENABLE_COURSEWARE_SEARCH'] = True # Enable dashboard search for tests FEATURES['ENABLE_DASHBOARD_SEARCH'] = True +# Enable cross-section Next button for tests +FEATURES['ENABLE_NEXT_BUTTON_ACROSS_SECTIONS'] = True + # Use MockSearchEngine as the search engine for test scenario SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine" # Path at which to store the mock index diff --git a/lms/envs/common.py b/lms/envs/common.py index a4ee7256db..5987089036 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -371,7 +371,10 @@ FEATURES = { # This is the default, but can be disabled if all history # lives in the Extended table, saving the frontend from # making multiple queries. - 'ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES': True + 'ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES': True, + + # Enable Next Button to jump sequences in Sequence Navigation bar. + 'ENABLE_NEXT_BUTTON_ACROSS_SECTIONS': False, } # Ignore static asset files on import which match this pattern diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html index f3ddc1938f..250045120e 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -1,6 +1,6 @@ <%! from django.utils.translation import ugettext as _ %> -
+