Skip to content

feat: [FC-0092] Add short label to assignment #36970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ormsbee merged 1 commit into from
Sep 18, 2025
Merged

feat: [FC-0092] Add short label to assignment #36970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lms/djangoapps/courseware/courses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor

@e0d e0d Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was concerned here because we're changing the return type of a function that felt very platform core.

get_assignments_grades

However, it looks like this is only called by one view in the mobile API. So, my concern has changed from breaking other callers to wonder why we have a special case for getting this data for mobile.

Copy link
Contributor

@Serj-N Serj-N Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, there is only one caller, so this change shouldn't be a problem.
Not sure what you mean by "special case" though. These labels are needed for a new mobile screen, to be used for buttons in the Assignments tab. If that doesn't answer your question, could you please be more specific?

Copy link
Contributor

@e0d e0d Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that I don't want the mobile application and the web application to have different views and assignments and progress.

Copy link
Contributor

@Serj-N Serj-N Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that in the web app, the instructor may create short labels that will be longer than 3 characters,
and they will be a pain to work with in the mobile UI. Hence the difference in constructing these labels on mobile vs. web.
I believe, @marcotuts may provide more details and/or correct us on this.

- 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))

Expand Down Expand Up @@ -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']
Copy link
Contributor

@NiedielnitsevIvan NiedielnitsevIvan Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the docstring according to these changes.

Copy link
Contributor Author

@KyryloKireiev KyryloKireiev Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated



def get_first_component_of_block(block_key, block_data):
Expand Down
24 changes: 16 additions & 8 deletions lms/djangoapps/grades/rest_api/v1/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
]
Expand All @@ -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,
}
]
+ [
Expand All @@ -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)
]
Expand All @@ -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',
Expand All @@ -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,
}
]
)
Expand Down
7 changes: 5 additions & 2 deletions lms/djangoapps/grades/tests/test_course_grade_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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',
Expand Down
24 changes: 20 additions & 4 deletions lms/djangoapps/mobile_api/course_info/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -383,17 +383,20 @@ 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],
) -> None:
"""
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':
Expand All @@ -403,19 +406,32 @@ 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(
{
'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):
Expand Down
6 changes: 4 additions & 2 deletions lms/djangoapps/mobile_api/tests/test_course_info_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
)

Expand Down
9 changes: 6 additions & 3 deletions xmodule/graders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})"
Expand All @@ -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)

Expand All @@ -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(
Expand Down
99 changes: 89 additions & 10 deletions xmodule/tests/test_graders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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",
),
},
Expand Down Expand Up @@ -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):
Expand Down
Loading