Return the actual value for Roster.last_updated when available

Return the greatest date we have for a given roster or None for cases
where we still base the roster data on launches.

lms/services/dashboard.py

@@ -1,3 +1,5 @@
+from datetime import datetime
+
 from pyramid.httpexceptions import HTTPNotFound, HTTPUnauthorized
 from sqlalchemy import Select, select, union
 
@@ -178,16 +180,16 @@ def get_request_admin_organizations(self, request) -> list[Organization]:
 
     def get_assignment_roster(
         self, assignment: Assignment, h_userids: list[str] | None = None
-    ) -> Select[tuple[LMSUser, bool]]:
+    ) -> tuple[datetime | None, Select[tuple[LMSUser, bool]]]:
         rosters_enabled = (
             assignment.course
             and assignment.course.application_instance.settings.get(
                 "dashboard", "rosters"
             )
         )
-        if rosters_enabled and self._roster_service.assignment_roster_exists(
-            assignment
-        ):
+        roster_last_updated = self._roster_service.assignment_roster_exists(assignment)
+
+        if rosters_enabled and roster_last_updated:
             # If rostering is enabled and we do have the data, use it
             query = self._roster_service.get_assignment_roster(
                 assignment,
@@ -207,7 +209,7 @@ def get_assignment_roster(
             ).add_columns(True)
 
         # Always return the results, no matter the source, sorted
-        return query.order_by(LMSUser.display_name, LMSUser.id)
+        return roster_last_updated, query.order_by(LMSUser.display_name, LMSUser.id)
 
 
 def factory(_context, request):

lms/services/roster.py

@@ -1,3 +1,4 @@
+from datetime import datetime
 from logging import getLogger
 
 from sqlalchemy import Select, func, select, text, update
@@ -64,14 +65,17 @@ def get_course_roster(
 
         return select(LMSUser).where(LMSUser.id.in_(roster_query))
 
-    def assignment_roster_exists(self, assignment: Assignment) -> bool:
-        """Check if we have roster data for the given assignment."""
-        return bool(
-            self._db.scalar(
-                select(AssignmentRoster)
-                .where(AssignmentRoster.assignment_id == assignment.id)
-                .limit(1)
-            )
+    def assignment_roster_exists(self, assignment: Assignment) -> datetime | None:
+        """
+        Check if we have roster data for the given assignment.
+
+        In case we have roster data, return the last updated timestamp, None otherwise.
+        """
+        return self._db.scalar(
+            select(AssignmentRoster.updated)
+            .where(AssignmentRoster.assignment_id == assignment.id)
+            .order_by(AssignmentRoster.updated.desc())
+            .limit(1)
         )
 
     def get_assignment_roster(

lms/views/dashboard/api/user.py

@@ -3,6 +3,7 @@
 
 from marshmallow import fields, validate
 from pyramid.view import view_config
+from sqlalchemy import Select
 
 from lms.js_config_types import (
     AnnotationMetrics,
@@ -84,7 +85,7 @@ def __init__(self, request) -> None:
         schema=ListUsersSchema,
     )
     def students(self) -> APIStudents:
-        students_query = self._students_query(
+        _, students_query = self._students_query(
             assignment_ids=self.request.parsed_params.get("assignment_ids"),
             segment_authority_provided_ids=self.request.parsed_params.get(
                 "segment_authority_provided_ids"
@@ -141,7 +142,7 @@ def students_metrics(self) -> APIRoster:
         stats_by_user = {s["userid"]: s for s in stats}
         students: list[RosterEntry] = []
 
-        users_query = self._students_query(
+        roster_last_updated, users_query = self._students_query(
             assignment_ids=[assignment.id],
             segment_authority_provided_ids=request_segment_authority_provided_ids,
             h_userids=request_h_userids,
@@ -179,9 +180,7 @@ def students_metrics(self) -> APIRoster:
         if assignment.auto_grading_config:
             students = self._add_auto_grading_data(assignment, 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)
+        return APIRoster(students=students, last_updated=roster_last_updated)
 
     def _add_auto_grading_data(
         self, assignment: Assignment, api_students: list[RosterEntry]
@@ -211,7 +210,7 @@ def _students_query(
         assignment_ids: list[int],
         segment_authority_provided_ids: list[str],
         h_userids: list[str] | None = None,
-    ):
+    ) -> tuple[datetime | None, Select]:
         course_ids = self.request.parsed_params.get("course_ids")
         # Single assigment fetch
         if (
@@ -234,7 +233,7 @@ def _students_query(
                 self.request, course_id=course_ids[0]
             )
 
-            return self.user_service.get_users_for_course(
+            return None, self.user_service.get_users_for_course(
                 role_scope=RoleScope.COURSE,
                 role_type=RoleType.LEARNER,
                 course_id=course.id,
@@ -246,7 +245,7 @@ def _students_query(
         )
         # Full organization fetch
         if not course_ids and not assignment_ids and not segment_authority_provided_ids:
-            return self.user_service.get_users_for_organization(
+            return None, self.user_service.get_users_for_organization(
                 role_scope=RoleScope.COURSE,
                 role_type=RoleType.LEARNER,
                 h_userids=h_userids,
@@ -257,7 +256,7 @@ def _students_query(
                 admin_organization_ids=[org.id for org in admin_organizations],
             )
 
-        return self.user_service.get_users(
+        return None, self.user_service.get_users(
             role_scope=RoleScope.COURSE,
             role_type=RoleType.LEARNER,
             course_ids=self.request.parsed_params.get("course_ids"),

tests/unit/lms/services/dashboard_test.py

@@ -218,34 +218,37 @@ def test_get_request_admin_organizations_for_staff(
         assert svc.get_request_admin_organizations(pyramid_request) == [organization]
 
     def test_get_assignment_roster_with_roster_disabled(
-        self, svc, application_instance, user_service
+        self, svc, application_instance, user_service, roster_service
     ):
         assignment = factories.Assignment(
             course=factories.Course(application_instance=application_instance)
         )
+        roster_service.assignment_roster_exists.return_value = None
 
-        roster = svc.get_assignment_roster(assignment, sentinel.h_userids)
+        last_updated, roster = svc.get_assignment_roster(assignment, sentinel.h_userids)
 
         user_service.get_users_for_assignment.assert_called_once_with(
             role_scope=RoleScope.COURSE,
             role_type=RoleType.LEARNER,
             assignment_id=assignment.id,
             h_userids=sentinel.h_userids,
         )
+
         assert (
             roster
             == user_service.get_users_for_assignment.return_value.add_columns.return_value.order_by.return_value
         )
+        assert not last_updated
 
-    def test_get_assignment_roster_with(
+    def test_get_assignment_roster_with_roster_enabled(
         self, svc, application_instance, roster_service
     ):
         application_instance.settings.set("dashboard", "rosters", True)
         assignment = factories.Assignment(
             course=factories.Course(application_instance=application_instance)
         )
 
-        roster = svc.get_assignment_roster(assignment, sentinel.h_userids)
+        last_updated, roster = svc.get_assignment_roster(assignment, sentinel.h_userids)
 
         roster_service.get_assignment_roster.assert_called_once_with(
             assignment,
@@ -257,6 +260,7 @@ def test_get_assignment_roster_with(
             roster
             == roster_service.get_assignment_roster.return_value.order_by.return_value
         )
+        assert last_updated == roster_service.assignment_roster_exists.return_value
 
     @pytest.fixture()
     def svc(

tests/unit/lms/services/roster_test.py

@@ -1,3 +1,4 @@
+from datetime import datetime
 from unittest.mock import Mock, sentinel
 
 import pytest
@@ -13,7 +14,7 @@
 class TestRosterService:
     @pytest.mark.parametrize(
         "create_roster,expected",
-        [(True, True), (False, False)],
+        [(True, datetime(2021, 1, 1)), (False, None)],
     )
     def test_assignment_roster_exists(
         self, svc, assignment, db_session, create_roster, expected
@@ -23,6 +24,7 @@ def test_assignment_roster_exists(
 
         if create_roster:
             factories.AssignmentRoster(
+                updated=datetime(2021, 1, 1),
                 lms_user=lms_user,
                 assignment=assignment,
                 lti_role=lti_role,

tests/unit/lms/views/dashboard/api/user_test.py

@@ -111,6 +111,7 @@ def test_students_metrics(
         else:
             db_session.flush()
             dashboard_service.get_assignment_roster.return_value = (
+                None,
                 select(User)
                 .where(
                     User.id.in_(
@@ -124,7 +125,7 @@ def test_students_metrics(
                         ]
                     )
                 )
-                .add_columns(True)
+                .add_columns(True),
             )
 
         db_session.flush()
@@ -217,6 +218,7 @@ def test_students_metrics_with_auto_grading(
 
         db_session.flush()
         dashboard_service.get_assignment_roster.return_value = (
+            None,
             select(User)
             .where(
                 User.id.in_(
@@ -226,7 +228,7 @@ def test_students_metrics_with_auto_grading(
                     ]
                 )
             )
-            .add_columns(True)
+            .add_columns(True),
         )
         dashboard_service.get_request_assignment.return_value = assignment
         h_api.get_annotation_counts.return_value = annotation_counts_response