diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index a8333dd52ed2..2c46248456f2 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -811,7 +811,9 @@ def get_assignments_grades(user, course_id, cache_timeout): course_id (CourseLocator): The course key. cache_timeout (int): Cache timeout in seconds Returns: - list (ReadSubsectionGrade, ZeroSubsectionGrade): The list with assignments grades. + tuple: + - list[Union[ReadSubsectionGrade, ZeroSubsectionGrade]]: List of subsection grades. + - list[dict]: List of dictionaries with section-level grade breakdown and assignment info. """ is_staff = bool(has_access(user, 'staff', course_id)) @@ -842,7 +844,7 @@ def get_assignments_grades(user, course_id, cache_timeout): log.warning(f'Could not get grades for the course: {course_id}, error: {err}') return [] - return subsection_grades + return subsection_grades, course_grade.grader_result()['section_breakdown'] def get_first_component_of_block(block_key, block_data): diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py index 656e7e6b4396..b5a849c1ea05 100644 --- a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py +++ b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py @@ -279,7 +279,8 @@ def setUpClass(cls): { 'category': 'Homework', 'detail': f'Homework {i} Unreleased - 0% (?/?)', - 'label': f'HW {i:02d}', 'percent': .0 + 'label': f'HW {i:02d}', 'percent': .0, + 'sequential_id': None, } for i in range(1, 11) ] @@ -289,14 +290,16 @@ def setUpClass(cls): 'detail': 'Homework 11 Unreleased - 0% (?/?)', 'label': 'HW 11', 'mark': {'detail': 'The lowest 2 Homework scores are dropped.'}, - 'percent': 0.0 + 'percent': 0.0, + 'sequential_id': None, }, { 'category': 'Homework', 'detail': 'Homework 12 Unreleased - 0% (?/?)', 'label': 'HW 12', 'mark': {'detail': 'The lowest 2 Homework scores are dropped.'}, - 'percent': 0.0 + 'percent': 0.0, + 'sequential_id': None, } ] + [ @@ -311,7 +314,8 @@ def setUpClass(cls): { 'category': 'Lab', 'detail': f'Lab {i} Unreleased - 0% (?/?)', - 'label': f'Lab {i:02d}', 'percent': .0 + 'label': f'Lab {i:02d}', 'percent': .0, + 'sequential_id': None, } for i in range(1, 11) ] @@ -321,14 +325,16 @@ def setUpClass(cls): 'detail': 'Lab 11 Unreleased - 0% (?/?)', 'label': 'Lab 11', 'mark': {'detail': 'The lowest 2 Lab scores are dropped.'}, - 'percent': 0.0 + 'percent': 0.0, + 'sequential_id': None, }, { 'category': 'Lab', 'detail': 'Lab 12 Unreleased - 0% (?/?)', 'label': 'Lab 12', 'mark': {'detail': 'The lowest 2 Lab scores are dropped.'}, - 'percent': 0.0 + 'percent': 0.0, + 'sequential_id': None, }, { 'category': 'Lab', @@ -342,14 +348,16 @@ def setUpClass(cls): 'detail': 'Midterm Exam = 0.00%', 'label': 'Midterm', 'percent': 0.0, - 'prominent': True + 'prominent': True, + 'sequential_id': None, }, { 'category': 'Final Exam', 'detail': 'Final Exam = 0.00%', 'label': 'Final', 'percent': 0.0, - 'prominent': True + 'prominent': True, + 'sequential_id': None, } ] ) diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 606c935e408f..d7d3a20c1ff0 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -162,6 +162,7 @@ def test_course_grade_summary(self): with mock_get_score(1, 2): self.subsection_grade_factory.update(self.course_structure[self.sequence.location]) course_grade = CourseGradeFactory().update(self.request.user, self.course) + subsection_grades = list(course_grade.subsection_grades.values()) actual_summary = course_grade.summary @@ -187,13 +188,15 @@ def test_course_grade_summary(self): 'category': 'Homework', 'detail': 'Homework 1 - Test Sequential X with an & Ampersand - 50.00% (1/2)', 'label': 'HW 01', - 'percent': 0.5 + 'percent': 0.5, + 'sequential_id': str(subsection_grades[0].location), }, { 'category': 'Homework', 'detail': 'Homework 2 - Test Sequential A - 0.00% (0/1)', 'label': 'HW 02', - 'percent': 0.0 + 'percent': 0.0, + 'sequential_id': str(subsection_grades[1].location), }, { 'category': 'Homework', diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index caa6a991dc46..fc818c646825 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -3,7 +3,7 @@ """ import logging -from typing import Dict, Optional, Union +from typing import Dict, List, Optional, Union import django from django.contrib.auth import get_user_model @@ -383,8 +383,8 @@ def list(self, request, **kwargs): # pylint: disable=W0221 response.data.update(course_data) return response - @staticmethod def _extend_sequential_info_with_assignment_progress( + self, requested_user: User, course_id: CourseKey, blocks_info_data: Dict[str, Dict], @@ -392,8 +392,11 @@ def _extend_sequential_info_with_assignment_progress( """ Extends sequential xblock info with assignment's name and progress. """ - subsection_grades = get_assignments_grades(requested_user, course_id, BLOCK_STRUCTURE_CACHE_TIMEOUT) + subsection_grades, section_breakdown = ( + get_assignments_grades(requested_user, course_id, BLOCK_STRUCTURE_CACHE_TIMEOUT) + ) grades_with_locations = {str(grade.location): grade for grade in subsection_grades} + id_to_label = self._id_to_label(section_breakdown) for block_id, block_info in blocks_info_data.items(): if block_info['type'] == 'sequential': @@ -403,8 +406,9 @@ def _extend_sequential_info_with_assignment_progress( points_earned = graded_total.earned if graded_total else 0 points_possible = graded_total.possible if graded_total else 0 assignment_type = grade.format + label = id_to_label.get(block_id) else: - points_earned, points_possible, assignment_type = 0, 0, None + points_earned, points_possible, assignment_type, label = 0, 0, None, None block_info.update( { @@ -412,10 +416,22 @@ def _extend_sequential_info_with_assignment_progress( 'assignment_type': assignment_type, 'num_points_earned': points_earned, 'num_points_possible': points_possible, + 'short_label': label, } } ) + @staticmethod + def _id_to_label(section_breakdown: List[Dict]) -> Dict[str, str]: + """ + Get `sequential_id` to assignment `label` mapping. + """ + return { + section['sequential_id']: section['label'] + for section in section_breakdown + if 'sequential_id' in section and section['sequential_id'] is not None + } + @mobile_view() class CourseEnrollmentDetailsView(APIView): diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 56c020ec8fa3..efb3f7d9fdbb 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -409,12 +409,14 @@ def test_extend_sequential_info_with_assignment_progress_get_only_sequential(sel { 'assignment_type': 'Lecture Sequence', 'num_points_earned': 0.0, - 'num_points_possible': 0.0 + 'num_points_possible': 0.0, + 'short_label': None, }, { 'assignment_type': None, 'num_points_earned': 0.0, - 'num_points_possible': 0.0 + 'num_points_possible': 0.0, + 'short_label': None, }, ) diff --git a/xmodule/graders.py b/xmodule/graders.py index 5261b9f4479a..001113882438 100644 --- a/xmodule/graders.py +++ b/xmodule/graders.py @@ -380,11 +380,12 @@ def grade(self, grade_sheet, generate_random_scores=False): earned = random.randint(2, 15) possible = random.randint(earned, 15) section_name = _("Generated") - + sequential_id = None else: earned = scores[i].graded_total.earned possible = scores[i].graded_total.possible section_name = scores[i].display_name + sequential_id = str(scores[i].location) percentage = scores[i].percent_graded summary_format = "{section_type} {index} - {name} - {percent:.2%} ({earned:.3n}/{possible:.3n})" @@ -403,10 +404,11 @@ def grade(self, grade_sheet, generate_random_scores=False): index=i + self.starting_index, section_type=self.section_type ) + sequential_id = None short_label = labeler(i + self.starting_index) breakdown.append({'percent': percentage, 'label': short_label, - 'detail': summary, 'category': self.category}) + 'detail': summary, 'category': self.category, 'sequential_id': sequential_id}) total_percent, dropped_indices = self.total_with_drops(breakdown) @@ -427,7 +429,8 @@ def grade(self, grade_sheet, generate_random_scores=False): ) total_label = f"{self.short_label}" breakdown = [{'percent': total_percent, 'label': total_label, - 'detail': total_detail, 'category': self.category, 'prominent': True}, ] + 'detail': total_detail, 'category': self.category, 'prominent': True, + 'sequential_id': str(scores[0].location) if scores else None}, ] else: # Translators: "Homework Average = 0%" total_detail = _("{section_type} Average = {percent:.2%}").format( diff --git a/xmodule/tests/test_graders.py b/xmodule/tests/test_graders.py index 5073052b1ba8..5e2444a05533 100644 --- a/xmodule/tests/test_graders.py +++ b/xmodule/tests/test_graders.py @@ -70,9 +70,10 @@ class MockGrade: """ Mock class for SubsectionGrade object. """ - def __init__(self, graded_total, display_name): + def __init__(self, graded_total, location, display_name): self.graded_total = graded_total self.display_name = display_name + self.location = location @property def percent_graded(self): @@ -81,27 +82,64 @@ def percent_graded(self): common_fields = dict(graded=True, first_attempted=datetime.now()) test_gradesheet = { 'Homework': { - 'hw1': MockGrade(AggregatedScore(tw_earned=2, tw_possible=20.0, **common_fields), display_name='hw1'), - 'hw2': MockGrade(AggregatedScore(tw_earned=16, tw_possible=16.0, **common_fields), display_name='hw2'), + 'hw1': MockGrade( + AggregatedScore(tw_earned=2, tw_possible=20.0, **common_fields), + location='location_hw1_mock', + display_name='hw1' + ), + 'hw2': MockGrade( + AggregatedScore(tw_earned=16, tw_possible=16.0, **common_fields), + location='location_hw2_mock', + display_name='hw2' + ), }, # The dropped scores should be from the assignments that don't exist yet 'Lab': { # Dropped - 'lab1': MockGrade(AggregatedScore(tw_earned=1, tw_possible=2.0, **common_fields), display_name='lab1'), - 'lab2': MockGrade(AggregatedScore(tw_earned=1, tw_possible=1.0, **common_fields), display_name='lab2'), - 'lab3': MockGrade(AggregatedScore(tw_earned=1, tw_possible=1.0, **common_fields), display_name='lab3'), + 'lab1': MockGrade( + AggregatedScore(tw_earned=1, tw_possible=2.0, **common_fields), + location='location_lab1_mock', + display_name='lab1' + ), + 'lab2': MockGrade( + AggregatedScore(tw_earned=1, tw_possible=1.0, **common_fields), + location='location_lab2_mock', + display_name='lab2' + ), + 'lab3': MockGrade( + AggregatedScore(tw_earned=1, tw_possible=1.0, **common_fields), + location='location_lab3_mock', + display_name='lab3' + ), # Dropped - 'lab4': MockGrade(AggregatedScore(tw_earned=5, tw_possible=25.0, **common_fields), display_name='lab4'), + 'lab4': MockGrade( + AggregatedScore(tw_earned=5, tw_possible=25.0, **common_fields), + location='location_lab4_mock', + display_name='lab4' + ), # Dropped - 'lab5': MockGrade(AggregatedScore(tw_earned=3, tw_possible=4.0, **common_fields), display_name='lab5'), - 'lab6': MockGrade(AggregatedScore(tw_earned=6, tw_possible=7.0, **common_fields), display_name='lab6'), - 'lab7': MockGrade(AggregatedScore(tw_earned=5, tw_possible=6.0, **common_fields), display_name='lab7'), + 'lab5': MockGrade( + AggregatedScore(tw_earned=3, tw_possible=4.0, **common_fields), + location='location_lab5_mock', + display_name='lab5' + ), + 'lab6': MockGrade( + AggregatedScore(tw_earned=6, tw_possible=7.0, **common_fields), + location='location_lab6_mock', + display_name='lab6' + ), + 'lab7': MockGrade( + AggregatedScore(tw_earned=5, tw_possible=6.0, **common_fields), + location='location_lab7_mock', + display_name='lab7' + ), }, 'Midterm': { 'midterm': MockGrade( AggregatedScore(tw_earned=50.5, tw_possible=100, **common_fields), + location='location_midterm_mock', display_name="Midterm Exam", ), }, @@ -337,6 +375,47 @@ def test_grader_with_invalid_conf(self, invalid_conf, expected_error_message): graders.grader_from_conf([invalid_conf]) assert expected_error_message in str(error.value) + def test_sequential_location_in_section_breakdown(self): + homework_grader = graders.AssignmentFormatGrader("Homework", 12, 2) + lab_grader = graders.AssignmentFormatGrader("Lab", 7, 3) + midterm_grader = graders.AssignmentFormatGrader("Midterm", 1, 0) + + weighted_grader = graders.WeightedSubsectionsGrader([ + (homework_grader, homework_grader.category, 0.25), + (lab_grader, lab_grader.category, 0.25), + (midterm_grader, midterm_grader.category, 0.5), + ]) + + expected_sequential_ids = [ + 'location_hw1_mock', + 'location_hw2_mock', + None, + None, + None, + None, + None, + None, + None, + None, + None, + None, + None, + 'location_lab1_mock', + 'location_lab2_mock', + 'location_lab3_mock', + 'location_lab4_mock', + 'location_lab5_mock', + 'location_lab6_mock', + 'location_lab7_mock', + None, + 'location_midterm_mock', + ] + + graded = weighted_grader.grade(self.test_gradesheet) + + for i, section_breakdown in enumerate(graded['section_breakdown']): + assert expected_sequential_ids[i] == section_breakdown.get('sequential_id') + @ddt.ddt class ShowCorrectnessTest(unittest.TestCase):