diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py
index 446815132a..0d54ab4e7d 100644
--- a/lms/djangoapps/commerce/signals.py
+++ b/lms/djangoapps/commerce/signals.py
@@ -2,7 +2,7 @@
Signal handling functions for use with external commerce service.
"""
import logging
-from urlparse import urlparse
+from urlparse import urljoin
from django.conf import settings
from django.core.mail import EmailMultiAlternatives
@@ -39,7 +39,7 @@ def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kw
# 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.user.id,
course_enrollment.course_id,
)
@@ -90,7 +90,7 @@ def refund_seat(course_enrollment, request_user):
# 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)
+ "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
@@ -104,11 +104,27 @@ def refund_seat(course_enrollment, request_user):
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)
+
+ # 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, 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)
@@ -131,14 +147,14 @@ def send_refund_notification(course_enrollment, refund_ids):
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
+ "A refund request has been initiated for {username} ({email}). "
+ "To process this request, please visit the link(s) below."
).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]
+ refund_urls = [
+ urljoin(settings.ECOMMERCE_PUBLIC_URL_ROOT, '/dashboard/refunds/{}/'.format(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))
diff --git a/lms/djangoapps/commerce/tests/__init__.py b/lms/djangoapps/commerce/tests/__init__.py
index 0601b0cc17..4af326a662 100644
--- a/lms/djangoapps/commerce/tests/__init__.py
+++ b/lms/djangoapps/commerce/tests/__init__.py
@@ -12,7 +12,8 @@ from commerce import ecommerce_api_client
from student.tests.factories import UserFactory
-TEST_API_URL = 'http://example.com/api'
+TEST_PUBLIC_URL_ROOT = 'http://www.example.com'
+TEST_API_URL = 'http://www-internal.example.com/api'
TEST_API_SIGNING_KEY = 'edx'
TEST_BASKET_ID = 7
TEST_ORDER_NUMBER = '100004'
diff --git a/lms/djangoapps/commerce/tests/test_signals.py b/lms/djangoapps/commerce/tests/test_signals.py
index 0d354622a0..85eb7cb591 100644
--- a/lms/djangoapps/commerce/tests/test_signals.py
+++ b/lms/djangoapps/commerce/tests/test_signals.py
@@ -3,23 +3,25 @@ Tests for signal handling in commerce djangoapp.
"""
from django.test import TestCase
from django.test.utils import override_settings
-from urlparse import urlparse
+from course_modes.models import CourseMode
+import ddt
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 import TEST_PUBLIC_URL_ROOT, 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)
+@ddt.ddt
+@override_settings(
+ ECOMMERCE_PUBLIC_URL_ROOT=TEST_PUBLIC_URL_ROOT,
+ 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.
@@ -32,6 +34,7 @@ class TestRefundSignal(TestCase):
self.course_enrollment = CourseEnrollmentFactory(
user=self.student,
course_id=CourseKey.from_string('course-v1:org+course+run'),
+ mode=CourseMode.VERIFIED,
)
self.course_enrollment.refundable = mock.Mock(return_value=True)
@@ -42,7 +45,11 @@ class TestRefundSignal(TestCase):
"""
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)
+ @override_settings(
+ ECOMMERCE_PUBLIC_URL_ROOT=None,
+ ECOMMERCE_API_URL=None,
+ ECOMMERCE_API_SIGNING_KEY=None,
+ )
def test_no_service(self):
"""
Ensure that the receiver quietly bypasses attempts to initiate
@@ -108,7 +115,7 @@ class TestRefundSignal(TestCase):
Ensure that expected authorization issues are logged as warnings.
"""
with mock_create_refund(status=403):
- refund_seat(self.course_enrollment, None)
+ refund_seat(self.course_enrollment, UserFactory())
self.assertTrue(mock_log_warning.called)
@mock.patch('commerce.signals.log.exception')
@@ -122,14 +129,14 @@ class TestRefundSignal(TestCase):
self.assertTrue(mock_log_exception.called)
@mock.patch('commerce.signals.send_refund_notification')
- def test_notification(self, mock_send_notificaton):
+ def test_notification(self, mock_send_notification):
"""
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)
+ self.assertTrue(mock_send_notification.called)
@mock.patch('commerce.signals.send_refund_notification')
def test_notification_no_refund(self, mock_send_notification):
@@ -141,6 +148,27 @@ class TestRefundSignal(TestCase):
self.send_signal()
self.assertFalse(mock_send_notification.called)
+ @mock.patch('commerce.signals.send_refund_notification')
+ @ddt.data(
+ CourseMode.HONOR,
+ CourseMode.PROFESSIONAL,
+ CourseMode.AUDIT,
+ CourseMode.NO_ID_PROFESSIONAL_MODE,
+ CourseMode.CREDIT_MODE,
+ )
+ def test_notification_not_verified(self, mode, mock_send_notification):
+ """
+ Ensure the notification function is NOT triggered when the
+ unenrollment is for any mode other than verified (i.e. any mode other
+ than one for which refunds are presently supported). See the
+ TODO associated with XCOM-371 in the signals module in the commerce
+ package for more information.
+ """
+ self.course_enrollment.mode = mode
+ with mock_create_refund(status=200, response=[1, 2, 3]):
+ 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):
@@ -185,14 +213,15 @@ class TestRefundSignal(TestCase):
)
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]:
+ for exp in [r'{0}/dashboard/refunds/{1}/'.format(TEST_PUBLIC_URL_ROOT, 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 exp in [r'a href="{0}/dashboard/refunds/{1}/"'.format(TEST_PUBLIC_URL_ROOT, refund_id)
for refund_id in refund_ids]:
self.assertRegexpMatches(html_body, exp)
diff --git a/lms/envs/aws.py b/lms/envs/aws.py
index 69df5d519f..7d8418cf19 100644
--- a/lms/envs/aws.py
+++ b/lms/envs/aws.py
@@ -583,6 +583,7 @@ CDN_VIDEO_URLS = ENV_TOKENS.get('CDN_VIDEO_URLS', CDN_VIDEO_URLS)
ONLOAD_BEACON_SAMPLE_RATE = ENV_TOKENS.get('ONLOAD_BEACON_SAMPLE_RATE', ONLOAD_BEACON_SAMPLE_RATE)
##### ECOMMERCE API CONFIGURATION SETTINGS #####
+ECOMMERCE_PUBLIC_URL_ROOT = ENV_TOKENS.get('ECOMMERCE_PUBLIC_URL_ROOT', ECOMMERCE_PUBLIC_URL_ROOT)
ECOMMERCE_API_URL = ENV_TOKENS.get('ECOMMERCE_API_URL', ECOMMERCE_API_URL)
ECOMMERCE_API_SIGNING_KEY = AUTH_TOKENS.get('ECOMMERCE_API_SIGNING_KEY', ECOMMERCE_API_SIGNING_KEY)
ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TIMEOUT)
diff --git a/lms/envs/common.py b/lms/envs/common.py
index baa3b8f4c2..077c5a2647 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -2317,6 +2317,7 @@ ACCOUNT_VISIBILITY_CONFIGURATION = {
}
# E-Commerce API Configuration
+ECOMMERCE_PUBLIC_URL_ROOT = None
ECOMMERCE_API_URL = None
ECOMMERCE_API_SIGNING_KEY = None
ECOMMERCE_API_TIMEOUT = 5