Auto setting of verification deadlines with manual overrides

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)

common/djangoapps/course_modes/migrations/0003_auto_20151113_1443.py

+# -*- 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),
+        ),
+    ]

common/djangoapps/course_modes/migrations/0004_auto_20151113_1457.py

+# -*- 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,
+            },
+        ),
+    ]

common/djangoapps/course_modes/models.py

 """
 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)

common/djangoapps/course_modes/signals.py

+"""
+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)

common/djangoapps/course_modes/startup.py

+"""
+Setup the signals on startup.
+"""
+import course_modes.signals  # pylint: disable=unused-import

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)

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)

common/djangoapps/course_modes/tests/test_signals.py

+"""
+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))

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)

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()

lms/djangoapps/verify_student/migrations/0003_auto_20151113_1443.py

+# -*- 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),
+        ),
+    ]

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

lms/djangoapps/verify_student/signals.py

+"""
+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

lms/djangoapps/verify_student/startup.py

+"""
+Setup the signals on startup.
+"""
+import lms.djangoapps.verify_student.signals  # pylint: disable=unused-import

lms/djangoapps/verify_student/tests/test_signals.py

+"""
+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)