Skip to content

Commit

Permalink
Expose roster information on the user metrics endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
marcospri committed Nov 27, 2024
1 parent 6d1005a commit c6bf186
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 10 deletions.
13 changes: 13 additions & 0 deletions lms/js_config_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,19 @@ class APIAssignments(TypedDict):
pagination: NotRequired[Pagination]


class RosterEntry(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]

Expand Down
20 changes: 13 additions & 7 deletions lms/views/dashboard/api/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -137,7 +139,7 @@ def students_metrics(self) -> APIStudents:
)
# Organize the H stats by userid for quick access
stats_by_user = {s["userid"]: s for s in stats}
students: list[APIStudent] = []
students: list[RosterEntry] = []

users_query = self._students_query(
assignment_ids=[assignment.id],
Expand All @@ -148,7 +150,8 @@ def students_metrics(self) -> APIStudents:
for user in self.request.db.scalars(users_query).all():
if s := stats_by_user.get(user.h_userid):
# We seen this student in H, get all the data from there
api_student = APIStudent(
api_student = RosterEntry(
active=True,
h_userid=user.h_userid,
lms_id=user.user_id,
display_name=s["display_name"],
Expand All @@ -161,7 +164,8 @@ def students_metrics(self) -> APIStudents:
else:
# We haven't seen this user H,
# use LMS DB's data and set 0s for all annotation related fields.
api_student = APIStudent(
api_student = RosterEntry(
active=True,
h_userid=user.h_userid,
lms_id=user.user_id,
display_name=user.display_name,
Expand All @@ -174,11 +178,13 @@ def students_metrics(self) -> APIStudents:
if assignment.auto_grading_config:
students = self._add_auto_grading_data(assignment, students)

return {"students": students}
# We are not exposing the roster info 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=students, last_updated=None)

def _add_auto_grading_data(
self, assignment: Assignment, api_students: list[APIStudent]
) -> list[APIStudent]:
self, assignment: Assignment, api_students: list[RosterEntry]
) -> list[RosterEntry]:
"""Augment APIStudent with auto-grading data."""
last_sync_grades = self.auto_grading_service.get_last_grades(assignment)

Expand Down
17 changes: 14 additions & 3 deletions tests/unit/lms/views/dashboard/api/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ def test_students_metrics(
expected = {
"students": [
{
"active": True,
"h_userid": student.h_userid,
"lms_id": student.user_id,
"display_name": student.display_name,
Expand All @@ -140,6 +141,7 @@ def test_students_metrics(
},
},
{
"active": True,
"h_userid": student_no_annos.h_userid,
"lms_id": student_no_annos.user_id,
"display_name": student_no_annos.display_name,
Expand All @@ -150,6 +152,7 @@ def test_students_metrics(
},
},
{
"active": True,
"h_userid": student_no_annos_no_name.h_userid,
"lms_id": student_no_annos_no_name.user_id,
"display_name": None,
Expand All @@ -159,7 +162,8 @@ def test_students_metrics(
"last_activity": None,
},
},
]
],
"last_updated": None,
}
assert response == expected

Expand Down Expand Up @@ -221,6 +225,7 @@ def test_students_metrics_with_auto_grading(
expected = {
"students": [
{
"active": True,
"h_userid": student.h_userid,
"lms_id": student.user_id,
"display_name": student.display_name,
Expand All @@ -231,6 +236,7 @@ def test_students_metrics_with_auto_grading(
},
},
{
"active": True,
"h_userid": student_no_annos.h_userid,
"lms_id": student_no_annos.user_id,
"display_name": student_no_annos.display_name,
Expand All @@ -241,6 +247,7 @@ def test_students_metrics_with_auto_grading(
},
},
{
"active": True,
"h_userid": student_no_annos_no_name.h_userid,
"lms_id": student_no_annos_no_name.user_id,
"display_name": None,
Expand All @@ -250,7 +257,8 @@ def test_students_metrics_with_auto_grading(
"last_activity": None,
},
},
]
],
"last_updated": None,
}
calls = []

Expand All @@ -266,7 +274,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["annotation_metrics"],
)
)

auto_grading_service.calculate_grade.assert_has_calls(calls)
Expand Down

0 comments on commit c6bf186

Please sign in to comment.