Merge pull request #111 from edx/peter-fogg/course-run-mktg-url

Add marketing_url to course runs.

course_discovery/apps/api/serializers.py

@@ -12,6 +12,26 @@ from course_discovery.apps.course_metadata.models import (
 User = get_user_model()
 
 
+def get_marketing_url_for_user(user, marketing_url):
+    """
+    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.
+
+    Returns:
+        str | None
+    """
+    if marketing_url is None:
+        return None
+    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):
     modified = serializers.DateTimeField()
 
@@ -122,6 +142,7 @@ class CourseRunSerializer(TimestampModelSerializer):
     seats = SeatSerializer(many=True)
     instructors = PersonSerializer(many=True)
     staff = PersonSerializer(many=True)
+    marketing_url = serializers.SerializerMethodField()
 
     class Meta(object):
         model = CourseRun
@@ -129,9 +150,12 @@ class CourseRunSerializer(TimestampModelSerializer):
             'course', 'key', 'title', 'short_description', 'full_description', 'start', 'end',
             'enrollment_start', 'enrollment_end', 'announcement', 'image', 'video', 'seats',
             'content_language', 'transcript_languages', 'instructors', 'staff',
-            'pacing_type', 'min_effort', 'max_effort', 'modified',
+            'pacing_type', 'min_effort', 'max_effort', 'modified', 'marketing_url',
         )
 
+    def get_marketing_url(self, obj):
+        return get_marketing_url_for_user(self.context['request'].user, obj.marketing_url)
+
 
 class ContainedCourseRunsSerializer(serializers.Serializer):  # pylint: disable=abstract-method
     course_runs = serializers.DictField(
@@ -161,14 +185,7 @@ class CourseSerializer(TimestampModelSerializer):
         )
 
     def get_marketing_url(self, obj):
-        if obj.marketing_url is None:
-            return None
-        user = self.context['request'].user
-        params = urlencode({
-            'utm_source': user.username,
-            'utm_medium': user.referral_tracking_id,
-        })
-        return '{url}?{params}'.format(url=obj.marketing_url, params=params)
+        return get_marketing_url_for_user(self.context['request'].user, obj.marketing_url)
 
 
 class CourseSerializerExcludingClosedRuns(CourseSerializer):

course_discovery/apps/api/tests/test_serializers.py

@@ -23,6 +23,13 @@ def json_date_format(datetime_obj):
     return datetime.strftime(datetime_obj, "%Y-%m-%dT%H:%M:%S.%fZ")
 
 
+def make_request():
+    user = UserFactory()
+    request = APIRequestFactory().get('/')
+    request.user = user
+    return request
+
+
 class CatalogSerializerTests(TestCase):
     def test_data(self):
         user = UserFactory()
@@ -59,7 +66,7 @@ class CourseSerializerTests(TestCase):
         image = course.image
         video = course.video
 
-        request = self._make_request()
+        request = make_request()
 
         CourseRunFactory.create_batch(3, course=course)
         serializer = CourseSerializer(course, context={'request': request})
@@ -78,7 +85,7 @@ class CourseSerializerTests(TestCase):
             'owners': [],
             'sponsors': [],
             'modified': json_date_format(course.modified),  # pylint: disable=no-member
-            'course_runs': CourseRunSerializer(course.course_runs, many=True).data,
+            'course_runs': CourseRunSerializer(course.course_runs, many=True, context={'request': request}).data,
             'marketing_url': '{url}?{params}'.format(
                 url=course.marketing_url,
                 params=urlencode({
@@ -96,23 +103,18 @@ class CourseSerializerTests(TestCase):
         parameters if the course has no marketing URL.
         """
         course = CourseFactory(marketing_url=None)
-        request = self._make_request()
+        request = make_request()
         serializer = CourseSerializer(course, context={'request': request})
         self.assertEqual(serializer.data['marketing_url'], None)
 
-    def _make_request(self):
-        user = UserFactory()
-        request = APIRequestFactory().get('/')
-        request.user = user
-        return request
-
 
 class CourseRunSerializerTests(TestCase):
     def test_data(self):
+        request = make_request()
         course_run = CourseRunFactory()
         image = course_run.image
         video = course_run.video
-        serializer = CourseRunSerializer(course_run)
+        serializer = CourseRunSerializer(course_run, context={'request': request})
 
         expected = {
             'course': course_run.course.key,
@@ -135,11 +137,28 @@ class CourseRunSerializerTests(TestCase):
             'instructors': [],
             'staff': [],
             'seats': [],
-            'modified': json_date_format(course_run.modified)  # pylint: disable=no-member
+            'modified': json_date_format(course_run.modified),  # pylint: disable=no-member
+            'marketing_url': '{url}?{params}'.format(
+                url=course_run.marketing_url,
+                params=urlencode({
+                    'utm_source': request.user.username,
+                    'utm_medium': request.user.referral_tracking_id,
+                })
+            ),
         }
 
         self.assertDictEqual(serializer.data, expected)
 
+    def test_data_url_none(self):
+        """
+        Verify that the course run serializer does not attempt to add URL
+        parameters if the course has no marketing URL.
+        """
+        course_run = CourseRunFactory(marketing_url=None)
+        request = make_request()
+        serializer = CourseRunSerializer(course_run, context={'request': request})
+        self.assertEqual(serializer.data['marketing_url'], None)
+
 
 class ContainedCourseRunsSerializerTests(TestCase):
     def test_data(self):

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

@@ -4,7 +4,7 @@ import ddt
 
 from django.db.models.functions import Lower
 from rest_framework.reverse import reverse
-from rest_framework.test import APITestCase
+from rest_framework.test import APITestCase, APIRequestFactory
 
 from course_discovery.apps.api.serializers import CourseRunSerializer
 from course_discovery.apps.core.tests.factories import UserFactory
@@ -21,6 +21,8 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
         self.client.force_authenticate(self.user)
         self.course_run = CourseRunFactory()
         self.refresh_index()
+        self.request = APIRequestFactory().get('/')
+        self.request.user = self.user
 
     def test_get(self):
         """ Verify the endpoint returns the details for a single course. """
@@ -28,7 +30,7 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
 
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, CourseRunSerializer(self.course_run).data)
+        self.assertEqual(response.data, CourseRunSerializer(self.course_run, context={'request': self.request}).data)
 
     def test_list(self):
         """ Verify the endpoint returns a list of all catalogs. """
@@ -38,7 +40,11 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
         self.assertEqual(response.status_code, 200)
         self.assertListEqual(
             response.data['results'],
-            CourseRunSerializer(CourseRun.objects.all().order_by(Lower('key')), many=True).data
+            CourseRunSerializer(
+                CourseRun.objects.all().order_by(Lower('key')),
+                many=True,
+                context={'request': self.request}
+            ).data
         )
 
     def test_list_query(self):
@@ -52,7 +58,12 @@ class CourseRunViewSetTests(ElasticsearchTestMixin, APITestCase):
         response = self.client.get(url)
         actual_sorted = sorted(response.data['results'], key=lambda course_run: course_run['key'])
         expected_sorted = sorted(
-            CourseRunSerializer(course_runs, many=True).data, key=lambda course_run: course_run['key']
+            CourseRunSerializer(
+                course_runs,
+                many=True,
+                context={'request': self.request}
+            ).data,
+            key=lambda course_run: course_run['key']
         )
         self.assertListEqual(actual_sorted, expected_sorted)
 

course_discovery/apps/course_metadata/data_loaders.py

@@ -298,7 +298,6 @@ class DrupalApiDataLoader(AbstractDataLoader):
 
         course.full_description = self.clean_html(body['description'])
         course.short_description = self.clean_html(body['subtitle'])
-        course.marketing_url = urljoin(settings.MARKETING_URL_ROOT, body['course_about_uri'])
 
         level_type, __ = LevelType.objects.get_or_create(name=body['level']['title'])
         course.level_type = level_type
@@ -347,6 +346,7 @@ class DrupalApiDataLoader(AbstractDataLoader):
             return None
         course_run.language = self.get_language_tag(body)
         course_run.course = course
+        course_run.marketing_url = urljoin(settings.MARKETING_URL_ROOT, body['course_about_uri'])
 
         self.set_staff(course_run, body)
 

course_discovery/apps/course_metadata/migrations/0003_auto_20160523_1422.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_metadata', '0002_auto_20160406_1644'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='courserun',
+            name='marketing_url',
+            field=models.URLField(max_length=255, blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='historicalcourserun',
+            name='marketing_url',
+            field=models.URLField(max_length=255, blank=True, null=True),
+        ),
+    ]

course_discovery/apps/course_metadata/models.py

@@ -222,6 +222,7 @@ class CourseRun(TimeStampedModel):
     syllabus = models.ForeignKey(SyllabusItem, default=None, null=True, blank=True)
     image = models.ForeignKey(Image, default=None, null=True, blank=True)
     video = models.ForeignKey(Video, default=None, null=True, blank=True)
+    marketing_url = models.URLField(max_length=255, null=True, blank=True)
 
     history = HistoricalRecords()
 

course_discovery/apps/course_metadata/tests/factories.py

@@ -112,6 +112,7 @@ class CourseRunFactory(factory.DjangoModelFactory):
     min_effort = FuzzyInteger(1, 10)
     max_effort = FuzzyInteger(10, 20)
     pacing_type = FuzzyChoice([name for name, __ in CourseRun.PACING_CHOICES])
+    marketing_url = FuzzyText(prefix='https://example.com/test-course-url')
 
     class Meta:
         model = CourseRun

course_discovery/apps/course_metadata/tests/test_data_loaders.py

@@ -2,7 +2,7 @@
 import datetime
 import json
 from decimal import Decimal
-from urllib.parse import parse_qs, urlparse, urljoin
+from urllib.parse import parse_qs, urlparse
 
 import ddt
 import responses
@@ -612,7 +612,6 @@ class DrupalApiDataLoaderTests(DataLoaderTestMixin, TestCase):
         self.assertEqual(course.title, body['title'])
         self.assertEqual(course.full_description, self.loader.clean_html(body['description']))
         self.assertEqual(course.short_description, self.loader.clean_html(body['subtitle']))
-        self.assertEqual(course.marketing_url, urljoin(settings.MARKETING_URL_ROOT, body['course_about_uri']))
         self.assertEqual(course.level_type.name, body['level']['title'])
 
         self.assert_subjects_loaded(course, body)