diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index e0026329c0..db6ff974f3 100644 --- a/common/djangoapps/course_modes/models.py +++ b/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) diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 814291c711..69de9de4a8 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/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"), diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py index 495dd0b93e..c8acf13168 100644 --- a/lms/djangoapps/commerce/api/v1/models.py +++ b/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) diff --git a/lms/djangoapps/commerce/api/v1/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py index bdf842af73..7419fe8362 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/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()