From 9af7c045bdd4d1e83b35162589df2767134c070a Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Tue, 26 Nov 2024 16:26:31 +0100 Subject: [PATCH] Expose roster information on the user metrics endpoint --- lms/js_config_types.py | 15 +++ lms/views/dashboard/api/user.py | 13 +- .../unit/lms/views/dashboard/api/user_test.py | 115 +++++++++++------- 3 files changed, 95 insertions(+), 48 deletions(-) diff --git a/lms/js_config_types.py b/lms/js_config_types.py index b9795cb55a..44b9f48dd7 100644 --- a/lms/js_config_types.py +++ b/lms/js_config_types.py @@ -113,6 +113,21 @@ class APIAssignments(TypedDict): pagination: NotRequired[Pagination] +class RosterEntry(TypedDict): + student: APIStudent + + active: bool + "Whether or not this student is active in the course/assignment or roster." + + +class APIRoster(TypedDict): + students: list[RosterEntry] + + last_updated: datetime | None + """When was this roster last updated. + None indicates we don't have roster data, we rely on launches to determine the list of students.""" + + class APIStudents(TypedDict): students: list[APIStudent] diff --git a/lms/views/dashboard/api/user.py b/lms/views/dashboard/api/user.py index e3042d8694..e5fd81c569 100644 --- a/lms/views/dashboard/api/user.py +++ b/lms/views/dashboard/api/user.py @@ -6,9 +6,11 @@ from lms.js_config_types import ( AnnotationMetrics, + APIRoster, APIStudent, APIStudents, AutoGradingGrade, + RosterEntry, ) from lms.models import Assignment, Organization, RoleScope, RoleType, User from lms.security import Permissions @@ -108,7 +110,7 @@ def students(self) -> APIStudents: permission=Permissions.DASHBOARD_VIEW, schema=UsersMetricsSchema, ) - def students_metrics(self) -> APIStudents: + def students_metrics(self) -> APIRoster: """Fetch the stats for one particular assignment.""" assignment = self.dashboard_service.get_request_assignment( self.request, self.request.parsed_params["assignment_id"] @@ -174,7 +176,14 @@ def students_metrics(self) -> APIStudents: if assignment.auto_grading_config: students = self._add_auto_grading_data(assignment, students) - return {"students": students} + # We are not expsoging the roster infor here yet, just making the API changes to better coordinate with the frontend + # For now we mark every roster entry as active and we don't include any last_activity. + return APIRoster( + students=[ + RosterEntry(student=student, active=True) for student in students + ], + last_updated=None, + ) def _add_auto_grading_data( self, assignment: Assignment, api_students: list[APIStudent] diff --git a/tests/unit/lms/views/dashboard/api/user_test.py b/tests/unit/lms/views/dashboard/api/user_test.py index 29a09bd6d9..90b9610e53 100644 --- a/tests/unit/lms/views/dashboard/api/user_test.py +++ b/tests/unit/lms/views/dashboard/api/user_test.py @@ -130,36 +130,46 @@ def test_students_metrics( expected = { "students": [ { - "h_userid": student.h_userid, - "lms_id": student.user_id, - "display_name": student.display_name, - "annotation_metrics": { - "annotations": 4, - "replies": sentinel.replies, - "last_activity": datetime(2024, 1, 1), + "active": True, + "student": { + "h_userid": student.h_userid, + "lms_id": student.user_id, + "display_name": student.display_name, + "annotation_metrics": { + "annotations": 4, + "replies": sentinel.replies, + "last_activity": datetime(2024, 1, 1), + }, }, }, { - "h_userid": student_no_annos.h_userid, - "lms_id": student_no_annos.user_id, - "display_name": student_no_annos.display_name, - "annotation_metrics": { - "annotations": 0, - "replies": 0, - "last_activity": None, + "active": True, + "student": { + "h_userid": student_no_annos.h_userid, + "lms_id": student_no_annos.user_id, + "display_name": student_no_annos.display_name, + "annotation_metrics": { + "annotations": 0, + "replies": 0, + "last_activity": None, + }, }, }, { - "h_userid": student_no_annos_no_name.h_userid, - "lms_id": student_no_annos_no_name.user_id, - "display_name": None, - "annotation_metrics": { - "annotations": 0, - "replies": 0, - "last_activity": None, + "active": True, + "student": { + "h_userid": student_no_annos_no_name.h_userid, + "lms_id": student_no_annos_no_name.user_id, + "display_name": None, + "annotation_metrics": { + "annotations": 0, + "replies": 0, + "last_activity": None, + }, }, }, - ] + ], + "last_updated": None, } assert response == expected @@ -221,42 +231,52 @@ def test_students_metrics_with_auto_grading( expected = { "students": [ { - "h_userid": student.h_userid, - "lms_id": student.user_id, - "display_name": student.display_name, - "annotation_metrics": { - "annotations": 4, - "replies": sentinel.replies, - "last_activity": datetime(2024, 1, 1), + "student": { + "h_userid": student.h_userid, + "lms_id": student.user_id, + "display_name": student.display_name, + "annotation_metrics": { + "annotations": 4, + "replies": sentinel.replies, + "last_activity": datetime(2024, 1, 1), + }, }, + "active": True, }, { - "h_userid": student_no_annos.h_userid, - "lms_id": student_no_annos.user_id, - "display_name": student_no_annos.display_name, - "annotation_metrics": { - "annotations": 0, - "replies": 0, - "last_activity": None, + "student": { + "h_userid": student_no_annos.h_userid, + "lms_id": student_no_annos.user_id, + "display_name": student_no_annos.display_name, + "annotation_metrics": { + "annotations": 0, + "replies": 0, + "last_activity": None, + }, }, + "active": True, }, { - "h_userid": student_no_annos_no_name.h_userid, - "lms_id": student_no_annos_no_name.user_id, - "display_name": None, - "annotation_metrics": { - "annotations": 0, - "replies": 0, - "last_activity": None, + "student": { + "h_userid": student_no_annos_no_name.h_userid, + "lms_id": student_no_annos_no_name.user_id, + "display_name": None, + "annotation_metrics": { + "annotations": 0, + "replies": 0, + "last_activity": None, + }, }, + "active": True, }, - ] + ], + "last_updated": None, } calls = [] last_grades = auto_grading_service.get_last_grades.return_value for api_student in expected["students"]: - api_student["auto_grading_grade"] = { + api_student["student"]["auto_grading_grade"] = { "current_grade": auto_grading_service.calculate_grade.return_value, "last_grade": last_grades.get.return_value.grade if with_last_grade @@ -266,7 +286,10 @@ def test_students_metrics_with_auto_grading( else None, } calls.append( - call(assignment.auto_grading_config, api_student["annotation_metrics"]) + call( + assignment.auto_grading_config, + api_student["student"]["annotation_metrics"], + ) ) auto_grading_service.calculate_grade.assert_has_calls(calls)