Use publicly-accessible ecommerce URL root when constructing refund email

Parsing the ECOMMERCE_API_URL yields a URL root only accessible from within the VPC.

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

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'

lms/djangoapps/commerce/tests/test_signals.py

@@ -3,7 +3,6 @@ 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
@@ -11,15 +10,15 @@ 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)
+@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.
@@ -42,7 +41,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
@@ -185,14 +188,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)
 

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)

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