diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index abe377b98b..11933b0930 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -4,6 +4,8 @@ xModule implementation of a learning sequence # pylint: disable=abstract-method import collections +from datetime import datetime +from django.utils.timezone import UTC import json import logging from pkg_resources import resource_string @@ -38,13 +40,22 @@ class SequenceFields(object): # NOTE: Position is 1-indexed. This is silly, but there are now student # positions saved on prod, so it's not easy to fix. position = Integer(help="Last tab viewed in this sequence", scope=Scope.user_state) + due = Date( display_name=_("Due Date"), help=_("Enter the date by which problems are due."), scope=Scope.settings, ) - # Entrance Exam flag -- see cms/contentstore/views/entrance_exam.py for usage + hide_after_due = Boolean( + display_name=_("Hide sequence content After Due Date"), + help=_( + "If set, the sequence content is hidden for non-staff users after the due date has passed." + ), + default=False, + scope=Scope.settings, + ) + is_entrance_exam = Boolean( display_name=_("Is Entrance Exam"), help=_( @@ -97,16 +108,6 @@ class ProctoringFields(object): scope=Scope.settings, ) - hide_after_due = Boolean( - display_name=_("Hide Exam Results After Due Date"), - help=_( - "This setting overrides the default behavior of showing exam results after the due date has passed." - " Currently only supported for timed exams." - ), - default=False, - scope=Scope.settings, - ) - is_practice_exam = Boolean( display_name=_("Is Practice Exam"), help=_( @@ -177,73 +178,91 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): raise NotFoundError('Unexpected dispatch type') + @classmethod + def verify_current_content_visibility(cls, due, hide_after_due): + """ + Returns whether the content visibility policy passes + for the given due date and hide_after_due values and + the current date-time. + """ + return ( + not due or + not hide_after_due or + datetime.now(UTC()) < due + ) + def student_view(self, context): + context = context or {} + self._capture_basic_metrics() + banner_text = None + special_html_view = self._hidden_content_student_view(context) or self._special_exam_student_view() + if special_html_view: + masquerading_as_specific_student = context.get('specific_masquerade', False) + banner_text, special_html = special_html_view + if special_html and not masquerading_as_specific_student: + return Fragment(special_html) + return self._student_view(context, banner_text) + + def _special_exam_student_view(self): + """ + Checks whether this sequential is a special exam. If so, returns + a banner_text or the fragment to display depending on whether + staff is masquerading. + """ + if self.is_time_limited: + special_exam_html = self._time_limited_student_view() + if special_exam_html: + banner_text = _("This exam is hidden from the learner.") + return banner_text, special_exam_html + + def _hidden_content_student_view(self, context): + """ + Checks whether the content of this sequential is hidden from the + runtime user. If so, returns a banner_text or the fragment to + display depending on whether staff is masquerading. + """ + if not self._can_user_view_content(): + subsection_format = (self.format or _("subsection")).lower() # pylint: disable=no-member + + # Translators: subsection_format refers to the assignment + # type of the subsection, such as Homework, Lab, Exam, etc. + banner_text = _( + "Because the due date has passed, " + "this {subsection_format} is hidden from the learner." + ).format(subsection_format=subsection_format) + + hidden_content_html = self.system.render_template( + 'hidden_content.html', + { + 'subsection_format': subsection_format, + 'progress_url': context.get('progress_url'), + } + ) + + return banner_text, hidden_content_html + + def _can_user_view_content(self): + """ + Returns whether the runtime user can view the content + of this sequential. + """ + return ( + self.runtime.user_is_staff or + self.verify_current_content_visibility(self.due, self.hide_after_due) + ) + + def _student_view(self, context, banner_text=None): + """ + Returns the rendered student view of the content of this + sequential. If banner_text is given, it is added to the + content. + """ 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 context.get('requested_child') == 'first': - self.position = 1 - elif context.get('requested_child') == 'last': - self.position = len(display_items) or 1 - elif self.position is None or self.position > len(display_items): - self.position = 1 - - ## Returns a set of all types of all sub-children - contents = [] + self._update_position(context, len(display_items)) fragment = Fragment() - context = context or {} - - bookmarks_service = self.runtime.service(self, "bookmarks") - context["username"] = self.runtime.service(self, "user").get_current_user().opt_attrs['edx-platform.username'] - - parent_module = self.get_parent() - display_names = [ - parent_module.display_name_with_default, - self.display_name_with_default - ] - - # We do this up here because proctored exam functionality could bypass - # rendering after this section. - self._capture_basic_metrics() - - # Is this sequential part of a timed or proctored exam? - masquerading = context.get('specific_masquerade', False) - special_exam_html = None - if self.is_time_limited: - special_exam_html = self._time_limited_student_view(context) - - # Do we have an applicable alternate rendering - # from the edx_proctoring subsystem? - if special_exam_html and not masquerading: - fragment.add_content(special_exam_html) - return fragment - - for child in display_items: - is_bookmarked = bookmarks_service.is_bookmarked(usage_key=child.scope_ids.usage_id) - context["bookmarked"] = is_bookmarked - - progress = child.get_progress() - rendered_child = child.render(STUDENT_VIEW, context) - fragment.add_frag_resources(rendered_child) - - childinfo = { - 'content': rendered_child.content, - 'page_title': getattr(child, 'tooltip_title', ''), - 'progress_status': Progress.to_js_status_str(progress), - 'progress_detail': Progress.to_js_detail_str(progress), - 'type': child.get_icon_class(), - 'id': child.scope_ids.usage_id.to_deprecated_string(), - 'bookmarked': is_bookmarked, - 'path': " > ".join(display_names + [child.display_name_with_default]), - } - - contents.append(childinfo) - params = { - 'items': contents, + 'items': self._render_student_view_for_items(context, display_items, fragment), 'element_id': self.location.html_id(), 'item_id': self.location.to_deprecated_string(), 'position': self.position, @@ -251,17 +270,66 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): 'ajax_url': self.system.ajax_url, 'next_url': context.get('next_url'), 'prev_url': context.get('prev_url'), - 'override_hidden_exam': masquerading and special_exam_html is not None, + 'banner_text': banner_text, } - fragment.add_content(self.system.render_template("seq_module.html", params)) self._capture_full_seq_item_metrics(display_items) self._capture_current_unit_metrics(display_items) - # Get all descendant XBlock types and counts return fragment + def _update_position(self, context, number_of_display_items): + """ + Update the user's sequential position given the context and the + number_of_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 context.get('requested_child') == 'first': + self.position = 1 + elif context.get('requested_child') == 'last': + self.position = number_of_display_items or 1 + elif self.position is None or self.position > number_of_display_items: + self.position = 1 + + def _render_student_view_for_items(self, context, display_items, fragment): + """ + Updates the given fragment with rendered student views of the given + display_items. Returns a list of dict objects with information about + the given display_items. + """ + bookmarks_service = self.runtime.service(self, "bookmarks") + context["username"] = self.runtime.service(self, "user").get_current_user().opt_attrs['edx-platform.username'] + display_names = [ + self.get_parent().display_name_with_default, + self.display_name_with_default + ] + contents = [] + for item in display_items: + is_bookmarked = bookmarks_service.is_bookmarked(usage_key=item.scope_ids.usage_id) + context["bookmarked"] = is_bookmarked + + progress = item.get_progress() + rendered_item = item.render(STUDENT_VIEW, context) + fragment.add_frag_resources(rendered_item) + + iteminfo = { + 'content': rendered_item.content, + 'page_title': getattr(item, 'tooltip_title', ''), + 'progress_status': Progress.to_js_status_str(progress), + 'progress_detail': Progress.to_js_detail_str(progress), + 'type': item.get_icon_class(), + 'id': item.scope_ids.usage_id.to_deprecated_string(), + 'bookmarked': is_bookmarked, + 'path': " > ".join(display_names + [item.display_name_with_default]), + } + + contents.append(iteminfo) + + return contents + def _locations_in_subtree(self, node): """ The usage keys for all descendants of an XBlock/XModule as a flat list. @@ -328,7 +396,7 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): for block_type, count in curr_block_counts.items(): newrelic.agent.add_custom_parameter('seq.current.block_counts.{}'.format(block_type), count) - def _time_limited_student_view(self, context): + def _time_limited_student_view(self): """ Delegated rendering of a student view when in a time limited view. This ultimately calls down into edx_proctoring diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py index fedb839d5e..f98319bee9 100644 --- a/common/lib/xmodule/xmodule/tests/test_sequence.py +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -2,6 +2,10 @@ Tests for sequence module. """ # pylint: disable=no-member +from datetime import timedelta +import ddt +from django.utils.timezone import now +from freezegun import freeze_time from mock import Mock from xblock.reference.user_service import XBlockUser, UserService from xmodule.tests import get_test_system @@ -24,10 +28,15 @@ class StubUserService(UserService): return user +@ddt.ddt class SequenceBlockTestCase(XModuleXmlImportTest): """ Tests for the Sequence Module. """ + TODAY = now() + TOMORROW = TODAY + timedelta(days=1) + DAY_AFTER_TOMORROW = TOMORROW + timedelta(days=1) + @classmethod def setUpClass(cls): super(SequenceBlockTestCase, cls).setUpClass() @@ -54,13 +63,16 @@ class SequenceBlockTestCase(XModuleXmlImportTest): 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 + chapter_4 = xml.ChapterFactory.build(parent=course) # has 1 child sequence, with hide_after_due 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) + xml.SequenceFactory.build( # sequence_4_1 + parent=chapter_4, + hide_after_due=str(True), + due=str(cls.TOMORROW), + ) for _ in range(3): xml.VerticalFactory.build(parent=sequence_3_1) @@ -98,9 +110,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): def test_render_student_view(self): html = self._get_rendered_student_view( self.sequence_3_1, - requested_child=None, - next_url='NextSequential', - prev_url='PrevSequential' + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), ) self._assert_view_at_position(html, expected_position=1) self.assertIn(unicode(self.sequence_3_1.location), html) @@ -115,20 +125,15 @@ 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, next_url=None, prev_url=None): + def _get_rendered_student_view(self, sequence, requested_child=None, extra_context=None): """ Returns the rendered student view for the given sequence and the requested_child parameter. """ - return sequence.xmodule_runtime.render( - sequence, - STUDENT_VIEW, - { - 'requested_child': requested_child, - 'next_url': next_url, - 'prev_url': prev_url, - }, - ).content + context = {'requested_child': requested_child} + if extra_context: + context.update(extra_context) + return sequence.xmodule_runtime.render(sequence, STUDENT_VIEW, context).content def _assert_view_at_position(self, rendered_html, expected_position): """ @@ -140,3 +145,46 @@ class SequenceBlockTestCase(XModuleXmlImportTest): html = self._get_rendered_student_view(self.sequence_3_1, requested_child=None) for child in self.sequence_3_1.children: self.assertIn("'page_title': '{}'".format(child.name), html) + + def test_hidden_content_before_due(self): + html = self._get_rendered_student_view(self.sequence_4_1) + self.assertIn("seq_module.html", html) + self.assertIn("'banner_text': None", html) + + @freeze_time(DAY_AFTER_TOMORROW) + @ddt.data( + (None, 'subsection'), + ('Homework', 'homework'), + ) + @ddt.unpack + def test_hidden_content_past_due(self, format_type, expected_text): + progress_url = 'http://test_progress_link' + self._set_sequence_format(self.sequence_4_1, format_type) + html = self._get_rendered_student_view( + self.sequence_4_1, + extra_context=dict(progress_url=progress_url), + ) + self.assertIn("hidden_content.html", html) + self.assertIn(progress_url, html) + self.assertIn("'subsection_format': '{}'".format(expected_text), html) + + @freeze_time(DAY_AFTER_TOMORROW) + def test_masquerade_hidden_content_past_due(self): + self._set_sequence_format(self.sequence_4_1, "Homework") + html = self._get_rendered_student_view( + self.sequence_4_1, + extra_context=dict(specific_masquerade=True), + ) + self.assertIn("seq_module.html", html) + self.assertIn( + "'banner_text': 'Because the due date has passed, " + "this homework is hidden from the learner.'", + html + ) + + def _set_sequence_format(self, sequence, format_type): + """ + Sets the format field on the given sequence to the + given value. + """ + sequence._xmodule.format = format_type # pylint: disable=protected-access diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index ab8fd53bbc..58371b5c3d 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -3,6 +3,7 @@ API function for retrieving course blocks data """ from lms.djangoapps.course_blocks.api import get_course_blocks, COURSE_BLOCK_ACCESS_TRANSFORMERS +from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer from openedx.core.lib.block_structure.transformers import BlockStructureTransformers from .transformers.blocks_api import BlocksAPITransformer @@ -51,7 +52,7 @@ def get_blocks( # create ordered list of transformers, adding BlocksAPITransformer at end. transformers = BlockStructureTransformers() if user is not None: - transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS + [ProctoredExamTransformer()] + transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS + [ProctoredExamTransformer(), HiddenContentTransformer()] transformers += [ BlocksAPITransformer( block_counts, diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index 6de78d78da..57b8db9076 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -2,11 +2,8 @@ API entry point to the course_blocks app with top-level get_course_blocks function. """ -from django.core.cache import cache from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager -from openedx.core.lib.block_structure.manager import BlockStructureManager from openedx.core.lib.block_structure.transformers import BlockStructureTransformers -from xmodule.modulestore.django import modulestore from .transformers import ( library_content, diff --git a/lms/djangoapps/course_blocks/transformers/hidden_content.py b/lms/djangoapps/course_blocks/transformers/hidden_content.py new file mode 100644 index 0000000000..4e1597513e --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/hidden_content.py @@ -0,0 +1,102 @@ +""" +Visibility Transformer implementation. +""" +from datetime import datetime +from pytz import utc + +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer, FilteringTransformerMixin +from xmodule.seq_module import SequenceModule +from .utils import collect_merged_boolean_field, collect_merged_date_field + + +MAXIMUM_DATE = utc.localize(datetime.max) + + +class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransformer): + """ + A transformer that enforces the hide_after_due field on + blocks by removing children blocks from the block structure for + which the user does not have access. The due and hide_after_due + fields on a block is percolated down to its descendants, so that + all blocks enforce the hidden content settings from their ancestors. + + For a block with multiple parents, access is denied only if + access is denied from all its parents. + + Staff users are exempted from hidden content rules. + """ + VERSION = 1 + MERGED_DUE_DATE = 'merged_due_date' + MERGED_HIDE_AFTER_DUE = 'merged_hide_after_due' + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "hidden_content" + + @classmethod + def _get_merged_hide_after_due(cls, block_structure, block_key): + """ + Returns whether the block with the given block_key in the + given block_structure should be visible to staff only per + computed value from ancestry chain. + """ + return block_structure.get_transformer_block_field( + block_key, cls, cls.MERGED_HIDE_AFTER_DUE, False + ) + + @classmethod + def _get_merged_due_date(cls, block_structure, block_key): + """ + Returns the merged value for the start date for the block with + the given block_key in the given block_structure. + """ + return block_structure.get_transformer_block_field( + block_key, cls, cls.MERGED_DUE_DATE, False + ) + + @classmethod + def collect(cls, block_structure): + """ + Collects any information that's necessary to execute this + transformer's transform method. + """ + collect_merged_boolean_field( + block_structure, + transformer=cls, + xblock_field_name='hide_after_due', + merged_field_name=cls.MERGED_HIDE_AFTER_DUE, + ) + + collect_merged_date_field( + block_structure, + transformer=cls, + xblock_field_name='due', + merged_field_name=cls.MERGED_DUE_DATE, + default_date=MAXIMUM_DATE, + func_merge_parents=max, + func_merge_ancestors=min, + ) + + def transform_block_filters(self, usage_info, block_structure): + # Users with staff access bypass the Visibility check. + if usage_info.has_staff_access: + return [block_structure.create_universal_filter()] + + return [ + block_structure.create_removal_filter( + lambda block_key: self._is_block_hidden(block_structure, block_key), + ), + ] + + def _is_block_hidden(self, block_structure, block_key): + """ + Returns whether the block with the given block_key should + be hidden, given the current time. + """ + due = self._get_merged_due_date(block_structure, block_key) + hide_after_due = self._get_merged_hide_after_due(block_structure, block_key) + return not SequenceModule.verify_current_content_visibility(due, hide_after_due) diff --git a/lms/djangoapps/course_blocks/transformers/start_date.py b/lms/djangoapps/course_blocks/transformers/start_date.py index 81d1bb3397..5a8ccb09b1 100644 --- a/lms/djangoapps/course_blocks/transformers/start_date.py +++ b/lms/djangoapps/course_blocks/transformers/start_date.py @@ -5,7 +5,7 @@ from openedx.core.lib.block_structure.transformer import BlockStructureTransform from lms.djangoapps.courseware.access_utils import check_start_date from xmodule.course_metadata_utils import DEFAULT_START_DATE -from .utils import get_field_on_block +from .utils import collect_merged_date_field class StartDateTransformer(FilteringTransformerMixin, BlockStructureTransformer): @@ -36,7 +36,7 @@ class StartDateTransformer(FilteringTransformerMixin, BlockStructureTransformer) return "start_date" @classmethod - def get_merged_start_date(cls, block_structure, block_key): + def _get_merged_start_date(cls, block_structure, block_key): """ Returns the merged value for the start date for the block with the given block_key in the given block_structure. @@ -53,35 +53,15 @@ class StartDateTransformer(FilteringTransformerMixin, BlockStructureTransformer) """ block_structure.request_xblock_fields('days_early_for_beta') - for block_key in block_structure.topological_traversal(): - - # compute merged value of start date from all parents - parents = block_structure.get_parents(block_key) - min_all_parents_start_date = min( - cls.get_merged_start_date(block_structure, parent_key) - for parent_key in parents - ) if parents else None - - # set the merged value for this block - block_start = get_field_on_block(block_structure.get_xblock(block_key), 'start') - if min_all_parents_start_date is None: - # no parents so just use value on block or default - merged_start_value = block_start or DEFAULT_START_DATE - - elif not block_start: - # no value on this block so take value from parents - merged_start_value = min_all_parents_start_date - - else: - # max of merged-start-from-all-parents and this block - merged_start_value = max(min_all_parents_start_date, block_start) - - block_structure.set_transformer_block_field( - block_key, - cls, - cls.MERGED_START_DATE, - merged_start_value - ) + collect_merged_date_field( + block_structure, + transformer=cls, + xblock_field_name='start', + merged_field_name=cls.MERGED_START_DATE, + default_date=DEFAULT_START_DATE, + func_merge_parents=min, + func_merge_ancestors=max, + ) def transform_block_filters(self, usage_info, block_structure): # Users with staff access bypass the Start Date check. @@ -91,7 +71,7 @@ class StartDateTransformer(FilteringTransformerMixin, BlockStructureTransformer) removal_condition = lambda block_key: not check_start_date( usage_info.user, block_structure.get_xblock_field(block_key, 'days_early_for_beta'), - self.get_merged_start_date(block_structure, block_key), + self._get_merged_start_date(block_structure, block_key), usage_info.course_key, ) return [block_structure.create_removal_filter(removal_condition)] diff --git a/lms/djangoapps/course_blocks/transformers/tests/helpers.py b/lms/djangoapps/course_blocks/transformers/tests/helpers.py index af8b5afa77..2ce251f6d1 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/helpers.py +++ b/lms/djangoapps/course_blocks/transformers/tests/helpers.py @@ -255,7 +255,7 @@ class BlockParentsMapTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase) self, test_user, expected_user_accessible_blocks, - blocks_with_differing_access, + blocks_with_differing_access=None, transformers=None, ): """ @@ -272,7 +272,8 @@ class BlockParentsMapTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase) blocks_with_differing_access (set(int)): Set of blocks (indices) whose access will differ from the transformers result and the current implementation of - has_access. + has_access. If not provided, does not compare with + has_access results. transformers (BlockStructureTransformers): An optional collection of transformers that are to be executed. If not @@ -312,7 +313,6 @@ class BlockParentsMapTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase) # compute access results of the block block_structure_result = xblock_key in block_structure - has_access_result = bool(has_access(user, 'load', self.get_block(i), course_key=self.course.id)) # compare with expected value self.assertEquals( @@ -323,23 +323,25 @@ class BlockParentsMapTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase) ) ) - # compare with has_access_result - if i in blocks_with_differing_access: - self.assertNotEqual( - block_structure_result, - has_access_result, - "block structure ({0}) & has_access ({1}) results are equal for block {2} for user {3}".format( - block_structure_result, has_access_result, i, user.username + if blocks_with_differing_access: + # compare with has_access_result + has_access_result = bool(has_access(user, 'load', self.get_block(i), course_key=self.course.id)) + if i in blocks_with_differing_access: + self.assertNotEqual( + block_structure_result, + has_access_result, + "block structure ({0}) & has_access ({1}) results are equal for block {2} for user {3}".format( + block_structure_result, has_access_result, i, user.username + ) ) - ) - else: - self.assertEquals( - block_structure_result, - has_access_result, - "block structure ({0}) & has_access ({1}) results not equal for block {2} for user {3}".format( - block_structure_result, has_access_result, i, user.username + else: + self.assertEquals( + block_structure_result, + has_access_result, + "block structure ({0}) & has_access ({1}) results not equal for block {2} for user {3}".format( + block_structure_result, has_access_result, i, user.username + ) ) - ) self.client.logout() diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py b/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py new file mode 100644 index 0000000000..480480e94f --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py @@ -0,0 +1,83 @@ +""" +Tests for HiddenContentTransformer. +""" +from datetime import timedelta +import ddt +from django.utils.timezone import now +from nose.plugins.attrib import attr + +from ..hidden_content import HiddenContentTransformer +from .helpers import BlockParentsMapTestCase, update_block + + +@attr('shard_3') +@ddt.ddt +class HiddenContentTransformerTestCase(BlockParentsMapTestCase): + """ + VisibilityTransformer Test + """ + TRANSFORMER_CLASS_TO_TEST = HiddenContentTransformer + ALL_BLOCKS = {0, 1, 2, 3, 4, 5, 6} + + class DueDateType(object): + """ + Use constant enum types for deterministic ddt test method names (rather than dynamically generated timestamps) + """ + none = 1, + future = 2, + past = 3 + + TODAY = now() + PAST_DATE = TODAY - timedelta(days=30) + FUTURE_DATE = TODAY + timedelta(days=30) + + @classmethod + def due(cls, enum_value): + """ + Returns a start date for the given enum value + """ + if enum_value == cls.future: + return cls.FUTURE_DATE + elif enum_value == cls.past: + return cls.PAST_DATE + else: + return None + + # Following test cases are based on BlockParentsMapTestCase.parents_map + @ddt.data( + ({}, ALL_BLOCKS), + + ({0: DueDateType.none}, ALL_BLOCKS), + ({0: DueDateType.future}, ALL_BLOCKS), + ({1: DueDateType.none}, ALL_BLOCKS), + ({1: DueDateType.future}, ALL_BLOCKS), + ({4: DueDateType.none}, ALL_BLOCKS), + ({4: DueDateType.future}, ALL_BLOCKS), + + ({0: DueDateType.past}, {}), + ({1: DueDateType.past}, ALL_BLOCKS - {1, 3, 4}), + ({2: DueDateType.past}, ALL_BLOCKS - {2, 5}), + ({4: DueDateType.past}, ALL_BLOCKS - {4}), + + ({1: DueDateType.past, 2: DueDateType.past}, {0}), + ({1: DueDateType.none, 2: DueDateType.past}, ALL_BLOCKS - {2, 5}), + ({1: DueDateType.past, 2: DueDateType.none}, ALL_BLOCKS - {1, 3, 4}), + ) + @ddt.unpack + def test_hidden_content( + self, + hide_due_values, + expected_visible_blocks, + ): + for idx, due_date_type in hide_due_values.iteritems(): + block = self.get_block(idx) + block.due = self.DueDateType.due(due_date_type) + block.hide_after_due = True + update_block(block) + + self.assert_transform_results( + self.student, + expected_visible_blocks, + blocks_with_differing_access=None, + transformers=self.transformers, + ) diff --git a/lms/djangoapps/course_blocks/transformers/utils.py b/lms/djangoapps/course_blocks/transformers/utils.py index 9884951251..5ca498053b 100644 --- a/lms/djangoapps/course_blocks/transformers/utils.py +++ b/lms/djangoapps/course_blocks/transformers/utils.py @@ -10,7 +10,103 @@ def get_field_on_block(block, field_name, default_value=None): returns value from only a single parent chain (e.g., doesn't take a union in DAGs). """ - if block.fields[field_name].is_set_on(block): - return getattr(block, field_name) - else: - return default_value + try: + if block.fields[field_name].is_set_on(block): + return getattr(block, field_name) + except KeyError: + pass + return default_value + + +def collect_merged_boolean_field( + block_structure, + transformer, + xblock_field_name, + merged_field_name, +): + """ + Collects a boolean xBlock field of name xblock_field_name + for the given block_structure and transformer. The boolean + value is percolated down the hierarchy of the block_structure + and stored as a value of merged_field_name in the + block_structure. + + Assumes that the boolean field is False, by default. So, + the value is ANDed across all parents for blocks with + multiple parents and ORed across all ancestors down a single + hierarchy chain. + """ + + for block_key in block_structure.topological_traversal(): + # compute merged value of the boolean field from all parents + parents = block_structure.get_parents(block_key) + all_parents_merged_value = all( # pylint: disable=invalid-name + block_structure.get_transformer_block_field( + parent_key, transformer, merged_field_name, False, + ) + for parent_key in parents + ) if parents else False + + # set the merged value for this block + block_structure.set_transformer_block_field( + block_key, + transformer, + merged_field_name, + ( + all_parents_merged_value or + get_field_on_block( + block_structure.get_xblock(block_key), xblock_field_name, + False, + ) + ) + ) + + +def collect_merged_date_field( + block_structure, + transformer, + xblock_field_name, + merged_field_name, + default_date, + func_merge_parents=min, + func_merge_ancestors=max, +): + """ + Collects a date xBlock field of name xblock_field_name + for the given block_structure and transformer. The date + value is percolated down the hierarchy of the block_structure + and stored as a value of merged_field_name in the + block_structure. + """ + + for block_key in block_structure.topological_traversal(): + + parents = block_structure.get_parents(block_key) + block_date = get_field_on_block(block_structure.get_xblock(block_key), xblock_field_name) + if not parents: + # no parents so just use value on block or default + merged_date_value = block_date or default_date + + else: + # compute merged value of date from all parents + merged_all_parents_date = func_merge_parents( + block_structure.get_transformer_block_field( + parent_key, transformer, merged_field_name, default_date, + ) + for parent_key in parents + ) + + if not block_date: + # no value on this block so take value from parents + merged_date_value = merged_all_parents_date + + else: + # compute merged date of the block and the parent + merged_date_value = func_merge_ancestors(merged_all_parents_date, block_date) + + block_structure.set_transformer_block_field( + block_key, + transformer, + merged_field_name, + merged_date_value + ) diff --git a/lms/djangoapps/course_blocks/transformers/visibility.py b/lms/djangoapps/course_blocks/transformers/visibility.py index 98c3036657..1fad7ff57e 100644 --- a/lms/djangoapps/course_blocks/transformers/visibility.py +++ b/lms/djangoapps/course_blocks/transformers/visibility.py @@ -2,6 +2,7 @@ Visibility Transformer implementation. """ from openedx.core.lib.block_structure.transformer import BlockStructureTransformer, FilteringTransformerMixin +from .utils import collect_merged_boolean_field class VisibilityTransformer(FilteringTransformerMixin, BlockStructureTransformer): @@ -30,7 +31,7 @@ class VisibilityTransformer(FilteringTransformerMixin, BlockStructureTransformer return "visibility" @classmethod - def get_visible_to_staff_only(cls, block_structure, block_key): + def _get_visible_to_staff_only(cls, block_structure, block_key): """ Returns whether the block with the given block_key in the given block_structure should be visible to staff only per @@ -46,26 +47,12 @@ class VisibilityTransformer(FilteringTransformerMixin, BlockStructureTransformer Collects any information that's necessary to execute this transformer's transform method. """ - for block_key in block_structure.topological_traversal(): - - # compute merged value of visible_to_staff_only from all parents - parents = block_structure.get_parents(block_key) - all_parents_visible_to_staff_only = all( # pylint: disable=invalid-name - cls.get_visible_to_staff_only(block_structure, parent_key) - for parent_key in parents - ) if parents else False - - # set the merged value for this block - block_structure.set_transformer_block_field( - block_key, - cls, - cls.MERGED_VISIBLE_TO_STAFF_ONLY, - # merge visible_to_staff_only from all parents and this block - ( - all_parents_visible_to_staff_only or - block_structure.get_xblock(block_key).visible_to_staff_only - ) - ) + collect_merged_boolean_field( + block_structure, + transformer=cls, + xblock_field_name='visible_to_staff_only', + merged_field_name=cls.MERGED_VISIBLE_TO_STAFF_ONLY, + ) def transform_block_filters(self, usage_info, block_structure): # Users with staff access bypass the Visibility check. @@ -74,6 +61,6 @@ class VisibilityTransformer(FilteringTransformerMixin, BlockStructureTransformer return [ block_structure.create_removal_filter( - lambda block_key: self.get_visible_to_staff_only(block_structure, block_key), + lambda block_key: self._get_visible_to_staff_only(block_structure, block_key), ) ] diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index a361898eff..8fcb0f89fd 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -447,7 +447,7 @@ class CoursewareIndex(View): return "{url}?child={requested_child}".format( url=reverse( 'courseware_section', - args=[unicode(self.course.id), section_info['chapter_url_name'], section_info['url_name']], + args=[unicode(self.course_key), section_info['chapter_url_name'], section_info['url_name']], ), requested_child=requested_child, ) @@ -455,6 +455,7 @@ class CoursewareIndex(View): section_context = { 'activate_block_id': self.request.GET.get('activate_block_id'), 'requested_child': self.request.GET.get("child"), + 'progress_url': reverse('progress', kwargs={'course_id': unicode(self.course_key)}), } if previous_of_active_section: section_context['prev_url'] = _compute_section_url(previous_of_active_section, 'last') diff --git a/lms/templates/hidden_content.html b/lms/templates/hidden_content.html new file mode 100644 index 0000000000..7e6f4b5c04 --- /dev/null +++ b/lms/templates/hidden_content.html @@ -0,0 +1,26 @@ +<%page expression_filter="h"/> +<%! +from django.utils.translation import ugettext as _ +from openedx.core.djangolib.markup import HTML, Text +%> + +
+

+ ${_("The due date for this {subsection_format} has passed.").format( + subsection_format=subsection_format, + )} +

+
+

+ ${Text(_( + "Because the due date has passed, this {subsection_format} " + "is no longer available.{line_break}If you have completed this {subsection_format}, " + "your grade is available on the {link_start}progress page{link_end}." + )).format( + subsection_format=subsection_format, + line_break=HTML("
"), + link_start=HTML("").format(progress_url), + link_end=HTML(""), + )} +

+
diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html index 3500467a62..7182ba1a68 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -3,12 +3,12 @@
- % if override_hidden_exam: + % if banner_text:

- ${_("This exam is hidden from the learner.")} + ${banner_text}

diff --git a/setup.py b/setup.py index 0ae21d650b..5e72d514d6 100644 --- a/setup.py +++ b/setup.py @@ -49,6 +49,7 @@ setup( "start_date = lms.djangoapps.course_blocks.transformers.start_date:StartDateTransformer", "user_partitions = lms.djangoapps.course_blocks.transformers.user_partitions:UserPartitionTransformer", "visibility = lms.djangoapps.course_blocks.transformers.visibility:VisibilityTransformer", + "hidden_content = lms.djangoapps.course_blocks.transformers.hidden_content:HiddenContentTransformer", "course_blocks_api = lms.djangoapps.course_api.blocks.transformers.blocks_api:BlocksAPITransformer", "proctored_exam = lms.djangoapps.course_api.blocks.transformers.proctored_exam:ProctoredExamTransformer", "grades = lms.djangoapps.courseware.transformers.grades:GradesTransformer",