Fix hold back error
Also add try/except that prevents enroll failure caused by Schedule creation exception in the future. Address PR comments Split one-line docstrings on their own line Add period to the end of docstring summaries Use kwargs.get to be more defensive Disable pylint unused-argument about sender param
This commit is contained in:
Tyler Hallada
@@ -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
|
||||
|
||||
@@ -121,6 +121,18 @@ class CreateScheduleTests(SharedModuleStoreTestCase):
|
||||
else:
|
||||
self.assert_schedule_not_created()
|
||||
mock_track.assert_called_once()
|
||||
self.assertEquals(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
|
||||
|
||||
Reference in New Issue
Block a user