Commit adad6b2e by Nimisha Asthagiri Committed by GitHub

Merge pull request #14293 from edx/beryl/pre-collect-grade-report

Grade Report: Reuse collected block structure
parents 4a5426a6 04d4ab0e
...@@ -12,6 +12,7 @@ from lazy import lazy ...@@ -12,6 +12,7 @@ from lazy import lazy
from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.course_blocks.api import get_course_blocks
from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager
from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED
from xmodule import block_metadata_utils from xmodule import block_metadata_utils
...@@ -323,17 +324,23 @@ class CourseGradeFactory(object): ...@@ -323,17 +324,23 @@ class CourseGradeFactory(object):
""" """
Factory class to create Course Grade objects Factory class to create Course Grade objects
""" """
def create(self, student, course, read_only=True): def create(self, student, course, collected_block_structure=None, read_only=True):
""" """
Returns the CourseGrade object for the given student and course. Returns the CourseGrade object for the given student and course.
If read_only is True, doesn't save any updates to the grades. If read_only is True, doesn't save any updates to the grades.
Raises a PermissionDenied if the user does not have course access. Raises a PermissionDenied if the user does not have course access.
""" """
course_structure = get_course_blocks(student, course.location) course_structure = get_course_blocks(
student,
course.location,
collected_block_structure=collected_block_structure,
)
# if user does not have access to this course, throw an exception # if user does not have access to this course, throw an exception
if not self._user_has_access_to_course(course_structure): if not self._user_has_access_to_course(course_structure):
raise PermissionDenied("User does not have access to this course") raise PermissionDenied("User does not have access to this course")
return ( return (
self._get_saved_grade(student, course, course_structure) or self._get_saved_grade(student, course, course_structure) or
self._compute_and_update_grade(student, course, course_structure, read_only) self._compute_and_update_grade(student, course, course_structure, read_only)
...@@ -351,11 +358,17 @@ class CourseGradeFactory(object): ...@@ -351,11 +358,17 @@ class CourseGradeFactory(object):
If an error occurred, course_grade will be None and err_msg will be an If an error occurred, course_grade will be None and err_msg will be an
exception message. If there was no error, err_msg is an empty string. exception message. If there was no error, err_msg is an empty string.
""" """
# Pre-fetch the collected course_structure so:
# 1. Correctness: the same version of the course is used to
# compute the grade for all students.
# 2. Optimization: the collected course_structure is not
# retrieved from the data store multiple times.
collected_block_structure = get_block_structure_manager(course.id).get_collected()
for student in students: for student in students:
with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=[u'action:{}'.format(course.id)]): with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=[u'action:{}'.format(course.id)]):
try: try:
course_grade = CourseGradeFactory().create(student, course) course_grade = CourseGradeFactory().create(student, course, collected_block_structure)
yield self.GradeResult(student, course_grade, "") yield self.GradeResult(student, course_grade, "")
except Exception as exc: # pylint: disable=broad-except except Exception as exc: # pylint: disable=broad-except
......
...@@ -12,6 +12,7 @@ from courseware.model_data import set_score ...@@ -12,6 +12,7 @@ from courseware.model_data import set_score
from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.helpers import LoginEnrollmentTestCase
from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.course_blocks.api import get_course_blocks
from openedx.core.lib.block_structure.factory import BlockStructureFactory
from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.djangolib.testing.utils import get_mock_request
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from student.models import CourseEnrollment from student.models import CourseEnrollment
...@@ -68,7 +69,14 @@ class TestGradeIteration(SharedModuleStoreTestCase): ...@@ -68,7 +69,14 @@ class TestGradeIteration(SharedModuleStoreTestCase):
""" """
No students have grade entries. No students have grade entries.
""" """
all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) with patch.object(
BlockStructureFactory,
'create_from_cache',
wraps=BlockStructureFactory.create_from_cache
) as mock_create_from_cache:
all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students)
self.assertEquals(mock_create_from_cache.call_count, 1)
self.assertEqual(len(all_errors), 0) self.assertEqual(len(all_errors), 0)
for course_grade in all_course_grades.values(): for course_grade in all_course_grades.values():
self.assertIsNone(course_grade.letter_grade) self.assertIsNone(course_grade.letter_grade)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment