Return actual value for roster last_updated

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,18 @@ 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(
+        roster_last_updated = self._roster_service.assignment_roster_last_updated(
             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,
@@ -197,6 +201,8 @@ def get_assignment_roster(
             )
 
         else:
+            # If we are not going to return data from the roster, don't return the last updated date
+            roster_last_updated = None
             # Always fallback to fetch users that have launched the assignment at some point
             query = self._user_service.get_users_for_assignment(
                 role_scope=RoleScope.COURSE,
@@ -207,7 +213,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,13 @@ 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_last_updated(self, assignment: Assignment) -> datetime | None:
+        """Return the roster's last updated timestamp for given assignment, or None if we don't have roster data."""
+        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

@@ -217,46 +217,53 @@ def test_get_request_admin_organizations_for_staff(
 
         assert svc.get_request_admin_organizations(pyramid_request) == [organization]
 
+    @pytest.mark.parametrize("rosters_enabled", [True, False])
+    @pytest.mark.parametrize("roster_available", [True, False])
     def test_get_assignment_roster_with_roster_disabled(
-        self, svc, application_instance, user_service
-    ):
-        assignment = factories.Assignment(
-            course=factories.Course(application_instance=application_instance)
-        )
-
-        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
-        )
-
-    def test_get_assignment_roster_with(
-        self, svc, application_instance, roster_service
+        self,
+        svc,
+        application_instance,
+        user_service,
+        roster_service,
+        rosters_enabled,
+        roster_available,
     ):
-        application_instance.settings.set("dashboard", "rosters", True)
+        application_instance.settings.set("dashboard", "rosters", rosters_enabled)
         assignment = factories.Assignment(
             course=factories.Course(application_instance=application_instance)
         )
-
-        roster = svc.get_assignment_roster(assignment, sentinel.h_userids)
-
-        roster_service.get_assignment_roster.assert_called_once_with(
-            assignment,
-            role_scope=RoleScope.COURSE,
-            role_type=RoleType.LEARNER,
-            h_userids=sentinel.h_userids,
-        )
-        assert (
-            roster
-            == roster_service.get_assignment_roster.return_value.order_by.return_value
-        )
+        if not roster_available:
+            roster_service.assignment_roster_last_updated.return_value = None
+
+        last_updated, roster = svc.get_assignment_roster(assignment, sentinel.h_userids)
+
+        if not roster_available or not rosters_enabled:
+            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 not last_updated
+            assert (
+                roster
+                == user_service.get_users_for_assignment.return_value.add_columns.return_value.order_by.return_value
+            )
+        else:
+            roster_service.get_assignment_roster.assert_called_once_with(
+                assignment,
+                role_scope=RoleScope.COURSE,
+                role_type=RoleType.LEARNER,
+                h_userids=sentinel.h_userids,
+            )
+            assert (
+                last_updated
+                == roster_service.assignment_roster_last_updated.return_value
+            )
+            assert (
+                roster
+                == roster_service.get_assignment_roster.return_value.order_by.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,24 +14,25 @@
 class TestRosterService:
     @pytest.mark.parametrize(
         "create_roster,expected",
-        [(True, True), (False, False)],
+        [(True, datetime(2021, 1, 1)), (False, None)],
     )
-    def test_assignment_roster_exists(
+    def test_assignment_roster_last_updated(
         self, svc, assignment, db_session, create_roster, expected
     ):
         lms_user = factories.LMSUser()
         lti_role = factories.LTIRole()
 
         if create_roster:
             factories.AssignmentRoster(
+                updated=datetime(2021, 1, 1),
                 lms_user=lms_user,
                 assignment=assignment,
                 lti_role=lti_role,
                 active=True,
             )
         db_session.flush()
 
-        assert svc.assignment_roster_exists(assignment) == expected
+        assert svc.assignment_roster_last_updated(assignment) == expected
 
     @pytest.mark.parametrize("with_role_scope", [True, False])
     @pytest.mark.parametrize("with_role_type", [True, False])

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