From 0b173946eeee5c2f89bef45941d2701d3ffab3c9 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Wed, 27 May 2015 16:10:20 -0400 Subject: [PATCH] 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 | 14 +++++------ lms/djangoapps/commerce/tests/__init__.py | 3 ++- lms/djangoapps/commerce/tests/test_signals.py | 24 +++++++++++-------- lms/envs/aws.py | 1 + lms/envs/common.py | 1 + 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py index 446815132a..52be46758d 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 @@ -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 = ['{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..8495f073e2 100644 --- a/lms/djangoapps/commerce/tests/test_signals.py +++ b/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) 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