Merge pull request openedx#13884 from edx/hotfix/2016-11-01

Hotfix for grades issues in 2016-11-01 release

lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py

@@ -1,7 +1,7 @@
 """
 Tests for ProctoredExamTransformer.
 """
-from mock import patch
+from mock import patch, Mock
 from nose.plugins.attrib import attr
 
 import ddt
@@ -133,16 +133,18 @@ def test_special_exams_not_visible_to_non_staff(self):
         (
             'H',
             'A',
+            'B',
             ('course', 'A', 'B', 'C',)
         ),
         (
             'H',
             'ProctoredExam',
+            'D',
             ('course', 'A', 'B', 'C'),
         ),
     )
     @ddt.unpack
-    def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_completion):
+    def test_gated(self, gated_block_ref, gating_block_ref, gating_block_child, expected_blocks_before_completion):
         """
         First, checks that a student cannot see the gated block when it is gated by the gating block and no
         attempt has been made to complete the gating block.
@@ -162,14 +164,17 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c
         # clear the request cache to simulate a new request
         self.clear_caches()
 
-        # this call triggers reevaluation of prerequisites fulfilled by the gating block
-        lms_gating_api.evaluate_prerequisite(
-            self.course,
-            self.user,
-            self.blocks[gating_block_ref].location,
-            100.0,
-        )
-        with self.assertNumQueries(3):
+        # mock the api that the lms gating api calls to get the score for each block to always return 1 (ie 100%)
+        with patch('gating.api.get_module_score', Mock(return_value=1)):
+
+            # this call triggers reevaluation of prerequisites fulfilled by the parent of the
+            # block passed in, so we pass in a child of the gating block
+            lms_gating_api.evaluate_prerequisite(
+                self.course,
+                self.blocks[gating_block_child],
+                self.user.id,
+            )
+        with self.assertNumQueries(2):
             self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL)
 
     def test_staff_access(self):

lms/djangoapps/gating/api.py

@@ -10,14 +10,34 @@
 from openedx.core.lib.gating import api as gating_api
 from opaque_keys.edx.keys import UsageKey
 from lms.djangoapps.courseware.entrance_exams import get_entrance_exam_score
+from lms.djangoapps.grades.module_grades import get_module_score
 from util import milestones_helpers
 
 
 log = logging.getLogger(__name__)
 
 
+def _get_xblock_parent(xblock, category=None):
+    """
+    Returns the parent of the given XBlock. If an optional category is supplied,
+    traverses the ancestors of the XBlock and returns the first with the
+    given category.
+
+    Arguments:
+        xblock (XBlock): Get the parent of this XBlock
+        category (str): Find an ancestor with this category (e.g. sequential)
+    """
+    parent = xblock.get_parent()
+    if parent and category:
+        if parent.category == category:
+            return parent
+        else:
+            return _get_xblock_parent(parent, category)
+    return parent
+
+
 @gating_api.gating_enabled(default=False)
-def evaluate_prerequisite(course, user, subsection_usage_key, new_score):
+def evaluate_prerequisite(course, block, user_id):
     """
     Finds the parent subsection of the content in the course and evaluates
     any milestone relationships attached to that subsection. If the calculated
@@ -32,33 +52,39 @@ def evaluate_prerequisite(course, user, subsection_usage_key, new_score):
     Returns:
         None
     """
-    prereq_milestone = gating_api.get_gating_milestone(
-        course.id,
-        subsection_usage_key,
-        'fulfills'
-    )
-    if prereq_milestone:
-        gated_content_milestones = defaultdict(list)
-        for milestone in gating_api.find_gating_milestones(course.id, None, 'requires'):
-            gated_content_milestones[milestone['id']].append(milestone)
+    sequential = _get_xblock_parent(block, 'sequential')
+    if sequential:
+        prereq_milestone = gating_api.get_gating_milestone(
+            course.id,
+            sequential.location.for_branch(None),
+            'fulfills'
+        )
+        if prereq_milestone:
+            gated_content_milestones = defaultdict(list)
+            for milestone in gating_api.find_gating_milestones(course.id, None, 'requires'):
+                gated_content_milestones[milestone['id']].append(milestone)
 
-        gated_content = gated_content_milestones.get(prereq_milestone['id'])
-        if gated_content:
-            for milestone in gated_content:
-                # Default minimum score to 100
-                min_score = 100
-                requirements = milestone.get('requirements')
-                if requirements:
-                    try:
-                        min_score = int(requirements.get('min_score'))
-                    except (ValueError, TypeError):
-                        # Use default value of 100
-                        pass
+            gated_content = gated_content_milestones.get(prereq_milestone['id'])
+            if gated_content:
+                user = User.objects.get(id=user_id)
+                score = get_module_score(user, course, sequential) * 100
+                for milestone in gated_content:
+                    # Default minimum score to 100
+                    min_score = 100
+                    requirements = milestone.get('requirements')
+                    if requirements:
+                        try:
+                            min_score = int(requirements.get('min_score'))
+                        except (ValueError, TypeError):
+                            log.warning(
+                                'Failed to find minimum score for gating milestone %s, defaulting to 100',
+                                json.dumps(milestone)
+                            )
 
-                if new_score >= min_score:
-                    milestones_helpers.add_user_milestone({'id': user.id}, prereq_milestone)
-                else:
-                    milestones_helpers.remove_user_milestone({'id': user.id}, prereq_milestone)
+                    if score >= min_score:
+                        milestones_helpers.add_user_milestone({'id': user_id}, prereq_milestone)
+                    else:
+                        milestones_helpers.remove_user_milestone({'id': user_id}, prereq_milestone)
 
 
 def evaluate_entrance_exam(course, block, user_id):

lms/djangoapps/gating/signals.py

@@ -4,7 +4,7 @@
 from django.dispatch import receiver
 
 from gating import api as gating_api
-from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED, SUBSECTION_SCORE_CHANGED
+from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from xmodule.modulestore.django import modulestore
 
@@ -24,26 +24,5 @@ def handle_score_changed(**kwargs):
     """
     course = modulestore().get_course(CourseKey.from_string(kwargs.get('course_id')))
     block = modulestore().get_item(UsageKey.from_string(kwargs.get('usage_id')))
+    gating_api.evaluate_prerequisite(course, block, kwargs.get('user_id'))
     gating_api.evaluate_entrance_exam(course, block, kwargs.get('user_id'))
-
-
-@receiver(SUBSECTION_SCORE_CHANGED)
-def handle_subsection_score_changed(**kwargs):
-    """
-    Receives the SUBSECTION_SCORE_CHANGED signal and triggers the evaluation of
-    any milestone relationships which are attached to the updated content.
-    Arguments:
-        kwargs (dict): Contains user ID, course key, and content usage key
-    Returns:
-        None
-    """
-    course = kwargs['course']
-    if course.enable_subsection_gating:
-        subsection_grade = kwargs['subsection_grade']
-        new_score = subsection_grade.graded_total.earned / subsection_grade.graded_total.possible * 100.0
-        gating_api.evaluate_prerequisite(
-            course,
-            kwargs['user'],
-            subsection_grade.location,
-            new_score,
-        )