fix: avoid resetting a learner schedule to before a course starts
If a learner changes modes (like upgrades to a verified learner), we will reset their schedule for them. But if they did this before the course started, we would accidentally set their schedule to the current time. So when the course did start, they would already appear to be behind schedule. That's silly. So now we always look at course start time when resetting the learner's schedule. AA-426
This commit is contained in:
@@ -82,8 +82,8 @@ def reset_schedule_on_mode_change(sender, user, course_key, mode, **kwargs): #
|
||||
# If switching to audit, reset to when the user got access to course. This is for the case where a user
|
||||
# upgrades to verified (resetting their date), then later refunds the order and goes back to audit. We want
|
||||
# to make sure that audit users go back to their normal audit schedule access.
|
||||
use_availability_date = mode in CourseMode.AUDIT_MODES
|
||||
reset_self_paced_schedule(user, course_key, use_availability_date=use_availability_date)
|
||||
use_enrollment_date = mode in CourseMode.AUDIT_MODES
|
||||
reset_self_paced_schedule(user, course_key, use_enrollment_date=use_enrollment_date)
|
||||
|
||||
|
||||
def _calculate_upgrade_deadline(course_id, content_availability_date): # lint-amnesty, pylint: disable=missing-function-docstring
|
||||
|
||||
@@ -6,6 +6,8 @@ import datetime
|
||||
|
||||
import ddt
|
||||
from pytz import utc
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
from openedx.core.djangoapps.schedules.models import Schedule
|
||||
@@ -13,38 +15,39 @@ from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFact
|
||||
from openedx.core.djangoapps.schedules.utils import reset_self_paced_schedule
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@skip_unless_lms
|
||||
class ResetSelfPacedScheduleTests(SharedModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring
|
||||
def create_schedule(self, offset=0): # lint-amnesty, pylint: disable=missing-function-docstring
|
||||
self.config = ScheduleConfigFactory() # lint-amnesty, pylint: disable=attribute-defined-outside-init
|
||||
class ResetSelfPacedScheduleTests(SharedModuleStoreTestCase):
|
||||
"""Tests for reset_self_paced_schedule"""
|
||||
def create_schedule(self, enrollment_offset=0, course_start_offset=-100):
|
||||
"""Makes a course, schedule, and enrollment ready to test"""
|
||||
# pylint: disable=attribute-defined-outside-init
|
||||
self.config = ScheduleConfigFactory()
|
||||
|
||||
start = datetime.datetime.now(utc) - datetime.timedelta(days=100)
|
||||
self.course = CourseFactory.create(start=start, self_paced=True) # lint-amnesty, pylint: disable=attribute-defined-outside-init
|
||||
start = datetime.datetime.now(utc) + datetime.timedelta(days=course_start_offset)
|
||||
self.course = CourseFactory.create(start=start, self_paced=True)
|
||||
|
||||
self.enrollment = CourseEnrollmentFactory( # lint-amnesty, pylint: disable=attribute-defined-outside-init
|
||||
self.enrollment = CourseEnrollmentFactory(
|
||||
course_id=self.course.id,
|
||||
mode=CourseMode.AUDIT,
|
||||
)
|
||||
self.enrollment.created = start + datetime.timedelta(days=offset)
|
||||
self.enrollment.created = start + datetime.timedelta(days=enrollment_offset)
|
||||
self.enrollment.save()
|
||||
|
||||
self.schedule = self.enrollment.schedule # lint-amnesty, pylint: disable=attribute-defined-outside-init
|
||||
self.schedule = self.enrollment.schedule
|
||||
self.schedule.start_date = self.enrollment.created
|
||||
self.schedule.save()
|
||||
|
||||
self.user = self.enrollment.user # lint-amnesty, pylint: disable=attribute-defined-outside-init
|
||||
self.user = self.enrollment.user
|
||||
|
||||
def test_reset_to_now(self):
|
||||
self.create_schedule()
|
||||
original_start = self.schedule.start_date
|
||||
|
||||
with self.assertNumQueries(3):
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_availability_date=False)
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_enrollment_date=False)
|
||||
|
||||
self.schedule.refresh_from_db()
|
||||
assert self.schedule.start_date > original_start
|
||||
@@ -55,11 +58,11 @@ class ResetSelfPacedScheduleTests(SharedModuleStoreTestCase): # lint-amnesty, p
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_reset_to_start_date(self, offset, expected_offset):
|
||||
self.create_schedule(offset=offset)
|
||||
self.create_schedule(enrollment_offset=offset)
|
||||
expected_start = self.course.start + datetime.timedelta(days=expected_offset)
|
||||
|
||||
with self.assertNumQueries(3):
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_availability_date=True)
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_enrollment_date=True)
|
||||
|
||||
self.schedule.refresh_from_db()
|
||||
assert self.schedule.start_date.replace(microsecond=0) == expected_start.replace(microsecond=0)
|
||||
@@ -69,7 +72,16 @@ class ResetSelfPacedScheduleTests(SharedModuleStoreTestCase): # lint-amnesty, p
|
||||
self.create_schedule()
|
||||
self.schedule.delete()
|
||||
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_availability_date=False)
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_availability_date=True)
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_enrollment_date=False)
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_enrollment_date=True)
|
||||
|
||||
assert Schedule.objects.count() == 0
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_reset_before_course_starts(self, use_enrollment_date):
|
||||
self.create_schedule(enrollment_offset=-5, course_start_offset=5) # starts in 5 days, enrollment is now
|
||||
|
||||
reset_self_paced_schedule(self.user, self.course.id, use_enrollment_date=use_enrollment_date)
|
||||
|
||||
self.schedule.refresh_from_db()
|
||||
assert self.schedule.start_date == self.enrollment.course.start
|
||||
|
||||
@@ -34,7 +34,7 @@ class PrefixedDebugLoggerMixin: # lint-amnesty, pylint: disable=missing-class-d
|
||||
LOG.info(self.log_prefix + ': ' + message, *args, **kwargs)
|
||||
|
||||
|
||||
def reset_self_paced_schedule(user, course_key, use_availability_date=False):
|
||||
def reset_self_paced_schedule(user, course_key, use_enrollment_date=False):
|
||||
"""
|
||||
Reset the user's schedule if self-paced.
|
||||
|
||||
@@ -44,7 +44,7 @@ def reset_self_paced_schedule(user, course_key, use_availability_date=False):
|
||||
Arguments:
|
||||
user (User)
|
||||
course_key (CourseKey or str)
|
||||
use_availability_date (bool): if False, reset to now, else reset to when user got access to course material
|
||||
use_enrollment_date (bool): if False, reset to now, else reset to original enrollment creation date
|
||||
"""
|
||||
with transaction.atomic(savepoint=False):
|
||||
try:
|
||||
@@ -56,10 +56,13 @@ def reset_self_paced_schedule(user, course_key, use_availability_date=False):
|
||||
except Schedule.DoesNotExist:
|
||||
return
|
||||
|
||||
if use_availability_date:
|
||||
enrollment = schedule.enrollment
|
||||
schedule.start_date = max(enrollment.created, enrollment.course.start)
|
||||
schedule.save()
|
||||
if use_enrollment_date:
|
||||
new_start_date = schedule.enrollment.created
|
||||
else:
|
||||
schedule.start_date = datetime.datetime.now(pytz.utc)
|
||||
schedule.save()
|
||||
new_start_date = datetime.datetime.now(pytz.utc)
|
||||
|
||||
# Make sure we don't start the clock on the learner's schedule before the course even starts
|
||||
new_start_date = max(new_start_date, schedule.enrollment.course.start)
|
||||
|
||||
schedule.start_date = new_start_date
|
||||
schedule.save()
|
||||
|
||||
@@ -47,7 +47,7 @@ class ResetCourseDeadlinesViewTests(EventTestMixin, BaseCourseHomeTests, Masquer
|
||||
|
||||
def test_reset_deadlines_with_masquerade(self):
|
||||
""" Staff users should be able to masquerade as a learner and reset the learner's schedule """
|
||||
course = CourseFactory.create(self_paced=True)
|
||||
course = CourseFactory.create(self_paced=True, start=timezone.now() - datetime.timedelta(days=1))
|
||||
student_username = self.user.username
|
||||
student_user_id = self.user.id
|
||||
student_enrollment = CourseEnrollment.enroll(self.user, course.id)
|
||||
|
||||
@@ -71,7 +71,7 @@ class TestCourseOutlinePage(SharedModuleStoreTestCase, MasqueradeMixin):
|
||||
# pylint: disable=super-method-not-called
|
||||
with super().setUpClassAndTestData():
|
||||
cls.courses = []
|
||||
course = CourseFactory.create(self_paced=True)
|
||||
course = CourseFactory.create(self_paced=True, start=timezone.now() - datetime.timedelta(days=1))
|
||||
with cls.store.bulk_operations(course.id):
|
||||
chapter = ItemFactory.create(category='chapter', parent_location=course.location)
|
||||
sequential = ItemFactory.create(category='sequential', parent_location=chapter.location, graded=True, format="Homework") # lint-amnesty, pylint: disable=line-too-long
|
||||
|
||||
Reference in New Issue
Block a user