From cb3b7e1822ade2c56338bacdb3e05ed13f41b72a Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Tue, 4 Dec 2018 15:00:52 -0500 Subject: [PATCH] Make gradebook GET API more performant. --- lms/djangoapps/grades/api/serializers.py | 19 -- .../grades/api/v1/tests/test_views.py | 256 +++--------------- lms/djangoapps/grades/api/v1/views.py | 173 +++++------- lms/djangoapps/grades/course_grade_factory.py | 2 +- lms/djangoapps/grades/subsection_grade.py | 24 +- .../tests/views/test_course_updates.py | 2 +- 6 files changed, 140 insertions(+), 336 deletions(-) diff --git a/lms/djangoapps/grades/api/serializers.py b/lms/djangoapps/grades/api/serializers.py index f282b4e289..00d9e1e57b 100644 --- a/lms/djangoapps/grades/api/serializers.py +++ b/lms/djangoapps/grades/api/serializers.py @@ -34,37 +34,19 @@ class SectionBreakdownSerializer(serializers.Serializer): """ Serializer for the `section_breakdown` portion of a gradebook entry. """ - are_grades_published = serializers.BooleanField() - auto_grade = serializers.BooleanField() category = serializers.CharField() - chapter_name = serializers.CharField() - comment = serializers.CharField() - detail = serializers.CharField() displayed_value = serializers.CharField() is_graded = serializers.BooleanField() grade_description = serializers.CharField() - is_ag = serializers.BooleanField() - is_average = serializers.BooleanField() - is_manually_graded = serializers.BooleanField() label = serializers.CharField() letter_grade = serializers.CharField() module_id = serializers.CharField() percent = serializers.FloatField() score_earned = serializers.FloatField() score_possible = serializers.FloatField() - section_block_id = serializers.CharField() subsection_name = serializers.CharField() -class SimpleSerializer(serializers.BaseSerializer): - """ - A Serializer intended to take a dictionary of data and simply spit - that same dictionary back out as the "serialization". - """ - def to_representation(self, instance): - return instance - - class StudentGradebookEntrySerializer(serializers.Serializer): """ Serializer for student gradebook entry. @@ -79,4 +61,3 @@ class StudentGradebookEntrySerializer(serializers.Serializer): letter_grade = serializers.CharField() progress_page_url = serializers.CharField() section_breakdown = SectionBreakdownSerializer(many=True) - aggregates = SimpleSerializer() diff --git a/lms/djangoapps/grades/api/v1/tests/test_views.py b/lms/djangoapps/grades/api/v1/tests/test_views.py index 4b3a4d3029..d03e386986 100644 --- a/lms/djangoapps/grades/api/v1/tests/test_views.py +++ b/lms/djangoapps/grades/api/v1/tests/test_views.py @@ -437,6 +437,10 @@ class GradebookViewTestBase(GradeViewTestMixin, APITestCase): ), ], } + cls.course_data = CourseData(None, course=cls.course) + # we have to force the collection of course data from the block_structure API + # so that CourseGrade.course_data objects can later have a non-null effective_structure + _ = cls.course_data.collected_structure def get_url(self, course_key=None): """ @@ -462,6 +466,40 @@ class GradebookViewTest(GradebookViewTestBase): """ Tests for the gradebook view. """ + @classmethod + def setUpClass(cls): + super(GradebookViewTest, cls).setUpClass() + cls.mock_subsection_grades = { + cls.subsections[cls.chapter_1.location][0].location: cls.mock_subsection_grade( + cls.subsections[cls.chapter_1.location][0], + earned_all=1.0, + possible_all=2.0, + earned_graded=1.0, + possible_graded=2.0, + ), + cls.subsections[cls.chapter_1.location][1].location: cls.mock_subsection_grade( + cls.subsections[cls.chapter_1.location][1], + earned_all=1.0, + possible_all=2.0, + earned_graded=1.0, + possible_graded=2.0, + ), + cls.subsections[cls.chapter_2.location][0].location: cls.mock_subsection_grade( + cls.subsections[cls.chapter_2.location][0], + earned_all=1.0, + possible_all=2.0, + earned_graded=1.0, + possible_graded=2.0, + ), + cls.subsections[cls.chapter_2.location][1].location: cls.mock_subsection_grade( + cls.subsections[cls.chapter_2.location][1], + earned_all=1.0, + possible_all=2.0, + earned_graded=1.0, + possible_graded=2.0, + ), + } + def get_url(self, course_key=None, username=None, username_contains=None): # pylint: disable=arguments-differ """ Helper function to create the course gradebook API read url. @@ -473,7 +511,8 @@ class GradebookViewTest(GradebookViewTestBase): return "{0}?username_contains={1}".format(base_url, username_contains) return base_url - def mock_subsection_grade(self, subsection, **kwargs): + @staticmethod + def mock_subsection_grade(subsection, **kwargs): """ Helper function to mock a subsection grade. """ @@ -485,48 +524,8 @@ class GradebookViewTest(GradebookViewTestBase): """ Helper function to return a mock CourseGrade object. """ - course_data = CourseData(user, course=self.course) - course_grade = CourseGrade(user=user, course_data=course_data, **kwargs) - course_grade.chapter_grades = OrderedDict([ - (self.chapter_1.location, { - 'sections': [ - self.mock_subsection_grade( - self.subsections[self.chapter_1.location][0], - earned_all=1.0, - possible_all=2.0, - earned_graded=1.0, - possible_graded=2.0, - ), - self.mock_subsection_grade( - self.subsections[self.chapter_1.location][1], - earned_all=1.0, - possible_all=2.0, - earned_graded=1.0, - possible_graded=2.0, - ), - ], - 'display_name': 'Chapter 1', - }), - (self.chapter_2.location, { - 'sections': [ - self.mock_subsection_grade( - self.subsections[self.chapter_2.location][0], - earned_all=1.0, - possible_all=2.0, - earned_graded=1.0, - possible_graded=2.0, - ), - self.mock_subsection_grade( - self.subsections[self.chapter_2.location][1], - earned_all=1.0, - possible_all=2.0, - earned_graded=1.0, - possible_graded=2.0, - ), - ], - 'display_name': 'Chapter 2', - }), - ]) + course_grade = CourseGrade(user=user, course_data=self.course_data, **kwargs) + course_grade.subsection_grade = lambda key: self.mock_subsection_grades[key] return course_grade def expected_subsection_grades(self, letter_grade=None): @@ -535,91 +534,55 @@ class GradebookViewTest(GradebookViewTestBase): """ return [ OrderedDict([ - ('are_grades_published', True), - ('auto_grade', False), ('category', 'Homework'), - ('chapter_name', 'Chapter 1'), - ('comment', ''), - ('detail', ''), ('displayed_value', '0.50'), ('is_graded', True), ('grade_description', '(1.00/2.00)'), - ('is_ag', False), - ('is_average', False), - ('is_manually_graded', False), ('label', 'HW 01'), ('letter_grade', letter_grade), ('module_id', text_type(self.subsections[self.chapter_1.location][0].location)), ('percent', 0.5), ('score_earned', 1.0), ('score_possible', 2.0), - ('section_block_id', text_type(self.chapter_1.location)), ('subsection_name', 'HW 1') ]), OrderedDict([ - ('are_grades_published', True), - ('auto_grade', False), ('category', 'Lab'), - ('chapter_name', 'Chapter 1'), - ('comment', ''), - ('detail', ''), ('displayed_value', '0.50'), ('is_graded', True), ('grade_description', '(1.00/2.00)'), - ('is_ag', False), - ('is_average', False), - ('is_manually_graded', False), ('label', 'Lab 01'), ('letter_grade', letter_grade), ('module_id', text_type(self.subsections[self.chapter_1.location][1].location)), ('percent', 0.5), ('score_earned', 1.0), ('score_possible', 2.0), - ('section_block_id', text_type(self.chapter_1.location)), ('subsection_name', 'Lab 1') ]), OrderedDict([ - ('are_grades_published', True), - ('auto_grade', False), ('category', 'Homework'), - ('chapter_name', 'Chapter 2'), - ('comment', ''), - ('detail', ''), ('displayed_value', '0.50'), ('is_graded', True), ('grade_description', '(1.00/2.00)'), - ('is_ag', False), - ('is_average', False), - ('is_manually_graded', False), ('label', 'HW 02'), ('letter_grade', letter_grade), ('module_id', text_type(self.subsections[self.chapter_2.location][0].location)), ('percent', 0.5), ('score_earned', 1.0), ('score_possible', 2.0), - ('section_block_id', text_type(self.chapter_2.location)), ('subsection_name', 'HW 2') ]), OrderedDict([ - ('are_grades_published', True), - ('auto_grade', False), ('category', 'Lab'), - ('chapter_name', 'Chapter 2'), - ('comment', ''), - ('detail', ''), ('displayed_value', '0.50'), ('is_graded', True), ('grade_description', '(1.00/2.00)'), - ('is_ag', False), - ('is_average', False), - ('is_manually_graded', False), ('label', 'Lab 02'), ('letter_grade', letter_grade), ('module_id', text_type(self.subsections[self.chapter_2.location][1].location)), ('percent', 0.5), ('score_earned', 1.0), ('score_possible', 2.0), - ('section_block_id', text_type(self.chapter_2.location)), ('subsection_name', 'Lab 2') ]), ] @@ -644,16 +607,6 @@ class GradebookViewTest(GradebookViewTestBase): kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) )), ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), ]), OrderedDict([ ('course_id', text_type(self.course.id)), @@ -669,16 +622,6 @@ class GradebookViewTest(GradebookViewTestBase): kwargs=dict(course_id=text_type(self.course.id), student_id=self.other_student.id) )), ('section_breakdown', self.expected_subsection_grades()), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), ]), ] @@ -786,16 +729,6 @@ class GradebookViewTest(GradebookViewTestBase): kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) )), ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), ]) self.assertEqual(status.HTTP_200_OK, resp.status_code) @@ -828,16 +761,6 @@ class GradebookViewTest(GradebookViewTestBase): kwargs=dict(course_id=text_type(self.course.id), student_id=self.other_student.id) )), ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), ]), ] @@ -889,16 +812,6 @@ class GradebookViewTest(GradebookViewTestBase): kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) )), ('section_breakdown', self.expected_subsection_grades(letter_grade='A')), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), ]), ] @@ -957,91 +870,6 @@ class GradebookViewTest(GradebookViewTestBase): ) self._assert_empty_response(resp) - def test_ungraded_subsection(self): - """ - Tests that an ungraded subsection is returned with the response data, and that the default - subsection label is returned (since no short label is generated for ungraded content). - """ - with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade: - course_grade = self.mock_course_grade(self.student, passed=True, letter_grade='A', percent=0.85) - ungraded_subsection = ItemFactory.create( - category='sequential', - parent_location=self.chapter_2.location, - due=datetime(2017, 12, 18, 11, 30, 00), - display_name='HW 3', - format='Homework', - graded=False, - ) - subsection_grade = self.mock_subsection_grade( - ungraded_subsection, - earned_all=0.0, - possible_all=1.0, - earned_graded=0.0, - possible_graded=0.0, - ) - course_grade.chapter_grades[self.chapter_2.location]['sections'].append(subsection_grade) - mock_grade.return_value = course_grade - - with override_waffle_flag(self.waffle_flag, active=True): - self.login_staff() - resp = self.client.get( - self.get_url(course_key=self.course.id, username=self.student.username) - ) - - expected_subsection_breakdown = self.expected_subsection_grades(letter_grade='A') - expected_subsection_breakdown.append(OrderedDict([ - ('are_grades_published', True), - ('auto_grade', False), - ('category', 'Homework'), - ('chapter_name', 'Chapter 2'), - ('comment', ''), - ('detail', ''), - ('displayed_value', '0.00'), - ('is_graded', False), - ('grade_description', '(0.00/0.00)'), - ('is_ag', False), - ('is_average', False), - ('is_manually_graded', False), - ('label', 'HW 03'), - ('letter_grade', 'A'), - ('module_id', text_type(ungraded_subsection.location)), - ('percent', 0.0), - ('score_earned', 0.0), - ('score_possible', 0.0), - ('section_block_id', text_type(self.chapter_2.location)), - ('subsection_name', 'HW 3') - ])) - - expected_results = OrderedDict([ - ('course_id', text_type(self.course.id)), - ('email', self.student.email), - ('user_id', self.student.id), - ('username', self.student.username), - ('full_name', self.student.get_full_name()), - ('passed', True), - ('percent', 0.85), - ('letter_grade', 'A'), - ('progress_page_url', reverse( - 'student_progress', - kwargs=dict(course_id=text_type(self.course.id), student_id=self.student.id) - )), - ('section_breakdown', expected_subsection_breakdown), - ('aggregates', { - 'Lab': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - 'Homework': { - 'score_earned': 2.0, - 'score_possible': 4.0, - }, - }), - ]) - - self.assertEqual(status.HTTP_200_OK, resp.status_code) - actual_data = dict(resp.data) - self.assertEqual(expected_results, actual_data) - @ddt.data(None, 2, 3, 10, 60, 80) def test_page_size_parameter(self, page_size): user_size = 60 diff --git a/lms/djangoapps/grades/api/v1/views.py b/lms/djangoapps/grades/api/v1/views.py index e359d22256..3877d207b3 100644 --- a/lms/djangoapps/grades/api/v1/views.py +++ b/lms/djangoapps/grades/api/v1/views.py @@ -1,6 +1,6 @@ """ API v0 views. """ import logging -from collections import defaultdict, namedtuple +from collections import namedtuple from contextlib import contextmanager from functools import wraps @@ -316,42 +316,6 @@ class GradeViewMixin(DeveloperErrorViewMixin): raise AuthenticationFailed -class SubsectionLabelFinder(object): - """ - Finds the grader label (a short string identifying the section) of a graded section. - """ - def __init__(self, course_grade): - """ - Args: - course_grade: A CourseGrade object. - """ - self.section_summaries = [section for section in course_grade.summary.get('section_breakdown', [])] - - def _get_subsection_summary(self, display_name): - """ - Given a subsection's display_name and a breakdown of section grades from CourseGrade.summary, - return the summary data corresponding to the subsection with this display_name. - """ - for index, section in enumerate(self.section_summaries): - if display_name.lower() in section['detail'].lower(): - return index, section - return -1, None - - def get_label(self, display_name): - """ - Returns the grader short label corresponding to the display_name, or None - if no match was found. - """ - section_index, summary = self._get_subsection_summary(display_name) - if summary: - # It's possible that two subsections/assignments would have the same display name. - # since the grade summary and chapter_grades data are presumably in a sorted order, - # we'll take the first matching section summary and remove it from the pool of - # section_summaries. - self.section_summaries.pop(section_index) - return summary['label'] - - class CourseGradesView(GradeViewMixin, PaginatedAPIView): """ **Use Case** @@ -495,8 +459,6 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): * letter_grade: A letter grade as defined in grading policy (e.g. 'A' 'B' 'C' for 6.002x) or None * progress_page_url: A link to the user's progress page. * section_breakdown: A list of subsection grade details, as specified below. - * aggregates: A dict containing earned and possible scores (floats), broken down by subsection type - (e.g. "Exam", "Homework", "Lab"). A response for all user's grades in the course is paginated, and contains "count", "next" and "previous" keys, along with the actual data contained in a "results" list. @@ -544,16 +506,6 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): "subsection_name": "Demo Course Overview" }, ], - "aggregates": { - "Exam": { - "score_possible": 6.0, - "score_earned": 0.0 - }, - "Homework": { - "score_possible": 16.0, - "score_earned": 10.0 - } - } } **Paginated GET response** When requesting gradebook entries for all users, the response is paginated and contains the following values: @@ -577,62 +529,62 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): required_scopes = ['grades:read'] - def _section_breakdown(self, course, course_grade): + def _section_breakdown(self, course, graded_subsections, course_grade): """ - Given a course_grade, returns a list of grade data broken down by subsection - and a dictionary containing aggregate grade data by subsection format for the course. + Given a course_grade and a list of graded subsections for a given course, + returns a list of grade data broken down by subsection. Args: + course: A Course Descriptor object + graded_subsections: A list of graded subsection objects in the given course. course_grade: A CourseGrade object. """ breakdown = [] - aggregates = defaultdict(lambda: defaultdict(float)) - - # TODO: https://openedx.atlassian.net/browse/EDUCATOR-3559 - # Fields we may not need: - # ['are_grades_published', 'auto_grade', 'comment', 'detail', 'is_ag', 'is_average', 'is_manually_graded'] - # Some fields should be renamed: - # 'displayed_value' should maybe be 'description_percent' - # 'grade_description' should be 'description_ratio' - - label_finder = SubsectionLabelFinder(course_grade) default_labeler = get_default_short_labeler(course) - for chapter_location, section_data in course_grade.chapter_grades.items(): - for subsection_grade in section_data['sections']: - default_short_label = default_labeler(subsection_grade.format) - breakdown.append({ - 'are_grades_published': True, - 'auto_grade': False, - 'category': subsection_grade.format, - 'chapter_name': section_data['display_name'], - 'comment': '', - 'detail': '', - 'displayed_value': '{:.2f}'.format(subsection_grade.percent_graded), - 'is_graded': subsection_grade.graded, - 'grade_description': '({earned:.2f}/{possible:.2f})'.format( - earned=subsection_grade.graded_total.earned, - possible=subsection_grade.graded_total.possible, - ), - 'is_ag': False, - 'is_average': False, - 'is_manually_graded': False, - 'label': label_finder.get_label(subsection_grade.display_name) or default_short_label, - 'letter_grade': course_grade.letter_grade, - 'module_id': text_type(subsection_grade.location), - 'percent': subsection_grade.percent_graded, - 'score_earned': subsection_grade.graded_total.earned, - 'score_possible': subsection_grade.graded_total.possible, - 'section_block_id': text_type(chapter_location), - 'subsection_name': subsection_grade.display_name, - }) - if subsection_grade.graded and subsection_grade.graded_total.possible > 0: - aggregates[subsection_grade.format]['score_earned'] += subsection_grade.graded_total.earned - aggregates[subsection_grade.format]['score_possible'] += subsection_grade.graded_total.possible + for subsection in graded_subsections: + subsection_grade = course_grade.subsection_grade(subsection.location) + short_label = default_labeler(subsection_grade.format) - return breakdown, aggregates + graded_description = 'Not Attempted' + score_earned = 0 + score_possible = 0 - def _gradebook_entry(self, user, course, course_grade): + # For ZeroSubsectionGrades, we don't want to crawl the subsection's + # subtree to find the problem scores specific to this user + # (ZeroSubsectionGrade.attempted_graded is always False). + # We've already fetched the whole course structure in a non-specific way + # when creating `graded_subsections`. Looking at the problem scores + # specific to this user (the user in `course_grade.user`) would require + # us to re-fetch the user-specific course structure from the modulestore, + # which is a costly operation. + if subsection_grade.attempted_graded: + graded_description = '({earned:.2f}/{possible:.2f})'.format( + earned=subsection_grade.graded_total.earned, + possible=subsection_grade.graded_total.possible, + ) + score_earned = subsection_grade.graded_total.earned + score_possible = subsection_grade.graded_total.possible + + # TODO: https://openedx.atlassian.net/browse/EDUCATOR-3559 -- Some fields should be renamed, others removed: + # 'displayed_value' should maybe be 'description_percent' + # 'grade_description' should be 'description_ratio' + breakdown.append({ + 'category': subsection_grade.format, + 'displayed_value': '{:.2f}'.format(subsection_grade.percent_graded), + 'is_graded': subsection_grade.graded, + 'grade_description': graded_description, + 'label': short_label, + 'letter_grade': course_grade.letter_grade, + 'module_id': text_type(subsection_grade.location), + 'percent': subsection_grade.percent_graded, + 'score_earned': score_earned, + 'score_possible': score_possible, + 'subsection_name': subsection_grade.display_name, + }) + return breakdown + + def _gradebook_entry(self, user, course, graded_subsections, course_grade): """ Returns a dictionary of course- and subsection-level grade data for a given user in a given course. @@ -640,13 +592,13 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): Args: user: A User object. course: A Course Descriptor object. + graded_subsections: A list of graded subsections in the given course. course_grade: A CourseGrade object. """ user_entry = self._serialize_user_grade(user, course.id, course_grade) - breakdown, aggregates = self._section_breakdown(course, course_grade) + breakdown = self._section_breakdown(course, graded_subsections, course_grade) user_entry['section_breakdown'] = breakdown - user_entry['aggregates'] = aggregates user_entry['progress_page_url'] = reverse( 'student_progress', kwargs=dict(course_id=text_type(course.id), student_id=user.id) @@ -671,11 +623,17 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): course_key = get_course_key(request, course_id) course = get_course_with_access(request.user, 'staff', course_key, depth=None) + # We fetch the entire course structure up-front, and use this when iterating + # over users to determine their subsection grades. We purposely avoid fetching + # the user-specific course structure for each user, because that is very expensive. + course_data = CourseData(user=None, course=course) + graded_subsections = list(graded_subsections_for_course(course_data.collected_structure)) + if request.GET.get('username'): with self._get_user_or_raise(request, course_key) as grade_user: course_grade = CourseGradeFactory().read(grade_user, course) - entry = self._gradebook_entry(grade_user, course, course_grade) + entry = self._gradebook_entry(grade_user, course, graded_subsections, course_grade) serializer = StudentGradebookEntrySerializer(entry) return Response(serializer.data) else: @@ -697,14 +655,29 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): users = self._paginate_users(course_key, filter_kwargs, related_models) with bulk_gradebook_view_context(course_key, users): - for user, course_grade, exc in CourseGradeFactory().iter(users, course_key=course_key): + for user, course_grade, exc in CourseGradeFactory().iter( + users, course_key=course_key, collected_block_structure=course_data.collected_structure + ): if not exc: - entries.append(self._gradebook_entry(user, course, course_grade)) + entries.append(self._gradebook_entry(user, course, graded_subsections, course_grade)) serializer = StudentGradebookEntrySerializer(entries, many=True) return self.get_paginated_response(serializer.data) +def graded_subsections_for_course(course_structure): + """ + Given a course block structure, yields the subsections of the course that are graded. + Args: + course_structure: A course structure object. Not user-specific. + """ + for chapter_key in course_structure.get_children(course_structure.root_block_usage_key): + for subsection_key in course_structure.get_children(chapter_key): + subsection = course_structure[subsection_key] + if subsection.graded: + yield subsection + + GradebookUpdateResponseItem = namedtuple('GradebookUpdateResponseItem', ['user_id', 'usage_id', 'success', 'reason']) diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index 359d1c1ab7..49c2ed3a75 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -120,7 +120,7 @@ class CourseGradeFactory(object): 'user': user, 'course': course_data.course, 'collected_block_structure': course_data.collected_structure, - 'course_key': course_data.course_key + 'course_key': course_data.course_key, } if force_update: kwargs['force_update_subsections'] = True diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 6b4e60bb18..80e3384d0e 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -5,6 +5,7 @@ from abc import ABCMeta from collections import OrderedDict from logging import getLogger +from django.utils.html import escape from lazy import lazy from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade @@ -23,7 +24,7 @@ class SubsectionGradeBase(object): def __init__(self, subsection): self.location = subsection.location - self.display_name = block_metadata_utils.display_name_with_default_escaped(subsection) + self.display_name = escape(block_metadata_utils.display_name_with_default(subsection)) self.url_name = block_metadata_utils.url_name_for_block(subsection) self.format = getattr(subsection, 'format', '') @@ -89,10 +90,22 @@ class ZeroSubsectionGrade(SubsectionGradeBase): @property def all_total(self): + """ + Returns the total score (earned and possible) amongst all problems (graded and ungraded) in this subsection. + NOTE: This will traverse this subsection's subtree to determine + problem scores. If self.course_data.structure is currently null, this means + we will first fetch the user-specific course structure from the data store! + """ return self._aggregate_scores[0] @property def graded_total(self): + """ + Returns the total score (earned and possible) amongst all graded problems in this subsection. + NOTE: This will traverse this subsection's subtree to determine + problem scores. If self.course_data.structure is currently null, this means + we will first fetch the user-specific course structure from the data store! + """ return self._aggregate_scores[1] @lazy @@ -105,6 +118,9 @@ class ZeroSubsectionGrade(SubsectionGradeBase): Overrides the problem_scores member variable in order to return empty scores for all scorable problems in the course. + NOTE: The use of `course_data.structure` here is very intentional. + It means we look through the user-specific subtree of this subsection, + taking into account which problems are visible to the user. """ locations = OrderedDict() # dict of problem locations to ProblemScore for block_key in self.course_data.structure.post_order_traversal( @@ -194,6 +210,12 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade): @lazy def problem_scores(self): + """ + Returns the scores of the problem blocks that compose this subsection. + NOTE: The use of `course_data.structure` here is very intentional. + It means we look through the user-specific subtree of this subsection, + taking into account which problems are visible to the user. + """ problem_scores = OrderedDict() for block in self.model.visible_blocks.blocks: problem_score = self._compute_block_score( diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 90230ff743..ab504262f8 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -5,7 +5,7 @@ from datetime import datetime from courseware.courses import get_course_info_usage_key from django.urls import reverse -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_experience.views.course_updates import STATUS_VISIBLE from student.models import CourseEnrollment