From de50cf88cdf3aa9f72e5b9e0b76aefc5cf82748b Mon Sep 17 00:00:00 2001
From: Marcos Prieto <marcospri@gmail.com>
Date: Wed, 27 Mar 2024 13:34:43 +0100
Subject: [PATCH] Include userid in the assignments stats response

This will allow the LMS side to match against it user records.
---
 h/services/bulk_api/lms_stats.py              | 24 ++++++++++++-------
 h/views/api/bulk/stats.py                     |  3 ++-
 .../h/services/bulk_api/lms_stats_test.py     |  1 +
 tests/unit/h/views/api/bulk/stats_test.py     |  3 ++-
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/h/services/bulk_api/lms_stats.py b/h/services/bulk_api/lms_stats.py
index de86633b084..d5c6709350b 100644
--- a/h/services/bulk_api/lms_stats.py
+++ b/h/services/bulk_api/lms_stats.py
@@ -11,6 +11,7 @@
 @dataclass
 class AssignmentStats:
     display_name: str
+    userid: str
     annotations: int
     replies: int
     last_activity: datetime
@@ -22,7 +23,7 @@ def __init__(self, db: Session, authorized_authority: str):
         self._authorized_authority = authorized_authority
 
     def _annotation_type_select(self):
-        """Build a select that tags each annotation row witht a type."""
+        """Build a select that tags each annotation row with a type."""
         return select(
             AnnotationSlim,
             case(
@@ -44,13 +45,13 @@ def assignment_stats(
         self, groups: list[str], assignment_id: dict
     ) -> list[AssignmentStats]:
         """
-        Get a list of groups.
+        Get basic stats per user for an LMS assignment.
 
         :param groups: List of "authority_provided_id" to filter groups by.
         :param assignment_id: ID of the assignment we are generating the stats for.
         """
 
-        annotation_query = (
+        annos_query = (
             self._annotation_type_select()
             .join(Group, Group.id == AnnotationSlim.group_id)
             .join(AnnotationMetadata)
@@ -74,15 +75,19 @@ def assignment_stats(
         query = (
             select(
                 User.display_name,
-                func.count(annotation_query.c.id)
-                .filter(annotation_query.c.type == "annotation")
+                # Unfortunally all thet magic around User.userid doesn't work in this context
+                func.concat("acct:", User.username, "@", User.authority).label(
+                    "userid"
+                ),
+                func.count(annos_query.c.id)
+                .filter(annos_query.c.type == "annotation")
                 .label("annotations"),
-                func.count(annotation_query.c.id)
-                .filter(annotation_query.c.type == "reply")
+                func.count(annos_query.c.id)
+                .filter(annos_query.c.type == "reply")
                 .label("replies"),
-                func.max(annotation_query.c.created).label("last_activity"),
+                func.max(annos_query.c.created).label("last_activity"),
             )
-            .join(annotation_query, annotation_query.c.user_id == User.id)
+            .join(annos_query, annos_query.c.user_id == User.id)
             .group_by(User.id)
             .where(
                 User.nipsa.is_(False),
@@ -92,6 +97,7 @@ def assignment_stats(
         results = self._db.execute(query)
         return [
             AssignmentStats(
+                userid=row.userid,
                 display_name=row.display_name,
                 annotations=row.annotations,
                 replies=row.replies,
diff --git a/h/views/api/bulk/stats.py b/h/views/api/bulk/stats.py
index ac42a708807..7645d08fdc3 100644
--- a/h/views/api/bulk/stats.py
+++ b/h/views/api/bulk/stats.py
@@ -1,12 +1,12 @@
 import json
 
 from importlib_resources import files
+from pyramid.response import Response
 
 from h.schemas.base import JSONSchema
 from h.security import Permission
 from h.services.bulk_api import BulkLMSStatsService
 from h.views.api.config import api_config
-from pyramid.response import Response
 
 
 class AssignmentStatsSchema(JSONSchema):
@@ -37,6 +37,7 @@ def assignment(request):
         json=[
             {
                 "display_name": row.display_name,
+                "userid": row.userid,
                 "annotations": row.annotations,
                 "replies": row.replies,
                 "last_activity": row.last_activity.isoformat(),
diff --git a/tests/unit/h/services/bulk_api/lms_stats_test.py b/tests/unit/h/services/bulk_api/lms_stats_test.py
index 9f76ca08a94..52eae205b17 100644
--- a/tests/unit/h/services/bulk_api/lms_stats_test.py
+++ b/tests/unit/h/services/bulk_api/lms_stats_test.py
@@ -52,6 +52,7 @@ def test_it(self, svc, factories):
 
         assert stats == [
             AssignmentStats(
+                userid=user.userid,
                 display_name=user.display_name,
                 annotations=1,
                 replies=1,
diff --git a/tests/unit/h/views/api/bulk/stats_test.py b/tests/unit/h/views/api/bulk/stats_test.py
index 16126fddcdf..957318d7b7d 100644
--- a/tests/unit/h/views/api/bulk/stats_test.py
+++ b/tests/unit/h/views/api/bulk/stats_test.py
@@ -1,7 +1,6 @@
 from datetime import datetime
 
 import pytest
-from h_matchers import Any
 
 from h.services.bulk_api.lms_stats import AssignmentStats
 from h.views.api.bulk.stats import AssignmentStatsSchema, assignment
@@ -23,6 +22,7 @@ def test_it(self, pyramid_request, valid_request, bulk_stats_service):
         bulk_stats_service.assignment_stats.return_value = [
             AssignmentStats(
                 display_name=f"display_name{i}",
+                userid=i,
                 annotations=i,
                 replies=i,
                 last_activity=datetime.now(),
@@ -40,6 +40,7 @@ def test_it(self, pyramid_request, valid_request, bulk_stats_service):
         return_data = [
             {
                 "display_name": row.display_name,
+                "userid": row.userid,
                 "annotations": row.annotations,
                 "replies": row.replies,
                 "last_activity": row.last_activity.isoformat(),