Merge pull request #10034 from mitocw/fix/gdm/hide_block_from_ccx

Fixed visibility of hidden blocks in CCX schedule

AUTHORS

@@ -246,3 +246,4 @@ Jamie Folsom <jfolsom@mit.edu>
 George Schneeloch <gschneel@mit.edu>
 Dustin Gadal <Dustin.Gadal@gmail.com>
 Robert Raposa <rraposa@edx.org>
+Giovanni Di Milia <gdimilia@mit.edu>

lms/djangoapps/ccx/tests/test_views.py

@@ -34,6 +34,7 @@ from student.tests.factories import (  # pylint: disable=import-error
 
 from xmodule.x_module import XModuleMixin
 from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase,
     SharedModuleStoreTestCase,
@@ -99,20 +100,20 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
             2010, 7, 7, 0, 0, tzinfo=pytz.UTC
         )
 
-        chapters = [
+        cls.chapters = [
             ItemFactory.create(start=start, parent=course) for _ in xrange(2)
         ]
-        sequentials = flatten([
+        cls.sequentials = flatten([
             [
                 ItemFactory.create(parent=chapter) for _ in xrange(2)
-            ] for chapter in chapters
+            ] for chapter in cls.chapters
         ])
-        verticals = flatten([
+        cls.verticals = flatten([
             [
                 ItemFactory.create(
                     due=due, parent=sequential, graded=True, format='Homework'
                 ) for _ in xrange(2)
-            ] for sequential in sequentials
+            ] for sequential in cls.sequentials
         ])
 
         # Trying to wrap the whole thing in a bulk operation fails because it
@@ -121,7 +122,7 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
             blocks = flatten([  # pylint: disable=unused-variable
                 [
                     ItemFactory.create(parent=vertical) for _ in xrange(2)
-                ] for vertical in verticals
+                ] for vertical in cls.verticals
             ])
 
     def setUp(self):
@@ -133,6 +134,8 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
         # Create instructor account
         self.coach = coach = AdminFactory.create()
         self.client.login(username=coach.username, password="test")
+        # create an instance of modulestore
+        self.mstore = modulestore()
 
     def make_coach(self):
         """
@@ -155,6 +158,31 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
         from django.core import mail
         return mail.outbox
 
+    def assert_elements_in_schedule(self, url, n_chapters=2, n_sequentials=4, n_verticals=8):
+        """
+        Helper function to count visible elements in the schedule
+        """
+        response = self.client.get(url)
+        # the schedule contains chapters
+        chapters = json.loads(response.mako_context['schedule'])  # pylint: disable=no-member
+        sequentials = flatten([chapter.get('children', []) for chapter in chapters])
+        verticals = flatten([sequential.get('children', []) for sequential in sequentials])
+        # check that the numbers of nodes at different level are the expected ones
+        self.assertEqual(n_chapters, len(chapters))
+        self.assertEqual(n_sequentials, len(sequentials))
+        self.assertEqual(n_verticals, len(verticals))
+        # extract the locations of all the nodes
+        all_elements = chapters + sequentials + verticals
+        return [elem['location'] for elem in all_elements if 'location' in elem]
+
+    def hide_node(self, node):
+        """
+        Helper function to set the node `visible_to_staff_only` property
+        to True and save the change
+        """
+        node.visible_to_staff_only = True
+        self.mstore.update_item(node, self.coach.id)
+
     def test_not_a_coach(self):
         """
         User is not a coach, should get Forbidden response.
@@ -196,6 +224,43 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
         self.assertEqual(response.status_code, 200)
         self.assertTrue(re.search('id="ccx-schedule"', response.content))
 
+    @SharedModuleStoreTestCase.modifies_courseware
+    @patch('ccx.views.render_to_response', intercept_renderer)
+    @patch('ccx.views.TODAY')
+    def test_get_ccx_schedule(self, today):
+        """
+        Gets CCX schedule and checks number of blocks in it.
+        Hides nodes at a different depth and checks that these nodes
+        are not in the schedule.
+        """
+        today.return_value = datetime.datetime(2014, 11, 25, tzinfo=pytz.UTC)
+        self.make_coach()
+        ccx = self.make_ccx()
+        url = reverse(
+            'ccx_coach_dashboard',
+            kwargs={
+                'course_id': CCXLocator.from_course_locator(
+                    self.course.id, ccx.id)
+            }
+        )
+        # all the elements are visible
+        self.assert_elements_in_schedule(url)
+        # hide a vertical
+        vertical = self.verticals[0]
+        self.hide_node(vertical)
+        locations = self.assert_elements_in_schedule(url, n_verticals=7)
+        self.assertNotIn(unicode(vertical.location), locations)
+        # hide a sequential
+        sequential = self.sequentials[0]
+        self.hide_node(sequential)
+        locations = self.assert_elements_in_schedule(url, n_sequentials=3, n_verticals=6)
+        self.assertNotIn(unicode(sequential.location), locations)
+        # hide a chapter
+        chapter = self.chapters[0]
+        self.hide_node(chapter)
+        locations = self.assert_elements_in_schedule(url, n_chapters=1, n_sequentials=2, n_verticals=4)
+        self.assertNotIn(unicode(chapter.location), locations)
+
     @patch('ccx.views.render_to_response', intercept_renderer)
     @patch('ccx.views.TODAY')
     def test_edit_schedule(self, today):

lms/djangoapps/ccx/views.py

@@ -347,8 +347,13 @@ def get_ccx_schedule(course, ccx):
         Recursive generator function which yields CCX schedule nodes.
         We convert dates to string to get them ready for use by the js date
         widgets, which use text inputs.
+        Visits students visible nodes only; nodes children of hidden ones
+        are skipped as well.
         """
         for child in node.get_children():
+            # in case the children are visible to staff only, skip them
+            if child.visible_to_staff_only:
+                continue
             start = get_override_for_ccx(ccx, child, 'start', None)
             if start:
                 start = str(start)[:-9]