diff --git a/common/djangoapps/course_modes/admin.py b/common/djangoapps/course_modes/admin.py index 8356f9134d..124f7ba6e6 100644 --- a/common/djangoapps/course_modes/admin.py +++ b/common/djangoapps/course_modes/admin.py @@ -14,7 +14,7 @@ from opaque_keys import InvalidKeyError from util.date_utils import get_time_display from xmodule.modulestore.django import modulestore -from course_modes.models import CourseMode +from course_modes.models import CourseMode, CourseModeExpirationConfig # Technically, we shouldn't be doing this, since verify_student is defined # in LMS, and course_modes is defined in common. @@ -66,12 +66,13 @@ class CourseModeForm(forms.ModelForm): default_tz = timezone(settings.TIME_ZONE) - if self.instance.expiration_datetime: + if self.instance._expiration_datetime: # pylint: disable=protected-access # django admin is using default timezone. To avoid time conversion from db to form # convert the UTC object to naive and then localize with default timezone. - expiration_datetime = self.instance.expiration_datetime.replace(tzinfo=None) - self.initial["expiration_datetime"] = default_tz.localize(expiration_datetime) - + _expiration_datetime = self.instance._expiration_datetime.replace( # pylint: disable=protected-access + tzinfo=None + ) + self.initial["_expiration_datetime"] = default_tz.localize(_expiration_datetime) # Load the verification deadline # Since this is stored on a model in verify student, we need to load it from there. # We need to munge the timezone a bit to get Django admin to display it without converting @@ -99,14 +100,14 @@ class CourseModeForm(forms.ModelForm): return course_key - def clean_expiration_datetime(self): + def clean__expiration_datetime(self): """ Ensure that the expiration datetime we save uses the UTC timezone. """ # django admin saving the date with default timezone to avoid time conversion from form to db # changes its tzinfo to UTC - if self.cleaned_data.get("expiration_datetime"): - return self.cleaned_data.get("expiration_datetime").replace(tzinfo=UTC) + if self.cleaned_data.get("_expiration_datetime"): + return self.cleaned_data.get("_expiration_datetime").replace(tzinfo=UTC) def clean_verification_deadline(self): """ @@ -122,7 +123,7 @@ class CourseModeForm(forms.ModelForm): """ cleaned_data = super(CourseModeForm, self).clean() mode_slug = cleaned_data.get("mode_slug") - upgrade_deadline = cleaned_data.get("expiration_datetime") + upgrade_deadline = cleaned_data.get("_expiration_datetime") verification_deadline = cleaned_data.get("verification_deadline") # Allow upgrade deadlines ONLY for the "verified" mode @@ -181,7 +182,7 @@ class CourseModeAdmin(admin.ModelAdmin): 'mode_display_name', 'min_price', 'currency', - 'expiration_datetime', + '_expiration_datetime', 'verification_deadline', 'sku' ) @@ -206,4 +207,12 @@ class CourseModeAdmin(admin.ModelAdmin): # in the Django admin list view. expiration_datetime_custom.short_description = "Upgrade Deadline" + +class CourseModeExpirationConfigAdmin(admin.ModelAdmin): + """Admin interface for the course mode auto expiration configuration. """ + + class Meta(object): + model = CourseModeExpirationConfig + admin.site.register(CourseMode, CourseModeAdmin) +admin.site.register(CourseModeExpirationConfig, CourseModeExpirationConfigAdmin) diff --git a/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py b/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py new file mode 100644 index 0000000000..04d20662aa --- /dev/null +++ b/common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_modes', '0002_coursemode_expiration_datetime_is_explicit'), + ] + + operations = [ + migrations.AlterField( + model_name='coursemode', + name='expiration_datetime_is_explicit', + field=models.BooleanField(default=False), + ), + ] diff --git a/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py b/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py new file mode 100644 index 0000000000..3504d15cbc --- /dev/null +++ b/common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals +from datetime import timedelta + +from django.db import migrations, models +import django.db.models.deletion +from django.conf import settings + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('course_modes', '0003_auto_20151113_1443'), + ] + + operations = [ + migrations.CreateModel( + name='CourseModeExpirationConfig', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('verification_window', models.DurationField(default=timedelta(10), help_text='The time period before a course ends in which a course mode will expire')), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + options={ + 'ordering': ('-change_date',), + 'abstract': False, + }, + ), + ] diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 022de21c21..b256dc44c2 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -1,15 +1,15 @@ """ Add and create new modes for running courses on this particular LMS """ +from datetime import datetime, timedelta import pytz -from datetime import datetime +from collections import namedtuple, defaultdict +from config_models.models import ConfigurationModel from django.core.exceptions import ValidationError from django.db import models -from collections import namedtuple, defaultdict -from django.utils.translation import ugettext_lazy as _ from django.db.models import Q - +from django.utils.translation import ugettext_lazy as _ from xmodule_django.models import CourseKeyField Mode = namedtuple('Mode', @@ -54,19 +54,20 @@ class CourseMode(models.Model): # For example, if there is a verified mode that expires on 1/1/2015, # then users will be able to upgrade into the verified mode before that date. # Once the date passes, users will no longer be able to enroll as verified. - expiration_datetime = models.DateTimeField( + _expiration_datetime = models.DateTimeField( default=None, null=True, blank=True, verbose_name=_(u"Upgrade Deadline"), help_text=_( u"OPTIONAL: After this date/time, users will no longer be able to enroll in this mode. " u"Leave this blank if users can enroll in this mode until enrollment closes for the course." ), + db_column='expiration_datetime', ) # The system prefers to set this automatically based on default settings. But # if the field is set manually we want a way to indicate that so we don't # overwrite the manual setting of the field. - expiration_datetime_is_explicit = models.BooleanField(default=True) + expiration_datetime_is_explicit = models.BooleanField(default=False) # DEPRECATED: the `expiration_date` field has been replaced by `expiration_datetime` expiration_date = models.DateField(default=None, null=True, blank=True) @@ -150,6 +151,17 @@ class CourseMode(models.Model): """ return self.mode_slug + @property + def expiration_datetime(self): + """ Return _expiration_datetime. """ + return self._expiration_datetime + + @expiration_datetime.setter + def expiration_datetime(self, new_datetime): + """ Saves datetime to _expiration_datetime and sets the explicit flag. """ + self.expiration_datetime_is_explicit = True + self._expiration_datetime = new_datetime + @classmethod def all_modes_for_courses(cls, course_id_list): """Find all modes for a list of course IDs, including expired modes. @@ -223,8 +235,8 @@ class CourseMode(models.Model): Q(course_id=course_id) & Q(min_price__gt=0) & ( - Q(expiration_datetime__isnull=True) | - Q(expiration_datetime__gte=now) + Q(_expiration_datetime__isnull=True) | + Q(_expiration_datetime__gte=now) ) ) return [mode.to_tuple() for mode in found_course_modes] @@ -259,7 +271,7 @@ class CourseMode(models.Model): # Filter out expired course modes if include_expired is not set if not include_expired: found_course_modes = found_course_modes.filter( - Q(expiration_datetime__isnull=True) | Q(expiration_datetime__gte=now) + Q(_expiration_datetime__isnull=True) | Q(_expiration_datetime__gte=now) ) # Credit course modes are currently not shown on the track selection page; @@ -633,3 +645,19 @@ class CourseModesArchive(models.Model): expiration_date = models.DateField(default=None, null=True, blank=True) expiration_datetime = models.DateTimeField(default=None, null=True, blank=True) + + +class CourseModeExpirationConfig(ConfigurationModel): + """ + Configuration for time period from end of course to auto-expire a course mode. + """ + verification_window = models.DurationField( + default=timedelta(days=10), + help_text=_( + "The time period before a course ends in which a course mode will expire" + ) + ) + + def __unicode__(self): + """ Returns the unicode date of the verification window. """ + return unicode(self.verification_window) diff --git a/common/djangoapps/course_modes/signals.py b/common/djangoapps/course_modes/signals.py new file mode 100644 index 0000000000..066004ab09 --- /dev/null +++ b/common/djangoapps/course_modes/signals.py @@ -0,0 +1,36 @@ +""" +Signal handler for setting default course mode expiration dates +""" +from django.core.exceptions import ObjectDoesNotExist +from django.dispatch.dispatcher import receiver +from xmodule.modulestore.django import SignalHandler, modulestore + +from .models import CourseMode, CourseModeExpirationConfig + + +@receiver(SignalHandler.course_published) +def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Catches the signal that a course has been published in Studio and + sets the verified mode dates to defaults. + """ + try: + verified_mode = CourseMode.objects.get(course_id=course_key, mode_slug=CourseMode.VERIFIED) + if _should_update_date(verified_mode): + course = modulestore().get_course(course_key) + if not course: + return None + verification_window = CourseModeExpirationConfig.current().verification_window + new_expiration_datetime = course.end - verification_window + + if verified_mode.expiration_datetime != new_expiration_datetime: + # Set the expiration_datetime without triggering the explicit flag + verified_mode._expiration_datetime = new_expiration_datetime # pylint: disable=protected-access + verified_mode.save() + except ObjectDoesNotExist: + pass + + +def _should_update_date(verified_mode): + """ Returns whether or not the verified mode should be updated. """ + return not(verified_mode is None or verified_mode.expiration_datetime_is_explicit) diff --git a/common/djangoapps/course_modes/startup.py b/common/djangoapps/course_modes/startup.py new file mode 100644 index 0000000000..c2e0f4d49d --- /dev/null +++ b/common/djangoapps/course_modes/startup.py @@ -0,0 +1,4 @@ +""" +Setup the signals on startup. +""" +import course_modes.signals # pylint: disable=unused-import diff --git a/common/djangoapps/course_modes/tests/test_admin.py b/common/djangoapps/course_modes/tests/test_admin.py index f65c3a6e9f..939c62d3d8 100644 --- a/common/djangoapps/course_modes/tests/test_admin.py +++ b/common/djangoapps/course_modes/tests/test_admin.py @@ -48,8 +48,8 @@ class AdminCourseModePageTest(ModuleStoreTestCase): 'mode_display_name': 'verified', 'min_price': 10, 'currency': 'usd', - 'expiration_datetime_0': expiration.date(), # due to django admin datetime widget passing as seperate vals - 'expiration_datetime_1': expiration.time(), + '_expiration_datetime_0': expiration.date(), # due to django admin datetime widget passing as separate vals + '_expiration_datetime_1': expiration.time(), } @@ -201,7 +201,7 @@ class AdminCourseModeFormTest(ModuleStoreTestCase): "course_id": unicode(self.course.id), "mode_slug": mode, "mode_display_name": mode, - "expiration_datetime": upgrade_deadline, + "_expiration_datetime": upgrade_deadline, "currency": "usd", "min_price": 10, }, instance=course_mode) diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 24a0dc3fde..c83ed6a00e 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -49,7 +49,7 @@ class CourseModeModelTest(TestCase): min_price=min_price, suggested_prices=suggested_prices, currency=currency, - expiration_datetime=expiration_datetime, + _expiration_datetime=expiration_datetime, ) def test_save(self): @@ -403,3 +403,21 @@ class CourseModeModelTest(TestCase): return dict(zip(dict_keys, display_values.get('verify_none'))) else: return dict(zip(dict_keys, display_values.get(dict_type))) + + def test_expiration_datetime_explicitly_set(self): + """ Verify that setting the expiration_date property sets the explicit flag. """ + verified_mode, __ = self.create_mode('verified', 'Verified Certificate') + now = datetime.now() + verified_mode.expiration_datetime = now + + self.assertTrue(verified_mode.expiration_datetime_is_explicit) + self.assertEqual(verified_mode.expiration_datetime, now) + + def test_expiration_datetime_not_explicitly_set(self): + """ Verify that setting the _expiration_date property does not set the explicit flag. """ + verified_mode, __ = self.create_mode('verified', 'Verified Certificate') + now = datetime.now() + verified_mode._expiration_datetime = now # pylint: disable=protected-access + + self.assertFalse(verified_mode.expiration_datetime_is_explicit) + self.assertEqual(verified_mode.expiration_datetime, now) diff --git a/common/djangoapps/course_modes/tests/test_signals.py b/common/djangoapps/course_modes/tests/test_signals.py new file mode 100644 index 0000000000..36d0dde1d2 --- /dev/null +++ b/common/djangoapps/course_modes/tests/test_signals.py @@ -0,0 +1,89 @@ +""" +Unit tests for the course_mode signals +""" + +from datetime import datetime, timedelta +from mock import patch + +import ddt +from pytz import UTC +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from course_modes.models import CourseMode +from course_modes.signals import _listen_for_course_publish + + +@ddt.ddt +class CourseModeSignalTest(ModuleStoreTestCase): + """ + Tests for the course_mode course_published signal. + """ + + def setUp(self): + super(CourseModeSignalTest, self).setUp() + self.end = datetime.now(tz=UTC).replace(microsecond=0) + timedelta(days=7) + self.course = CourseFactory.create(end=self.end) + CourseMode.objects.all().delete() + + def create_mode( + self, + mode_slug, + mode_name, + min_price=0, + suggested_prices='', + currency='usd', + expiration_datetime=None, + ): + """ + Create a new course mode + """ + return CourseMode.objects.get_or_create( + course_id=self.course.id, + mode_display_name=mode_name, + mode_slug=mode_slug, + min_price=min_price, + suggested_prices=suggested_prices, + currency=currency, + _expiration_datetime=expiration_datetime, + ) + + def test_no_verified_mode(self): + """ Verify expiration not updated by signal for non-verified mode. """ + course_mode, __ = self.create_mode('honor', 'honor') + + _listen_for_course_publish('store', self.course.id) + course_mode.refresh_from_db() + + self.assertIsNone(course_mode.expiration_datetime) + + @ddt.data(1, 14, 30) + def test_verified_mode(self, verification_window): + """ Verify signal updates expiration to configured time period before course end for verified mode. """ + course_mode, __ = self.create_mode('verified', 'verified') + self.assertIsNone(course_mode.expiration_datetime) + + with patch('course_modes.models.CourseModeExpirationConfig.current') as config: + instance = config.return_value + instance.verification_window = timedelta(days=verification_window) + + _listen_for_course_publish('store', self.course.id) + course_mode.refresh_from_db() + + self.assertEqual(course_mode.expiration_datetime, self.end - timedelta(days=verification_window)) + + @ddt.data(1, 14, 30) + def test_verified_mode_explicitly_set(self, verification_window): + """ Verify signal does not update expiration for verified mode with explicitly set expiration. """ + course_mode, __ = self.create_mode('verified', 'verified') + course_mode.expiration_datetime_is_explicit = True + self.assertIsNone(course_mode.expiration_datetime) + + with patch('course_modes.models.CourseModeExpirationConfig.current') as config: + instance = config.return_value + instance.verification_window = timedelta(days=verification_window) + + _listen_for_course_publish('store', self.course.id) + course_mode.refresh_from_db() + + self.assertEqual(course_mode.expiration_datetime, self.end - timedelta(days=verification_window)) diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py index 6440dc3083..6d042fe6f1 100644 --- a/lms/djangoapps/commerce/api/v1/models.py +++ b/lms/djangoapps/commerce/api/v1/models.py @@ -58,8 +58,9 @@ class Course(object): 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) + # Override the verification deadline for the course (not the individual modes) + if self.verification_deadline is not None: + VerificationDeadline.set_deadline(self.id, self.verification_deadline, is_explicit=True) for mode in self.modes: mode.course_id = self.id @@ -87,7 +88,8 @@ 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 + if posted_mode.expiration_datetime is not None: + merged_mode.expiration_datetime = posted_mode.expiration_datetime merged_mode.save() merged_modes.add(merged_mode) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 091a5f7bed..48e68a1338 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1531,7 +1531,7 @@ def financial_assistance_form(request): {'name': enrollment.course_overview.display_name, 'value': unicode(enrollment.course_id)} for enrollment in CourseEnrollment.enrollments_for_user(user).order_by('-created') if CourseMode.objects.filter( - Q(expiration_datetime__isnull=True) | Q(expiration_datetime__gt=datetime.now(UTC())), + Q(_expiration_datetime__isnull=True) | Q(_expiration_datetime__gt=datetime.now(UTC())), course_id=enrollment.course_id, mode_slug=CourseMode.VERIFIED ).exists() diff --git a/lms/djangoapps/verify_student/migrations/0003_auto_20151113_1443.py b/lms/djangoapps/verify_student/migrations/0003_auto_20151113_1443.py new file mode 100644 index 0000000000..6b29b475bd --- /dev/null +++ b/lms/djangoapps/verify_student/migrations/0003_auto_20151113_1443.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('verify_student', '0002_auto_20151124_1024'), + ] + + operations = [ + migrations.AlterField( + model_name='historicalverificationdeadline', + name='deadline_is_explicit', + field=models.BooleanField(default=False), + ), + migrations.AlterField( + model_name='verificationdeadline', + name='deadline_is_explicit', + field=models.BooleanField(default=False), + ), + ] diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 2b42ce05db..5e735c04f9 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -958,7 +958,7 @@ class VerificationDeadline(TimeStampedModel): # The system prefers to set this automatically based on default settings. But # if the field is set manually we want a way to indicate that so we don't # overwrite the manual setting of the field. - deadline_is_explicit = models.BooleanField(default=True) + deadline_is_explicit = models.BooleanField(default=False) # Maintain a history of changes to deadlines for auditing purposes history = HistoricalRecords() @@ -966,7 +966,7 @@ class VerificationDeadline(TimeStampedModel): ALL_DEADLINES_CACHE_KEY = "verify_student.all_verification_deadlines" @classmethod - def set_deadline(cls, course_key, deadline): + def set_deadline(cls, course_key, deadline, is_explicit=False): """ Configure the verification deadline for a course. @@ -984,11 +984,12 @@ class VerificationDeadline(TimeStampedModel): else: record, created = VerificationDeadline.objects.get_or_create( course_key=course_key, - defaults={"deadline": deadline} + defaults={"deadline": deadline, "deadline_is_explicit": is_explicit} ) if not created: record.deadline = deadline + record.deadline_is_explicit = is_explicit record.save() @classmethod diff --git a/lms/djangoapps/verify_student/signals.py b/lms/djangoapps/verify_student/signals.py new file mode 100644 index 0000000000..5e4b94b47c --- /dev/null +++ b/lms/djangoapps/verify_student/signals.py @@ -0,0 +1,24 @@ +""" +Signal handler for setting default course verification dates +""" +from django.core.exceptions import ObjectDoesNotExist +from django.dispatch.dispatcher import receiver +from xmodule.modulestore.django import SignalHandler, modulestore + +from .models import VerificationDeadline + + +@receiver(SignalHandler.course_published) +def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Catches the signal that a course has been published in Studio and + sets the verification deadline date to a default. + """ + try: + deadline = VerificationDeadline.objects.get(course_key=course_key) + if deadline and not deadline.deadline_is_explicit: + course = modulestore().get_course(course_key) + if course and deadline.deadline != course.end: + VerificationDeadline.set_deadline(course_key, course.end) + except ObjectDoesNotExist: + pass diff --git a/lms/djangoapps/verify_student/startup.py b/lms/djangoapps/verify_student/startup.py new file mode 100644 index 0000000000..8045c5f972 --- /dev/null +++ b/lms/djangoapps/verify_student/startup.py @@ -0,0 +1,4 @@ +""" +Setup the signals on startup. +""" +import lms.djangoapps.verify_student.signals # pylint: disable=unused-import diff --git a/lms/djangoapps/verify_student/tests/test_signals.py b/lms/djangoapps/verify_student/tests/test_signals.py new file mode 100644 index 0000000000..aa828880a1 --- /dev/null +++ b/lms/djangoapps/verify_student/tests/test_signals.py @@ -0,0 +1,49 @@ +""" +Unit tests for the VerificationDeadline signals +""" + +from datetime import datetime, timedelta + +from pytz import UTC +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from lms.djangoapps.verify_student.models import VerificationDeadline +from lms.djangoapps.verify_student.signals import _listen_for_course_publish + + +class VerificationDeadlineSignalTest(ModuleStoreTestCase): + """ + Tests for the VerificationDeadline signal + """ + + def setUp(self): + super(VerificationDeadlineSignalTest, self).setUp() + self.end = datetime.now(tz=UTC).replace(microsecond=0) + timedelta(days=7) + self.course = CourseFactory.create(end=self.end) + VerificationDeadline.objects.all().delete() + + def test_no_deadline(self): + """ Verify the signal does not raise error when no deadlines found. """ + _listen_for_course_publish('store', self.course.id) + + self.assertIsNone(_listen_for_course_publish('store', self.course.id)) + + def test_deadline(self): + """ Verify deadline is set to course end date by signal. """ + deadline = datetime.now(tz=UTC) - timedelta(days=7) + VerificationDeadline.set_deadline(self.course.id, deadline) + + _listen_for_course_publish('store', self.course.id) + self.assertEqual(VerificationDeadline.deadline_for_course(self.course.id), self.course.end) + + def test_deadline_explicit(self): + """ Verify deadline is unchanged by signal when explicitly set. """ + deadline = datetime.now(tz=UTC) - timedelta(days=7) + VerificationDeadline.set_deadline(self.course.id, deadline, is_explicit=True) + + _listen_for_course_publish('store', self.course.id) + + actual_deadline = VerificationDeadline.deadline_for_course(self.course.id) + self.assertNotEqual(actual_deadline, self.course.end) + self.assertEqual(actual_deadline, deadline)