diff --git a/.gitignore b/.gitignore index 4d20bc0d..2b3e2712 100644 --- a/.gitignore +++ b/.gitignore @@ -99,3 +99,6 @@ docs/_build/ #transifex exe tx + +# Python version +.python-version diff --git a/commerce_coordinator/apps/core/models.py b/commerce_coordinator/apps/core/models.py index d170b281..af8e8181 100644 --- a/commerce_coordinator/apps/core/models.py +++ b/commerce_coordinator/apps/core/models.py @@ -1,9 +1,13 @@ """ Core models. """ +import logging + from django.contrib.auth.models import AbstractUser from django.db import models from django.utils.translation import gettext_lazy as _ +logger = logging.getLogger(__name__) + class User(AbstractUser): """ @@ -36,3 +40,49 @@ def get_full_name(self): def __str__(self): return str(self.get_full_name()) + + def add_lms_user_id(self, called_from): + """ + If this user does not already have an LMS user id, look for the id in social auth. If the id can be found, + add it to the user and save the user. + The LMS user_id may already be present for the user. It may have been added from the jwt (see the + EDX_DRF_EXTENSIONS.JWT_PAYLOAD_USER_ATTRIBUTE_MAPPING settings) or by a previous call to this method. + Arguments: + called_from (String): Descriptive string describing the caller. This will be included in log messages. + """ + if not self.lms_user_id: + # Check for the LMS user id in social auth + lms_user_id_social_auth, social_auth_id = self._get_lms_user_id_from_social_auth() + if lms_user_id_social_auth: + self.lms_user_id = lms_user_id_social_auth + self.save() + log_message = ( + 'Saving lms_user_id from social auth with id %s ' + 'for user %s. Called from %s', + social_auth_id, + self.id, + called_from + ) + logger.info(log_message) + else: + log_message = f'No lms_user_id found in social auth for user {self.id}. Called from {called_from}' + logger.warning(log_message) + + def _get_lms_user_id_from_social_auth(self): + """ + Find the LMS user_id passed through social auth. Because a single user_id can be associated with multiple + provider/uid combinations, start by checking the most recently saved social auth entry. + Returns: + (lms_user_id, social_auth_id): a tuple containing the LMS user id and the id of the social auth entry + where the LMS user id was found. Returns None, None if the LMS user id was not found. + """ + try: + auth_entries = self.social_auth.order_by('-id') + if auth_entries: + for auth_entry in auth_entries: + lms_user_id_social_auth = auth_entry.extra_data.get('user_id') + if lms_user_id_social_auth: + return lms_user_id_social_auth, auth_entry.id + except Exception: # pylint: disable=broad-except + logger.warning('Exception retrieving lms_user_id from social_auth for user %s.', self.id, exc_info=True) + return None, None diff --git a/commerce_coordinator/apps/core/tests/test_models.py b/commerce_coordinator/apps/core/tests/test_models.py index b6cce081..b9d0dd65 100644 --- a/commerce_coordinator/apps/core/tests/test_models.py +++ b/commerce_coordinator/apps/core/tests/test_models.py @@ -1,5 +1,6 @@ """ Tests for core models. """ +import mock from django.test import TestCase from django_dynamic_fixture import G from social_django.models import UserSocialAuth @@ -43,3 +44,74 @@ def test_string(self): full_name = 'Bob' user = G(User, full_name=full_name) self.assertEqual(str(user), full_name) + + def test_add_lms_user_id(self): + ''' + Verify lms_user_id is added to user if exists in social_auth entry. + ''' + full_name = 'Kosmo Kramer' + user = G(User, full_name=full_name) + assert user.lms_user_id is None + + UserSocialAuth.objects.create( + user=user, + provider='edx-oauth2', + uid='1', + extra_data={'user_id': 1337, 'access_token': 'access_token_1'} + ) + + user.add_lms_user_id('Calling from test') + user.refresh_from_db() + assert user.lms_user_id == 1337 + + def test_add_lms_user_id_does_not_change_if_exists(self): + full_name = 'Elaine Benes' + user = G(User, full_name=full_name, lms_user_id=1234) + assert user.lms_user_id == 1234 + + UserSocialAuth.objects.create( + user=user, + provider='edx-oauth2', + uid='1', + extra_data={'user_id': 9999, 'access_token': 'access_token_1'} + ) + + user.add_lms_user_id('Calling from test') + user.refresh_from_db() + assert user.lms_user_id == 1234 + + def test_add_lms_user_id_not_added_if_no_auth_entries(self): + ''' + If no auth_entries in social_auth, lms_user_id should not be updated. + ''' + full_name = 'Newman...' + user = G(User, full_name=full_name) + assert user.lms_user_id is None + + assert not UserSocialAuth.objects.filter(user=user).exists() + + user.add_lms_user_id('Calling from test') + user.refresh_from_db() + assert user.lms_user_id is None + + @mock.patch('commerce_coordinator.apps.core.models.User._get_lms_user_id_from_social_auth') + def test_add_lms_user_id_not_added_if_get_from_social_auth_fails(self, mock_get_lms_user_id_from_social_auth): + """ + If fetching social auth entry excepts, lms_user_id should not be updated. + """ + full_name = 'Jerry the Mouse' + user = G(User, full_name=full_name) + assert user.lms_user_id is None + + UserSocialAuth.objects.create( + user=user, + provider='edx-oauth2', + uid='1', + extra_data={'user_id': 1337, 'access_token': 'access_token_1'} + ) + + mock_get_lms_user_id_from_social_auth.side_effect = Exception('Something went wrong') + with self.assertRaises(Exception): + user.add_lms_user_id('Calling from test') + user.refresh_from_db() + assert user.lms_user_id is None diff --git a/commerce_coordinator/apps/frontend_app_ecommerce/views.py b/commerce_coordinator/apps/frontend_app_ecommerce/views.py index 8abeeaa4..a46a98f9 100644 --- a/commerce_coordinator/apps/frontend_app_ecommerce/views.py +++ b/commerce_coordinator/apps/frontend_app_ecommerce/views.py @@ -39,6 +39,8 @@ class RedirectReceiptView(APIView): def get(self, request): """Get the order history for the authenticated user.""" + user = request.user + user.add_lms_user_id("RedirectReceiptView GET method") order_number = request.query_params.get('order_number', None) if not order_number: @@ -73,6 +75,8 @@ class UserOrdersView(APIView): def get(self, request): """Return paginated response of user's order history.""" + user = request.user + user.add_lms_user_id("UserOrdersView GET method") # build parameters params = { 'username': request.user.username, diff --git a/commerce_coordinator/apps/lms/tasks.py b/commerce_coordinator/apps/lms/tasks.py index 93b403e5..5e941fd7 100644 --- a/commerce_coordinator/apps/lms/tasks.py +++ b/commerce_coordinator/apps/lms/tasks.py @@ -4,15 +4,16 @@ from celery import shared_task from celery.utils.log import get_task_logger +from django.contrib.auth import get_user_model from requests import RequestException from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient -from commerce_coordinator.apps.core.models import User from commerce_coordinator.apps.lms.clients import LMSAPIClient # Use the special Celery logger for our tasks logger = get_task_logger(__name__) +User = get_user_model() @shared_task(bind=True, autoretry_for=(RequestException,), retry_kwargs={'max_retries': 5, 'countdown': 3}) diff --git a/commerce_coordinator/apps/lms/views.py b/commerce_coordinator/apps/lms/views.py index b938ca1f..d5b5981a 100644 --- a/commerce_coordinator/apps/lms/views.py +++ b/commerce_coordinator/apps/lms/views.py @@ -58,6 +58,12 @@ def get(self, request): logger.debug(f'{self.get.__qualname__} headers: {request.headers}.') try: + user = request.user + user.add_lms_user_id("PaymentPageRedirectView GET method") + logger.info( + f"Received request to redirect user having lms_user_id: {user.lms_user_id} to checkout" + f" with query params: {list(self.request.GET.lists())}" + ) return self._redirect_response_payment(request) except OpenEdxFilterException as e: logger.exception(f"Something went wrong! Exception raised in {self.get.__name__} with error {repr(e)}")