From 7717adcc6f1c6dd39f8aeafe10dec30506e9e687 Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Wed, 18 Sep 2024 14:17:13 +0200 Subject: [PATCH] Explicitly request a large number of results from BB grading API Due to a bug in Blackboard (see: https://github.com/hypothesis/lms/pull/5839) we fetch results for all students and just pick the right one. We didn't implement pagination together with that change so we are missing students that are part of the course but are present in other pages. For now will just request a large number of results and alert when we still hit the pagination limit. --- lms/services/lti_grading/_v13.py | 9 +++++++++ tests/unit/lms/services/lti_grading/_v13_test.py | 14 +++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lms/services/lti_grading/_v13.py b/lms/services/lti_grading/_v13.py index 9e799c1237..daceacbc32 100644 --- a/lms/services/lti_grading/_v13.py +++ b/lms/services/lti_grading/_v13.py @@ -46,7 +46,11 @@ def read_result(self, grading_id) -> GradingResult: if self._product_family == Family.BLACKBOARD: # There's currently a bug in Blackboard's LTIA implementation. # Using the user filter in the request removes the comment field in the response's body. + # So we'll remove the parameter and will search for the right student in the response. del params["user_id"] + # We explicitly add a big limit parameter to try to fetch as many results as possible + # without having to fetch more pages (which we have not implemented) + params["limit"] = 1000 try: response = self._ltia_service.request( @@ -67,6 +71,11 @@ def read_result(self, grading_id) -> GradingResult: # Due to the bug mentioned above we didn't use the API filtering by user for blackboard. # We'll filter ourselves here instead. results = [result for result in results if result["userId"] == grading_id] + if response.links.get("next"): + # We want to be notified if we are leaving results unfetched and likely start following these links + LOG.error( + "Blackboard grading reading result, container with paginated results" + ) if not results: return result diff --git a/tests/unit/lms/services/lti_grading/_v13_test.py b/tests/unit/lms/services/lti_grading/_v13_test.py index d4a6b5f9e8..eacb5c77a0 100644 --- a/tests/unit/lms/services/lti_grading/_v13_test.py +++ b/tests/unit/lms/services/lti_grading/_v13_test.py @@ -81,6 +81,7 @@ def test_read_result_blackboard( misc_plugin, lti_registration, ): + ltia_http_service.request.return_value = Mock(links={"next": None}) ltia_http_service.request.return_value.json.return_value = blackboard_response blackboard_svc.line_item_url = "https://lms.com/lineitems?param=1" @@ -91,7 +92,7 @@ def test_read_result_blackboard( "GET", "https://lms.com/lineitems/results?param=1", scopes=blackboard_svc.LTIA_SCOPES, - params={}, + params={"limit": 1000}, headers={"Accept": "application/vnd.ims.lis.v2.resultcontainer+json"}, ) assert ( @@ -104,6 +105,17 @@ def test_read_result_blackboard( ) assert result.comment == misc_plugin.clean_lms_grading_comment.return_value + def test_read_result_blackboard_pagination_limit( + self, blackboard_svc, ltia_http_service, blackboard_response, caplog + ): + ltia_http_service.request.return_value = Mock(links={"next": sentinel.next}) + ltia_http_service.request.return_value.json.return_value = blackboard_response + + result = blackboard_svc.read_result(sentinel.user_id) + + assert "paginated" in caplog.text + assert result + def test_get_score_maximum(self, svc, ltia_http_service, lti_registration): ltia_http_service.request.return_value.json.return_value = [ {"scoreMaximum": sentinel.score_max, "id": svc.line_item_url},