Skip to content

Commit

Permalink
Merge pull request #2026 from openedx/ENT-8268-fix-no-user-found-error
Browse files Browse the repository at this point in the history
[ENT-8268] - Unlink Canvas user if decommissioned on Canvas side
  • Loading branch information
hamzawaleed01 authored Feb 21, 2024
2 parents ab0ffd6 + 53a149d commit ba758ef
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 38 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ Change Log
Unreleased
----------

[4.12.1]
---------
* feat: unlink canvas user if not decommissioned on canvas side

[4.12.0]
---------
* feat: Remove history tables for integrated channels customers configurations.
Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Your project description goes here.
"""

__version__ = "4.12.0"
__version__ = "4.12.1"
36 changes: 29 additions & 7 deletions integrated_channels/canvas/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from django.apps import apps

from enterprise.models import EnterpriseCustomerUser
from integrated_channels.canvas.utils import CanvasUtil # pylint: disable=cyclic-import
from integrated_channels.exceptions import ClientError
from integrated_channels.integrated_channel.client import IntegratedChannelApiClient, IntegratedChannelHealthStatus
Expand Down Expand Up @@ -654,7 +655,7 @@ def _extract_integration_id(self, data):

return integration_id

def _search_for_canvas_user_by_email(self, user_email):
def _search_for_canvas_user_by_email(self, user_email): # pylint: disable=inconsistent-return-statements
"""
Helper method to make an api call to Canvas using the user's email as a search term.
Expand Down Expand Up @@ -687,13 +688,34 @@ def _search_for_canvas_user_by_email(self, user_email):
get_users_by_email_response = rsps.json()

try:
canvas_user_id = get_users_by_email_response[0]['id']
canvas_user_id = get_users_by_email_response[0]["id"]
return canvas_user_id
except (KeyError, IndexError) as error:
raise ClientError(
"No Canvas user ID found associated with email: {}".format(user_email),
HTTPStatus.NOT_FOUND.value
) from error
return canvas_user_id
# learner is decommissioned on Canvas side - unlink it from enterprise
try:
enterprise_customer = self.enterprise_configuration.enterprise_customer
# Unlink user from related Enterprise Customer
EnterpriseCustomerUser.objects.unlink_user(
enterprise_customer=enterprise_customer,
user_email=user_email,
)
raise ClientError(
"No Canvas user ID found associated with email: {} - User unlinked from enterprise now".format(
user_email
),
HTTPStatus.NOT_FOUND.value,
) from error
except Exception as e: # pylint: disable=broad-except
LOGGER.error(
generate_formatted_log(
self.enterprise_configuration.channel_code(),
self.enterprise_configuration.enterprise_customer.uuid,
None,
None,
f"Error occurred while unlinking a Canvas learner: {user_email}. "
f"Error: {e}",
)
)

def _get_canvas_user_courses_by_id(self, user_id):
"""Helper method to retrieve all courses that a Canvas user is enrolled in."""
Expand Down
2 changes: 2 additions & 0 deletions integrated_channels/integrated_channel/admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,7 @@ class IntegratedChannelAPIRequestLogAdmin(admin.ModelAdmin):
"payload",
]

list_per_page = 100

class Meta:
model = IntegratedChannelAPIRequestLogs
49 changes: 19 additions & 30 deletions tests/test_integrated_channels/test_canvas/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

from django.apps import apps

from enterprise.models import EnterpriseCustomerUser
from integrated_channels.canvas.client import MESSAGE_WHEN_COURSE_WAS_DELETED, CanvasAPIClient
from integrated_channels.canvas.utils import CanvasUtil
from integrated_channels.exceptions import ClientError
Expand Down Expand Up @@ -161,39 +162,27 @@ def test_expires_at_is_updated_after_session_expiry(self):
canvas_api_client._create_session() # pylint: disable=protected-access
assert canvas_api_client.expires_at > orig_expires_at

@responses.activate
def test_search_for_canvas_user_with_400(self):
"""
Test that we properly raise exceptions if the client can't find the edx user in Canvas while reporting
grades (assessment and course level reporting both use the same method of retrieval).
Test that we properly raise exception and unlink user if the client can't find the edx user in Canvas
while reporting grades (assessment and course level reporting both use the same method of retrieval).
"""
with responses.RequestsMock() as rsps:
rsps.add(
responses.GET,
self.canvas_users_url,
body="[]",
status=200
)
canvas_api_client = CanvasAPIClient(self.enterprise_config)

# Searching for canvas users will require the session to be created
rsps.add(
responses.POST,
self.oauth_url,
json=self._token_response(),
status=200
)
canvas_api_client._create_session() # pylint: disable=protected-access

with pytest.raises(ClientError) as client_error:
canvas_api_client._search_for_canvas_user_by_email(self.canvas_email) # pylint: disable=protected-access
assert IntegratedChannelAPIRequestLogs.objects.count() == 2
assert client_error.value.message == \
"Course: {course_id} not found registered in Canvas for Edx " \
"learner: {canvas_email}/Canvas learner: {canvas_user_id}.".format(
course_id=self.course_id,
canvas_email=self.canvas_email,
canvas_user_id=self.canvas_user_id
)
responses.add(
responses.POST, self.oauth_url, json=self._token_response(), status=200
)
responses.add(responses.GET, self.canvas_users_url, json=[], status=200)
canvas_api_client = CanvasAPIClient(self.enterprise_config)
canvas_api_client._create_session() # pylint: disable=protected-access
assert responses.calls[0].request.url == self.oauth_url

with mock.patch.object(
EnterpriseCustomerUser.objects, "unlink_user"
) as unlink_user_mock:
canvas_api_client._search_for_canvas_user_by_email(self.canvas_email) # pylint: disable=protected-access
unlink_user_mock.assert_called_once()
assert len(responses.calls) == 2
assert IntegratedChannelAPIRequestLogs.objects.count() == 2

def test_assessment_reporting_with_no_canvas_course_found(self):
"""
Expand Down

0 comments on commit ba758ef

Please sign in to comment.