Implement student-initiated refunds for external commerce service.
XCOM-306
This commit is contained in:
@@ -2,6 +2,7 @@ import threading
|
|||||||
|
|
||||||
_request_cache_threadlocal = threading.local()
|
_request_cache_threadlocal = threading.local()
|
||||||
_request_cache_threadlocal.data = {}
|
_request_cache_threadlocal.data = {}
|
||||||
|
_request_cache_threadlocal.request = None
|
||||||
|
|
||||||
|
|
||||||
class RequestCache(object):
|
class RequestCache(object):
|
||||||
@@ -9,11 +10,21 @@ class RequestCache(object):
|
|||||||
def get_request_cache(cls):
|
def get_request_cache(cls):
|
||||||
return _request_cache_threadlocal
|
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):
|
def clear_request_cache(self):
|
||||||
_request_cache_threadlocal.data = {}
|
_request_cache_threadlocal.data = {}
|
||||||
|
_request_cache_threadlocal.request = None
|
||||||
|
|
||||||
def process_request(self, request):
|
def process_request(self, request):
|
||||||
self.clear_request_cache()
|
self.clear_request_cache()
|
||||||
|
_request_cache_threadlocal.request = request
|
||||||
return None
|
return None
|
||||||
|
|
||||||
def process_response(self, request, response):
|
def process_response(self, request, response):
|
||||||
|
|||||||
@@ -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):
|
def ecommerce_api_client(user):
|
||||||
""" Returns an E-Commerce API client setup with authentication for the specified 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,
|
return EcommerceApiClient(settings.ECOMMERCE_API_URL, settings.ECOMMERCE_API_SIGNING_KEY, user.username,
|
||||||
user.profile.name, user.email, tracking_context=create_tracking_context(user))
|
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
|
||||||
|
|||||||
149
lms/djangoapps/commerce/signals.py
Normal file
149
lms/djangoapps/commerce/signals.py
Normal file
@@ -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 = ['<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()
|
||||||
@@ -93,3 +93,13 @@ class mock_basket_order(mock_ecommerce_api_endpoint): # pylint: disable=invalid
|
|||||||
|
|
||||||
def get_uri(self):
|
def get_uri(self):
|
||||||
return TEST_API_URL + '/baskets/{}/order/'.format(self.basket_id)
|
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/'
|
||||||
|
|||||||
200
lms/djangoapps/commerce/tests/test_signals.py
Normal file
200
lms/djangoapps/commerce/tests/test_signals.py
Normal file
@@ -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)
|
||||||
Reference in New Issue
Block a user