diff --git a/lms/djangoapps/courseware/admin.py b/lms/djangoapps/courseware/admin.py index 3b704565a8..17106b5149 100644 --- a/lms/djangoapps/courseware/admin.py +++ b/lms/djangoapps/courseware/admin.py @@ -1,13 +1,10 @@ -''' -django admin pages for courseware model -''' - +from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin from ratelimitbackend import admin -from courseware.models import OfflineComputedGrade, OfflineComputedGradeLog, StudentModule +from courseware import models -admin.site.register(StudentModule) - -admin.site.register(OfflineComputedGrade) - -admin.site.register(OfflineComputedGradeLog) +admin.site.register(models.DynamicUpgradeDeadlineConfiguration, ConfigurationModelAdmin) +admin.site.register(models.OfflineComputedGrade) +admin.site.register(models.OfflineComputedGradeLog) +admin.site.register(models.CourseDynamicUpgradeDeadlineConfiguration, KeyedConfigurationModelAdmin) +admin.site.register(models.StudentModule) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index eeed11b6d3..18d5e66968 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -18,7 +18,7 @@ from courseware.date_summary import ( VerifiedUpgradeDeadlineDate ) from courseware.model_data import FieldDataCache -from courseware.module_render import get_module, get_module_for_descriptor +from courseware.module_render import get_module from django.conf import settings from django.core.urlresolvers import reverse from django.http import Http404, QueryDict diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index ae0b3a0fdd..a8f80fb430 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -3,18 +3,21 @@ This module provides date summary blocks for the Course Info page. Each block gives information about a particular course-run-specific date which will be displayed to the user. """ -from datetime import datetime +import datetime from babel.dates import format_timedelta from django.core.urlresolvers import reverse -from django.utils.translation import ugettext as _ +from django.utils.functional import cached_property from django.utils.translation import get_language, to_locale, ugettext_lazy +from django.utils.translation import ugettext as _ from lazy import lazy from pytz import timezone, utc from course_modes.models import CourseMode +from courseware.models import CourseDynamicUpgradeDeadlineConfiguration, DynamicUpgradeDeadlineConfiguration from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment @@ -85,7 +88,7 @@ class DateSummary(object): if self.date is None: return '' locale = to_locale(get_language()) - delta = self.date - datetime.now(utc) + delta = self.date - datetime.datetime.now(utc) try: relative_date = format_timedelta(delta, locale=locale) # Babel doesn't have translations for Esperanto, so we get @@ -115,7 +118,7 @@ class DateSummary(object): future. """ if self.date is not None: - return datetime.now(utc).date() <= self.date.date() + return datetime.datetime.now(utc).date() <= self.date.date() return False def deadline_has_passed(self): @@ -124,7 +127,7 @@ class DateSummary(object): Returns False otherwise. """ deadline = self.date - return deadline is not None and deadline <= datetime.now(utc) + return deadline is not None and deadline <= datetime.datetime.now(utc) def __repr__(self): return u'DateSummary: "{title}" {date} is_enabled={is_enabled}'.format( @@ -149,7 +152,7 @@ class TodaysDate(DateSummary): @property def date(self): - return datetime.now(utc) + return datetime.datetime.now(utc) @property def title(self): @@ -181,7 +184,7 @@ class CourseEndDate(DateSummary): @property def description(self): - if datetime.now(utc) <= self.date: + if datetime.datetime.now(utc) <= self.date: mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course_id) if is_active and CourseMode.is_eligible_for_certificate(mode): return _('To earn a certificate, you must complete all requirements before this date.') @@ -217,6 +220,14 @@ class VerifiedUpgradeDeadlineDate(DateSummary): return ecommerce_service.get_checkout_page_url(course_mode.sku) return reverse('verify_student_upgrade_and_verify', args=(self.course_id,)) + @cached_property + def enrollment(self): + return CourseEnrollment.get_enrollment(self.user, self.course_id) + + @cached_property + def course_overview(self): + return CourseOverview.get_from_id(self.course_id) + @property def is_enabled(self): """ @@ -229,7 +240,12 @@ class VerifiedUpgradeDeadlineDate(DateSummary): if not is_enabled: return False - enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course_id) + enrollment_mode = None + is_active = None + + if self.enrollment: + enrollment_mode = self.enrollment.mode + is_active = self.enrollment.is_active # Return `true` if user is not enrolled in course if enrollment_mode is None and is_active is None: @@ -240,13 +256,40 @@ class VerifiedUpgradeDeadlineDate(DateSummary): @lazy def date(self): + deadline = None + try: - verified_mode = CourseMode.objects.get( - course_id=self.course_id, mode_slug=CourseMode.VERIFIED - ) - return verified_mode.expiration_datetime + verified_mode = CourseMode.objects.get(course_id=self.course_id, mode_slug=CourseMode.VERIFIED) + deadline = verified_mode.expiration_datetime except CourseMode.DoesNotExist: - return None + pass + + if self.course and self.course_overview.self_paced and self.enrollment: + global_config = DynamicUpgradeDeadlineConfiguration.current() + if global_config.enabled: + delta = global_config.deadline_days + + # Check if the given course has opted out of the feature + course_config = CourseDynamicUpgradeDeadlineConfiguration.current(self.course.id) + if course_config.enabled: + if course_config.opt_out: + return deadline + + delta = course_config.deadline_days + + # This represents the first date at which the learner can access the content. This will be the + # latter of either the enrollment date or the course's start date. + content_availability_date = max(self.enrollment.created, self.course_overview.start) + user_deadline = content_availability_date + datetime.timedelta(days=delta) + + # If the deadline from above is None, make sure we have a value for comparison + deadline = deadline or datetime.date.max + + # The user-specific deadline should never occur after the verified mode's expiration date, + # if one is set. + deadline = min(deadline, user_deadline) + + return deadline class VerificationDeadlineDate(DateSummary): diff --git a/lms/djangoapps/courseware/migrations/0002_coursedynamicupgradedeadlineconfiguration_dynamicupgradedeadlineconfiguration.py b/lms/djangoapps/courseware/migrations/0002_coursedynamicupgradedeadlineconfiguration_dynamicupgradedeadlineconfiguration.py new file mode 100644 index 0000000000..555670d236 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0002_coursedynamicupgradedeadlineconfiguration_dynamicupgradedeadlineconfiguration.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +from django.conf import settings +import django.db.models.deletion +import openedx.core.djangoapps.xmodule_django.models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('courseware', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='CourseDynamicUpgradeDeadlineConfiguration', + 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')), + ('course_id', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True)), + ('deadline_days', models.PositiveSmallIntegerField(default=21, help_text='Number of days a learner has to upgrade after content is made available')), + ('opt_out', models.BooleanField(default=False, help_text='Disable the dynamic upgrade deadline for this course run.')), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + ), + migrations.CreateModel( + name='DynamicUpgradeDeadlineConfiguration', + 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')), + ('deadline_days', models.PositiveSmallIntegerField(default=21, help_text='Number of days a learner has to upgrade after content is made available')), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + ), + ] diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 7e0c5fc5a8..00b58c82d8 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -15,10 +15,12 @@ ASSUMPTIONS: modules have unique IDs, even across different module_types import itertools import logging +from config_models.models import ConfigurationModel from django.conf import settings from django.contrib.auth.models import User from django.db import models from django.db.models.signals import post_save +from django.utils.translation import ugettext_lazy as _ from model_utils.models import TimeStampedModel import coursewarehistoryextended @@ -40,6 +42,7 @@ class ChunkingManager(models.Manager): :class:`~Manager` that adds an additional method :meth:`chunked_filter` to provide the ability to make select queries with specific chunk sizes. """ + class Meta(object): app_label = "courseware" @@ -130,16 +133,17 @@ class StudentModule(models.Model): return queryset def __repr__(self): - return 'StudentModule<%r>' % ({ - 'course_id': self.course_id, - 'module_type': self.module_type, - # We use the student_id instead of username to avoid a database hop. - # This can actually matter in cases where we're logging many of - # these (e.g. on a broken progress page). - 'student_id': self.student_id, - 'module_state_key': self.module_state_key, - 'state': str(self.state)[:20], - },) + return 'StudentModule<%r>' % ( + { + 'course_id': self.course_id, + 'module_type': self.module_type, + # We use the student_id instead of username to avoid a database hop. + # This can actually matter in cases where we're logging many of + # these (e.g. on a broken progress page). + 'student_id': self.student_id, + 'module_state_key': self.module_state_key, + 'state': str(self.state)[:20], + },) def __unicode__(self): return unicode(repr(self)) @@ -267,6 +271,7 @@ class XModuleUserStateSummaryField(XBlockFieldBase): """ Stores data set in the Scope.user_state_summary scope by an xmodule field """ + class Meta(object): app_label = "courseware" unique_together = (('usage_id', 'field_name'),) @@ -279,6 +284,7 @@ class XModuleStudentPrefsField(XBlockFieldBase): """ Stores data set in the Scope.preferences scope by an xmodule field """ + class Meta(object): app_label = "courseware" unique_together = (('student', 'module_type', 'field_name'),) @@ -293,6 +299,7 @@ class XModuleStudentInfoField(XBlockFieldBase): """ Stores data set in the Scope.preferences scope by an xmodule field """ + class Meta(object): app_label = "courseware" unique_together = (('student', 'field_name'),) @@ -310,11 +317,11 @@ class OfflineComputedGrade(models.Model): created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) updated = models.DateTimeField(auto_now=True, db_index=True) - gradeset = models.TextField(null=True, blank=True) # grades, stored as JSON + gradeset = models.TextField(null=True, blank=True) # grades, stored as JSON class Meta(object): app_label = "courseware" - unique_together = (('user', 'course_id'), ) + unique_together = (('user', 'course_id'),) def __unicode__(self): return "[OfflineComputedGrade] %s: %s (%s) = %s" % (self.user, self.course_id, self.created, self.gradeset) @@ -325,6 +332,7 @@ class OfflineComputedGradeLog(models.Model): Log of when offline grades are computed. Use this to be able to show instructor when the last computed grades were done. """ + class Meta(object): app_label = "courseware" ordering = ["-created"] @@ -332,7 +340,7 @@ class OfflineComputedGradeLog(models.Model): course_id = CourseKeyField(max_length=255, db_index=True) created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) - seconds = models.IntegerField(default=0) # seconds elapsed for computation + seconds = models.IntegerField(default=0) # seconds elapsed for computation nstudents = models.IntegerField(default=0) def __unicode__(self): @@ -355,3 +363,40 @@ class StudentFieldOverride(TimeStampedModel): field = models.CharField(max_length=255) value = models.TextField(default='null') + + +class DynamicUpgradeDeadlineConfiguration(ConfigurationModel): + """ Dynamic upgrade deadline configuration. + + This model controls the behavior of the dynamic upgrade deadline for self-paced courses. + """ + class Meta(object): + app_label = 'courseware' + + deadline_days = models.PositiveSmallIntegerField( + default=21, + help_text=_('Number of days a learner has to upgrade after content is made available') + ) + + +class CourseDynamicUpgradeDeadlineConfiguration(ConfigurationModel): + """ + Per-course run configuration for dynamic upgrade deadlines. + + This model controls dynamic upgrade deadlines on a per-course run level, allowing course runs to + have different deadlines or opt out of the functionality altogether. + """ + class Meta(object): + app_label = 'courseware' + + KEY_FIELDS = ('course_id',) + + course_id = CourseKeyField(max_length=255, db_index=True) + deadline_days = models.PositiveSmallIntegerField( + default=21, + help_text=_('Number of days a learner has to upgrade after content is made available') + ) + opt_out = models.BooleanField( + default=False, + help_text=_('Disable the dynamic upgrade deadline for this course run.') + ) diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 3ebc7f1239..240da2360c 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -15,18 +15,19 @@ from courseware.courses import get_course_date_blocks from courseware.date_summary import ( CourseEndDate, CourseStartDate, - DateSummary, TodaysDate, VerificationDeadlineDate, VerifiedUpgradeDeadlineDate ) +from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration from lms.djangoapps.verify_student.models import VerificationDeadline from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG -from student.tests.factories import CourseEnrollmentFactory, UserFactory +from student.tests.factories import CourseEnrollmentFactory, UserFactory, TEST_PASSWORD from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -56,12 +57,12 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): ): """Set up the course and user for this test.""" now = datetime.now(utc) - if create_user: - self.user = UserFactory.create(username='mrrobot', password='test') # pylint: disable=attribute-defined-outside-init - self.course = CourseFactory.create( # pylint: disable=attribute-defined-outside-init - start=now + timedelta(days=days_till_start) - ) + # pylint: disable=attribute-defined-outside-init + if create_user: + self.user = UserFactory() + + self.course = CourseFactory.create(start=now + timedelta(days=days_till_start)) if days_till_end is not None: self.course.end = now + timedelta(days=days_till_end) @@ -96,7 +97,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): def test_course_info_feature_flag(self): SelfPacedConfiguration(enable_course_home_improvements=False).save() self.setup_course_and_user() - self.client.login(username='mrrobot', password='test') + self.client.login(username=self.user.username, password=TEST_PASSWORD) url = reverse('info', args=(self.course.id,)) response = self.client.get(url) self.assertNotIn('date-summary', response.content) @@ -198,7 +199,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): def test_todays_date_no_timezone(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() - self.client.login(username='mrrobot', password='test') + self.client.login(username=self.user.username, password=TEST_PASSWORD) html_elements = [ '

Important Course Dates

', @@ -209,7 +210,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): 'data-string="Today is {date}"', 'data-timezone="None"' ] - url = reverse(url_name, args=(self.course.id, )) + url = reverse(url_name, args=(self.course.id,)) response = self.client.get(url, follow=True) for html in html_elements: self.assertContains(response, html) @@ -222,7 +223,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): def test_todays_date_timezone(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() - self.client.login(username='mrrobot', password='test') + self.client.login(username=self.user.username, password=TEST_PASSWORD) set_user_preference(self.user, "time_zone", "America/Los_Angeles") url = reverse(url_name, args=(self.course.id,)) response = self.client.get(url, follow=True) @@ -253,7 +254,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): def test_start_date_render(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() - self.client.login(username='mrrobot', password='test') + self.client.login(username=self.user.username, password=TEST_PASSWORD) url = reverse(url_name, args=(self.course.id,)) response = self.client.get(url, follow=True) html_elements = [ @@ -271,7 +272,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): def test_start_date_render_time_zone(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() - self.client.login(username='mrrobot', password='test') + self.client.login(username=self.user.username, password=TEST_PASSWORD) set_user_preference(self.user, "time_zone", "America/Los_Angeles") url = reverse(url_name, args=(self.course.id,)) response = self.client.get(url, follow=True) @@ -389,3 +390,62 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): ) block = VerificationDeadlineDate(self.course, self.user) self.assertEqual(block.relative_datestring, expected_date_string) + + def create_self_paced_course_run(self, **kwargs): + defaults = { + 'enroll_user': False, + 'days_till_upgrade_deadline': 100, + } + defaults.update(kwargs) + self.setup_course_and_user(**defaults) + self.course.self_paced = True + self.store.update_item(self.course, self.user.id) + overview = CourseOverview.get_from_id(self.course.id) + self.assertTrue(overview.self_paced) + + def test_date_with_self_paced(self): + """ The date returned for self-paced course runs should be dependent on the learner's enrollment date. """ + global_config = DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + + # Enrollments made before the course start should use the course start date as the content availability date + self.create_self_paced_course_run(days_till_start=3) + CourseEnrollmentFactory.create(course_id=self.course.id, user=self.user, mode=CourseMode.AUDIT) + block = VerifiedUpgradeDeadlineDate(self.course, self.user) + overview = CourseOverview.get_from_id(self.course.id) + expected = overview.start + timedelta(days=global_config.deadline_days) + self.assertEqual(block.date, expected) + + # Enrollments made after the course start should use the enrollment date as the content availability date + self.create_self_paced_course_run(days_till_start=-1) + enrollment = CourseEnrollmentFactory.create(course_id=self.course.id, user=self.user, mode=CourseMode.AUDIT) + block = VerifiedUpgradeDeadlineDate(self.course, self.user) + expected = enrollment.created + timedelta(days=global_config.deadline_days) + self.assertEqual(block.date, expected) + + # Courses should be able to override the deadline + course_config = CourseDynamicUpgradeDeadlineConfiguration.objects.create( + enabled=True, course_id=self.course.id, opt_out=False, deadline_days=3 + ) + block = VerifiedUpgradeDeadlineDate(self.course, self.user) + expected = enrollment.created + timedelta(days=course_config.deadline_days) + self.assertEqual(block.date, expected) + + # Disabling the functionality should result in the verified mode's expiration date being returned. + global_config.enabled = False + global_config.save() + block = VerifiedUpgradeDeadlineDate(self.course, self.user) + expected = CourseMode.objects.get(course_id=self.course.id, mode_slug=CourseMode.VERIFIED).expiration_datetime + self.assertEqual(block.date, expected) + + def test_date_with_self_paced_with_course_opt_out(self): + """ If the course run has opted out of the dynamic deadline, the course mode's deadline should be used. """ + self.create_self_paced_course_run(days_till_start=-1) + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + CourseEnrollmentFactory.create(course_id=self.course.id, user=self.user, mode=CourseMode.AUDIT) + + # Opt the course out of the dynamic upgrade deadline + CourseDynamicUpgradeDeadlineConfiguration.objects.create(enabled=True, course_id=self.course.id, opt_out=True) + + block = VerifiedUpgradeDeadlineDate(self.course, self.user) + expected = CourseMode.objects.get(course_id=self.course.id, mode_slug=CourseMode.VERIFIED).expiration_datetime + self.assertEqual(block.date, expected) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 4508797807..4f10ce7677 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -211,8 +211,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 143), - (ModuleStoreEnum.Type.split, 4, 143), + (ModuleStoreEnum.Type.mongo, 10, 144), + (ModuleStoreEnum.Type.split, 4, 144), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index 91ba8d813a..1374e50a27 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -148,9 +148,9 @@ class RenderXBlockTestMixin(object): return response @ddt.data( - ('vertical_block', ModuleStoreEnum.Type.mongo, 10), + ('vertical_block', ModuleStoreEnum.Type.mongo, 14), ('vertical_block', ModuleStoreEnum.Type.split, 6), - ('html_block', ModuleStoreEnum.Type.mongo, 11), + ('html_block', ModuleStoreEnum.Type.mongo, 15), ('html_block', ModuleStoreEnum.Type.split, 6), ) @ddt.unpack diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 39478c162e..e4b73175e8 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -160,7 +160,7 @@ class TestCourseHomePage(CourseHomePageTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(38, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(40, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url)