Merge pull request #9084 from edx/jsa/xcom-497

commerce/api: Don’t allow setting expiration of prof modes.

common/djangoapps/course_modes/models.py

@@ -4,6 +4,7 @@ Add and create new modes for running courses on this particular LMS
 import pytz
 from datetime import datetime
 
+from django.core.exceptions import ValidationError
 from django.db import models
 from collections import namedtuple, defaultdict
 from django.utils.translation import ugettext_lazy as _
@@ -106,8 +107,19 @@ class CourseMode(models.Model):
         """ meta attributes of this model """
         unique_together = ('course_id', 'mode_slug', 'currency')
 
+    def clean(self):
+        """
+        Object-level validation - implemented in this method so DRF serializers
+        catch errors in advance of a save() attempt.
+        """
+        if self.is_professional_slug(self.mode_slug) and self.expiration_datetime is not None:
+            raise ValidationError(
+                _(u"Professional education modes are not allowed to have expiration_datetime set.")
+            )
+
     def save(self, force_insert=False, force_update=False, using=None):
         # Ensure currency is always lowercase.
+        self.clean()  # ensure object-level validation is performed before we save.
         self.currency = self.currency.lower()
         super(CourseMode, self).save(force_insert, force_update, using)
 

common/djangoapps/course_modes/tests/test_models.py

@@ -6,12 +6,15 @@ Replace this with more appropriate tests for your application.
 """
 
 from datetime import datetime, timedelta
-import pytz
-import ddt
+import itertools
 
+import ddt
+from django.core.exceptions import ValidationError
+from django.test import TestCase
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from opaque_keys.edx.locator import CourseLocator
-from django.test import TestCase
+import pytz
+
 from course_modes.models import CourseMode, Mode
 
 
@@ -26,7 +29,15 @@ class CourseModeModelTest(TestCase):
         self.course_key = SlashSeparatedCourseKey('Test', 'TestCourse', 'TestCourseRun')
         CourseMode.objects.all().delete()
 
-    def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices='', currency='usd'):
+    def create_mode(
+            self,
+            mode_slug,
+            mode_name,
+            min_price=0,
+            suggested_prices='',
+            currency='usd',
+            expiration_datetime=None,
+    ):
         """
         Create a new course mode
         """
@@ -37,6 +48,7 @@ class CourseModeModelTest(TestCase):
             min_price=min_price,
             suggested_prices=suggested_prices,
             currency=currency,
+            expiration_datetime=expiration_datetime,
         )
 
     def test_save(self):
@@ -264,6 +276,29 @@ class CourseModeModelTest(TestCase):
         else:
             self.assertFalse(CourseMode.is_verified_slug(mode_slug))
 
+    @ddt.data(*itertools.product(
+        (
+            CourseMode.HONOR,
+            CourseMode.AUDIT,
+            CourseMode.VERIFIED,
+            CourseMode.PROFESSIONAL,
+            CourseMode.NO_ID_PROFESSIONAL_MODE
+        ),
+        (datetime.now(), None),
+    ))
+    @ddt.unpack
+    def test_invalid_mode_expiration(self, mode_slug, exp_dt):
+        is_error_expected = CourseMode.is_professional_slug(mode_slug) and exp_dt is not None
+        try:
+            self.create_mode(mode_slug=mode_slug, mode_name=mode_slug.title(), expiration_datetime=exp_dt)
+            self.assertFalse(is_error_expected, "Expected a ValidationError to be thrown.")
+        except ValidationError, exc:
+            self.assertTrue(is_error_expected, "Did not expect a ValidationError to be thrown.")
+            self.assertEqual(
+                exc.messages,
+                [u"Professional education modes are not allowed to have expiration_datetime set."],
+            )
+
     @ddt.data(
         ("verified", "verify_need_to_verify"),
         ("verified", "verify_submitted"),

lms/djangoapps/commerce/api/v1/models.py

@@ -66,6 +66,7 @@ class Course(object):
             merged_mode.min_price = posted_mode.min_price
             merged_mode.currency = posted_mode.currency
             merged_mode.sku = posted_mode.sku
+            merged_mode.expiration_datetime = posted_mode.expiration_datetime
 
             merged_modes.add(merged_mode)
             merged_mode_keys.add(merged_mode.mode_slug)

lms/djangoapps/commerce/api/v1/tests/test_views.py

@@ -1,4 +1,6 @@
 """ Commerce API v1 view tests. """
+from datetime import datetime
+import itertools
 import json
 
 import ddt
@@ -6,6 +8,7 @@ from django.conf import settings
 from django.contrib.auth.models import Permission
 from django.core.urlresolvers import reverse
 from django.test.utils import override_settings
+from rest_framework.utils.encoders import JSONEncoder
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -31,12 +34,17 @@ class CourseApiViewTestMixin(object):
     @staticmethod
     def _serialize_course_mode(course_mode):
         """ Serialize a CourseMode to a dict. """
+        # encode the datetime (if nonempty) using DRF's encoder, simplifying
+        # equality assertions.
+        expires = course_mode.expiration_datetime
+        if expires is not None:
+            expires = JSONEncoder().default(expires)
         return {
             u'name': course_mode.mode_slug,
             u'currency': course_mode.currency.lower(),
             u'price': course_mode.min_price,
             u'sku': course_mode.sku,
-            u'expires': course_mode.expiration_datetime,
+            u'expires': expires,
         }
 
 
@@ -112,7 +120,14 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase)
         """ Verify the view supports updating a course. """
         permission = Permission.objects.get(name='Can change course mode')
         self.user.user_permissions.add(permission)
-        expected_course_mode = CourseMode(mode_slug=u'verified', min_price=200, currency=u'USD', sku=u'ABC123')
+        expiration_datetime = datetime.now()
+        expected_course_mode = CourseMode(
+            mode_slug=u'verified',
+            min_price=200,
+            currency=u'USD',
+            sku=u'ABC123',
+            expiration_datetime=expiration_datetime
+        )
         expected = {
             u'id': unicode(self.course.id),
             u'modes': [self._serialize_course_mode(expected_course_mode)]
@@ -144,6 +159,34 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase)
         # The existing CourseMode should have been removed.
         self.assertFalse(CourseMode.objects.filter(id=self.course_mode.id).exists())
 
+    @ddt.data(*itertools.product(
+        ('honor', 'audit', 'verified', 'professional', 'no-id-professional'),
+        (datetime.now(), None),
+    ))
+    @ddt.unpack
+    def test_update_professional_expiration(self, mode_slug, expiration_datetime):
+        """ Verify that pushing a mode with a professional certificate and an expiration datetime
+        will be rejected (this is not allowed). """
+        permission = Permission.objects.get(name='Can change course mode')
+        self.user.user_permissions.add(permission)
+
+        mode = self._serialize_course_mode(
+            CourseMode(
+                mode_slug=mode_slug,
+                min_price=500,
+                currency=u'USD',
+                sku=u'ABC123',
+                expiration_datetime=expiration_datetime
+            )
+        )
+        course_id = unicode(self.course.id)
+        payload = {u'id': course_id, u'modes': [mode]}
+        path = reverse('commerce_api:v1:courses:retrieve_update', args=[course_id])
+
+        expected_status = 400 if CourseMode.is_professional_slug(mode_slug) and expiration_datetime is not None else 200
+        response = self.client.put(path, json.dumps(payload), content_type=JSON_CONTENT_TYPE)
+        self.assertEqual(response.status_code, expected_status)
+
     def assert_can_create_course(self, **request_kwargs):
         """ Verify a course can be created by the view. """
         course = CourseFactory.create()