diff --git a/lms/services/h_api.py b/lms/services/h_api.py index dad2c88c19..a847659b75 100644 --- a/lms/services/h_api.py +++ b/lms/services/h_api.py @@ -169,32 +169,24 @@ def get_groups( authority_provided_id=group["authority_provided_id"] ) - def get_assignment_stats( - self, group_authority_ids: list[str], resource_link_id: str + def get_annotation_counts( + self, + group_authority_ids: list[str], + group_by: str, + h_userids: list[str] | None = None, + resource_link_id: str | None = None, ): - response = self._api_request( - "POST", - path="bulk/stats/users", - body=json.dumps( - { - "filter": { - "groups": group_authority_ids, - "assignment_id": resource_link_id, - } - } - ), - headers={ - "Content-Type": "application/vnd.hypothesis.v1+json", - }, - stream=False, - ) - return response.json() + filters = { + "groups": group_authority_ids, + "assignment_id": resource_link_id, + } + if h_userids: + filters["h_userids"] = h_userids - def get_course_stats(self, group_authority_ids: list[str]): response = self._api_request( "POST", - path="bulk/stats/assignments", - body=json.dumps({"filter": {"groups": group_authority_ids}}), + path="bulk/lms/annotations", + body=json.dumps({"group_by": group_by, "filter": filters}), headers={ "Content-Type": "application/vnd.hypothesis.v1+json", }, diff --git a/lms/views/dashboard/api/assignment.py b/lms/views/dashboard/api/assignment.py index dd08536b7b..444a6f45e6 100644 --- a/lms/views/dashboard/api/assignment.py +++ b/lms/views/dashboard/api/assignment.py @@ -42,11 +42,11 @@ def assignment(self) -> APIAssignment: def assignment_stats(self) -> APIStudents: """Fetch the stats for one particular assignment.""" assignment = get_request_assignment(self.request, self.assignment_service) - stats = self.h_api.get_assignment_stats( + stats = self.h_api.get_annotation_counts( [g.authority_provided_id for g in assignment.groupings], - assignment.resource_link_id, + group_by="user", + resource_link_id=assignment.resource_link_id, ) - # Organize the H stats by userid for quick access stats_by_user = {s["userid"]: s for s in stats} students: list[APIStudent] = [] diff --git a/lms/views/dashboard/api/course.py b/lms/views/dashboard/api/course.py index 3126c5932b..6c6dc31bcc 100644 --- a/lms/views/dashboard/api/course.py +++ b/lms/views/dashboard/api/course.py @@ -8,6 +8,7 @@ APICourses, CourseMetrics, ) +from lms.models import RoleScope, RoleType from lms.security import Permissions from lms.services.h_api import HAPI from lms.services.organization import OrganizationService @@ -71,11 +72,16 @@ def course(self) -> APICourse: ) def course_assignments(self) -> APIAssignments: course = get_request_course(self.request, self.course_service) + course_students = self.course_service.get_members( + course, role_scope=RoleScope.COURSE, role_type=RoleType.LEARNER + ) - stats = self.h_api.get_course_stats( - # Annotations in the course group an any children + stats = self.h_api.get_annotation_counts( + # Annotations in the course group and any children [course.authority_provided_id] - + [child.authority_provided_id for child in course.children] + + [child.authority_provided_id for child in course.children], + group_by="assignment", + h_userids=[s.h_userid for s in course_students], ) # Organize the H stats by assignment ID for quick access stats_by_assignment = {s["assignment_id"]: s for s in stats} diff --git a/tests/unit/lms/services/h_api_test.py b/tests/unit/lms/services/h_api_test.py index 5b82820cd6..ff370d01f7 100644 --- a/tests/unit/lms/services/h_api_test.py +++ b/tests/unit/lms/services/h_api_test.py @@ -132,13 +132,16 @@ def test_get_annotations( assert result == expected_result @pytest.mark.parametrize( - "method,url,args,payload", + "kwargs,payload", [ ( - "get_assignment_stats", - "https://h.example.com/private/api/bulk/stats/users", - (["group_1", "group_2"], "assignment_id"), { + "group_authority_ids": ["group_1", "group_2"], + "group_by": "user", + "resource_link_id": "assignment_id", + }, + { + "group_by": "user", "filter": { "groups": ["group_1", "group_2"], "assignment_id": "assignment_id", @@ -146,23 +149,31 @@ def test_get_annotations( }, ), ( - "get_course_stats", - "https://h.example.com/private/api/bulk/stats/assignments", - (["group_1", "group_2"],), { - "filter": {"groups": ["group_1", "group_2"]}, + "group_authority_ids": ["group_1", "group_2"], + "group_by": "user", + "resource_link_id": "assignment_id", + "h_userids": ["user_1", "user_2"], + }, + { + "group_by": "user", + "filter": { + "groups": ["group_1", "group_2"], + "assignment_id": "assignment_id", + "h_userids": ["user_1", "user_2"], + }, }, ), ], ) - def test_get_stats_endpoints(self, h_api, http_service, method, url, args, payload): + def test_get_annotation_counts(self, h_api, http_service, kwargs, payload): http_service.request.return_value = factories.requests.Response(raw="{}") - getattr(h_api, method)(*args) + h_api.get_annotation_counts(**kwargs) http_service.request.assert_called_once_with( method="POST", - url=url, + url="https://h.example.com/private/api/bulk/lms/annotations", auth=("TEST_CLIENT_ID", "TEST_CLIENT_SECRET"), headers={ "Content-Type": "application/vnd.hypothesis.v1+json", diff --git a/tests/unit/lms/views/dashboard/api/assignment_test.py b/tests/unit/lms/views/dashboard/api/assignment_test.py index 67af5d05cf..2d1b756416 100644 --- a/tests/unit/lms/views/dashboard/api/assignment_test.py +++ b/tests/unit/lms/views/dashboard/api/assignment_test.py @@ -59,14 +59,15 @@ def test_assignment_stats(self, views, pyramid_request, assignment_service, h_ap }, ] - h_api.get_assignment_stats.return_value = stats + h_api.get_annotation_counts.return_value = stats response = views.assignment_stats() assignment_service.get_by_id.assert_called_once_with(sentinel.id) - h_api.get_assignment_stats.assert_called_once_with( + h_api.get_annotation_counts.assert_called_once_with( [g.authority_provided_id for g in assignment.groupings], - assignment.resource_link_id, + group_by="user", + resource_link_id=assignment.resource_link_id, ) expected = { "students": [ diff --git a/tests/unit/lms/views/dashboard/api/course_test.py b/tests/unit/lms/views/dashboard/api/course_test.py index de53c0edd4..18a27bf235 100644 --- a/tests/unit/lms/views/dashboard/api/course_test.py +++ b/tests/unit/lms/views/dashboard/api/course_test.py @@ -75,6 +75,7 @@ def test_course_assignments( assignment, assignment_with_no_annos, ] + course_service.get_members.return_value = factories.User.create_batch(5) db_session.flush() @@ -88,12 +89,14 @@ def test_course_assignments( }, ] - h_api.get_course_stats.return_value = stats + h_api.get_annotation_counts.return_value = stats response = views.course_assignments() - h_api.get_course_stats.assert_called_once_with( - [course.authority_provided_id, section.authority_provided_id] + h_api.get_annotation_counts.assert_called_once_with( + [course.authority_provided_id, section.authority_provided_id], + group_by="assignment", + h_userids=[u.h_userid for u in course_service.get_members.return_value], ) assert response == { "assignments": [