Merge pull request #8127 from edx/jsa/xcom-306

Implement student-initiated refunds for external commerce service.

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):

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

lms/djangoapps/commerce/signals.py

+"""
+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 = ['<a href="{0}">{0}</a>'.format(url) for url in refund_urls]
+    html_body = '<p>{}</p>'.format('<br>'.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()

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/'

lms/djangoapps/commerce/tests/test_signals.py

+"""
+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)