Skip to content

Commit

Permalink
Ignore known error codes from the rostering services
Browse files Browse the repository at this point in the history
We'll tackle this individually as we gather more information are able to
reproduce and fix them.
  • Loading branch information
marcospri committed Sep 4, 2024
1 parent 0a271c2 commit ab31da2
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 47 deletions.
20 changes: 6 additions & 14 deletions lms/services/roster.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,23 +122,15 @@ def fetch_assignment_roster(self, assignment: Assignment) -> None:
if (
err.response_body
and (
# Error message from Canvas
"Requested ResourceLink bound to unexpected external tool"
"Requested ResourceLink bound to unexpected external tool" # Canvas, unknown reason
in err.response_body
)
# In cases for which we have different assignment IDs
and assignment.resource_link_id != assignment.lti_v13_resource_link_id
):
# The LMS (Canvas) doesn't seem to be able to link our tool to this assignment
# Try the same call but with the LTI1.1 assignment identifier.
LOG.info("Try to fetch roster with the LTI1.1 identifier")
roster = self._lti_names_roles_service.get_context_memberships(
lti_registration,
lms_course.lti_context_memberships_url,
resource_link_id=assignment.resource_link_id,
)
else:
raise
LOG.error("Fetching assignment roster failed: %s", err.response_body)
# We ignore this type of error, just stop here.
return

raise

# Insert any users we might be missing in the DB
lms_users_by_lti_user_id = {
Expand Down
40 changes: 7 additions & 33 deletions tests/unit/lms/services/roster_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import Mock, call, sentinel
from unittest.mock import Mock, sentinel

import pytest
from h_matchers import Any
Expand Down Expand Up @@ -128,45 +128,19 @@ def test_fetch_assignment_roster(
assert not roster[3].active

def test_fetch_assignment_roster_retries_with_lti_v11_id(
self,
svc,
lti_names_roles_service,
lti_v13_application_instance,
assignment,
names_and_roles_roster_response,
lti_role_service,
self, svc, lti_names_roles_service, assignment
):
lti_role_service.get_roles.return_value = [
factories.LTIRole(value="ROLE1"),
factories.LTIRole(value="ROLE2"),
]

lti_names_roles_service.get_context_memberships.side_effect = [
lti_names_roles_service.get_context_memberships.side_effect = (
ExternalRequestError(
response=Mock(
text="Requested ResourceLink bound to unexpected external tool"
)
),
names_and_roles_roster_response,
]

svc.fetch_assignment_roster(assignment)

lti_names_roles_service.get_context_memberships.assert_has_calls(
[
call(
lti_v13_application_instance.lti_registration,
"SERVICE_URL",
assignment.lti_v13_resource_link_id,
),
call(
lti_v13_application_instance.lti_registration,
"SERVICE_URL",
assignment.resource_link_id,
),
]
)
)

# Method finishes without re-raising the exception
assert not svc.fetch_assignment_roster(assignment)

def test_fetch_assignment_roster_raises_external_request_error(
self, svc, lti_names_roles_service, assignment
):
Expand Down

0 comments on commit ab31da2

Please sign in to comment.