From eb73057d90a8e270cd9b85b7f3129ba6e2d0bb8a Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Mon, 19 Jun 2017 13:36:59 -0400 Subject: [PATCH] EDUCATOR-333: Grade each course subsection exactly once; ensure that our stashed, unsaved subsection grades are unique. --- lms/djangoapps/grades/new/course_grade.py | 6 +++++- lms/djangoapps/grades/new/subsection_grade_factory.py | 11 +++++++---- lms/djangoapps/grades/tests/test_new.py | 9 +++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 1783db8ef7..745c6434b4 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/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 diff --git a/lms/djangoapps/grades/new/subsection_grade_factory.py b/lms/djangoapps/grades/new/subsection_grade_factory.py index 6a97b31705..ba464b8c50 100644 --- a/lms/djangoapps/grades/new/subsection_grade_factory.py +++ b/lms/djangoapps/grades/new/subsection_grade_factory.py @@ -1,3 +1,4 @@ +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): """ diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index 172658806f..d1dbbac161 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/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()