Merge pull request #14324 from edx/efischer/revertins

Revert #14225 hide_after_due for self-paced courses

cms/djangoapps/contentstore/views/item.py

@@ -981,7 +981,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
         "release_date": release_date,
         "visibility_state": visibility_state,
         "has_explicit_staff_lock": xblock.fields['visible_to_staff_only'].is_set_on(xblock),
-        "self_paced": is_self_paced(course),
         "start": xblock.fields['start'].to_json(xblock.start),
         "graded": xblock.graded,
         "due_date": get_default_time_display(xblock.due),

cms/static/js/views/modals/course_outline_modals.js

@@ -712,10 +712,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview',
             return $.extend(
                 {},
                 AbstractVisibilityEditor.prototype.getContext.call(this),
-                {
-                    hide_after_due: this.modelVisibility() === 'hide_after_due',
-                    self_paced: this.model.get('self_paced') === true
-                }
+                {hide_after_due: this.modelVisibility() === 'hide_after_due'}
             );
         }
     });

cms/templates/js/content-visibility-editor.underscore

@@ -12,19 +12,9 @@
         <li class="field-radio">
             <label class="label">
                 <input class="input input-radio" name="content-visibility" type="radio" value="hide_after_due" aria-described-by="hide_after_due_description">
-                <% if (self_paced) { %>
-                    <%- gettext('Hide content after course end date') %>
-                <% } else { %>
-                    <%- gettext('Hide content after due date') %>
-                <% } %>
+                <%- gettext('Hide content after due date') %>
             </label>
-            <p class='field-message' id='hide_after_due_description'>
-                <% if (self_paced) { %>
-                    <%- gettext('After the course\'s end date has passed, learners can no longer access subsection content. The subsection remains included in grade calculations.') %>
-                <% } else { %>
-                    <%- gettext('After the subsection\'s due date has passed, learners can no longer access its content. The subsection remains included in grade calculations.') %>
-                <% } %>
-            </p>
+            <p class='field-message' id='hide_after_due_description'> <%- gettext('After the subsection\'s due date has passed, learners can no longer access its content. The subsection remains included in grade calculations.') %> </p>
         </li>
         <li class="field-radio">
             <label class="label">
@@ -39,7 +29,7 @@
     <% if (hasExplicitStaffLock && !ancestorLocked) { %>
         <p class="tip tip-warning">
             <%- interpolate(
-                gettext('If you select an option other than "%(hide_label)s", published units in this subsection become available to learners unless they are explicitly hidden.'),
+                gettext('If you select an option other than "%(hide_label)s", after the subsection release date has passed, published units in this subsection will become available to learners unless units are explicitly hidden.'),
                 { hide_label: hide_label },
                 true
             ) %>

cms/templates/js/course-outline.underscore

@@ -201,12 +201,7 @@ if (is_proctored_exam) {
                 <p>
                     <% if (xblockInfo.get('hide_after_due')) { %>
                         <span class="icon fa fa-eye-slash" aria-hidden="true"></span>
-                        <span class="status-hide-after-due-value">
-                        <% if (course.get('self_paced')) { %>
-                            <%- gettext("Subsection is hidden after course end date") %> </span>
-                        <% } else { %>
-                            <%- gettext("Subsection is hidden after due date") %> </span>
-                        <% } %>
+                        <span class="status-hide-after-due-value"> <%- gettext("Subsection is hidden after due date") %> </span>
                     <% } %>
                 </p>
             </div>

common/lib/xmodule/xmodule/seq_module.py

@@ -202,16 +202,16 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
         raise NotFoundError('Unexpected dispatch type')
 
     @classmethod
-    def verify_current_content_visibility(cls, date, hide_after_date):
+    def verify_current_content_visibility(cls, due, hide_after_due):
         """
         Returns whether the content visibility policy passes
-        for the given date and hide_after_date values and
+        for the given due date and hide_after_due values and
         the current date-time.
         """
         return (
-            not date or
-            not hide_after_date or
-            datetime.now(UTC()) < date
+            not due or
+            not hide_after_due or
+            datetime.now(UTC()) < due
         )
 
     def student_view(self, context):
@@ -246,17 +246,20 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
         runtime user. If so, returns a banner_text or the fragment to
         display depending on whether staff is masquerading.
         """
-        course = self._get_course()
-        if not self._can_user_view_content(course):
-            if course.self_paced:
-                banner_text = _("Because the course has ended, this assignment is hidden from the learner.")
-            else:
-                banner_text = _("Because the due date has passed, this assignment is hidden from the learner.")
+        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',
                 {
-                    'self_paced': course.self_paced,
+                    'subsection_format': subsection_format,
                     'progress_url': context.get('progress_url'),
                 }
             )
@@ -277,15 +280,14 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
             if content_milestones and self.runtime.user_is_staff:
                 return banner_text
 
-    def _can_user_view_content(self, course):
+    def _can_user_view_content(self):
         """
         Returns whether the runtime user can view the content
         of this sequential.
         """
-        hidden_date = course.end if course.self_paced else self.due
         return (
             self.runtime.user_is_staff or
-            self.verify_current_content_visibility(hidden_date, self.hide_after_due)
+            self.verify_current_content_visibility(self.due, self.hide_after_due)
         )
 
     def _student_view(self, context, banner_text=None):

common/lib/xmodule/xmodule/tests/test_sequence.py

@@ -3,46 +3,49 @@ 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, patch
-from xmodule.seq_module import SequenceModule
+from mock import Mock
 from xmodule.tests import get_test_system
 from xmodule.tests.helpers import StubUserService
-from xmodule.tests.xml import factories as xml, XModuleXmlImportTest
+from xmodule.tests.xml import XModuleXmlImportTest
+from xmodule.tests.xml import factories as xml
 from xmodule.x_module import STUDENT_VIEW
-
-TODAY = now()
-DUE_DATE = TODAY + timedelta(days=7)
-PAST_DUE_BEFORE_END_DATE = TODAY + timedelta(days=14)
-COURSE_END_DATE = TODAY + timedelta(days=21)
+from xmodule.seq_module import SequenceModule
 
 
+@ddt.ddt
 class SequenceBlockTestCase(XModuleXmlImportTest):
     """
-    Base class for tests of Sequence Module.
+    Tests for the Sequence Module.
     """
-    def setUp(self):
-        super(SequenceBlockTestCase, self).setUp()
+    TODAY = now()
+    TOMORROW = TODAY + timedelta(days=1)
+    DAY_AFTER_TOMORROW = TOMORROW + timedelta(days=1)
 
-        course_xml = self._set_up_course_xml()
-        self.course = self.process_xml(course_xml)
-        self._set_up_module_system(self.course)
+    @classmethod
+    def setUpClass(cls):
+        super(SequenceBlockTestCase, cls).setUpClass()
 
-        for chapter_index in range(len(self.course.get_children())):
-            chapter = self._set_up_block(self.course, chapter_index)
-            setattr(self, 'chapter_{}'.format(chapter_index + 1), chapter)
+        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 = self._set_up_block(chapter, sequence_index)
-                setattr(self, 'sequence_{}_{}'.format(chapter_index + 1, sequence_index + 1), sequence)
+                sequence = cls._set_up_block(chapter, sequence_index)
+                setattr(cls, 'sequence_{}_{}'.format(chapter_index + 1, sequence_index + 1), sequence)
 
-    @staticmethod
-    def _set_up_course_xml():
+    @classmethod
+    def _set_up_course_xml(cls):
         """
         Sets up and returns XML course structure.
         """
-        course = xml.CourseFactory.build(end=str(COURSE_END_DATE))
+        course = xml.CourseFactory.build()
 
         chapter_1 = xml.ChapterFactory.build(parent=course)  # has 2 child sequences
         xml.ChapterFactory.build(parent=course)  # has 0 child sequences
@@ -55,7 +58,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
         xml.SequenceFactory.build(  # sequence_4_1
             parent=chapter_4,
             hide_after_due=str(True),
-            due=str(DUE_DATE),
+            due=str(cls.TOMORROW),
         )
 
         for _ in range(3):
@@ -63,13 +66,14 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
 
         return course
 
-    def _set_up_block(self, parent, index_in_parent):
+    @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]
 
-        self._set_up_module_system(block)
+        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
@@ -77,7 +81,8 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
         block.parent = parent.location
         return block
 
-    def _set_up_module_system(self, block):
+    @classmethod
+    def _set_up_module_system(cls, block):
         """
         Sets up the test module system for the given block.
         """
@@ -85,28 +90,6 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
         module_system.descriptor_runtime = block._runtime  # pylint: disable=protected-access
         block.xmodule_runtime = module_system
 
-    def _get_rendered_student_view(self, sequence, requested_child=None, extra_context=None, self_paced=False):
-        """
-        Returns the rendered student view for the given sequence and the
-        requested_child parameter.
-        """
-        context = {'requested_child': requested_child}
-        if extra_context:
-            context.update(extra_context)
-
-        # The render operation will ask modulestore for the current course to get some data. As these tests were
-        # originally not written to be compatible with a real modulestore, we've mocked out the relevant return values.
-        with patch.object(SequenceModule, '_get_course') as mock_course:
-            self.course.self_paced = self_paced
-            mock_course.return_value = self.course
-            return sequence.xmodule_runtime.render(sequence, STUDENT_VIEW, context).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_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
@@ -129,6 +112,22 @@ 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=None, extra_context=None):
+        """
+        Returns the rendered student view for the given sequence and the
+        requested_child parameter.
+        """
+        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):
+        """
+        Verifies that the rendered view contains the expected position.
+        """
+        self.assertIn("'position': {}".format(expected_position), rendered_html)
+
     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:
@@ -139,18 +138,26 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
         self.assertIn("seq_module.html", html)
         self.assertIn("'banner_text': None", html)
 
-    @freeze_time(COURSE_END_DATE)
-    def test_hidden_content_past_due(self):
+    @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(COURSE_END_DATE)
+    @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),
@@ -158,23 +165,13 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
         self.assertIn("seq_module.html", html)
         self.assertIn(
             "'banner_text': 'Because the due date has passed, "
-            "this assignment is hidden from the learner.'",
+            "this homework is hidden from the learner.'",
             html
         )
 
-    @freeze_time(PAST_DUE_BEFORE_END_DATE)
-    def test_hidden_content_self_paced_past_due_before_end(self):
-        html = self._get_rendered_student_view(self.sequence_4_1, self_paced=True)
-        self.assertIn("seq_module.html", html)
-        self.assertIn("'banner_text': None", html)
-
-    @freeze_time(COURSE_END_DATE + timedelta(days=7))
-    def test_hidden_content_self_paced_past_end(self):
-        progress_url = 'http://test_progress_link'
-        html = self._get_rendered_student_view(
-            self.sequence_4_1,
-            extra_context=dict(progress_url=progress_url),
-            self_paced=True,
-        )
-        self.assertIn("hidden_content.html", html)
-        self.assertIn(progress_url, 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

common/test/acceptance/pages/lms/courseware.py

@@ -209,13 +209,13 @@ class CoursewarePage(CoursePage):
         """
         return self.q(css="div.proctored-exam.completed").visible
 
-    def content_hidden_past_due_date(self):
+    def content_hidden_past_due_date(self, content_type="subsection"):
         """
         Returns whether the "the due date for this ___ has passed" message is present.
         ___ is the type of the hidden content, and defaults to subsection.
         This being true implies "the ___ contents are hidden because their due date has passed".
         """
-        message = "this assignment is no longer available"
+        message = "The due date for this {0} has passed.".format(content_type)
         if self.q(css="div.seq_content").is_present():
             return False
         for html in self.q(css="div.hidden-content").html:

lms/djangoapps/course_blocks/transformers/hidden_content.py

@@ -25,7 +25,7 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor
 
     Staff users are exempted from hidden content rules.
     """
-    VERSION = 2
+    VERSION = 1
     MERGED_DUE_DATE = 'merged_due_date'
     MERGED_HIDE_AFTER_DUE = 'merged_hide_after_due'
 
@@ -41,7 +41,7 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor
     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 hidden after due date per
+        given block_structure should be visible to staff only per
         computed value from ancestry chain.
         """
         return block_structure.get_transformer_block_field(
@@ -81,8 +81,6 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor
             func_merge_ancestors=min,
         )
 
-        block_structure.request_xblock_fields(u'self_paced', u'end')
-
     def transform_block_filters(self, usage_info, block_structure):
         # Users with staff access bypass the Visibility check.
         if usage_info.has_staff_access:
@@ -99,10 +97,6 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor
         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)
-        self_paced = block_structure[block_structure.root_block_usage_key].self_paced
-        if self_paced:
-            hidden_date = block_structure[block_structure.root_block_usage_key].end
-        else:
-            hidden_date = self._get_merged_due_date(block_structure, block_key)
-        return not SequenceModule.verify_current_content_visibility(hidden_date, hide_after_due)
+        return not SequenceModule.verify_current_content_visibility(due, hide_after_due)

lms/djangoapps/courseware/tests/test_views.py

@@ -199,7 +199,7 @@ class IndexQueryTestCase(ModuleStoreTestCase):
     NUM_PROBLEMS = 20
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 9),
+        (ModuleStoreEnum.Type.mongo, 8),
         (ModuleStoreEnum.Type.split, 4),
     )
     @ddt.unpack

lms/templates/hidden_content.html

@@ -5,35 +5,22 @@ from openedx.core.djangolib.markup import HTML, Text
 %>
 
 <div class="sequence hidden-content proctored-exam completed">
-    <h3>
-        % if self_paced:
-            ${_("The course has ended.")}
-        % else:
-            ${_("The due date for this assignment has passed.")}
-        % endif
-    </h3>
-    <hr>
-    <p>
-        % if self_paced:
-            ${Text(_(
-                "Because the course has ended, this assignment is no longer "
-                "available.{line_break}If you have completed this assignment, your "
-                "grade is available on the {link_start}progress page{link_end}."
-            )).format(
-                line_break=HTML("<br>"),
-                link_start=HTML("<a href='{}'>").format(progress_url),
-                link_end=HTML("</a>"),
-            )}
-        % else:
-            ${Text(_(
-                "Because the due date has passed, this assignment is no longer "
-                "available.{line_break}If you have completed this assignment, your "
-                "grade is available on the {link_start}progress page{link_end}."
-            )).format(
-                line_break=HTML("<br>"),
-                link_start=HTML("<a href='{}'>").format(progress_url),
-                link_end=HTML("</a>"),
-            )}
-        % endif
-    </p>
+  <h3>
+      ${_("The due date for this {subsection_format} has passed.").format(
+        subsection_format=subsection_format,
+      )}
+  </h3>
+  <hr>
+  <p>
+      ${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("<br>"),
+          link_start=HTML("<a href='{}'>").format(progress_url),
+          link_end=HTML("</a>"),
+      )}
+  </p>
 </div>