Merge pull request #14021 from edx/beryl/null-subsection-error

Handle DAGs with inaccessible Subsections on Problem Score update

lms/djangoapps/grades/tasks.py

@@ -123,17 +123,18 @@ def _update_subsection_grades(
 
     try:
         for subsection_usage_key in subsections_to_update:
-            subsection_grade = subsection_grade_factory.update(
-                course_structure[subsection_usage_key],
-                only_if_higher,
-            )
-            SUBSECTION_SCORE_CHANGED.send(
-                sender=recalculate_subsection_grade,
-                course=course,
-                course_structure=course_structure,
-                user=student,
-                subsection_grade=subsection_grade,
-            )
+            if subsection_usage_key in course_structure:
+                subsection_grade = subsection_grade_factory.update(
+                    course_structure[subsection_usage_key],
+                    only_if_higher,
+                )
+                SUBSECTION_SCORE_CHANGED.send(
+                    sender=recalculate_subsection_grade,
+                    course=course,
+                    course_structure=course_structure,
+                    user=student,
+                    subsection_grade=subsection_grade,
+                )
 
     except DatabaseError as exc:
         raise _retry_recalculate_subsection_grade(

lms/djangoapps/grades/tests/test_new.py

@@ -436,12 +436,6 @@ class TestVariedMetadata(ProblemSubmissionTestMixin, ModuleStoreTestCase):
         self.request = get_mock_request(UserFactory())
         self.client.login(username=self.request.user.username, password="test")
         CourseEnrollment.enroll(self.request.user, self.course.id)
-        course_structure = get_course_blocks(self.request.user, self.course.location)
-        self.subsection_factory = SubsectionGradeFactory(
-            self.request.user,
-            course_structure=course_structure,
-            course=self.course,
-        )
 
     def _get_altered_metadata(self, alterations):
         """
@@ -473,7 +467,13 @@ class TestVariedMetadata(ProblemSubmissionTestMixin, ModuleStoreTestCase):
         """
 
         self.submit_question_answer(u'problem', {u'2_1': u'Correct'})
-        return self.subsection_factory.create(self.sequence)
+        course_structure = get_course_blocks(self.request.user, self.course.location)
+        subsection_factory = SubsectionGradeFactory(
+            self.request.user,
+            course_structure=course_structure,
+            course=self.course,
+        )
+        return subsection_factory.create(self.sequence)
 
     @ddt.data(
         ({}, 1.25, 2.5),

lms/djangoapps/grades/tests/test_tasks.py

@@ -46,8 +46,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             PersistentGradesEnabledFlag.objects.create(enabled=False)
 
         self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter")
-        self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Open Sequential")
-        self.problem = ItemFactory.create(parent=self.sequential, category='problem', display_name='problem')
+        self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Sequential1")
+        self.problem = ItemFactory.create(parent=self.sequential, category='problem', display_name='Problem')
 
         self.problem_score_changed_kwargs = OrderedDict([
             ('points_earned', 1.0),
@@ -95,13 +95,13 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             mock_task_apply.assert_called_once_with(kwargs=self.recalculate_subsection_grade_kwargs)
 
     @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
-    def test_subsection_update_triggers_course_update(self, mock_course_signal):
+    def test_subsection_update_triggers_signal(self, mock_subsection_signal):
         """
-        Ensures that the subsection update operation also updates the course grade.
+        Ensures that the subsection update operation triggers a signal.
         """
         self.set_up_course()
         self._apply_recalculate_subsection_grade()
-        self.assertTrue(mock_course_signal.called)
+        self.assertTrue(mock_subsection_signal.called)
 
     @ddt.data(
         (ModuleStoreEnum.Type.mongo, 1),
@@ -115,6 +115,30 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
             with check_mongo_calls(2) and self.assertNumQueries(20 + added_queries):
                 self._apply_recalculate_subsection_grade()
 
+    @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
+    def test_other_inaccessible_subsection(self, mock_subsection_signal):
+        self.set_up_course()
+        accessible_seq = ItemFactory.create(parent=self.chapter, category='sequential')
+        inaccessible_seq = ItemFactory.create(parent=self.chapter, category='sequential', visible_to_staff_only=True)
+
+        # Update problem to have 2 additional sequential parents.
+        # So in total, 3 sequential parents, with one inaccessible.
+        for sequential in (accessible_seq, inaccessible_seq):
+            sequential.children = [self.problem.location]
+            modulestore().update_item(sequential, self.user.id)  # pylint: disable=no-member
+
+        # Make sure the signal is sent for only the 2 accessible sequentials.
+        self._apply_recalculate_subsection_grade()
+        self.assertEquals(mock_subsection_signal.call_count, 2)
+        sequentials_signalled = {
+            args[1]['subsection_grade'].location
+            for args in mock_subsection_signal.call_args_list
+        }
+        self.assertSetEqual(
+            sequentials_signalled,
+            {self.sequential.location, accessible_seq.location},
+        )
+
     def test_single_call_to_create_block_structure(self):
         self.set_up_course()
         self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id))

openedx/core/lib/block_structure/block_structure.py

@@ -445,7 +445,7 @@ class BlockStructureBlockData(BlockStructure):
         """
         Returns the BlockData associated with the given key.
         """
-        return self._block_data_map.get(usage_key)
+        return self._block_data_map[usage_key]
 
     def get_xblock_field(self, usage_key, field_name, default=None):
         """

openedx/core/lib/block_structure/tests/test_block_structure.py

@@ -131,6 +131,7 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin):
         block_structure = BlockStructureModulestoreData(root_block_usage_key=0)
         for block in blocks:
             block_structure._add_xblock(block.location, block)
+            block_structure._get_or_create_block(block.location)
 
         # request fields
         fields = ["field1", "field2", "field3"]