From 6c83bff5be9f733b7df9bda8930f2c37cbcbd1e4 Mon Sep 17 00:00:00 2001 From: jsa Date: Mon, 27 Jul 2015 15:46:15 -0400 Subject: [PATCH 1/2] commerce/api: pass expiration_datetime when updating modes --- lms/djangoapps/commerce/api/v1/models.py | 1 + .../commerce/api/v1/tests/test_views.py | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py index 235b1385d7..f2903477c5 100644 --- a/lms/djangoapps/commerce/api/v1/models.py +++ b/lms/djangoapps/commerce/api/v1/models.py @@ -49,6 +49,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 36b264894c..c1d1e67746 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -1,4 +1,5 @@ """ Commerce API v1 view tests. """ +from datetime import datetime import json import ddt @@ -6,6 +7,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 +33,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 +119,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)] From 03717f3940ade9662de7356962856d0e3319fbe8 Mon Sep 17 00:00:00 2001 From: jsa Date: Mon, 27 Jul 2015 12:15:43 -0400 Subject: [PATCH 2/2] =?UTF-8?q?commerce/api:=20Don=E2=80=99t=20allow=20set?= =?UTF-8?q?ting=20expiration=20of=20prof=20modes.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XCOM-497 --- common/djangoapps/course_modes/models.py | 12 ++++++ .../course_modes/tests/test_models.py | 43 +++++++++++++++++-- .../commerce/api/v1/tests/test_views.py | 29 +++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) 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/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py index c1d1e67746..e1905fd0a8 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -1,5 +1,6 @@ """ Commerce API v1 view tests. """ from datetime import datetime +import itertools import json import ddt @@ -158,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()