From 519364995698187788129ee2011766de73f8b6da Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 7 Sep 2017 10:51:30 -0400 Subject: [PATCH] Update Schedules when start date changes on non-live courses --- .../certificates/tests/test_signals.py | 24 ++--- .../certificates/tests/test_webview_views.py | 3 +- .../content/course_overviews/signals.py | 29 +++++- .../tests/test_course_overviews.py | 52 +---------- .../course_overviews/tests/test_signals.py | 91 ++++++++++++++++++ .../core/djangoapps/models/course_details.py | 7 -- openedx/core/djangoapps/schedules/models.py | 2 - openedx/core/djangoapps/schedules/signals.py | 92 +++++++++++++------ openedx/core/djangoapps/schedules/tasks.py | 30 +++++- .../schedules/tests/test_signals.py | 61 +++++++++++- openedx/core/djangoapps/signals/handlers.py | 16 ++-- openedx/core/djangoapps/signals/signals.py | 4 +- 12 files changed, 289 insertions(+), 122 deletions(-) create mode 100644 openedx/core/djangoapps/content/course_overviews/tests/test_signals.py diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index a6f76a91af..7712cf1922 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -5,12 +5,12 @@ and disabling for instructor-paced courses. import mock from certificates import api as certs_api -from certificates.models import \ - CertificateGenerationConfiguration, \ - CertificateWhitelist, \ - GeneratedCertificate, \ - CertificateStatuses -from openedx.core.djangoapps.signals.handlers import _listen_for_course_pacing_changed +from certificates.models import ( + CertificateGenerationConfiguration, + CertificateWhitelist, + GeneratedCertificate, + CertificateStatuses, +) from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.tests.utils import mock_passing_grade from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification @@ -25,12 +25,11 @@ class SelfGeneratedCertsSignalTest(ModuleStoreTestCase): """ Tests for enabling/disabling self-generated certificates according to course-pacing. """ + ENABLED_SIGNALS = ['course_published'] def setUp(self): super(SelfGeneratedCertsSignalTest, self).setUp() SelfPacedConfiguration(enabled=True).save() - self.course = CourseFactory.create(self_paced=True) - # Enable the feature CertificateGenerationConfiguration.objects.create(enabled=True) def test_cert_generation_flag_on_pacing_toggle(self): @@ -38,18 +37,15 @@ class SelfGeneratedCertsSignalTest(ModuleStoreTestCase): Verify that signal enables or disables self-generated certificates according to course-pacing. """ - #self-generation of cert disables by default + self.course = CourseFactory.create(self_paced=False, emit_signals=True) self.assertFalse(certs_api.cert_generation_enabled(self.course.id)) - _listen_for_course_pacing_changed('store', self.course.id, self.course.self_paced) - #verify that self-generation of cert is enabled for self-paced course + self.course.self_paced = True + self.store.update_item(self.course, self.user.id) self.assertTrue(certs_api.cert_generation_enabled(self.course.id)) self.course.self_paced = False self.store.update_item(self.course, self.user.id) - - _listen_for_course_pacing_changed('store', self.course.id, self.course.self_paced) - # verify that self-generation of cert is disabled for instructor-paced course self.assertFalse(certs_api.cert_generation_enabled(self.course.id)) diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 8de0f4fb59..d72acf19f4 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -12,8 +12,8 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.test.client import Client, RequestFactory from django.test.utils import override_settings +from freezegun import freeze_time from util.date_utils import strftime_localized -from django.utils.translation import ugettext as _ from mock import patch from nose.plugins.attrib import attr @@ -795,6 +795,7 @@ class CertificatesViewsTests(CommonCertificatesTestCase): self.assertIn('course_title_0', response.content) self.assertIn('Signatory_Title 0', response.content) + @freeze_time('2017-09-10 00:00:00Z') @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) @ddt.data( (datetime.datetime.now() - datetime.timedelta(days=1), True), diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py index f32cf24848..0cd0da4e4c 100644 --- a/openedx/core/djangoapps/content/course_overviews/signals.py +++ b/openedx/core/djangoapps/content/course_overviews/signals.py @@ -4,6 +4,7 @@ Signal handler for invalidating cached course overviews from django.dispatch.dispatcher import receiver from .models import CourseOverview +from openedx.core.djangoapps.signals.signals import COURSE_PACING_CHANGED, COURSE_START_DATE_CHANGED from xmodule.modulestore.django import SignalHandler @@ -13,7 +14,9 @@ def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable Catches the signal that a course has been published in Studio and updates the corresponding CourseOverview cache entry. """ - CourseOverview.load_from_module_store(course_key) + previous_course_overview = CourseOverview.get_from_ids_if_exists([course_key]).get(course_key) + updated_course_overview = CourseOverview.load_from_module_store(course_key) + _check_for_course_changes(previous_course_overview, updated_course_overview) @receiver(SignalHandler.course_deleted) @@ -27,3 +30,27 @@ def _listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable= from cms.djangoapps.contentstore.courseware_index import CourseAboutSearchIndexer # Delete course entry from Course About Search_index CourseAboutSearchIndexer.remove_deleted_items(course_key) + + +def _check_for_course_changes(previous_course_overview, updated_course_overview): + if previous_course_overview: + _check_for_course_date_changes(previous_course_overview, updated_course_overview) + _check_for_pacing_changes(previous_course_overview, updated_course_overview) + + +def _check_for_course_date_changes(previous_course_overview, updated_course_overview): + if previous_course_overview.start != updated_course_overview.start: + COURSE_START_DATE_CHANGED.send( + sender=None, + updated_course_overview=updated_course_overview, + previous_start_date=previous_course_overview.start, + ) + + +def _check_for_pacing_changes(previous_course_overview, updated_course_overview): + if previous_course_overview.self_paced != updated_course_overview.self_paced: + COURSE_PACING_CHANGED.send( + sender=None, + updated_course_overview=updated_course_overview, + previous_self_paced=previous_course_overview.self_paced, + ) diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py index b94458e04d..92537f08d2 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py @@ -33,7 +33,7 @@ from xmodule.error_module import ErrorDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls, check_mongo_calls_range +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls_range from ..models import CourseOverview, CourseOverviewImageSet, CourseOverviewImageConfig @@ -270,56 +270,6 @@ class CourseOverviewTestCase(ModuleStoreTestCase): course = CourseFactory.create(default_store=modulestore_type, run="TestRun", **kwargs) self.check_course_overview_against_course(course) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_overview_cache_invalidation(self, modulestore_type): - """ - Tests that when a course is published or deleted, the corresponding - course_overview is removed from the cache. - - Arguments: - modulestore_type (ModuleStoreEnum.Type): type of store to create the - course in. - """ - with self.store.default_store(modulestore_type): - - # Create a course where mobile_available is True. - course = CourseFactory.create(mobile_available=True, default_store=modulestore_type) - course_overview_1 = CourseOverview.get_from_id(course.id) - self.assertTrue(course_overview_1.mobile_available) - - # Set mobile_available to False and update the course. - # This fires a course_published signal, which should be caught in signals.py, which should in turn - # delete the corresponding CourseOverview from the cache. - course.mobile_available = False - with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): - self.store.update_item(course, ModuleStoreEnum.UserID.test) - - # Make sure that when we load the CourseOverview again, mobile_available is updated. - course_overview_2 = CourseOverview.get_from_id(course.id) - self.assertFalse(course_overview_2.mobile_available) - - # Verify that when the course is deleted, the corresponding CourseOverview is deleted as well. - with self.assertRaises(CourseOverview.DoesNotExist): - self.store.delete_course(course.id, ModuleStoreEnum.UserID.test) - CourseOverview.get_from_id(course.id) - - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_overview_caching(self, modulestore_type): - """ - Tests that CourseOverview structures are actually getting cached. - - Arguments: - modulestore_type (ModuleStoreEnum.Type): type of store to create the - course in. - """ - - # Creating a new course will trigger a publish event and the course will be cached - course = CourseFactory.create(default_store=modulestore_type, emit_signals=True) - - # The cache will be hit and mongo will not be queried - with check_mongo_calls(0): - CourseOverview.get_from_id(course.id) - @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) def test_get_non_existent_course(self, modulestore_type): """ diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py new file mode 100644 index 0000000000..f97f3719df --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py @@ -0,0 +1,91 @@ +import datetime +import ddt +from mock import patch +from nose.plugins.attrib import attr + +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls + +from ..models import CourseOverview + + +@ddt.ddt +@attr(shard=3) +class CourseOverviewSignalsTestCase(ModuleStoreTestCase): + """ + Tests for CourseOverview signals. + """ + ENABLED_SIGNALS = ['course_deleted', 'course_published'] + TODAY = datetime.datetime.utcnow() + NEXT_WEEK = TODAY + datetime.timedelta(days=7) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_caching(self, modulestore_type): + """ + Tests that CourseOverview structures are actually getting cached. + + Arguments: + modulestore_type (ModuleStoreEnum.Type): type of store to create the + course in. + """ + # Creating a new course will trigger a publish event and the course will be cached + course = CourseFactory.create(default_store=modulestore_type, emit_signals=True) + + # The cache will be hit and mongo will not be queried + with check_mongo_calls(0): + CourseOverview.get_from_id(course.id) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_cache_invalidation(self, modulestore_type): + """ + Tests that when a course is published or deleted, the corresponding + course_overview is removed from the cache. + + Arguments: + modulestore_type (ModuleStoreEnum.Type): type of store to create the + course in. + """ + with self.store.default_store(modulestore_type): + + # Create a course where mobile_available is True. + course = CourseFactory.create(mobile_available=True, default_store=modulestore_type) + course_overview_1 = CourseOverview.get_from_id(course.id) + self.assertTrue(course_overview_1.mobile_available) + + # Set mobile_available to False and update the course. + # This fires a course_published signal, which should be caught in signals.py, which should in turn + # delete the corresponding CourseOverview from the cache. + course.mobile_available = False + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.store.update_item(course, ModuleStoreEnum.UserID.test) + + # Make sure that when we load the CourseOverview again, mobile_available is updated. + course_overview_2 = CourseOverview.get_from_id(course.id) + self.assertFalse(course_overview_2.mobile_available) + + # Verify that when the course is deleted, the corresponding CourseOverview is deleted as well. + with self.assertRaises(CourseOverview.DoesNotExist): + self.store.delete_course(course.id, ModuleStoreEnum.UserID.test) + CourseOverview.get_from_id(course.id) + + def assert_changed_signal_sent(self, field_name, initial_value, changed_value, mock_signal): + course = CourseFactory.create(emit_signals=True, **{field_name: initial_value}) + + # changing display name doesn't fire the signal + course.display_name = course.display_name + u'changed' + self.store.update_item(course, ModuleStoreEnum.UserID.test) + self.assertFalse(mock_signal.called) + + # changing the given field fires the signal + setattr(course, field_name, changed_value) + self.store.update_item(course, ModuleStoreEnum.UserID.test) + self.assertTrue(mock_signal.called) + + @patch('openedx.core.djangoapps.signals.signals.COURSE_START_DATE_CHANGED.send') + def test_start_changed(self, mock_signal): + self.assert_changed_signal_sent('start', self.TODAY, self.NEXT_WEEK, mock_signal) + + @patch('openedx.core.djangoapps.signals.signals.COURSE_PACING_CHANGED.send') + def test_pacing_changed(self, mock_signal): + self.assert_changed_signal_sent('self_paced', True, False, mock_signal) diff --git a/openedx/core/djangoapps/models/course_details.py b/openedx/core/djangoapps/models/course_details.py index dee667849a..e24bac7565 100644 --- a/openedx/core/djangoapps/models/course_details.py +++ b/openedx/core/djangoapps/models/course_details.py @@ -9,7 +9,6 @@ from django.conf import settings from xmodule.fields import Date from xmodule.modulestore.exceptions import ItemNotFoundError from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration -from openedx.core.djangoapps.signals.signals import COURSE_PACING_CHANGE from openedx.core.lib.courses import course_image_url from xmodule.modulestore.django import modulestore @@ -190,7 +189,6 @@ class CourseDetails(object): descriptor = module_store.get_course(course_key) dirty = False - is_pacing_changed = False # In the descriptor's setter, the date is converted to JSON # using Date's to_json method. Calling to_json on something that @@ -283,15 +281,10 @@ class CourseDetails(object): and jsondict['self_paced'] != descriptor.self_paced): descriptor.self_paced = jsondict['self_paced'] dirty = True - is_pacing_changed = True if dirty: module_store.update_item(descriptor, user.id) - # fires a signal indicating that the course pacing has changed - if is_pacing_changed: - COURSE_PACING_CHANGE.send(sender=None, course_key=course_key, course_self_paced=descriptor.self_paced) - # NOTE: below auto writes to the db w/o verifying that any of # the fields actually changed to make faster, could compare # against db or could have client send over a list of which diff --git a/openedx/core/djangoapps/schedules/models.py b/openedx/core/djangoapps/schedules/models.py index e1391101bf..bf2df3e41a 100644 --- a/openedx/core/djangoapps/schedules/models.py +++ b/openedx/core/djangoapps/schedules/models.py @@ -1,5 +1,3 @@ -from collections import namedtuple - from django.db import models from django.utils.translation import ugettext_lazy as _ from django_extensions.db.models import TimeStampedModel diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index f40f95e107..1b6e4320b8 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -3,13 +3,18 @@ import logging from django.db.models.signals import post_save from django.dispatch import receiver +from django.utils import timezone from course_modes.models import CourseMode from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration +from edx_ace.utils import date +from openedx.core.djangoapps.signals.signals import COURSE_START_DATE_CHANGED from openedx.core.djangoapps.theming.helpers import get_current_site from openedx.core.djangoapps.waffle_utils import WaffleFlagNamespace, CourseWaffleFlag from student.models import CourseEnrollment from .models import Schedule, ScheduleConfig +from .tasks import update_course_schedules + log = logging.getLogger(__name__) @@ -45,39 +50,11 @@ def create_schedule(sender, **kwargs): log.debug('Schedules: Creation only enabled for self-paced courses') return - delta = None - global_config = DynamicUpgradeDeadlineConfiguration.current() - if global_config.enabled: - # Use the default from this model whether or not the feature is enabled - delta = global_config.deadline_days - - # Check if the course has a deadline override - course_config = CourseDynamicUpgradeDeadlineConfiguration.current(enrollment.course_id) - if course_config.enabled: - delta = course_config.deadline_days - - upgrade_deadline = None - # 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(enrollment.created, enrollment.course_overview.start) - if delta is not None: - upgrade_deadline = content_availability_date + datetime.timedelta(days=delta) - - course_upgrade_deadline = None - try: - verified_mode = CourseMode.verified_mode_for_course(enrollment.course_id) - except CourseMode.DoesNotExist: - pass - else: - if verified_mode: - course_upgrade_deadline = verified_mode.expiration_datetime - - if course_upgrade_deadline is not None and upgrade_deadline is not None: - # The content availability-based deadline should never occur after the verified mode's - # expiration date, if one is set. - upgrade_deadline = min(upgrade_deadline, course_upgrade_deadline) + upgrade_deadline = _calculate_upgrade_deadline(enrollment.course_id, content_availability_date) Schedule.objects.create( enrollment=enrollment, @@ -87,3 +64,60 @@ def create_schedule(sender, **kwargs): log.debug('Schedules: created a new schedule starting at %s with an upgrade deadline of %s', content_availability_date, upgrade_deadline) + + +@receiver(COURSE_START_DATE_CHANGED, dispatch_uid="update_schedules_on_course_start_changed") +def update_schedules_on_course_start_changed(sender, updated_course_overview, previous_start_date, **kwargs): + """ + Updates all course schedules if course hasn't started yet. + """ + if previous_start_date > timezone.now(): + upgrade_deadline = _calculate_upgrade_deadline( + updated_course_overview.id, + content_availability_date=updated_course_overview.start, + ) + update_course_schedules.apply_async( + kwargs=dict( + course_id=unicode(updated_course_overview.id), + new_start_date_str=date.serialize(updated_course_overview.start), + new_upgrade_deadline_str=date.serialize(upgrade_deadline), + ), + ) + + +def _calculate_upgrade_deadline(course_id, content_availability_date): + upgrade_deadline = None + + delta = _get_upgrade_deadline_delta_setting(course_id) + if delta is not None: + upgrade_deadline = content_availability_date + datetime.timedelta(days=delta) + if upgrade_deadline is not None: + # The content availability-based deadline should never occur + # after the verified mode's expiration date, if one is set. + try: + verified_mode = CourseMode.verified_mode_for_course(course_id) + except CourseMode.DoesNotExist: + pass + else: + if verified_mode: + course_mode_upgrade_deadline = verified_mode.expiration_datetime + if course_mode_upgrade_deadline is not None: + upgrade_deadline = min(upgrade_deadline, course_mode_upgrade_deadline) + + return upgrade_deadline + + +def _get_upgrade_deadline_delta_setting(course_id): + delta = None + + global_config = DynamicUpgradeDeadlineConfiguration.current() + if global_config.enabled: + # Use the default from this model whether or not the feature is enabled + delta = global_config.deadline_days + + # Check if the course has a deadline + course_config = CourseDynamicUpgradeDeadlineConfiguration.current(course_id) + if course_config.enabled: + delta = course_config.deadline_days + + return delta diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index fbffff2939..e2a409e45e 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -5,19 +5,47 @@ from urlparse import urlparse from celery.task import task from django.conf import settings from django.contrib.sites.models import Site +from django.core.exceptions import ValidationError from django.core.urlresolvers import reverse +from django.db.utils import DatabaseError from django.utils.http import urlquote +from logging import getLogger + from edx_ace import ace from edx_ace.message import MessageType, Message from edx_ace.recipient import Recipient from edx_ace.utils.date import deserialize - from edxmako.shortcuts import marketing_link +from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig +log = getLogger(__name__) + + ROUTING_KEY = getattr(settings, 'ACE_ROUTING_KEY', None) +KNOWN_RETRY_ERRORS = ( # Errors we expect occasionally that could resolve on retry + DatabaseError, + ValidationError, +) + + +@task(bind=True, default_retry_delay=30, routing_key=ROUTING_KEY) +def update_course_schedules(self, **kwargs): + course_key = CourseKey.from_string(kwargs['course_id']) + new_start_date = deserialize(kwargs['new_start_date_str']) + new_upgrade_deadline = deserialize(kwargs['new_upgrade_deadline_str']) + + try: + Schedule.objects.filter(enrollment__course_id=course_key).update( + start=new_start_date, + upgrade_deadline=new_upgrade_deadline + ) + except Exception as exc: # pylint: disable=broad-except + if not isinstance(exc, KNOWN_RETRY_ERRORS): + log.exception("Unexpected failure: task id: %s, kwargs=%s".format(self.request.id, kwargs)) + raise self.retry(kwargs=kwargs, exc=exc) class RecurringNudge(MessageType): diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index f879bdee19..11c5454cc0 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -9,7 +9,9 @@ from openedx.core.djangoapps.schedules.signals import SCHEDULE_WAFFLE_FLAG from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.djangolib.testing.utils import skip_unless_lms +from student.models import CourseEnrollment from student.tests.factories import CourseEnrollmentFactory +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from ..models import Schedule @@ -21,13 +23,13 @@ from ..tests.factories import ScheduleConfigFactory class CreateScheduleTests(SharedModuleStoreTestCase): def assert_schedule_created(self): - course = create_course_run(self_paced=True) + course = _create_course_run(self_paced=True) enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) self.assertIsNotNone(enrollment.schedule) self.assertIsNone(enrollment.schedule.upgrade_deadline) def assert_schedule_not_created(self): - course = create_course_run(self_paced=True) + course = _create_course_run(self_paced=True) enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) with self.assertRaises(Schedule.DoesNotExist): enrollment.schedule @@ -70,19 +72,64 @@ class CreateScheduleTests(SharedModuleStoreTestCase): site = SiteFactory.create() mock_get_current_site.return_value = site ScheduleConfigFactory.create(site=site, enabled=True, create_schedules=True) - course = create_course_run(self_paced=False) + course = _create_course_run(self_paced=False) enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) with self.assertRaises(Schedule.DoesNotExist): enrollment.schedule -def create_course_run(self_paced=True): +@skip_unless_lms +class UpdateScheduleTests(SharedModuleStoreTestCase): + ENABLED_SIGNALS = ['course_published'] + VERIFICATION_DEADLINE_DAYS = 14 + + def setUp(self): + super(UpdateScheduleTests, self).setUp() + self.site = SiteFactory.create() + ScheduleConfigFactory.create(site=self.site) + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True, deadline_days=self.VERIFICATION_DEADLINE_DAYS) + + def assert_schedule_dates(self, schedule, expected_start): + self.assertEquals(_strip_secs(schedule.start), _strip_secs(expected_start)) + self.assertEquals( + _strip_secs(schedule.upgrade_deadline), + _strip_secs(expected_start) + datetime.timedelta(days=self.VERIFICATION_DEADLINE_DAYS), + ) + + @patch('openedx.core.djangoapps.schedules.signals.get_current_site') + def test_schedule_updated(self, mock_get_current_site): + mock_get_current_site.return_value = self.site + + course = _create_course_run(self_paced=True, start_day_offset=5) + enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) + + self.assert_schedule_dates(enrollment.schedule, enrollment.course_overview.start) + course.start = course.start + datetime.timedelta(days=3) + self.store.update_item(course, ModuleStoreEnum.UserID.test) + enrollment = CourseEnrollment.objects.get(id=enrollment.id) + self.assert_schedule_dates(enrollment.schedule, course.start) + + @patch('openedx.core.djangoapps.schedules.signals.get_current_site') + def test_schedule_not_updated(self, mock_get_current_site): + mock_get_current_site.return_value = self.site + + course = _create_course_run(self_paced=True, start_day_offset=-5) + enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) + self.assert_schedule_dates(enrollment.schedule, enrollment.created) + + course.start = course.start + datetime.timedelta(days=3) + self.store.update_item(course, ModuleStoreEnum.UserID.test) + self.assert_schedule_dates(enrollment.schedule, enrollment.created) + + +def _create_course_run(self_paced=True, start_day_offset=-1): """ Create a new course run and course modes. Both audit and verified `CourseMode` objects will be created for the course run. """ now = datetime.datetime.now(utc) - course = CourseFactory.create(start=now + datetime.timedelta(days=-1), self_paced=self_paced) + start = now + datetime.timedelta(days=start_day_offset) + course = CourseFactory.create(start=start, self_paced=self_paced) CourseModeFactory( course_id=course.id, @@ -95,3 +142,7 @@ def create_course_run(self_paced=True): ) return course + + +def _strip_secs(timestamp): + return timestamp.replace(second=0, microsecond=0) diff --git a/openedx/core/djangoapps/signals/handlers.py b/openedx/core/djangoapps/signals/handlers.py index adfd1bdaae..0b518e7d51 100644 --- a/openedx/core/djangoapps/signals/handlers.py +++ b/openedx/core/djangoapps/signals/handlers.py @@ -3,32 +3,28 @@ This module contains all general use or cross-use handlers. """ import logging -from celery.task import task from django.dispatch import receiver from certificates.models import CertificateGenerationCourseSetting -from opaque_keys.edx.keys import CourseKey -from signals import COURSE_PACING_CHANGE +from signals import COURSE_PACING_CHANGED log = logging.getLogger(__name__) -@receiver(COURSE_PACING_CHANGE, dispatch_uid="course_pacing_changed") -def _listen_for_course_pacing_changed(sender, course_key, course_self_paced, **kwargs): # pylint: disable=unused-argument +@receiver(COURSE_PACING_CHANGED, dispatch_uid="update_cert_settings_on_pacing_change") +def _update_cert_settings_on_pacing_change(sender, updated_course_overview, **kwargs): # pylint: disable=unused-argument """ Catches the signal that course pacing has changed and enable/disable the self-generated certificates according to course-pacing. """ - toggle_self_generated_certs.delay(unicode(course_key), course_self_paced) - log.info(u'Certificate Generation Setting Toggled for {course} via pacing change'.format( - course=course_key + toggle_self_generated_certs(updated_course_overview.id, updated_course_overview.self_paced) + log.info(u'Certificate Generation Setting Toggled for {course_id} via pacing change'.format( + course_id=updated_course_overview.id )) -@task def toggle_self_generated_certs(course_key, course_self_paced): """ Enable or disable self-generated certificates for a course according to pacing. """ - course_key = CourseKey.from_string(course_key) CertificateGenerationCourseSetting.set_enabled_for_course(course_key, course_self_paced) diff --git a/openedx/core/djangoapps/signals/signals.py b/openedx/core/djangoapps/signals/signals.py index 1fe73fee4e..7bdb3656fd 100644 --- a/openedx/core/djangoapps/signals/signals.py +++ b/openedx/core/djangoapps/signals/signals.py @@ -23,4 +23,6 @@ COURSE_GRADE_NOW_PASSED = Signal( # Signal that indicates that a user has become verified LEARNER_NOW_VERIFIED = Signal(providing_args=['user']) -COURSE_PACING_CHANGE = Signal(providing_args=["course_key", "course_self_paced"]) +COURSE_PACING_CHANGED = Signal(providing_args=["updated_course_overview", "previous_self_paced"]) + +COURSE_START_DATE_CHANGED = Signal(providing_args=["updated_course_overview", "previous_start_date"])