Merge pull request #13019 from mitocw/fix/aq/emit_course_published_event

Fixed progress page update on save ccx and grade book crash issue

lms/djangoapps/ccx/api/v0/views.py

@@ -17,6 +17,7 @@ from rest_framework_oauth.authentication import OAuth2Authentication
 
 from ccx_keys.locator import CCXLocator
 from courseware import courses
+from xmodule.modulestore.django import SignalHandler
 from edx_rest_framework_extensions.authentication import JwtAuthentication
 from instructor.enrollment import (
     enroll_email,
@@ -491,7 +492,7 @@ class CCXListView(GenericAPIView):
             make_user_coach(coach, master_course_key)
 
             # pull the ccx course key
-            ccx_course_key = CCXLocator.from_course_locator(master_course_object.id, ccx_course_object.id)
+            ccx_course_key = CCXLocator.from_course_locator(master_course_object.id, unicode(ccx_course_object.id))
             # enroll the coach in the newly created ccx
             email_params = get_email_params(
                 master_course_object,
@@ -517,6 +518,14 @@ class CCXListView(GenericAPIView):
             )
 
         serializer = self.get_serializer(ccx_course_object)
+
+        # using CCX object as sender here.
+        responses = SignalHandler.course_published.send(
+            sender=ccx_course_object,
+            course_key=ccx_course_key
+        )
+        for rec, response in responses:
+            log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response)
         return Response(
             status=status.HTTP_201_CREATED,
             data=serializer.data
@@ -760,6 +769,14 @@ class CCXDetailView(GenericAPIView):
                 # enroll the coach to the newly created ccx
                 assign_coach_role_to_ccx(ccx_course_key, coach, master_course_object.id)
 
+        # using CCX object as sender here.
+        responses = SignalHandler.course_published.send(
+            sender=ccx_course_object,
+            course_key=ccx_course_key
+        )
+        for rec, response in responses:
+            log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response)
+
         return Response(
             status=status.HTTP_204_NO_CONTENT,
         )

lms/djangoapps/ccx/tasks.py

@@ -33,7 +33,7 @@ def send_ccx_course_published(course_key):
     course_key = CourseLocator.from_string(course_key)
     for ccx in CustomCourseForEdX.objects.filter(course_id=course_key):
         try:
-            ccx_key = CCXLocator.from_course_locator(course_key, ccx.id)
+            ccx_key = CCXLocator.from_course_locator(course_key, unicode(ccx.id))
         except InvalidKeyError:
             log.info('Attempt to publish course with deprecated id. Course: %s. CCX: %s', course_key, ccx.id)
             continue

lms/djangoapps/ccx/tests/test_views.py

@@ -7,6 +7,7 @@ import re
 import pytz
 import ddt
 import urlparse
+from dateutil.tz import tzutc
 from mock import patch, MagicMock
 from nose.plugins.attrib import attr
 
@@ -67,7 +68,7 @@ from lms.djangoapps.ccx.tests.utils import (
 )
 from lms.djangoapps.ccx.utils import (
     ccx_course,
-    is_email
+    is_email,
 )
 from lms.djangoapps.ccx.views import get_date
 
@@ -133,6 +134,16 @@ def setup_students_and_grades(context):
                     )
 
 
+def unhide(unit):
+    """
+    Recursively unhide a unit and all of its children in the CCX
+    schedule.
+    """
+    unit['hidden'] = False
+    for child in unit.get('children', ()):
+        unhide(child)
+
+
 class TestAdminAccessCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
     """
     Tests for Custom Courses views.
@@ -178,6 +189,121 @@ class TestAdminAccessCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
 
 
 @attr('shard_1')
+@override_settings(
+    XBLOCK_FIELD_DATA_WRAPPERS=['lms.djangoapps.courseware.field_overrides:OverrideModulestoreFieldData.wrap'],
+    MODULESTORE_FIELD_OVERRIDE_PROVIDERS=['ccx.overrides.CustomCoursesForEdxOverrideProvider'],
+)
+class TestCCXProgressChanges(CcxTestCase, LoginEnrollmentTestCase):
+    """
+    Tests ccx schedule changes in progress page
+    """
+    @classmethod
+    def setUpClass(cls):
+        """
+        Set up tests
+        """
+        super(TestCCXProgressChanges, cls).setUpClass()
+        start = datetime.datetime(2016, 7, 1, 0, 0, tzinfo=tzutc())
+        due = datetime.datetime(2016, 7, 8, 0, 0, tzinfo=tzutc())
+
+        cls.course = course = CourseFactory.create(enable_ccx=True, start=start)
+        chapter = ItemFactory.create(start=start, parent=course, category=u'chapter')
+        sequential = ItemFactory.create(
+            parent=chapter,
+            start=start,
+            due=due,
+            category=u'sequential',
+            metadata={'graded': True, 'format': 'Homework'}
+        )
+        vertical = ItemFactory.create(
+            parent=sequential,
+            start=start,
+            due=due,
+            category=u'vertical',
+            metadata={'graded': True, 'format': 'Homework'}
+        )
+
+        # Trying to wrap the whole thing in a bulk operation fails because it
+        # doesn't find the parents. But we can at least wrap this part...
+        with cls.store.bulk_operations(course.id, emit_signals=False):
+            flatten([ItemFactory.create(
+                parent=vertical,
+                start=start,
+                due=due,
+                category="problem",
+                data=StringResponseXMLFactory().build_xml(answer='foo'),
+                metadata={'rerandomize': 'always'}
+            )] for _ in xrange(2))
+
+    def assert_progress_summary(self, ccx_course_key, due):
+        """
+        assert signal and schedule update.
+        """
+        student = UserFactory.create(is_staff=False, password="test")
+        CourseEnrollment.enroll(student, ccx_course_key)
+        self.assertTrue(
+            CourseEnrollment.objects.filter(course_id=ccx_course_key, user=student).exists()
+        )
+
+        # login as student
+        self.client.login(username=student.username, password="test")
+        progress_page_response = self.client.get(
+            reverse('progress', kwargs={'course_id': ccx_course_key})
+        )
+        grade_summary = progress_page_response.mako_context['courseware_summary']  # pylint: disable=no-member
+        chapter = grade_summary[0]
+        section = chapter['sections'][0]
+        progress_page_due_date = section['due'].strftime("%Y-%m-%d %H:%M")
+        self.assertEqual(progress_page_due_date, due)
+
+    @patch('ccx.views.render_to_response', intercept_renderer)
+    @patch('courseware.views.views.render_to_response', intercept_renderer)
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def test_edit_schedule(self):
+        """
+        Get CCX schedule, modify it, save it.
+        """
+        self.make_coach()
+        ccx = self.make_ccx()
+        ccx_course_key = CCXLocator.from_course_locator(self.course.id, unicode(ccx.id))
+        self.client.login(username=self.coach.username, password="test")
+
+        url = reverse('ccx_coach_dashboard', kwargs={'course_id': ccx_course_key})
+        response = self.client.get(url)
+
+        schedule = json.loads(response.mako_context['schedule'])  # pylint: disable=no-member
+        self.assertEqual(len(schedule), 1)
+
+        unhide(schedule[0])
+
+        # edit schedule
+        date = datetime.datetime.now() - datetime.timedelta(days=5)
+        start = date.strftime("%Y-%m-%d %H:%M")
+        due = (date + datetime.timedelta(days=3)).strftime("%Y-%m-%d %H:%M")
+
+        schedule[0]['start'] = start
+        schedule[0]['children'][0]['start'] = start
+        schedule[0]['children'][0]['due'] = due
+        schedule[0]['children'][0]['children'][0]['start'] = start
+        schedule[0]['children'][0]['children'][0]['due'] = due
+
+        url = reverse('save_ccx', kwargs={'course_id': ccx_course_key})
+        response = self.client.post(url, json.dumps(schedule), content_type='application/json')
+
+        self.assertEqual(response.status_code, 200)
+
+        schedule = json.loads(response.content)['schedule']
+        self.assertEqual(schedule[0]['hidden'], False)
+        self.assertEqual(schedule[0]['start'], start)
+        self.assertEqual(schedule[0]['children'][0]['start'], start)
+        self.assertEqual(schedule[0]['children'][0]['due'], due)
+        self.assertEqual(schedule[0]['children'][0]['children'][0]['due'], due)
+        self.assertEqual(schedule[0]['children'][0]['children'][0]['start'], start)
+
+        self.assert_progress_summary(ccx_course_key, due)
+
+
+@attr('shard_1')
 @ddt.ddt
 class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
     """
@@ -384,15 +510,6 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
             'save_ccx',
             kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)})
 
-        def unhide(unit):
-            """
-            Recursively unhide a unit and all of its children in the CCX
-            schedule.
-            """
-            unit['hidden'] = False
-            for child in unit.get('children', ()):
-                unhide(child)
-
         unhide(schedule[0])
         schedule[0]['start'] = u'2014-11-20 00:00'
         schedule[0]['children'][0]['due'] = u'2014-12-25 00:00'  # what a jerk!
@@ -1017,11 +1134,18 @@ class TestCCXGrades(FieldOverrideTestMixin, SharedModuleStoreTestCase, LoginEnro
 
         # create a ccx locator and retrieve the course structure using that key
         # which emulates how a student would get access.
-        self.ccx_key = CCXLocator.from_course_locator(self._course.id, ccx.id)
+        self.ccx_key = CCXLocator.from_course_locator(self._course.id, unicode(ccx.id))
         self.course = get_course_by_id(self.ccx_key, depth=None)
         setup_students_and_grades(self)
         self.client.login(username=coach.username, password="test")
         self.addCleanup(RequestCache.clear_request_cache)
+        from xmodule.modulestore.django import SignalHandler
+
+        # using CCX object as sender here.
+        SignalHandler.course_published.send(
+            sender=ccx,
+            course_key=self.ccx_key
+        )
 
     @patch('ccx.views.render_to_response', intercept_renderer)
     @patch('instructor.views.gradebook_api.MAX_STUDENTS_PER_PAGE_GRADE_BOOK', 1)

lms/djangoapps/ccx/tests/utils.py

@@ -80,7 +80,7 @@ class CcxTestCase(SharedModuleStoreTestCase):
         """
         super(CcxTestCase, self).setUp()
         # Create instructor account
-        self.coach = UserFactory.create()
+        self.coach = UserFactory.create(password="test")
         # create an instance of modulestore
         self.mstore = modulestore()
 

lms/djangoapps/ccx/views.py

@@ -36,6 +36,7 @@ from opaque_keys.edx.keys import CourseKey
 from ccx_keys.locator import CCXLocator
 from student.roles import CourseCcxCoachRole
 from student.models import CourseEnrollment
+from xmodule.modulestore.django import SignalHandler
 
 from instructor.views.api import _split_input_list
 from instructor.views.gradebook_api import get_grade_book_page
@@ -132,7 +133,7 @@ def dashboard(request, course, ccx=None):
         if ccx:
             url = reverse(
                 'ccx_coach_dashboard',
-                kwargs={'course_id': CCXLocator.from_course_locator(course.id, ccx.id)}
+                kwargs={'course_id': CCXLocator.from_course_locator(course.id, unicode(ccx.id))}
             )
             return redirect(url)
 
@@ -217,7 +218,7 @@ def create_ccx(request, course, ccx=None):
             for vertical in sequential.get_children():
                 override_field_for_ccx(ccx, vertical, hidden, True)
 
-    ccx_id = CCXLocator.from_course_locator(course.id, ccx.id)
+    ccx_id = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
 
     url = reverse('ccx_coach_dashboard', kwargs={'course_id': ccx_id})
 
@@ -233,6 +234,15 @@ def create_ccx(request, course, ccx=None):
 
     assign_coach_role_to_ccx(ccx_id, request.user, course.id)
     add_master_course_staff_to_ccx(course, ccx_id, ccx.display_name)
+
+    # using CCX object as sender here.
+    responses = SignalHandler.course_published.send(
+        sender=ccx,
+        course_key=CCXLocator.from_course_locator(course.id, unicode(ccx.id))
+    )
+    for rec, response in responses:
+        log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response)
+
     return redirect(url)
 
 
@@ -324,6 +334,14 @@ def save_ccx(request, course, ccx=None):
     if changed:
         override_field_for_ccx(ccx, course, 'grading_policy', policy)
 
+    # using CCX object as sender here.
+    responses = SignalHandler.course_published.send(
+        sender=ccx,
+        course_key=CCXLocator.from_course_locator(course.id, unicode(ccx.id))
+    )
+    for rec, response in responses:
+        log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response)
+
     return HttpResponse(
         json.dumps({
             'schedule': get_ccx_schedule(course, ccx),
@@ -345,9 +363,17 @@ def set_grading_policy(request, course, ccx=None):
     override_field_for_ccx(
         ccx, course, 'grading_policy', json.loads(request.POST['policy']))
 
+    # using CCX object as sender here.
+    responses = SignalHandler.course_published.send(
+        sender=ccx,
+        course_key=CCXLocator.from_course_locator(course.id, unicode(ccx.id))
+    )
+    for rec, response in responses:
+        log.info('Signal fired when course is published. Receiver: %s. Response: %s', rec, response)
+
     url = reverse(
         'ccx_coach_dashboard',
-        kwargs={'course_id': CCXLocator.from_course_locator(course.id, ccx.id)}
+        kwargs={'course_id': CCXLocator.from_course_locator(course.id, unicode(ccx.id))}
     )
     return redirect(url)
 
@@ -448,7 +474,7 @@ def ccx_invite(request, course, ccx=None):
     identifiers_raw = request.POST.get('student-ids')
     identifiers = _split_input_list(identifiers_raw)
     email_students = 'email-students' in request.POST
-    course_key = CCXLocator.from_course_locator(course.id, ccx.id)
+    course_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
     email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name)
 
     ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, ccx.coach)
@@ -471,7 +497,7 @@ def ccx_student_management(request, course, ccx=None):
     student_id = request.POST.get('student-id', '')
     email_students = 'email-students' in request.POST
     identifiers = [student_id]
-    course_key = CCXLocator.from_course_locator(course.id, ccx.id)
+    course_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
     email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name)
 
     errors = ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, ccx.coach)
@@ -494,7 +520,7 @@ def ccx_gradebook(request, course, ccx=None):
     if not ccx:
         raise Http404
 
-    ccx_key = CCXLocator.from_course_locator(course.id, ccx.id)
+    ccx_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
     with ccx_course(ccx_key) as course:
         prep_course_for_grading(course, request)
         student_info, page = get_grade_book_page(request, course, course_key=ccx_key)
@@ -522,7 +548,7 @@ def ccx_grades_csv(request, course, ccx=None):
     if not ccx:
         raise Http404
 
-    ccx_key = CCXLocator.from_course_locator(course.id, ccx.id)
+    ccx_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
     with ccx_course(ccx_key) as course:
         prep_course_for_grading(course, request)