diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py index 0641b85ef6..be014afeed 100644 --- a/lms/djangoapps/commerce/api/v1/models.py +++ b/lms/djangoapps/commerce/api/v1/models.py @@ -7,6 +7,7 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from course_modes.models import CourseMode +from verify_student.models import VerificationDeadline log = logging.getLogger(__name__) @@ -17,9 +18,10 @@ class Course(object): modes = None _deleted_modes = None - def __init__(self, id, modes): # pylint: disable=invalid-name,redefined-builtin + def __init__(self, id, modes, verification_deadline=None): # pylint: disable=invalid-name,redefined-builtin self.id = CourseKey.from_string(unicode(id)) # pylint: disable=invalid-name self.modes = list(modes) + self.verification_deadline = verification_deadline self._deleted_modes = [] @property @@ -55,6 +57,10 @@ class Course(object): @transaction.commit_on_success def save(self, *args, **kwargs): # pylint: disable=unused-argument """ Save the CourseMode objects to the database. """ + + # Update the verification deadline for the course (not the individual modes) + VerificationDeadline.set_deadline(self.id, self.verification_deadline) + for mode in self.modes: mode.course_id = self.id mode.mode_display_name = self.get_mode_display_name(mode) @@ -66,6 +72,8 @@ class Course(object): def update(self, attrs): """ Update the model with external data (usually passed via API call). """ + self.verification_deadline = attrs.get('verification_deadline') + existing_modes = {mode.mode_slug: mode for mode in self.modes} merged_modes = set() merged_mode_keys = set() @@ -100,7 +108,8 @@ class Course(object): course_modes = CourseMode.objects.filter(course_id=course_id) if course_modes: - return cls(unicode(course_id), list(course_modes)) + verification_deadline = VerificationDeadline.deadline_for_course(course_id) + return cls(course_id, list(course_modes), verification_deadline=verification_deadline) return None diff --git a/lms/djangoapps/commerce/api/v1/serializers.py b/lms/djangoapps/commerce/api/v1/serializers.py index 37e8890ab1..fa8c2754f3 100644 --- a/lms/djangoapps/commerce/api/v1/serializers.py +++ b/lms/djangoapps/commerce/api/v1/serializers.py @@ -1,4 +1,7 @@ """ API v1 serializers. """ +from datetime import datetime + +import pytz from rest_framework import serializers from commerce.api.v1.models import Course @@ -26,11 +29,35 @@ class CourseSerializer(serializers.Serializer): """ Course serializer. """ id = serializers.CharField() # pylint: disable=invalid-name name = serializers.CharField(read_only=True) + verification_deadline = serializers.DateTimeField(blank=True) modes = CourseModeSerializer(many=True, allow_add_remove=True) + def validate(self, attrs): + """ Ensure the verification deadline occurs AFTER the course mode enrollment deadlines. """ + verification_deadline = attrs.get('verification_deadline', None) + + if verification_deadline: + upgrade_deadline = None + + # Find the earliest upgrade deadline + for mode in attrs['modes']: + expires = mode.expiration_datetime + if expires: + # If we don't already have an upgrade_deadline value, use datetime.max so that we can actually + # complete the comparison. + upgrade_deadline = min(expires, upgrade_deadline or datetime.max.replace(tzinfo=pytz.utc)) + + # In cases where upgrade_deadline is None (e.g. the verified professional mode), allow a verification + # deadline to be set anyway. + if upgrade_deadline is not None and verification_deadline < upgrade_deadline: + raise serializers.ValidationError( + 'Verification deadline must be after the course mode upgrade deadlines.') + + return attrs + def restore_object(self, attrs, instance=None): if instance is None: - return Course(attrs['id'], attrs['modes']) + return Course(attrs['id'], attrs['modes'], attrs['verification_deadline']) instance.update(attrs) return instance diff --git a/lms/djangoapps/commerce/api/v1/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py index 2ef0602b96..365a286b0d 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -8,12 +8,14 @@ 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 +import pytz from rest_framework.utils.encoders import JSONEncoder from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from course_modes.models import CourseMode from student.tests.factories import UserFactory +from verify_student.models import VerificationDeadline PASSWORD = 'test' JSON_CONTENT_TYPE = 'application/json' @@ -31,30 +33,37 @@ class CourseApiViewTestMixin(object): self.course_mode = CourseMode.objects.create(course_id=self.course.id, mode_slug=u'verified', min_price=100, currency=u'USD', sku=u'ABC123') - @staticmethod - def _serialize_course_mode(course_mode): + @classmethod + def _serialize_datetime(cls, dt): # pylint: disable=invalid-name + """ Serializes datetime values using Django REST Framework's encoder. + + Use this to simplify equality assertions. + """ + if dt: + return JSONEncoder().default(dt) + return None + + @classmethod + def _serialize_course_mode(cls, 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': expires, + u'expires': cls._serialize_datetime(course_mode.expiration_datetime), } @classmethod - def _serialize_course(cls, course, modes=None): + def _serialize_course(cls, course, modes=None, verification_deadline=None): """ Serializes a course to a Python dict. """ modes = modes or [] + verification_deadline = verification_deadline or VerificationDeadline.deadline_for_course(course.id) return { u'id': unicode(course.id), u'name': unicode(course.display_name), + u'verification_deadline': cls._serialize_datetime(verification_deadline), u'modes': [cls._serialize_course_mode(mode) for mode in modes] } @@ -91,6 +100,9 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) self.user = UserFactory.create() self.client.login(username=self.user.username, password=PASSWORD) + permission = Permission.objects.get(name='Can change course mode') + self.user.user_permissions.add(permission) + @ddt.data('get', 'post', 'put') def test_authentication_required(self, method): """ Verify only authenticated users can access the view. """ @@ -100,6 +112,7 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) @ddt.data('post', 'put') def test_authorization_required(self, method): + self.user.user_permissions.clear() """ Verify create/edit operations require appropriate permissions. """ response = getattr(self.client, method)(self.path, content_type=JSON_CONTENT_TYPE) self.assertEqual(response.status_code, 403) @@ -119,32 +132,72 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) response = self.client.get(path, content_type=JSON_CONTENT_TYPE) self.assertEqual(response.status_code, 404) - def test_update(self): - """ Verify the view supports updating a course. """ - permission = Permission.objects.get(name='Can change course mode') - self.user.user_permissions.add(permission) - expiration_datetime = datetime.now() + def _get_update_response_and_expected_data(self, mode_expiration, verification_deadline): + """ Returns expected data and response for course update. """ expected_course_mode = CourseMode( mode_slug=u'verified', min_price=200, currency=u'USD', sku=u'ABC123', - expiration_datetime=expiration_datetime + expiration_datetime=mode_expiration ) - expected = self._serialize_course(self.course, [expected_course_mode]) + expected = self._serialize_course(self.course, [expected_course_mode], verification_deadline) + # Sanity check: The API should return HTTP status 200 for updates response = self.client.put(self.path, json.dumps(expected), content_type=JSON_CONTENT_TYPE) + + return response, expected + + def test_update(self): + """ Verify the view supports updating a course. """ + # Sanity check: Ensure no verification deadline is set + self.assertIsNone(VerificationDeadline.deadline_for_course(self.course.id)) + + # Generate the expected data + verification_deadline = datetime(year=2020, month=12, day=31, tzinfo=pytz.utc) + expiration_datetime = datetime.now(pytz.utc) + response, expected = self._get_update_response_and_expected_data(expiration_datetime, verification_deadline) + + # Sanity check: The API should return HTTP status 200 for updates self.assertEqual(response.status_code, 200) + # Verify the course and modes are returned as JSON actual = json.loads(response.content) self.assertEqual(actual, expected) + # Verify the verification deadline is updated + self.assertEqual(VerificationDeadline.deadline_for_course(self.course.id), verification_deadline) + + def test_update_invalid_dates(self): + """ + Verify the API does not allow the verification deadline to be set before the course mode upgrade deadlines. + """ + expiration_datetime = datetime.now(pytz.utc) + verification_deadline = datetime(year=1915, month=5, day=7, tzinfo=pytz.utc) + response, __ = self._get_update_response_and_expected_data(expiration_datetime, verification_deadline) + self.assertEqual(response.status_code, 400) + + # Verify the error message is correct + actual = json.loads(response.content) + expected = { + 'non_field_errors': ['Verification deadline must be after the course mode upgrade deadlines.'] + } + self.assertEqual(actual, expected) + + def test_update_verification_deadline_without_expiring_modes(self): + """ Verify verification deadline can be set if no course modes expire. + + This accounts for the verified professional mode, which requires verification but should never expire. + """ + verification_deadline = datetime(year=1915, month=5, day=7, tzinfo=pytz.utc) + response, __ = self._get_update_response_and_expected_data(None, verification_deadline) + + self.assertEqual(response.status_code, 200) + self.assertEqual(VerificationDeadline.deadline_for_course(self.course.id), verification_deadline) + def test_update_overwrite(self): """ Verify that data submitted via PUT overwrites/deletes modes that are not included in the body of the request. """ - permission = Permission.objects.get(name='Can change course mode') - self.user.user_permissions.add(permission) - course_id = unicode(self.course.id) expected_course_mode = CourseMode(mode_slug=u'credit', min_price=500, currency=u'USD', sku=u'ABC123') expected = self._serialize_course(self.course, [expected_course_mode]) @@ -165,9 +218,6 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) 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, diff --git a/lms/djangoapps/commerce/api/v1/views.py b/lms/djangoapps/commerce/api/v1/views.py index ad72561bed..34b58c7915 100644 --- a/lms/djangoapps/commerce/api/v1/views.py +++ b/lms/djangoapps/commerce/api/v1/views.py @@ -41,3 +41,8 @@ class CourseRetrieveUpdateView(RetrieveUpdateAPIView): return course raise Http404 + + def pre_save(self, obj): + # There is nothing to pre-save. The default behavior changes the Course.id attribute from + # a CourseKey to a string, which is not desired. + pass