From aa93585e4e5372c31c7a46d0179aa92bf70c8996 Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Fri, 2 Oct 2015 14:28:32 -0400 Subject: [PATCH 1/2] Added myself to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 7f7769ff15..37f80cad0d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -246,3 +246,4 @@ Jamie Folsom George Schneeloch Dustin Gadal Robert Raposa +Giovanni Di Milia From f2a1d0e54b5252e134947e92bf2dd50e35495f56 Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Fri, 2 Oct 2015 13:21:06 -0400 Subject: [PATCH 2/2] Coaches can't see blocks "hide from students" If a block is marked as "hide from students" cannot be seen in the CCX schedule --- lms/djangoapps/ccx/tests/test_views.py | 77 ++++++++++++++++++++++++-- lms/djangoapps/ccx/views.py | 5 ++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index fcce7146a8..ad0d52da3f 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/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): diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 5cc41f7625..4c54a8e1f3 100644 --- a/lms/djangoapps/ccx/views.py +++ b/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]