Add exclude_utm querystring parameter (#357)

Calls to course, course run, and program APIs can use this parameter to prevent UTM parameters from being appended to marketing URLs. ECOM-5782.

course_discovery/apps/api/filters.py

@@ -11,6 +11,7 @@ from dry_rest_permissions.generics import DRYPermissionFiltersBase
 from guardian.shortcuts import get_objects_for_user
 from rest_framework.exceptions import PermissionDenied, NotFound
 
+from course_discovery.apps.api.utils import cast2int
 from course_discovery.apps.core.models import Partner
 from course_discovery.apps.course_metadata.models import Course, CourseRun, Program
 
@@ -105,14 +106,7 @@ class CharListFilter(django_filters.CharFilter):
 
 class FilterSetMixin:
     def _apply_filter(self, name, queryset, value):
-        try:
-            if int(value):
-                queryset = getattr(queryset, name)()
-        except ValueError:
-            logger.exception('The "%s" filter requires an integer value of either 0 or 1. %s is invalid', name, value)
-            raise
-
-        return queryset
+        return getattr(queryset, name)() if cast2int(value, name) else queryset
 
     def filter_active(self, queryset, value):
         return self._apply_filter('active', queryset, value)

course_discovery/apps/api/serializers.py

@@ -107,24 +107,30 @@ SELECT_RELATED_FIELDS = {
 }
 
 
-def get_marketing_url_for_user(user, marketing_url):
+def get_marketing_url_for_user(user, marketing_url, exclude_utm=False):
     """
     Return the given marketing URL with affiliate query parameters for the user.
 
     Arguments:
-        user (User): the user to use to construct the query parameters.
-        marketing_url (str | None): the base URL.
+        user (User): Used to construct UTM query parameters.
+        marketing_url (str | None): Base URL to which UTM parameters may be appended.
+
+    Keyword Arguments:
+        exclude_utm (bool): Whether to exclude UTM parameters from marketing URLs.
 
     Returns:
         str | None
     """
-    if marketing_url is None:
+    if not marketing_url:
         return None
-    params = urlencode({
-        'utm_source': user.username,
-        'utm_medium': user.referral_tracking_id,
-    })
-    return '{url}?{params}'.format(url=marketing_url, params=params)
+    elif exclude_utm:
+        return marketing_url
+    else:
+        params = urlencode({
+            'utm_source': user.username,
+            'utm_medium': user.referral_tracking_id,
+        })
+        return '{url}?{params}'.format(url=marketing_url, params=params)
 
 
 class TimestampModelSerializer(serializers.ModelSerializer):
@@ -343,7 +349,11 @@ class CourseRunSerializer(TimestampModelSerializer):
         )
 
     def get_marketing_url(self, obj):
-        return get_marketing_url_for_user(self.context['request'].user, obj.marketing_url)
+        return get_marketing_url_for_user(
+            self.context['request'].user,
+            obj.marketing_url,
+            exclude_utm=self.context.get('exclude_utm')
+        )
 
     def get_instructors(self, obj):  # pylint: disable=unused-argument
         return []
@@ -405,7 +415,11 @@ class CourseSerializer(TimestampModelSerializer):
         )
 
     def get_marketing_url(self, obj):
-        return get_marketing_url_for_user(self.context['request'].user, obj.marketing_url)
+        return get_marketing_url_for_user(
+            self.context['request'].user,
+            obj.marketing_url,
+            exclude_utm=self.context.get('exclude_utm')
+        )
 
 
 class CourseWithProgramsSerializer(CourseSerializer):
@@ -447,7 +461,10 @@ class ProgramCourseSerializer(CourseSerializer):
         return CourseRunSerializer(
             course_runs,
             many=True,
-            context={'request': self.context.get('request')}
+            context={
+                'request': self.context.get('request'),
+                'exclude_utm': self.context.get('exclude_utm'),
+            }
         ).data
 
 
@@ -510,8 +527,9 @@ class ProgramSerializer(serializers.ModelSerializer):
             many=True,
             context={
                 'request': self.context.get('request'),
-                'program': program,
                 'published_course_runs_only': self.context.get('published_course_runs_only'),
+                'exclude_utm': self.context.get('exclude_utm'),
+                'program': program,
                 'course_runs': course_runs,
             }
         )

course_discovery/apps/api/tests/test_serializers.py

@@ -129,6 +129,14 @@ class CourseSerializerTests(TestCase):
 
         self.assertDictEqual(serializer.data, expected)
 
+    def test_exclude_utm(self):
+        request = make_request()
+        course = CourseFactory()
+        CourseRunFactory.create_batch(3, course=course)
+        serializer = CourseWithProgramsSerializer(course, context={'request': request, 'exclude_utm': 1})
+
+        self.assertEqual(serializer.data['marketing_url'], course.marketing_url)
+
 
 class CourseRunSerializerTests(TestCase):
     def test_data(self):
@@ -174,6 +182,13 @@ class CourseRunSerializerTests(TestCase):
 
         self.assertDictEqual(serializer.data, expected)
 
+    def test_exclude_utm(self):
+        request = make_request()
+        course_run = CourseRunFactory()
+        serializer = CourseRunSerializer(course_run, context={'request': request, 'exclude_utm': 1})
+
+        self.assertEqual(serializer.data['marketing_url'], course_run.marketing_url)
+
 
 class CourseRunWithProgramsSerializerTests(TestCase):
     def test_data(self):

course_discovery/apps/api/tests/test_utils.py

+import ddt
+from django.test import TestCase
+import mock
+
+from course_discovery.apps.api.utils import cast2int
+
+
+LOGGER_PATH = 'course_discovery.apps.api.utils.logger.exception'
+
+
+@ddt.ddt
+class Cast2IntTests(TestCase):
+    name = 'foo'
+
+    @ddt.data(
+        ('0', 0),
+        ('1', 1),
+        (None, None),
+    )
+    @ddt.unpack
+    def test_cast_success(self, value, expected):
+        self.assertEqual(cast2int(value, self.name), expected)
+
+    @ddt.data('beep', '1.1')
+    def test_cast_failure(self, value):
+        with mock.patch(LOGGER_PATH) as mock_logger:
+            with self.assertRaises(ValueError):
+                cast2int(value, self.name)
+
+        self.assertTrue(mock_logger.called)

course_discovery/apps/api/utils.py

+import logging
+
+
+logger = logging.getLogger(__name__)
+
+
+def cast2int(value, name):
+    """
+    Attempt to cast the provided value to an integer.
+
+    Arguments:
+        value (str): A value to cast to an integer.
+        name (str): A name to log if casting fails.
+
+    Raises:
+        ValueError, if the provided value can't be converted. A helpful
+            error message is logged first.
+
+    Returns:
+        int | None
+    """
+    if value is None:
+        return value
+
+    try:
+        return int(value)
+    except ValueError:
+        logger.exception('The "%s" parameter requires an integer value. "%s" is invalid.', name, value)
+        raise

course_discovery/apps/api/v1/tests/test_views/mixins.py

@@ -8,12 +8,15 @@ from rest_framework.test import APIRequestFactory
 
 from course_discovery.apps.api.serializers import (
     CatalogSerializer, CourseWithProgramsSerializer, CourseSerializerExcludingClosedRuns,
-    FlattenedCourseRunWithCourseSerializer
+    CourseRunWithProgramsSerializer, ProgramSerializer, FlattenedCourseRunWithCourseSerializer
 )
 
 
 class SerializationMixin(object):
     def _get_request(self, format=None):
+        if getattr(self, 'request', None):
+            return self.request
+
         query_data = {}
         if format:
             query_data['format'] = format
@@ -21,20 +24,30 @@ class SerializationMixin(object):
         request.user = self.user
         return request
 
-    def _serialize_object(self, serializer, obj, many=False, format=None):
-        return serializer(obj, many=many, context={'request': self._get_request(format)}).data
+    def _serialize_object(self, serializer, obj, many=False, format=None, extra_context=None):
+        context = {'request': self._get_request(format)}
+        if extra_context:
+            context.update(extra_context)
+
+        return serializer(obj, many=many, context=context).data
+
+    def serialize_catalog(self, catalog, many=False, format=None, extra_context=None):
+        return self._serialize_object(CatalogSerializer, catalog, many, format, extra_context)
+
+    def serialize_course(self, course, many=False, format=None, extra_context=None):
+        return self._serialize_object(CourseWithProgramsSerializer, course, many, format, extra_context)
 
-    def serialize_catalog(self, catalog, many=False, format=None):
-        return self._serialize_object(CatalogSerializer, catalog, many, format)
+    def serialize_course_run(self, run, many=False, format=None, extra_context=None):
+        return self._serialize_object(CourseRunWithProgramsSerializer, run, many, format, extra_context)
 
-    def serialize_course(self, course, many=False, format=None):
-        return self._serialize_object(CourseWithProgramsSerializer, course, many, format)
+    def serialize_program(self, program, many=False, format=None, extra_context=None):
+        return self._serialize_object(ProgramSerializer, program, many, format, extra_context)
 
-    def serialize_catalog_course(self, course, many=False, format=None):
-        return self._serialize_object(CourseSerializerExcludingClosedRuns, course, many, format)
+    def serialize_catalog_course(self, course, many=False, format=None, extra_context=None):
+        return self._serialize_object(CourseSerializerExcludingClosedRuns, course, many, format, extra_context)
 
-    def serialize_catalog_flat_course_run(self, course_run, many=False, format=None):
-        return self._serialize_object(FlattenedCourseRunWithCourseSerializer, course_run, many, format)
+    def serialize_catalog_flat_course_run(self, course_run, many=False, format=None, extra_context=None):
+        return self._serialize_object(FlattenedCourseRunWithCourseSerializer, course_run, many, format, extra_context)
 
 
 class OAuth2Mixin(object):

course_discovery/apps/api/v1/tests/test_views/test_course_runs.py

@@ -8,7 +8,7 @@ from django.db.models.functions import Lower
 from rest_framework.reverse import reverse
 from rest_framework.test import APITestCase, APIRequestFactory
 
-from course_discovery.apps.api.serializers import CourseRunWithProgramsSerializer
+from course_discovery.apps.api.v1.tests.test_views.mixins import SerializationMixin
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
 from course_discovery.apps.course_metadata.models import CourseRun
@@ -16,7 +16,7 @@ from course_discovery.apps.course_metadata.tests.factories import CourseRunFacto
 
 
 @ddt.ddt
-class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
+class CourseRunViewSetTests(SerializationMixin, ElasticsearchTestMixin, APITestCase):
     def setUp(self):
         super(CourseRunViewSetTests, self).setUp()
         self.user = UserFactory(is_staff=True, is_superuser=True)
@@ -28,9 +28,6 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
         self.request = APIRequestFactory().get('/')
         self.request.user = self.user
 
-    def serialize_course_run(self, course_run, **kwargs):
-        return CourseRunWithProgramsSerializer(course_run, context={'request': self.request}, **kwargs).data
-
     def test_get(self):
         """ Verify the endpoint returns the details for a single course. """
         url = reverse('api:v1:course_run-detail', kwargs={'key': self.course_run.key})
@@ -78,11 +75,14 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
         response = self.client.get(url)
         self.assertEqual(response.status_code, 400)
 
-    def assert_list_results(self, url, expected):
+    def assert_list_results(self, url, expected, extra_context=None):
         expected = sorted(expected, key=lambda course_run: course_run.key.lower())
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertListEqual(response.data['results'], self.serialize_course_run(expected, many=True))
+        self.assertListEqual(
+            response.data['results'],
+            self.serialize_course_run(expected, many=True, extra_context=extra_context)
+        )
 
     def test_filter_by_keys(self):
         """ Verify the endpoint returns a list of course runs filtered by the specified keys. """
@@ -126,6 +126,11 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
         url = reverse('api:v1:course_run-list') + '?active=1'
         self.assert_list_results(url, expected)
 
+    def test_list_exclude_utm(self):
+        """ Verify the endpoint returns marketing URLs without UTM parameters. """
+        url = reverse('api:v1:course_run-list') + '?exclude_utm=1'
+        self.assert_list_results(url, CourseRun.objects.all(), extra_context={'exclude_utm': 1})
+
     def test_contains_single_course_run(self):
         """ Verify that a single course_run is contained in a query """
         qs = urllib.parse.urlencode({

course_discovery/apps/api/v1/tests/test_views/test_courses.py

@@ -9,6 +9,8 @@ from course_discovery.apps.course_metadata.tests.factories import CourseFactory
 
 
 class CourseViewSetTests(SerializationMixin, APITestCase):
+    maxDiff = None
+
     def setUp(self):
         super(CourseViewSetTests, self).setUp()
         self.user = UserFactory(is_staff=True, is_superuser=True)
@@ -58,3 +60,14 @@ class CourseViewSetTests(SerializationMixin, APITestCase):
         with self.assertNumQueries(35):
             response = self.client.get(url)
             self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True))
+
+    def test_list_exclude_utm(self):
+        """ Verify the endpoint returns marketing URLs without UTM parameters. """
+        url = reverse('api:v1:course-list') + '?exclude_utm=1'
+
+        response = self.client.get(url)
+        context = {'exclude_utm': 1}
+        self.assertEqual(
+            response.data['results'],
+            self.serialize_course([self.course], many=True, extra_context=context)
+        )

course_discovery/apps/api/v1/tests/test_views/test_programs.py

@@ -2,7 +2,7 @@ import ddt
 from django.core.urlresolvers import reverse
 from rest_framework.test import APITestCase, APIRequestFactory
 
-from course_discovery.apps.api.serializers import ProgramSerializer
+from course_discovery.apps.api.v1.tests.test_views.mixins import SerializationMixin
 from course_discovery.apps.core.tests.factories import USER_PASSWORD, UserFactory
 from course_discovery.apps.core.tests.helpers import make_image_file
 from course_discovery.apps.course_metadata.choices import ProgramStatus
@@ -14,7 +14,7 @@ from course_discovery.apps.course_metadata.tests.factories import (
 
 
 @ddt.ddt
-class ProgramViewSetTests(APITestCase):
+class ProgramViewSetTests(SerializationMixin, APITestCase):
     list_path = reverse('api:v1:program-list')
 
     def setUp(self):
@@ -50,7 +50,7 @@ class ProgramViewSetTests(APITestCase):
 
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, ProgramSerializer(program, context={'request': self.request}).data)
+        self.assertEqual(response.data, self.serialize_program(program))
 
     def test_authentication(self):
         """ Verify the endpoint requires the user to be authenticated. """
@@ -89,7 +89,7 @@ class ProgramViewSetTests(APITestCase):
         with self.assertNumQueries(55):
             self.assert_retrieve_success(program)
 
-    def assert_list_results(self, url, expected, expected_query_count):
+    def assert_list_results(self, url, expected, expected_query_count, extra_context=None):
         """
         Asserts the results serialized/returned at the URL matches those that are expected.
         Args:
@@ -108,7 +108,7 @@ class ProgramViewSetTests(APITestCase):
 
         self.assertEqual(
             response.data['results'],
-            ProgramSerializer(expected, many=True, context={'request': self.request}).data
+            self.serialize_program(expected, many=True, extra_context=extra_context)
         )
 
     def test_list(self):
@@ -154,3 +154,9 @@ class ProgramViewSetTests(APITestCase):
         expected = programs if is_marketable else []
         self.assertEqual(list(Program.objects.marketable()), expected)
         self.assert_list_results(url, expected, expected_query_count)
+
+    def test_list_exclude_utm(self):
+        """ Verify the endpoint returns marketing URLs without UTM parameters. """
+        url = self.list_path + '?exclude_utm=1'
+        program = self.create_program()
+        self.assert_list_results(url, [program], 33, extra_context={'exclude_utm': 1})

course_discovery/apps/api/v1/views.py

@@ -26,6 +26,7 @@ from course_discovery.apps.api import serializers
 from course_discovery.apps.api.exceptions import InvalidPartnerError
 from course_discovery.apps.api.pagination import PageNumberPagination
 from course_discovery.apps.api.renderers import AffiliateWindowXMLRenderer, CourseRunCSVRenderer
+from course_discovery.apps.api.utils import cast2int
 from course_discovery.apps.catalogs.models import Catalog
 from course_discovery.apps.core.utils import SearchQuerySetWrapper
 from course_discovery.apps.course_metadata.constants import COURSE_ID_REGEX, COURSE_RUN_ID_REGEX
@@ -35,6 +36,13 @@ logger = logging.getLogger(__name__)
 User = get_user_model()
 
 
+def get_query_param(request, name):
+    """
+    Get a query parameter and cast it to an integer.
+    """
+    return cast2int(request.query_params.get(name), name)
+
+
 def prefetch_related_objects_for_courses(queryset):
     """
     Pre-fetches the related objects that will be serialized with a `Course`.
@@ -222,6 +230,14 @@ class CourseViewSet(viewsets.ReadOnlyModelViewSet):
 
         return queryset.order_by(Lower('key'))
 
+    def get_serializer_context(self, *args, **kwargs):
+        context = super().get_serializer_context(*args, **kwargs)
+        context.update({
+            'exclude_utm': get_query_param(self.request, 'exclude_utm'),
+        })
+
+        return context
+
     def list(self, request, *args, **kwargs):
         """ List all courses.
          ---
@@ -238,6 +254,12 @@ class CourseViewSet(viewsets.ReadOnlyModelViewSet):
               type: string
               paramType: query
               multiple: false
+            - name: exclude_utm
+              description: Exclude UTM parameters from marketing URLs.
+              required: false
+              type: integer
+              paramType: query
+              multiple: false
         """
         return super(CourseViewSet, self).list(request, *args, **kwargs)
 
@@ -284,6 +306,14 @@ class CourseRunViewSet(viewsets.ReadOnlyModelViewSet):
             queryset = queryset.prefetch_related(*serializers.PREFETCH_FIELDS['course_run'])
             return queryset
 
+    def get_serializer_context(self, *args, **kwargs):
+        context = super().get_serializer_context(*args, **kwargs)
+        context.update({
+            'exclude_utm': get_query_param(self.request, 'exclude_utm'),
+        })
+
+        return context
+
     def list(self, request, *args, **kwargs):
         """ List all courses runs.
         ---
@@ -320,6 +350,12 @@ class CourseRunViewSet(viewsets.ReadOnlyModelViewSet):
               type: integer
               paramType: query
               multiple: false
+            - name: exclude_utm
+              description: Exclude UTM parameters from marketing URLs.
+              required: false
+              type: integer
+              paramType: query
+              multiple: false
         """
         return super(CourseRunViewSet, self).list(request, *args, **kwargs)
 
@@ -388,7 +424,11 @@ class ProgramViewSet(viewsets.ReadOnlyModelViewSet):
 
     def get_serializer_context(self, *args, **kwargs):
         context = super().get_serializer_context(*args, **kwargs)
-        context['published_course_runs_only'] = int(self.request.GET.get('published_course_runs_only', 0))
+        context.update({
+            'published_course_runs_only': get_query_param(self.request, 'published_course_runs_only'),
+            'exclude_utm': get_query_param(self.request, 'exclude_utm'),
+        })
+
         return context
 
     def list(self, request, *args, **kwargs):
@@ -414,6 +454,12 @@ class ProgramViewSet(viewsets.ReadOnlyModelViewSet):
               type: integer
               paramType: query
               mulitple: false
+            - name: exclude_utm
+              description: Exclude UTM parameters from marketing URLs.
+              required: false
+              type: integer
+              paramType: query
+              multiple: false
         """
         return super(ProgramViewSet, self).list(request, *args, **kwargs)