diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index 8a8e28249c..53603acf45 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -27,59 +27,31 @@ log = logging.getLogger(__name__) @receiver(post_save, sender=CourseEnrollment, dispatch_uid='create_schedule_for_enrollment') -def create_schedule(sender, **kwargs): - if not kwargs['created']: - # only create schedules when enrollment records are created - return +def create_schedule(sender, **kwargs): # pylint: disable=unused-argument + """ + When a CourseEnrollment is created, create a Schedule if configured. + """ + try: + enrollment = kwargs.get('instance') + enrollment_created = kwargs.get('created') - current_site = get_current_site() - if current_site is None: - log.debug('Schedules: No current site') - return + schedule_details = _create_schedule(enrollment, enrollment_created) - enrollment = kwargs['instance'] - schedule_config = ScheduleConfig.current(current_site) - if ( - not schedule_config.create_schedules - and not CREATE_SCHEDULE_WAFFLE_FLAG.is_enabled(enrollment.course_id) - ): - log.debug('Schedules: Creation not enabled for this course or for this site') - return - - if not enrollment.course_overview.self_paced: - log.debug('Schedules: Creation only enabled for self-paced courses') - return - - # 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) - - upgrade_deadline = _calculate_upgrade_deadline(enrollment.course_id, content_availability_date) - - if course_has_highlights(enrollment.course_id): - experience_type = ScheduleExperience.EXPERIENCES.course_updates - else: - experience_type = ScheduleExperience.EXPERIENCES.default - - if _should_randomly_suppress_schedule_creation( - schedule_config, - enrollment, - upgrade_deadline, - experience_type, - content_availability_date, - ): - return - - schedule = Schedule.objects.create( - enrollment=enrollment, - start=content_availability_date, - upgrade_deadline=upgrade_deadline - ) - - ScheduleExperience(schedule=schedule, experience_type=experience_type).save() - - log.debug('Schedules: created a new schedule starting at %s with an upgrade deadline of %s and experience type: %s', - content_availability_date, upgrade_deadline, ScheduleExperience.EXPERIENCES[experience_type]) + if schedule_details: + log.debug( + 'Schedules: created a new schedule starting at ' + + '%s with an upgrade deadline of %s and experience type: %s', + schedule_details['content_availability_date'], + schedule_details['upgrade_deadline'], + ScheduleExperience.EXPERIENCES[schedule_details['experience_type']] + ) + except Exception: # pylint: disable=broad-except + # We do not want to block the creation of a CourseEnrollment because of an error in creating a Schedule. + # No Schedule is acceptable, but no CourseEnrollment is not. + log.exception('Encountered error in creating a Schedule for CourseEnrollment for user {} in course {}'.format( + enrollment.user.id if (enrollment and enrollment.user) else None, + enrollment.course_id if enrollment else None + )) @receiver(COURSE_START_DATE_CHANGED, dispatch_uid="update_schedules_on_course_start_changed") @@ -167,9 +139,9 @@ def _should_randomly_suppress_schedule_creation( if upgrade_deadline: upgrade_deadline_str = upgrade_deadline.isoformat() analytics.track( - 'edx.bi.schedule.suppressed', - { - 'user_id': enrollment.user.id, + user_id=enrollment.user.id, + event='edx.bi.schedule.suppressed', + properties={ 'course_id': unicode(enrollment.course_id), 'experience_type': experience_type, 'upgrade_deadline': upgrade_deadline_str, @@ -179,3 +151,70 @@ def _should_randomly_suppress_schedule_creation( return True return False + + +def _create_schedule(enrollment, enrollment_created): + """ + Checks configuration and creates Schedule with ScheduleExperience for this enrollment. + """ + if not enrollment_created: + # only create schedules when enrollment records are created + return + + current_site = get_current_site() + if current_site is None: + log.debug('Schedules: No current site') + return + + schedule_config = ScheduleConfig.current(current_site) + if ( + not schedule_config.create_schedules and + not CREATE_SCHEDULE_WAFFLE_FLAG.is_enabled(enrollment.course_id) + ): + log.debug('Schedules: Creation not enabled for this course or for this site') + return + + if not enrollment.course_overview.self_paced: + log.debug('Schedules: Creation only enabled for self-paced courses') + return + + # 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) + upgrade_deadline = _calculate_upgrade_deadline(enrollment.course_id, content_availability_date) + experience_type = _get_experience_type(enrollment) + + if _should_randomly_suppress_schedule_creation( + schedule_config, + enrollment, + upgrade_deadline, + experience_type, + content_availability_date, + ): + return + + schedule = Schedule.objects.create( + enrollment=enrollment, + start=content_availability_date, + upgrade_deadline=upgrade_deadline + ) + + ScheduleExperience(schedule=schedule, experience_type=experience_type).save() + + return { + 'content_availability_date': content_availability_date, + 'upgrade_deadline': upgrade_deadline, + 'experience_type': experience_type, + } + + +def _get_experience_type(enrollment): + """ + Returns the ScheduleExperience type for the given enrollment. + + Schedules will receive the Course Updates experience if the course has any section highlights defined. + """ + if course_has_highlights(enrollment.course_id): + return ScheduleExperience.EXPERIENCES.course_updates + else: + return ScheduleExperience.EXPERIENCES.default diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index 167744e4c5..c17fba705d 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -1,5 +1,6 @@ import datetime import ddt +import pytest from mock import patch from pytz import utc @@ -31,9 +32,9 @@ class CreateScheduleTests(SharedModuleStoreTestCase): course_id=course.id, mode=CourseMode.AUDIT, ) - self.assertIsNotNone(enrollment.schedule) - self.assertIsNone(enrollment.schedule.upgrade_deadline) - self.assertEquals(enrollment.schedule.experience.experience_type, experience_type) + assert enrollment.schedule is not None + assert enrollment.schedule.upgrade_deadline is None + assert enrollment.schedule.experience.experience_type == experience_type def assert_schedule_not_created(self): course = _create_course_run(self_paced=True) @@ -41,7 +42,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): course_id=course.id, mode=CourseMode.AUDIT, ) - with self.assertRaises(Schedule.DoesNotExist): + with pytest.raises(Schedule.DoesNotExist, message="Expecting Schedule to not exist"): enrollment.schedule @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) @@ -84,7 +85,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): ScheduleConfigFactory.create(site=site, enabled=True, create_schedules=True) course = _create_course_run(self_paced=False) enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) - with self.assertRaises(Schedule.DoesNotExist): + with pytest.raises(Schedule.DoesNotExist, message="Expecting Schedule to not exist"): enrollment.schedule @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) @@ -117,10 +118,22 @@ class CreateScheduleTests(SharedModuleStoreTestCase): mock_get_current_site.return_value = schedule_config.site if expect_schedule_created: self.assert_schedule_created() - self.assertFalse(mock_track.called) + assert not mock_track.called else: self.assert_schedule_not_created() mock_track.assert_called_once() + assert mock_track.call_args[1].get('event') == 'edx.bi.schedule.suppressed' + + @patch('openedx.core.djangoapps.schedules.signals.log.exception') + @patch('openedx.core.djangoapps.schedules.signals.Schedule.objects.create') + def test_create_schedule_error(self, mock_create_schedule, mock_log, mock_get_current_site): + site = SiteFactory.create() + mock_get_current_site.return_value = site + ScheduleConfigFactory.create(site=site) + mock_create_schedule.side_effect = ValueError('Fake error') + self.assert_schedule_not_created() + mock_log.assert_called_once() + assert 'Encountered error in creating a Schedule for CourseEnrollment' in mock_log.call_args[0][0] @ddt.ddt @@ -137,9 +150,9 @@ class UpdateScheduleTests(SharedModuleStoreTestCase): 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), + assert _strip_secs(schedule.start) == _strip_secs(expected_start) + assert ( + _strip_secs(schedule.upgrade_deadline) == _strip_secs(expected_start) + datetime.timedelta(days=self.VERIFICATION_DEADLINE_DAYS), )