Join data from H for the dashboard with LMS DB records

Use the LMS DB data to:

- Filter out teachers
- Include students that didn't make an annotation

lms/models/__init__.py

@@ -23,7 +23,7 @@
 from lms.models.jwt_oauth2_token import JWTOAuth2Token
 from lms.models.lti_params import CLAIM_PREFIX, LTIParams
 from lms.models.lti_registration import LTIRegistration
-from lms.models.lti_role import LTIRole, LTIRoleOverride
+from lms.models.lti_role import LTIRole, LTIRoleOverride, RoleScope, RoleType
 from lms.models.lti_user import LTIUser, display_name
 from lms.models.oauth2_token import OAuth2Token
 from lms.models.organization import Organization

lms/models/assignment.py

@@ -81,6 +81,10 @@ class Assignment(CreatedUpdatedMixin, Base):
     groupings = association_proxy("assignment_grouping", "grouping")
     """List of groupings this assigments is related to."""
 
+    membership = sa.orm.relationship(
+        "AssignmentMembership", lazy="dynamic", viewonly=True
+    )
+
     def get_canvas_mapped_file_id(self, file_id):
         return self.extra.get("canvas_file_mappings", {}).get(file_id, file_id)
 

lms/services/assignment.py

@@ -1,13 +1,15 @@
 import logging
 
-from sqlalchemy.orm import Session
+from sqlalchemy.orm import Session, joinedload
 
 from lms.models import (
     Assignment,
     AssignmentGrouping,
     AssignmentMembership,
     Grouping,
     LTIRole,
+    RoleScope,
+    RoleType,
     User,
 )
 from lms.services.upsert import bulk_upsert
@@ -198,11 +200,19 @@ def get_by_id(self, id_: int) -> Assignment | None:
 
     def is_member(self, assignment: Assignment, user: User) -> bool:
         """Check if a user is a member of an assignment."""
-        return bool(
-            self._db.query(AssignmentMembership)
-            .filter_by(user=user, assignment=assignment)
-            .one_or_none()
-        )
+        return bool(assignment.membership.filter_by(user=user).one_or_none())
+
+    def get_members(
+        self, assignment, role_type: RoleType, role_scope: RoleScope
+    ) -> list[User]:
+        return [
+            member.user
+            for member in assignment.membership.options(
+                joinedload(AssignmentMembership.user)
+            )
+            .join(LTIRole)
+            .where(LTIRole.scope == role_scope, LTIRole.type == role_type)
+        ]
 
 
 def factory(_context, request):

lms/views/dashboard.py

@@ -1,6 +1,7 @@
 from pyramid.httpexceptions import HTTPFound, HTTPNotFound, HTTPUnauthorized
 from pyramid.view import view_config
 
+from lms.models import RoleScope, RoleType
 from lms.security import Permissions
 from lms.services.h_api import HAPI
 from lms.validation.authentication import BearerTokenSchema
@@ -59,15 +60,39 @@ def api_assignment_stats(self):
             [g.authority_provided_id for g in assignment.groupings],
             assignment.resource_link_id,
         )
-        return [
-            {
-                "display_name": s["display_name"],
-                "annotations": s["annotations"],
-                "last_activity": s["last_activity"],
-                "replies": s["replies"],
-            }
-            for s in stats
-        ]
+
+        # Organize the H stats by userid for quick access
+        stats_by_user = {s["userid"]: s for s in stats}
+        student_stats = []
+
+        # Iterate over all the students we have in the DB
+        for user in self.assignment_service.get_members(
+            assignment, role_scope=RoleScope.COURSE, role_type=RoleType.LEARNER
+        ):
+            if s := stats_by_user.get(user.h_userid):
+                # We seen this student in H, get all the data from there
+                student_stats.append(
+                    {
+                        "display_name": s["display_name"],
+                        "annotations": s["annotations"],
+                        "replies": s["replies"],
+                        "last_activity": s["last_activity"],
+                    }
+                )
+            else:
+                # We haven't seen this user H,
+                # use LMS DB's data and set 0s for all annotation related fields.
+                student_stats.append(
+                    {
+                        "display_name": user.display_name
+                        or f"Student {user.user_id[:10]}",
+                        "annotations": 0,
+                        "replies": 0,
+                        "last_activity": None,
+                    }
+                )
+
+        return student_stats
 
     def get_request_assignment(self):
         assignment = self.assignment_service.get_by_id(self.request.matchdict["id_"])

tests/unit/lms/services/assignment_test.py

@@ -4,7 +4,7 @@
 import pytest
 from h_matchers import Any
 
-from lms.models import AssignmentGrouping, AssignmentMembership
+from lms.models import AssignmentGrouping, AssignmentMembership, RoleScope
 from lms.services.assignment import AssignmentService, factory
 from tests import factories
 
@@ -226,6 +226,27 @@ def test_is_member(self, svc, db_session):
         assert svc.is_member(assignment, user)
         assert not svc.is_member(assignment, other_user)
 
+    def test_get_members(self, svc, db_session):
+        factories.User()  # User not in assignment
+        assignment = factories.Assignment()
+        user = factories.User()
+        lti_role = factories.LTIRole(scope=RoleScope.COURSE)
+        factories.AssignmentMembership.create(
+            assignment=assignment, user=user, lti_role=lti_role
+        )
+        # User in assignment with other role
+        factories.AssignmentMembership.create(
+            assignment=assignment,
+            user=factories.User(),
+            lti_role=factories.LTIRole(scope=RoleScope.SYSTEM),
+        )
+
+        db_session.flush()
+
+        assert svc.get_members(
+            assignment, role_scope=lti_role.scope, role_type=lti_role.type
+        ) == [user]
+
     @pytest.fixture
     def svc(self, db_session, misc_plugin):
         return AssignmentService(db_session, misc_plugin)

tests/unit/lms/views/dashboard_test.py

@@ -57,17 +57,36 @@ def test_assignment_show(self, views, pyramid_request, assignment_service):
     def test_api_assignment_stats(
         self, views, pyramid_request, assignment_service, h_api
     ):
+        # User returned by the stats endpoint
+        student = factories.User()
+        # User with no annotations
+        student_no_annos = factories.User(display_name="Homer")
+        # User with no annotations and no name
+        student_no_annos_no_name = factories.User(display_name=None)
+
         pyramid_request.matchdict["id_"] = sentinel.id
         assignment = factories.Assignment()
+        assignment_service.get_members.return_value = [
+            student,
+            student_no_annos,
+            student_no_annos_no_name,
+        ]
         assignment_service.get_by_id.return_value = assignment
         stats = [
+            {
+                "display_name": student.display_name,
+                "annotations": sentinel.annotations,
+                "replies": sentinel.replies,
+                "userid": student.h_userid,
+                "last_activity": sentinel.last_activity,
+            },
             {
                 "display_name": sentinel.display_name,
                 "annotations": sentinel.annotations,
                 "replies": sentinel.replies,
+                "userid": "TEACHER",
                 "last_activity": sentinel.last_activity,
-                "extra_data": sentinel.extra_data,
-            }
+            },
         ]
 
         h_api.get_assignment_stats.return_value = stats
@@ -81,12 +100,23 @@ def test_api_assignment_stats(
         )
         assert response == [
             {
-                "display_name": s["display_name"],
-                "annotations": s["annotations"],
-                "replies": s["replies"],
-                "last_activity": s["last_activity"],
-            }
-            for s in stats
+                "display_name": student.display_name,
+                "annotations": sentinel.annotations,
+                "replies": sentinel.replies,
+                "last_activity": sentinel.last_activity,
+            },
+            {
+                "display_name": student_no_annos.display_name,
+                "annotations": 0,
+                "replies": 0,
+                "last_activity": None,
+            },
+            {
+                "display_name": f"Student {student_no_annos_no_name.user_id[:10]}",
+                "annotations": 0,
+                "replies": 0,
+                "last_activity": None,
+            },
         ]
 
     def test_get_request_assignment_404(