Skip to content

E
edx-platform
Commit 7c39978b by Clinton Blackburn Committed by Clinton Blackburn
Options

Automated refund approval process 

Refunds are now processed automatically. If the automated processing fails, the system falls back to the previous behavior of notifying the Support Team. Additionally, these calls are all made by the service user rather than the learner.

ECOM-6541
parent 1189867d
Showing with 157 additions and 85 deletions
lms/djangoapps/commerce/migrations/0005_commerceconfiguration_enable_automatic_refund_approval.py 0 → 100644
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('commerce', '0004_auto_20160531_0950'),
]
operations = [
migrations.AddField(
model_name='commerceconfiguration',
name='enable_automatic_refund_approval',
field=models.BooleanField(default=True, help_text='Automatically approve valid refund requests, without manual processing'),
),
]
lms/djangoapps/commerce/models.py
......@@ -39,6 +39,10 @@ class CommerceConfiguration(ConfigurationModel):
default=DEFAULT_RECEIPT_PAGE_URL,
help_text=_('Path to order receipt page.')
)
enable_automatic_refund_approval = models.BooleanField(
default=True,
help_text=_('Automatically approve valid refund requests, without manual processing')
)
def __unicode__(self):
return "Commerce configuration"
......
lms/djangoapps/commerce/signals.py
......@@ -9,23 +9,24 @@ from urlparse import urljoin
import requests
from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.models import AnonymousUser
from django.dispatch import receiver
from django.utils.translation import ugettext as _
from edx_rest_api_client.exceptions import HttpClientError
from request_cache.middleware import RequestCache
from student.models import UNENROLL_DONE
from commerce.models import CommerceConfiguration
from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, is_commerce_service_configured
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.theming import helpers as theming_helpers
from request_cache.middleware import RequestCache
from student.models import UNENROLL_DONE
log = logging.getLogger(__name__)
# pylint: disable=unused-argument
@receiver(UNENROLL_DONE)
def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False,
**kwargs): # pylint: disable=unused-argument
def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kwargs):
"""
Signal receiver for unenrollments, used to automatically initiate refunds
when applicable.
......@@ -40,12 +41,12 @@ def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False,
request_user = get_request_user() or course_enrollment.user
if isinstance(request_user, AnonymousUser):
# Assume the request was initiated via server-to-server
# api call (presumably Otto). In this case we cannot
# API call (presumably Otto). In this case we cannot
# construct a client to call Otto back anyway, because
# the client does not work anonymously, and furthermore,
# there's certainly no need to inform Otto about this request.
return
refund_seat(course_enrollment, request_user)
refund_seat(course_enrollment)
except: # pylint: disable=bare-except
# don't assume the signal was fired with `send_robust`.
# avoid blowing up other signal handlers by gracefully
......@@ -69,78 +70,76 @@ def get_request_user():
return getattr(request, 'user', None)
def refund_seat(course_enrollment, request_user):
def refund_seat(course_enrollment):
"""
Attempt to initiate a refund for any orders associated with the seat being
unenrolled, using the commerce service.
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.
exceptions.SlumberBaseException: for any unhandled HTTP error during communication with the E-Commerce Service.
exceptions.Timeout: if the attempt to reach the commerce service timed out.
"""
User = get_user_model() # pylint:disable=invalid-name
course_key_str = unicode(course_enrollment.course_id)
unenrolled_user = course_enrollment.user
enrollee = 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.id, unenrolled_user.id, course_key_str)
return []
else:
# no other error is anticipated, so re-raise the Exception
raise exc
service_user = User.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME)
api_client = ecommerce_api_client(service_user)
log.info('Attempting to create a refund for user [%s], course [%s]...', enrollee.id, course_key_str)
refund_ids = api_client.refunds.post({'course_id': course_key_str, 'username': enrollee.username})
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,
)
# XCOM-371: this is a temporary measure to suppress refund-related email
# notifications to students and support@) for free enrollments. This
# condition should be removed when the CourseEnrollment.refundable() logic
# is updated to be more correct, or when we implement better handling (and
# notifications) in Otto for handling reversal of $0 transactions.
if course_enrollment.mode != 'verified':
# 'verified' is the only enrollment mode that should presently
# result in opening a refund request.
log.info(
"Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]",
course_enrollment.user.id,
course_enrollment.course_id,
course_enrollment.mode,
)
log.info('Refund successfully opened for user [%s], course [%s]: %r', enrollee.id, course_key_str, refund_ids)
config = CommerceConfiguration.current()
if config.enable_automatic_refund_approval:
refunds_requiring_approval = []
for refund_id in refund_ids:
try:
# NOTE: Approve payment only because the user has already been unenrolled. Additionally, this
# ensures we don't tie up an additional web worker when the E-Commerce Service tries to unenroll
# the learner
api_client.refunds(refund_id).process.put({'action': 'approve_payment_only'})
log.info('Refund [%d] successfully approved.', refund_id)
except: # pylint: disable=bare-except
log.exception('Failed to automatically approve refund [%d]!', refund_id)
refunds_requiring_approval.append(refund_id)
else:
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)
refunds_requiring_approval = refund_ids
if refunds_requiring_approval:
# XCOM-371: this is a temporary measure to suppress refund-related email
# notifications to students and support for free enrollments. This
# condition should be removed when the CourseEnrollment.refundable() logic
# is updated to be more correct, or when we implement better handling (and
# notifications) in Otto for handling reversal of $0 transactions.
if course_enrollment.mode != 'verified':
# 'verified' is the only enrollment mode that should presently
# result in opening a refund request.
log.info(
'Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]',
course_enrollment.user.id,
course_enrollment.course_id,
course_enrollment.mode,
)
else:
try:
send_refund_notification(course_enrollment, refunds_requiring_approval)
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)
log.info('No refund opened for user [%s], course [%s]', enrollee.id, course_key_str)
return refund_ids
......
lms/djangoapps/commerce/tests/mocks.py
......@@ -26,7 +26,7 @@ class mock_ecommerce_api_endpoint(object):
host = settings.ECOMMERCE_API_URL.strip('/')
def __init__(self, response=None, status=200, expect_called=True, exception=None):
def __init__(self, response=None, status=200, expect_called=True, exception=None, reset_on_exit=True):
"""
Keyword Arguments:
response: a JSON-serializable Python type representing the desired response body.
......@@ -34,11 +34,13 @@ class mock_ecommerce_api_endpoint(object):
expect_called: a boolean indicating whether an API request was expected; set
to False if we should ensure that no request arrived.
exception: raise this exception instead of returning an HTTP response when called.
reset_on_exit (bool): Indicates if `httpretty` should be reset after the decorator exits.
"""
self.response = response or self.default_response
self.status = status
self.expect_called = expect_called
self.exception = exception
self.reset_on_exit = reset_on_exit
def get_uri(self):
"""
......@@ -74,7 +76,8 @@ class mock_ecommerce_api_endpoint(object):
def __exit__(self, exc_type, exc_val, exc_tb):
assert self.expect_called == (httpretty.last_request().headers != {})
httpretty.disable()
httpretty.reset()
if self.reset_on_exit:
httpretty.reset()
class mock_create_basket(mock_ecommerce_api_endpoint):
......@@ -119,6 +122,20 @@ class mock_create_refund(mock_ecommerce_api_endpoint):
return '/refunds/'
class mock_process_refund(mock_ecommerce_api_endpoint):
""" Mocks calls to E-Commerce API client refund process method. """
default_response = []
method = httpretty.PUT
def __init__(self, refund_id, **kwargs):
super(mock_process_refund, self).__init__(**kwargs)
self.refund_id = refund_id
def get_path(self):
return '/refunds/{}/process/'.format(self.refund_id)
class mock_order_endpoint(mock_ecommerce_api_endpoint):
""" Mocks calls to E-Commerce API client basket order method. """
......
lms/djangoapps/commerce/tests/test_signals.py
......@@ -11,17 +11,17 @@ from urlparse import urljoin
import ddt
import httpretty
import mock
from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from django.test import TestCase
from django.test.utils import override_settings
from opaque_keys.edx.keys import CourseKey
from requests import Timeout
from commerce.signals import (
refund_seat, send_refund_notification, generate_refund_notification_body, create_zendesk_ticket
)
from commerce.models import CommerceConfiguration
from commerce.signals import send_refund_notification, generate_refund_notification_body, create_zendesk_ticket
from commerce.tests import JSON
from commerce.tests.mocks import mock_create_refund
from commerce.tests.mocks import mock_create_refund, mock_process_refund
from course_modes.models import CourseMode
from student.models import UNENROLL_DONE
from student.tests.factories import UserFactory, CourseEnrollmentFactory
......@@ -40,6 +40,10 @@ class TestRefundSignal(TestCase):
def setUp(self):
super(TestRefundSignal, self).setUp()
# Ensure the E-Commerce service user exists
UserFactory(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME, is_staff=True)
self.requester = UserFactory(username="test-requester")
self.student = UserFactory(
username="test-student",
......@@ -52,6 +56,10 @@ class TestRefundSignal(TestCase):
)
self.course_enrollment.refundable = mock.Mock(return_value=True)
self.config = CommerceConfiguration.current()
self.config.enable_automatic_refund_approval = True
self.config.save()
def send_signal(self, skip_refund=False):
"""
DRY helper: emit the UNENROLL_DONE signal, as is done in
......@@ -84,7 +92,7 @@ class TestRefundSignal(TestCase):
"""
self.send_signal()
self.assertTrue(mock_refund_seat.called)
self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment, self.student))
self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
# if skip_refund is set to True in the signal, we should not try to initiate a refund.
mock_refund_seat.reset_mock()
......@@ -106,21 +114,21 @@ class TestRefundSignal(TestCase):
# 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))
self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
# 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))
self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
# 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))
self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,))
# HTTP user is another server (AnonymousUser): do not try to initiate a refund at all.
mock_get_request_user.return_value = AnonymousUser()
......@@ -128,15 +136,6 @@ class TestRefundSignal(TestCase):
self.send_signal()
self.assertFalse(mock_refund_seat.called)
@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, UserFactory())
self.assertTrue(mock_log_warning.called)
@mock.patch('commerce.signals.log.exception')
def test_error_logging(self, mock_log_exception):
"""
......@@ -148,14 +147,48 @@ class TestRefundSignal(TestCase):
self.assertTrue(mock_log_exception.called)
@mock.patch('commerce.signals.send_refund_notification')
def test_notification(self, mock_send_notification):
def test_notification_when_approval_fails(self, mock_send_notification):
"""
Ensure the notification function is triggered when refunds are
initiated
Ensure the notification function is triggered when refunds are initiated, and cannot be automatically approved.
"""
with mock_create_refund(status=200, response=[1, 2, 3]):
refund_id = 1
failed_refund_id = 2
with mock_create_refund(status=201, response=[refund_id, failed_refund_id]):
with mock_process_refund(refund_id, reset_on_exit=False):
with mock_process_refund(failed_refund_id, status=500, reset_on_exit=False):
self.send_signal()
self.assertTrue(mock_send_notification.called)
mock_send_notification.assert_called_with(self.course_enrollment, [failed_refund_id])
@mock.patch('commerce.signals.send_refund_notification')
def test_notification_if_automatic_approval_disabled(self, mock_send_notification):
"""
Ensure the notification is always sent if the automatic approval functionality is disabled.
"""
refund_id = 1
self.config.enable_automatic_refund_approval = False
self.config.save()
with mock_create_refund(status=201, response=[refund_id]):
self.send_signal()
self.assertTrue(mock_send_notification.called)
mock_send_notification.assert_called_with(self.course_enrollment, [refund_id])
@mock.patch('commerce.signals.send_refund_notification')
def test_no_notification_after_approval(self, mock_send_notification):
"""
Ensure the notification function is triggered when refunds are initiated, and cannot be automatically approved.
"""
refund_id = 1
with mock_create_refund(status=201, response=[refund_id]):
with mock_process_refund(refund_id, reset_on_exit=False):
self.send_signal()
self.assertFalse(mock_send_notification.called)
last_request = httpretty.last_request()
self.assertDictEqual(json.loads(last_request.body), {'action': 'approve_payment_only'})
@mock.patch('commerce.signals.send_refund_notification')
def test_notification_no_refund(self, mock_send_notification):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment