Skip to content

E
edx-platform
Commit 036537f1 by Nimisha Asthagiri
Options

Merge pull request #12263 from edx/tnl/next-button-fix 

Fix Next Button's 404 error
parents 0f1366a7 2396cc49
Showing with 158 additions and 240 deletions
common/lib/xmodule/xmodule/js/src/sequence/display.coffee
......@@ -251,7 +251,7 @@ class @Sequence
tab_count: @num_contents
widget_placement: widget_placement
if (direction == 'next') and (@position == @contents.length)
if (direction == 'next') and (@position >= @contents.length)
window.location.href = @nextUrl
else if (direction == 'previous') and (@position == 1)
window.location.href = @prevUrl
......
common/lib/xmodule/xmodule/seq_module.py
......@@ -176,7 +176,7 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
if context.get('requested_child') == 'first':
self.position = 1
elif context.get('requested_child') == 'last':
self.position = len(display_items) or None
self.position = len(display_items) or 1
elif self.position is None or self.position > len(display_items):
self.position = 1
......@@ -237,16 +237,8 @@ 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'),
),
'next_url': context.get('next_url'),
'prev_url': context.get('prev_url'),
}
fragment.add_content(self.system.render_template("seq_module.html", params))
......@@ -455,88 +447,3 @@ 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
common/lib/xmodule/xmodule/tests/test_sequence.py
......@@ -8,7 +8,7 @@ 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
from xmodule.seq_module import SequenceModule
class StubUserService(UserService):
......@@ -96,11 +96,16 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
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)
html = self._get_rendered_student_view(
self.sequence_3_1,
requested_child=None,
next_url='NextSequential',
prev_url='PrevSequential'
)
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)
self.assertIn("'next_url': 'NextSequential'", html)
self.assertIn("'prev_url': 'PrevSequential'", html)
def test_student_view_first_child(self):
html = self._get_rendered_student_view(self.sequence_3_1, requested_child='first')
......@@ -110,7 +115,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
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):
def _get_rendered_student_view(self, sequence, requested_child, next_url=None, prev_url=None):
"""
Returns the rendered student view for the given sequence and the
requested_child parameter.
......@@ -119,8 +124,9 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
sequence,
STUDENT_VIEW,
{
'redirect_url_func': lambda course_key, block_location, child: unicode(block_location),
'requested_child': requested_child,
'next_url': next_url,
'prev_url': prev_url,
},
).content
......@@ -130,38 +136,6 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
"""
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)
def test_tooltip(self):
html = self._get_rendered_student_view(self.sequence_3_1, requested_child=None)
for child in self.sequence_3_1.children:
......
common/test/acceptance/tests/lms/test_lms_courseware.py
......@@ -432,12 +432,20 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest, EventsTestMixin):
XBlockFixtureDesc('sequential', 'Test Subsection 1,2').add_children(
XBlockFixtureDesc('problem', 'Test Problem 3', data='<problem>problem 3 dummy body</problem>'),
),
XBlockFixtureDesc(
'sequential', 'Test HIDDEN Subsection', metadata={'visible_to_staff_only': True}
).add_children(
XBlockFixtureDesc('problem', 'Test HIDDEN Problem', data='<problem>hidden problem</problem>'),
),
),
XBlockFixtureDesc('chapter', 'Test Section 2').add_children(
XBlockFixtureDesc('sequential', 'Test Subsection 2,1').add_children(
XBlockFixtureDesc('problem', 'Test Problem 4', data='<problem>problem 4 dummy body</problem>'),
),
),
XBlockFixtureDesc('chapter', 'Test HIDDEN Section', metadata={'visible_to_staff_only': True}).add_children(
XBlockFixtureDesc('sequential', 'Test HIDDEN Subsection'),
),
).install()
# Auto-auth register for the course.
......
lms/djangoapps/course_api/blocks/serializers.py
"""
Serializers for Course Blocks related return objects.
"""
from django.conf import settings
from rest_framework import serializers
from rest_framework.reverse import reverse
......@@ -44,7 +45,7 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho
),
}
if 'lti_url' in self.context['requested_fields']:
if settings.FEATURES.get("ENABLE_LTI_PROVIDER") and 'lti_url' in self.context['requested_fields']:
data['lti_url'] = reverse(
'lti_provider_launch',
kwargs={'course_id': unicode(block_key.course_key), 'usage_id': unicode(block_key)},
......
lms/djangoapps/courseware/module_render.py
......@@ -147,7 +147,7 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
user, request, course, field_data_cache, course.id, course=course
)
if course_module is None:
return None
return None, None, None
toc_chapters = list()
chapters = course_module.get_display_items()
......@@ -163,9 +163,12 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
if not user_must_complete_entrance_exam(request, user, course):
required_content = [content for content in required_content if not content == course.entrance_exam_id]
previous_of_active_section, next_of_active_section = None, None
last_processed_section, last_processed_chapter = None, None
found_active_section = False
for chapter in chapters:
# Only show required content, if there is required content
# chapter.hide_from_toc is read-only (boo)
# chapter.hide_from_toc is read-only (bool)
display_id = slugify(chapter.display_name_with_default_escaped)
local_hide_from_toc = False
if required_content:
......@@ -178,78 +181,39 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
sections = list()
for section in chapter.get_display_items():
active = (chapter.url_name == active_chapter and
section.url_name == active_section)
# Skip the current section if it is gated
# skip the section if it is gated/hidden from the user
if gated_content and unicode(section.location) in gated_content:
continue
if section.hide_from_toc:
continue
is_section_active = (chapter.url_name == active_chapter and section.url_name == active_section)
if is_section_active:
found_active_section = True
section_context = {
'display_name': section.display_name_with_default_escaped,
'url_name': section.url_name,
'format': section.format if section.format is not None else '',
'due': section.due,
'active': is_section_active,
'graded': section.graded,
}
_add_timed_exam_info(user, course, section, section_context)
# update next and previous of active section, if applicable
if is_section_active:
if last_processed_section:
previous_of_active_section = last_processed_section.copy()
previous_of_active_section['chapter_url_name'] = last_processed_chapter.url_name
elif found_active_section and not next_of_active_section:
next_of_active_section = section_context.copy()
next_of_active_section['chapter_url_name'] = chapter.url_name
sections.append(section_context)
last_processed_section = section_context
last_processed_chapter = chapter
if not section.hide_from_toc:
section_context = {
'display_name': section.display_name_with_default_escaped,
'url_name': section.url_name,
'format': section.format if section.format is not None else '',
'due': section.due,
'active': active,
'graded': section.graded,
}
#
# Add in rendering context if exam is a timed exam (which includes proctored)
#
section_is_time_limited = (
getattr(section, 'is_time_limited', False) and
settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False)
)
if section_is_time_limited:
# We need to import this here otherwise Lettuce test
# harness fails. When running in 'harvest' mode, the
# test service appears to get into trouble with
# circular references (not sure which as edx_proctoring.api
# doesn't import anything from edx-platform). Odd thing
# is that running: manage.py lms runserver --settings=acceptance
# works just fine, it's really a combination of Lettuce and the
# 'harvest' management command
#
# One idea is that there is some coupling between
# lettuce and the 'terrain' Djangoapps projects in /common
# This would need more investigation
from edx_proctoring.api import get_attempt_status_summary
#
# call into edx_proctoring subsystem
# to get relevant proctoring information regarding this
# level of the courseware
#
# This will return None, if (user, course_id, content_id)
# is not applicable
#
timed_exam_attempt_context = None
try:
timed_exam_attempt_context = get_attempt_status_summary(
user.id,
unicode(course.id),
unicode(section.location)
)
except Exception, ex: # pylint: disable=broad-except
# safety net in case something blows up in edx_proctoring
# as this is just informational descriptions, it is better
# to log and continue (which is safe) than to have it be an
# unhandled exception
log.exception(ex)
if timed_exam_attempt_context:
# yes, user has proctoring context about
# this level of the courseware
# so add to the accordion data context
section_context.update({
'proctoring': timed_exam_attempt_context,
})
sections.append(section_context)
toc_chapters.append({
'display_name': chapter.display_name_with_default_escaped,
'display_id': display_id,
......@@ -257,7 +221,61 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_
'sections': sections,
'active': chapter.url_name == active_chapter
})
return toc_chapters
return toc_chapters, previous_of_active_section, next_of_active_section
def _add_timed_exam_info(user, course, section, section_context):
"""
Add in rendering context if exam is a timed exam (which includes proctored)
"""
section_is_time_limited = (
getattr(section, 'is_time_limited', False) and
settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False)
)
if section_is_time_limited:
# We need to import this here otherwise Lettuce test
# harness fails. When running in 'harvest' mode, the
# test service appears to get into trouble with
# circular references (not sure which as edx_proctoring.api
# doesn't import anything from edx-platform). Odd thing
# is that running: manage.py lms runserver --settings=acceptance
# works just fine, it's really a combination of Lettuce and the
# 'harvest' management command
#
# One idea is that there is some coupling between
# lettuce and the 'terrain' Djangoapps projects in /common
# This would need more investigation
from edx_proctoring.api import get_attempt_status_summary
#
# call into edx_proctoring subsystem
# to get relevant proctoring information regarding this
# level of the courseware
#
# This will return None, if (user, course_id, content_id)
# is not applicable
#
timed_exam_attempt_context = None
try:
timed_exam_attempt_context = get_attempt_status_summary(
user.id,
unicode(course.id),
unicode(section.location)
)
except Exception, ex: # pylint: disable=broad-except
# safety net in case something blows up in edx_proctoring
# as this is just informational descriptions, it is better
# to log and continue (which is safe) than to have it be an
# unhandled exception
log.exception(ex)
if timed_exam_attempt_context:
# yes, user has proctoring context about
# this level of the courseware
# so add to the accordion data context
section_context.update({
'proctoring': timed_exam_attempt_context,
})
def get_module(user, request, usage_key, field_data_cache,
......
lms/djangoapps/courseware/tests/test_entrance_exam.py
......@@ -583,7 +583,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest
self.request.user,
self.entrance_exam
)
return toc_for_course(
toc, __, __ = toc_for_course(
self.request.user,
self.request,
self.course,
......@@ -591,6 +591,7 @@ class EntranceExamTestCases(LoginEnrollmentTestCase, ModuleStoreTestCase, Milest
self.exam_1.url_name,
self.field_data_cache
)
return toc
def answer_entrance_exam_problem(course, request, problem, user=None):
......
lms/djangoapps/courseware/tests/test_module_render.py
......@@ -668,11 +668,13 @@ class TestTOC(ModuleStoreTestCase):
course = self.store.get_course(self.toy_course.id, depth=2)
with check_mongo_calls(toc_finds):
actual = render.toc_for_course(
actual, prev_sequential, next_sequential = 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)
# Mongo makes 3 queries to load the course to depth 2:
# - 1 for the course
......@@ -707,11 +709,13 @@ class TestTOC(ModuleStoreTestCase):
'url_name': 'secret:magic', 'display_name': 'secret:magic', 'display_id': 'secretmagic'}])
with check_mongo_calls(toc_finds):
actual = render.toc_for_course(
actual, prev_sequential, next_sequential = 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')
@attr('shard_1')
......@@ -852,7 +856,7 @@ class TestProctoringRendering(SharedModuleStoreTestCase):
"""
self._setup_test_data(enrollment_mode, is_practice_exam, attempt_status)
actual = render.toc_for_course(
actual, prev_sequential, next_sequential = render.toc_for_course(
self.request.user,
self.request,
self.toy_course,
......@@ -867,6 +871,8 @@ class TestProctoringRendering(SharedModuleStoreTestCase):
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")
@ddt.data(
(
......@@ -1108,7 +1114,7 @@ class TestGatedSubsectionRendering(SharedModuleStoreTestCase, MilestonesTestCase
"""
Test generation of TOC for a course with a gated subsection
"""
actual = render.toc_for_course(
actual, prev_sequential, next_sequential = render.toc_for_course(
self.request.user,
self.request,
self.course,
......@@ -1119,6 +1125,8 @@ class TestGatedSubsectionRendering(SharedModuleStoreTestCase, MilestonesTestCase
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)
@attr('shard_1')
......
lms/djangoapps/courseware/tests/test_views.py
......@@ -35,6 +35,7 @@ from commerce.models import CommerceConfiguration
from course_modes.models import CourseMode
from course_modes.tests.factories import CourseModeFactory
from courseware.model_data import set_score
from courseware.module_render import toc_for_course
from courseware.testutils import RenderXBlockTestMixin
from courseware.tests.factories import StudentModuleFactory
from courseware.url_helpers import get_redirect_url
......@@ -415,16 +416,6 @@ class ViewsTestCase(ModuleStoreTestCase):
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()
mock_module.descriptor.id = 'Underwater Basketweaving'
......@@ -926,10 +917,10 @@ class TestAccordionDueDate(BaseDueDateTests):
def get_text(self, course):
""" Returns the HTML for the accordion """
return views.render_accordion(
self.request.user, self.request, course,
unicode(course.get_children()[0].scope_ids.usage_id), None, None
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)
@attr('shard_1')
......
lms/djangoapps/courseware/url_helpers.py
......@@ -7,13 +7,12 @@ from xmodule.modulestore.django import modulestore
from django.core.urlresolvers import reverse
def get_redirect_url(course_key, usage_key, child=None):
def get_redirect_url(course_key, usage_key):
""" 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,8 +49,4 @@ def get_redirect_url(course_key, usage_key, child=None):
)
redirect_url += "?{}".format(urlencode({'activate_block_id': unicode(final_target_id)}))
if child:
redirect_url += "&child={}".format(child)
return redirect_url
lms/djangoapps/courseware/views.py
......@@ -158,7 +158,7 @@ def courses(request):
)
def render_accordion(user, request, course, chapter, section, field_data_cache):
def render_accordion(request, course, toc):
"""
Draws navigation bar. Takes current position in accordion as
parameter.
......@@ -169,9 +169,6 @@ def render_accordion(user, request, course, chapter, section, field_data_cache):
Returns the html string
"""
# grab the table of contents
toc = toc_for_course(user, request, course, chapter, section, field_data_cache)
context = dict([
('toc', toc),
('course_id', course.id.to_deprecated_string()),
......@@ -442,7 +439,6 @@ def _index_bulk_op(request, course_key, chapter, section, position):
context = {
'csrf': csrf(request)['csrf_token'],
'accordion': render_accordion(user, request, course, chapter, section, field_data_cache),
'COURSE_TITLE': course.display_name_with_default_escaped,
'course': course,
'init': '',
......@@ -455,6 +451,8 @@ def _index_bulk_op(request, course_key, chapter, section, position):
'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)
......@@ -472,7 +470,6 @@ def _index_bulk_op(request, course_key, chapter, section, position):
if course_has_entrance_exam(course):
exam_chapter = get_entrance_exam_content(request, course)
if exam_chapter:
exam_section = None
if exam_chapter.get_children():
exam_section = exam_chapter.get_children()[0]
if exam_section:
......@@ -556,12 +553,30 @@ def _index_bulk_op(request, course_key, chapter, section, position):
# 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'),
'redirect_url_func': get_redirect_url,
'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['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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment