diff --git a/common/djangoapps/request_cache/middleware.py b/common/djangoapps/request_cache/middleware.py index 4b5ce114b0..72c6d4114e 100644 --- a/common/djangoapps/request_cache/middleware.py +++ b/common/djangoapps/request_cache/middleware.py @@ -2,6 +2,7 @@ import threading _request_cache_threadlocal = threading.local() _request_cache_threadlocal.data = {} +_request_cache_threadlocal.request = None class RequestCache(object): @@ -9,11 +10,21 @@ class RequestCache(object): def get_request_cache(cls): return _request_cache_threadlocal + @classmethod + def get_current_request(cls): + """ + Get a reference to the HttpRequest object, if we are presently + servicing one. + """ + return _request_cache_threadlocal.request + def clear_request_cache(self): _request_cache_threadlocal.data = {} + _request_cache_threadlocal.request = None def process_request(self, request): self.clear_request_cache() + _request_cache_threadlocal.request = request return None def process_response(self, request, response): diff --git a/lms/djangoapps/commerce/__init__.py b/lms/djangoapps/commerce/__init__.py index 8fcbe9ea51..53d676cd5e 100644 --- a/lms/djangoapps/commerce/__init__.py +++ b/lms/djangoapps/commerce/__init__.py @@ -13,7 +13,19 @@ def create_tracking_context(user): } +def is_commerce_service_configured(): + """ + Return a Boolean indicating whether or not configuration is present to use + the external commerce service. + """ + return bool(settings.ECOMMERCE_API_URL and settings.ECOMMERCE_API_SIGNING_KEY) + + def ecommerce_api_client(user): """ Returns an E-Commerce API client setup with authentication for the specified user. """ return EcommerceApiClient(settings.ECOMMERCE_API_URL, settings.ECOMMERCE_API_SIGNING_KEY, user.username, user.profile.name, user.email, tracking_context=create_tracking_context(user)) + + +# this is here to support registering the signals in signals.py +from commerce import signals # pylint: disable=unused-import diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py new file mode 100644 index 0000000000..5781167a6c --- /dev/null +++ b/lms/djangoapps/commerce/signals.py @@ -0,0 +1,149 @@ +""" +Signal handling functions for use with external commerce service. +""" +import logging +from urlparse import urlparse + +from django.conf import settings +from django.core.mail import EmailMultiAlternatives +from django.dispatch import receiver +from django.utils.translation import ugettext as _ +from ecommerce_api_client.exceptions import HttpClientError +from microsite_configuration import microsite +from request_cache.middleware import RequestCache +from student.models import UNENROLL_DONE + +from commerce import ecommerce_api_client, is_commerce_service_configured + +log = logging.getLogger(__name__) + + +@receiver(UNENROLL_DONE) +def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kwargs): # pylint: disable=unused-argument + """ + Signal receiver for unenrollments, used to automatically initiate refunds + when applicable. + + N.B. this signal is also consumed by lms.djangoapps.shoppingcart. + """ + if not is_commerce_service_configured() or skip_refund: + return + + if course_enrollment and course_enrollment.refundable(): + try: + request_user = get_request_user() or course_enrollment.user + refund_seat(course_enrollment, request_user) + except: # pylint: disable=bare-except + # don't assume the signal was fired with `send_robust`. + # avoid blowing up other signal handlers by gracefully + # trapping the Exception and logging an error. + log.exception( + "Unexpected exception while attempting to initiate refund for user [%s], course [%s]", + course_enrollment.user, + course_enrollment.course_id, + ) + + +def get_request_user(): + """ + Helper to get the authenticated user from the current HTTP request (if + applicable). + + If the requester of an unenrollment is not the same person as the student + being unenrolled, we authenticate to the commerce service as the requester. + """ + request = RequestCache.get_current_request() + return getattr(request, 'user', None) + + +def refund_seat(course_enrollment, request_user): + """ + Attempt to initiate a refund for any orders associated with the seat being + unenrolled, using the commerce service. + + Arguments: + course_enrollment (CourseEnrollment): a student enrollment + request_user: the user as whom to authenticate to the commerce service + when attempting to initiate the refund. + + Returns: + A list of the external service's IDs for any refunds that were initiated + (may be empty). + + Raises: + exceptions.SlumberBaseException: for any unhandled HTTP error during + communication with the commerce service. + exceptions.Timeout: if the attempt to reach the commerce service timed + out. + + """ + course_key_str = unicode(course_enrollment.course_id) + unenrolled_user = course_enrollment.user + + try: + refund_ids = ecommerce_api_client(request_user or unenrolled_user).refunds.post( + course_id=course_key_str, + username=unenrolled_user.username, + ) + except HttpClientError, exc: + if exc.response.status_code == 403 and request_user != unenrolled_user: + # this is a known limitation; commerce service does not presently + # support the case of a non-superusers initiating a refund on + # behalf of another user. + log.warning("User [%s] was not authorized to initiate a refund for user [%s] " + "upon unenrollment from course [%s]", request_user, unenrolled_user, course_key_str) + return [] + else: + # no other error is anticipated, so re-raise the Exception + raise exc + + if refund_ids: + # at least one refundable order was found. + log.info( + "Refund successfully opened for user [%s], course [%s]: %r", + unenrolled_user.id, + course_key_str, + refund_ids, + ) + try: + send_refund_notification(course_enrollment, refund_ids) + except: # pylint: disable=bare-except + # don't break, just log a warning + log.warning("Could not send email notification for refund.", exc_info=True) + else: + # no refundable orders were found. + log.debug("No refund opened for user [%s], course [%s]", unenrolled_user.id, course_key_str) + + return refund_ids + + +def send_refund_notification(course_enrollment, refund_ids): + """ + Issue an email notification to the configured email recipient about a + newly-initiated refund request. + + This function does not do any exception handling; callers are responsible + for capturing and recovering from any errors. + """ + if microsite.is_request_in_microsite(): + # this is not presently supported with the external service. + raise NotImplementedError("Unable to send refund processing emails to microsite teams.") + + for_user = course_enrollment.user + subject = _("[Refund] User-Requested Refund") + message = _( + "A refund request has been initiated for {username} ({email}). To process this request, please visit the link(s) below." # pylint: disable=line-too-long + ).format(username=for_user.username, email=for_user.email) + + # TODO: this manipulation of API url is temporary, pending the introduction + # of separate configuration items for the service's base url and api path. + ecommerce_url = '://'.join(urlparse(settings.ECOMMERCE_API_URL)[:2]) + + refund_urls = ["{}/dashboard/refunds/{}/".format(ecommerce_url, refund_id) for refund_id in refund_ids] + text_body = '\r\n'.join([message] + refund_urls + ['']) + refund_links = ['{0}'.format(url) for url in refund_urls] + html_body = '

{}

'.format('
'.join([message] + refund_links)) + + email_message = EmailMultiAlternatives(subject, text_body, for_user.email, [settings.PAYMENT_SUPPORT_EMAIL]) + email_message.attach_alternative(html_body, "text/html") + email_message.send() diff --git a/lms/djangoapps/commerce/tests/mocks.py b/lms/djangoapps/commerce/tests/mocks.py index 9f12805ce0..1eee94b056 100644 --- a/lms/djangoapps/commerce/tests/mocks.py +++ b/lms/djangoapps/commerce/tests/mocks.py @@ -93,3 +93,13 @@ class mock_basket_order(mock_ecommerce_api_endpoint): # pylint: disable=invalid def get_uri(self): return TEST_API_URL + '/baskets/{}/order/'.format(self.basket_id) + + +class mock_create_refund(mock_ecommerce_api_endpoint): # pylint: disable=invalid-name + """ Mocks calls to E-Commerce API client refund creation method. """ + + default_response = [] + method = httpretty.POST + + def get_uri(self): + return TEST_API_URL + '/refunds/' diff --git a/lms/djangoapps/commerce/tests/test_signals.py b/lms/djangoapps/commerce/tests/test_signals.py new file mode 100644 index 0000000000..0d354622a0 --- /dev/null +++ b/lms/djangoapps/commerce/tests/test_signals.py @@ -0,0 +1,200 @@ +""" +Tests for signal handling in commerce djangoapp. +""" +from django.test import TestCase +from django.test.utils import override_settings +from urlparse import urlparse + +import mock +from opaque_keys.edx.keys import CourseKey +from student.models import UNENROLL_DONE +from student.tests.factories import UserFactory, CourseEnrollmentFactory + +from commerce.signals import refund_seat, send_refund_notification +from commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY +from commerce.tests.mocks import mock_create_refund + + +# TODO: this is temporary. See the corresponding TODO in signals.send_refund_notification +TEST_BASE_URL = '://'.join(urlparse(TEST_API_URL)[:2]) + + +@override_settings(ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY) +class TestRefundSignal(TestCase): + """ + Exercises logic triggered by the UNENROLL_DONE signal. + """ + + def setUp(self): + super(TestRefundSignal, self).setUp() + self.requester = UserFactory(username="test-requester") + self.student = UserFactory(username="test-student", email="test-student@example.com") + self.course_enrollment = CourseEnrollmentFactory( + user=self.student, + course_id=CourseKey.from_string('course-v1:org+course+run'), + ) + self.course_enrollment.refundable = mock.Mock(return_value=True) + + def send_signal(self, skip_refund=False): + """ + DRY helper: emit the UNENROLL_DONE signal, as is done in + common.djangoapps.student.models after a successful unenrollment. + """ + UNENROLL_DONE.send(sender=None, course_enrollment=self.course_enrollment, skip_refund=skip_refund) + + @override_settings(ECOMMERCE_API_URL=None, ECOMMERCE_API_SIGNING_KEY=None) + def test_no_service(self): + """ + Ensure that the receiver quietly bypasses attempts to initiate + refunds when there is no external service configured. + """ + with mock.patch('commerce.signals.refund_seat') as mock_refund_seat: + self.send_signal() + self.assertFalse(mock_refund_seat.called) + + @mock.patch('commerce.signals.refund_seat') + def test_receiver(self, mock_refund_seat): + """ + Ensure that the UNENROLL_DONE signal triggers correct calls to + refund_seat(), when it is appropriate to do so. + + TODO (jsa): ideally we would assert that the signal receiver got wired + up independently of the import statement in this module. I'm not aware + of any reliable / sane way to do this. + """ + self.send_signal() + self.assertTrue(mock_refund_seat.called) + self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student)) + + # if skip_refund is set to True in the signal, we should not try to initiate a refund. + mock_refund_seat.reset_mock() + self.send_signal(skip_refund=True) + self.assertFalse(mock_refund_seat.called) + + # if the course_enrollment is not refundable, we should not try to initiate a refund. + mock_refund_seat.reset_mock() + self.course_enrollment.refundable = mock.Mock(return_value=False) + self.send_signal() + self.assertFalse(mock_refund_seat.called) + + @mock.patch('commerce.signals.refund_seat') + @mock.patch('commerce.signals.get_request_user', return_value=None) + def test_requester(self, mock_get_request_user, mock_refund_seat): + """ + Ensure the right requester is specified when initiating refunds. + """ + # no HTTP request/user: auth to commerce service as the unenrolled student. + self.send_signal() + self.assertTrue(mock_refund_seat.called) + self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student)) + + # HTTP user is the student: auth to commerce service as the unenrolled student. + mock_get_request_user.return_value = self.student + mock_refund_seat.reset_mock() + self.send_signal() + self.assertTrue(mock_refund_seat.called) + self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student)) + + # HTTP user is another user: auth to commerce service as the requester. + mock_get_request_user.return_value = self.requester + mock_refund_seat.reset_mock() + self.send_signal() + self.assertTrue(mock_refund_seat.called) + self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.requester)) + + @mock.patch('commerce.signals.log.warning') + def test_not_authorized_warning(self, mock_log_warning): + """ + Ensure that expected authorization issues are logged as warnings. + """ + with mock_create_refund(status=403): + refund_seat(self.course_enrollment, None) + self.assertTrue(mock_log_warning.called) + + @mock.patch('commerce.signals.log.exception') + def test_error_logging(self, mock_log_exception): + """ + Ensure that unexpected Exceptions are logged as errors (but do not + break program flow). + """ + with mock_create_refund(status=500): + self.send_signal() + self.assertTrue(mock_log_exception.called) + + @mock.patch('commerce.signals.send_refund_notification') + def test_notification(self, mock_send_notificaton): + """ + Ensure the notification function is triggered when refunds are + initiated + """ + with mock_create_refund(status=200, response=[1, 2, 3]): + self.send_signal() + self.assertTrue(mock_send_notificaton.called) + + @mock.patch('commerce.signals.send_refund_notification') + def test_notification_no_refund(self, mock_send_notification): + """ + Ensure the notification function is NOT triggered when no refunds are + initiated + """ + with mock_create_refund(status=200, response=[]): + self.send_signal() + self.assertFalse(mock_send_notification.called) + + @mock.patch('commerce.signals.send_refund_notification', side_effect=Exception("Splat!")) + @mock.patch('commerce.signals.log.warning') + def test_notification_error(self, mock_log_warning, mock_send_notification): + """ + Ensure an error occuring during notification does not break program + flow, but a warning is logged. + """ + with mock_create_refund(status=200, response=[1, 2, 3]): + self.send_signal() + self.assertTrue(mock_send_notification.called) + self.assertTrue(mock_log_warning.called) + + @mock.patch('commerce.signals.microsite.is_request_in_microsite', return_value=True) + def test_notification_microsite(self, mock_is_request_in_microsite): # pylint: disable=unused-argument + """ + Ensure the notification function raises an Exception if used in the + context of microsites. + """ + with self.assertRaises(NotImplementedError): + send_refund_notification(self.course_enrollment, [1, 2, 3]) + + @override_settings(PAYMENT_SUPPORT_EMAIL='payment@example.com') + @mock.patch('commerce.signals.EmailMultiAlternatives') + def test_notification_content(self, mock_email_class): + """ + Ensure the email sender, recipient, subject, content type, and content + are all correct. + """ + # mock_email_class is the email message class/constructor. + # mock_message is the instance returned by the constructor. + # we need to make assertions regarding both. + mock_message = mock.MagicMock() + mock_email_class.return_value = mock_message + + refund_ids = [1, 2, 3] + send_refund_notification(self.course_enrollment, refund_ids) + + # check headers and text content + self.assertEqual( + mock_email_class.call_args[0], + ("[Refund] User-Requested Refund", mock.ANY, self.student.email, ['payment@example.com']), + ) + text_body = mock_email_class.call_args[0][1] + # check for a URL for each refund + for exp in [r'{0}/dashboard/refunds/{1}/'.format(TEST_BASE_URL, refund_id) for refund_id in refund_ids]: + self.assertRegexpMatches(text_body, exp) + + # check HTML content + self.assertEqual(mock_message.attach_alternative.call_args[0], (mock.ANY, "text/html")) + html_body = mock_message.attach_alternative.call_args[0][0] + # check for a link to each refund + for exp in [r'a href="{0}/dashboard/refunds/{1}/"'.format(TEST_BASE_URL, refund_id) + for refund_id in refund_ids]: + self.assertRegexpMatches(html_body, exp) + + # make sure we actually SEND the message too. + self.assertTrue(mock_message.send.called)