Merge pull request #14780 from edx/efischer/ora_grades

Clear Student State: ORA/Robust Grades fixes

lms/djangoapps/grades/signals/handlers.py

 """
 Grades related signals.
 """
-
+from contextlib import contextmanager
 from logging import getLogger
 
 from django.dispatch import receiver
@@ -109,6 +109,25 @@ def submissions_score_reset_handler(sender, **kwargs):  # pylint: disable=unused
     )
 
 
+@contextmanager
+def disconnect_submissions_signal_receiver(signal):
+    """
+    Context manager to be used for temporarily disconnecting edx-submission's set or reset signal.
+    """
+    if signal == score_set:
+        handler = submissions_score_set_handler
+    else:
+        if signal != score_reset:
+            raise ValueError("This context manager only deal with score_set and score_reset signals.")
+        handler = submissions_score_reset_handler
+
+    signal.disconnect(handler)
+    try:
+        yield
+    finally:
+        signal.connect(handler)
+
+
 @receiver(SCORE_PUBLISHED)
 def score_published_handler(sender, block, user, raw_earned, raw_possible, only_if_higher, **kwargs):  # pylint: disable=unused-argument
     """

lms/djangoapps/grades/tests/test_signals.py

@@ -9,6 +9,7 @@ import ddt
 from django.test import TestCase
 from mock import patch, MagicMock
 import pytz
+from submissions.models import score_set, score_reset
 from util.date_utils import to_timestamp
 
 from ..constants import ScoreDatabaseTableEnum
@@ -16,8 +17,10 @@ from ..signals.handlers import (
     enqueue_subsection_update,
     submissions_score_set_handler,
     submissions_score_reset_handler,
+    disconnect_submissions_signal_receiver,
     problem_raw_score_changed_handler,
 )
+from ..signals.signals import PROBLEM_RAW_SCORE_CHANGED
 
 UUID_REGEX = re.compile(ur'%(hex)s{8}-%(hex)s{4}-%(hex)s{4}-%(hex)s{4}-%(hex)s{12}' % {'hex': u'[0-9a-f]'})
 
@@ -214,3 +217,37 @@ class ScoreChangedSignalRelayTest(TestCase):
                 u'usage_id:block-v1:block-key, user_id:1. Task [*UUID*]'
             ).format(time=FROZEN_NOW_DATETIME)
         )
+
+    @ddt.data(
+        [score_set, 'lms.djangoapps.grades.signals.handlers.submissions_score_set_handler', SUBMISSION_SET_KWARGS],
+        [score_reset, 'lms.djangoapps.grades.signals.handlers.submissions_score_reset_handler', SUBMISSION_RESET_KWARGS]
+    )
+    @ddt.unpack
+    def test_disconnect_manager(self, signal, handler, kwargs):
+        """
+        Tests to confirm the disconnect_submissions_signal_receiver context manager is working correctly.
+        """
+        handler_mock = self.setup_patch(handler, None)
+
+        # Receiver connected before we start
+        signal.send(None, **kwargs)
+        handler_mock.assert_called_once()
+        handler_mock.reset_mock()
+
+        # Disconnect is functioning
+        with disconnect_submissions_signal_receiver(signal):
+            signal.send(None, **kwargs)
+            handler_mock.assert_not_called()
+            handler_mock.reset_mock()
+
+        # And we reconnect properly afterwards
+        signal.send(None, **kwargs)
+        handler_mock.assert_called_once()
+
+    def test_disconnect_manager_bad_arg(self):
+        """
+        Tests that the disconnect context manager errors when given an invalid signal.
+        """
+        with self.assertRaises(ValueError):
+            with disconnect_submissions_signal_receiver(PROBLEM_RAW_SCORE_CHANGED):
+                pass

lms/djangoapps/instructor/enrollment.py

@@ -16,12 +16,14 @@ from django.utils.translation import override as override_language
 from eventtracking import tracker
 import pytz
 
+from lms.djangoapps.grades.signals.handlers import disconnect_submissions_signal_receiver
 from lms.djangoapps.grades.signals.signals import PROBLEM_RAW_SCORE_CHANGED
 from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum
 from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.user_api.models import UserPreference
 from submissions import api as sub_api  # installed from the edx-submissions repository
+from submissions.models import score_set
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
@@ -245,12 +247,13 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user
             # Inform these blocks of the reset and allow them to handle their data.
             clear_student_state = getattr(block, "clear_student_state", None)
             if callable(clear_student_state):
-                clear_student_state(
-                    user_id=user_id,
-                    course_id=unicode(course_id),
-                    item_id=unicode(module_state_key),
-                    requesting_user_id=requesting_user_id
-                )
+                with disconnect_submissions_signal_receiver(score_set):
+                    clear_student_state(
+                        user_id=user_id,
+                        course_id=unicode(course_id),
+                        item_id=unicode(module_state_key),
+                        requesting_user_id=requesting_user_id
+                    )
                 submission_cleared = True
     except ItemNotFoundError:
         block = None
@@ -290,12 +293,13 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user
                 'event_transaction_type': unicode(grade_update_root_type),
             }
         )
-        _fire_score_changed_for_block(
-            course_id,
-            student,
-            block,
-            module_state_key,
-        )
+        if not submission_cleared:
+            _fire_score_changed_for_block(
+                course_id,
+                student,
+                block,
+                module_state_key,
+            )
     else:
         _reset_module_attempts(module_to_reset)
 

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -401,7 +401,9 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
     # Disable the score change signal to prevent other components from being
     # pulled into tests.
     @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send')
-    def test_delete_submission_scores(self, _mock_signal):
+    @mock.patch('lms.djangoapps.grades.signals.handlers.submissions_score_set_handler')
+    @mock.patch('lms.djangoapps.grades.signals.handlers.submissions_score_reset_handler')
+    def test_delete_submission_scores(self, _mock_send_signal, mock_set_receiver, mock_reset_receiver):
         user = UserFactory()
         problem_location = self.course_key.make_usage_key('dummy', 'module')
 
@@ -430,6 +432,10 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
             delete_module=True,
         )
 
+        # Make sure our grades signal receivers handled the reset properly
+        mock_set_receiver.assert_not_called()
+        mock_reset_receiver.assert_called_once()
+
         # Verify that the student's scores have been reset in the submissions API
         score = sub_api.get_score(student_item)
         self.assertIs(score, None)

openedx/core/djangoapps/ccxcon/api.py

@@ -16,9 +16,9 @@ from rest_framework.status import (
 )
 
 from lms.djangoapps.courseware.courses import get_course_by_id
-from lms.djangoapps.instructor.access import list_with_level
 from openedx.core.djangoapps.models.course_details import CourseDetails
 from student.models import anonymous_id_for_user
+from student.roles import CourseInstructorRole
 from .models import CCXCon
 
 log = logging.getLogger(__name__)
@@ -122,7 +122,7 @@ def course_info_to_ccxcon(course_key):
     )
 
     # get the entire list of instructors
-    course_instructors = list_with_level(course, 'instructor')
+    course_instructors = CourseInstructorRole(course.id).users_with_role()
     # get anonymous ids for each of them
     course_instructors_ids = [anonymous_id_for_user(user, course_key) for user in course_instructors]
     # extract the course details

requirements/edx/github.txt

@@ -75,8 +75,8 @@ git+https://github.com/edx/lettuce.git@0.2.20.002#egg=lettuce==0.2.20.002
 -e git+https://github.com/edx/event-tracking.git@0.2.1#egg=event-tracking==0.2.1
 -e git+https://github.com/edx/django-splash.git@v0.2#egg=django-splash==0.2
 -e git+https://github.com/edx/acid-block.git@e46f9cda8a03e121a00c7e347084d142d22ebfb7#egg=acid-xblock
-git+https://github.com/edx/edx-ora2.git@1.3.1#egg=ora2==1.3.1
--e git+https://github.com/edx/edx-submissions.git@1.1.6#egg=edx-submissions==1.1.6
+git+https://github.com/edx/edx-ora2.git@1.3.2#egg=ora2==1.3.2
+-e git+https://github.com/edx/edx-submissions.git@1.2.0#egg=edx-submissions==1.2.0
 git+https://github.com/edx/ease.git@release-2015-07-14#egg=ease==0.1.3
 git+https://github.com/edx/edx-val.git@0.0.13#egg=edxval==0.0.13
 git+https://github.com/pmitros/RecommenderXBlock.git@v1.1#egg=recommender-xblock==1.1