From 777ba8821891a37b8cec8e48c2d727473bb1d099 Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Thu, 18 Sep 2025 17:46:02 +0300 Subject: [PATCH] feat: [AXM-2398] Add short label to assignment (#36970) Rationale: The instructor may create short labels that are longer than 3 characters, and they can be hard to work with in the mobile UI. Thus, on mobile, it was decided to add short labels to the api response by getting them from section breakdown, which ensures they are consistent with the labels the user sees in the Grading section. --- lms/djangoapps/courseware/courses.py | 6 +- .../grades/rest_api/v1/tests/test_views.py | 24 +++-- .../grades/tests/test_course_grade_factory.py | 7 +- .../mobile_api/course_info/views.py | 24 ++++- .../tests/test_course_info_views.py | 6 +- xmodule/graders.py | 9 +- xmodule/tests/test_graders.py | 99 +++++++++++++++++-- 7 files changed, 144 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index a8333dd52e..2c46248456 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 656e7e6b43..b5a849c1ea 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 @@ class SectionGradesBreakdownTest(GradeViewTestMixin, APITestCase): { '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 @@ class SectionGradesBreakdownTest(GradeViewTestMixin, APITestCase): '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 @@ class SectionGradesBreakdownTest(GradeViewTestMixin, APITestCase): { '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 @@ class SectionGradesBreakdownTest(GradeViewTestMixin, APITestCase): '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 @@ class SectionGradesBreakdownTest(GradeViewTestMixin, APITestCase): '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 606c935e40..d7d3a20c1f 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 @@ class TestCourseGradeFactory(GradeTestBase): 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 @@ class TestCourseGradeFactory(GradeTestBase): '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 caa6a991dc..fc818c6468 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -3,7 +3,7 @@ Views for course info API """ 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 @@ class BlocksInfoInCourseView(BlocksInCourseView): 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 @@ class BlocksInfoInCourseView(BlocksInCourseView): """ 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 @@ class BlocksInfoInCourseView(BlocksInCourseView): 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 @@ class BlocksInfoInCourseView(BlocksInCourseView): '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 56c020ec8f..efb3f7d9fd 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 @@ class TestBlocksInfoInCourseView(TestBlocksInCourseView, MilestonesTestCaseMixin { '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 5261b9f447..0011138824 100644 --- a/xmodule/graders.py +++ b/xmodule/graders.py @@ -380,11 +380,12 @@ class AssignmentFormatGrader(CourseGrader): 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 @@ class AssignmentFormatGrader(CourseGrader): 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 @@ class AssignmentFormatGrader(CourseGrader): ) 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 5073052b1b..5e2444a055 100644 --- a/xmodule/tests/test_graders.py +++ b/xmodule/tests/test_graders.py @@ -70,9 +70,10 @@ class GraderTest(unittest.TestCase): """ 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 @@ class GraderTest(unittest.TestCase): 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 @@ class GraderTest(unittest.TestCase): 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):