EDUCATOR-333: Grade each course subsection exactly once; ensure that our...

EDUCATOR-333: Grade each course subsection exactly once; ensure that our stashed, unsaved subsection grades are unique.

lms/djangoapps/grades/new/course_grade.py

@@ -13,6 +13,10 @@ from .subsection_grade import ZeroSubsectionGrade
 from .subsection_grade_factory import SubsectionGradeFactory
 
 
+def uniqueify(iterable):
+    return OrderedDict([(item, None) for item in iterable]).keys()
+
+
 class CourseGradeBase(object):
     """
     Base class for Course Grades.
@@ -168,7 +172,7 @@ class CourseGradeBase(object):
         """
         return [
             self._get_subsection_grade(course_structure[subsection_key])
-            for subsection_key in course_structure.get_children(chapter_key)
+            for subsection_key in uniqueify(course_structure.get_children(chapter_key))
         ]
 
     @abstractmethod

lms/djangoapps/grades/new/subsection_grade_factory.py

+from collections import OrderedDict
 from logging import getLogger
 
 from lazy import lazy
@@ -25,7 +26,7 @@ class SubsectionGradeFactory(object):
         self.course_data = course_data or CourseData(student, course=course, structure=course_structure)
 
         self._cached_subsection_grades = None
-        self._unsaved_subsection_grades = []
+        self._unsaved_subsection_grades = OrderedDict()
 
     def create(self, subsection, read_only=False):
         """
@@ -47,7 +48,7 @@ class SubsectionGradeFactory(object):
                 )
                 if should_persist_grades(self.course_data.course_key):
                     if read_only:
-                        self._unsaved_subsection_grades.append(subsection_grade)
+                        self._unsaved_subsection_grades[subsection_grade.location] = subsection_grade
                     else:
                         grade_model = subsection_grade.create_model(self.student)
                         self._update_saved_subsection_grade(subsection.location, grade_model)
@@ -57,8 +58,10 @@ class SubsectionGradeFactory(object):
         """
         Bulk creates all the unsaved subsection_grades to this point.
         """
-        SubsectionGrade.bulk_create_models(self.student, self._unsaved_subsection_grades, self.course_data.course_key)
-        self._unsaved_subsection_grades = []
+        SubsectionGrade.bulk_create_models(
+            self.student, self._unsaved_subsection_grades.values(), self.course_data.course_key
+        )
+        self._unsaved_subsection_grades.clear()
 
     def update(self, subsection, only_if_higher=None):
         """

lms/djangoapps/grades/tests/test_new.py

@@ -84,6 +84,15 @@ class GradeTestBase(SharedModuleStoreTestCase):
                 display_name="Test Problem",
                 data=problem_xml
             )
+            # AED 2017-06-19: make cls.sequence belong to multiple parents,
+            # so we can test that DAGs with this shape are handled correctly.
+            cls.chapter_2 = ItemFactory.create(
+                parent=cls.course,
+                category='chapter',
+                display_name='Test Chapter 2'
+            )
+            cls.chapter_2.children.append(cls.sequence.location)
+            cls.store.update_item(cls.chapter_2, UserFactory().id)
 
     def setUp(self):
         super(GradeTestBase, self).setUp()