diff --git a/AUTHORS b/AUTHORS index 2feb253c1ecd..37baa411e1cb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -171,3 +171,4 @@ Filippo Valsorda Ivica Ceraj Jason Zhu Marceau Cnudde +Braden MacDonald diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4ce1290b666c..f10ef6215b66 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +LMS: Do not allow individual due dates to be earlier than the normal due date. LMS-6563 + Blades: Course teams can turn off Chinese Caching from Studio. BLD-1207 LMS: Instructors can request and see content of previous bulk emails sent in the instructor dashboard. diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 59227e9825a2..a776a07fc7a5 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -47,7 +47,8 @@ from shoppingcart.models import CourseRegistrationCode, RegistrationCodeRedemption, Order, PaidCourseRegistration, Coupon from course_modes.models import CourseMode -from .test_tools import msk_from_problem_urlname, get_extended_due +from .test_tools import msk_from_problem_urlname +from ..views.tools import get_extended_due @common_exceptions_400 @@ -2219,11 +2220,13 @@ def setUp(self): """ Fixtures. """ + super(TestDueDateExtensions, self).setUp() + due = datetime.datetime(2010, 5, 12, 2, 42, tzinfo=utc) course = CourseFactory.create() week1 = ItemFactory.create(due=due) week2 = ItemFactory.create(due=due) - week3 = ItemFactory.create(due=due) + week3 = ItemFactory.create() # No due date course.children = [week1.location.to_deprecated_string(), week2.location.to_deprecated_string(), week3.location.to_deprecated_string()] @@ -2283,6 +2286,7 @@ def setUp(self): self.week1 = week1 self.homework = homework self.week2 = week2 + self.week3 = week3 self.user1 = user1 self.user2 = user2 @@ -2300,6 +2304,32 @@ def test_change_due_date(self): self.assertEqual(datetime.datetime(2013, 12, 30, 0, 0, tzinfo=utc), get_extended_due(self.course, self.week1, self.user1)) + def test_change_to_invalid_due_date(self): + url = reverse('change_due_date', kwargs={'course_id': self.course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'student': self.user1.username, + 'url': self.week1.location.to_deprecated_string(), + 'due_datetime': '01/01/2009 00:00' + }) + self.assertEqual(response.status_code, 400, response.content) + self.assertEqual( + None, + get_extended_due(self.course, self.week1, self.user1) + ) + + def test_change_nonexistent_due_date(self): + url = reverse('change_due_date', kwargs={'course_id': self.course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'student': self.user1.username, + 'url': self.week3.location.to_deprecated_string(), + 'due_datetime': '12/30/2013 00:00' + }) + self.assertEqual(response.status_code, 400, response.content) + self.assertEqual( + None, + get_extended_due(self.course, self.week3, self.user1) + ) + def test_reset_date(self): self.test_change_due_date() url = reverse('reset_due_date', kwargs={'course_id': self.course.id.to_deprecated_string()}) @@ -2308,8 +2338,38 @@ def test_reset_date(self): 'url': self.week1.location.to_deprecated_string(), }) self.assertEqual(response.status_code, 200, response.content) - self.assertEqual(None, - get_extended_due(self.course, self.week1, self.user1)) + self.assertEqual( + None, + get_extended_due(self.course, self.week1, self.user1) + ) + + def test_reset_nonexistent_extension(self): + url = reverse('reset_due_date', kwargs={'course_id': self.course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'student': self.user1.username, + 'url': self.week1.location.to_deprecated_string(), + }) + self.assertEqual(response.status_code, 400, response.content) + + def test_reset_extension_to_deleted_date(self): + """ + Test that we can delete a due date extension after deleting the normal + due date, without causing an error. + """ + self.test_change_due_date() + self.week1.due = None + self.week1 = self.store.update_item(self.week1, self.user1.id) + # Now, week1's normal due date is deleted but the extension still exists. + url = reverse('reset_due_date', kwargs={'course_id': self.course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'student': self.user1.username, + 'url': self.week1.location.to_deprecated_string(), + }) + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual( + None, + get_extended_due(self.course, self.week1, self.user1) + ) def test_show_unit_extensions(self): self.test_change_due_date() diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 722b57a81c02..aa216625a7ee 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -170,6 +170,7 @@ def setUp(self): course = CourseFactory.create() week1 = ItemFactory.create(due=due, parent=course) week2 = ItemFactory.create(due=due, parent=course) + week3 = ItemFactory.create(parent=course) homework = ItemFactory.create( parent=week1, @@ -192,10 +193,11 @@ def setUp(self): self.week1 = week1 self.homework = homework self.week2 = week2 + self.week3 = week3 self.user = user self.extended_due = functools.partial( - get_extended_due, course, student=user) + tools.get_extended_due, course, student=user) def test_set_due_date_extension(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=utc) @@ -207,13 +209,26 @@ def test_set_due_date_extension_create_studentmodule(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=utc) user = UserFactory.create() # No student modules for this user tools.set_due_date_extension(self.course, self.week1, user, extended) - extended_due = functools.partial(get_extended_due, self.course, student=user) + extended_due = functools.partial(tools.get_extended_due, self.course, student=user) self.assertEqual(extended_due(self.week1), extended) self.assertEqual(extended_due(self.homework), extended) + def test_set_due_date_extension_invalid_date(self): + extended = datetime.datetime(2009, 1, 1, 0, 0, tzinfo=utc) + with self.assertRaises(tools.DashboardError): + tools.set_due_date_extension(self.course, self.week1, self.user, extended) + + def test_set_due_date_extension_no_date(self): + extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=utc) + with self.assertRaises(tools.DashboardError): + tools.set_due_date_extension(self.course, self.week3, self.user, extended) + def test_reset_due_date_extension(self): + extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=utc) + tools.set_due_date_extension(self.course, self.week1, self.user, extended) tools.set_due_date_extension(self.course, self.week1, self.user, None) self.assertEqual(self.extended_due(self.week1), None) + self.assertEqual(self.extended_due(self.homework), None) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @@ -329,22 +344,6 @@ def test_dump_student_extensions(self): "Extended Due Date": "2013-12-25 00:00"}]) -def get_extended_due(course, unit, student): - """ - Get the extended due date out of a student's state for a particular unit. - """ - student_module = StudentModule.objects.get( - student_id=student.id, - course_id=course.id, - module_state_key=unit.location - ) - - state = json.loads(student_module.state) - extended = state.get('extended_due', None) - if extended: - return DATE_FIELD.from_json(extended) - - def msk_from_problem_urlname(course_id, urlname, block_type='problem'): """ Convert a 'problem urlname' to a module state key (db field) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 49a5a4248c5f..72990cbea456 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1416,11 +1416,17 @@ def reset_due_date(request, course_id): student = get_student_from_identifier(request.GET.get('student')) unit = find_unit(course, request.GET.get('url')) set_due_date_extension(course, unit, student, None) + if not getattr(unit, "due", None): + # It's possible the normal due date was deleted after an extension was granted: + return JsonResponse( + _("Successfully removed invalid due date extension (unit has no due date).") + ) + original_due_date_str = unit.due.strftime('%Y-%m-%d %H:%M') return JsonResponse(_( 'Successfully reset due date for student {0} for {1} ' 'to {2}').format(student.profile.name, _display_unit(unit), - unit.due.strftime('%Y-%m-%d %H:%M'))) + original_due_date_str)) @handle_dashboard_error diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py index d195db7a4c41..44919ca06479 100644 --- a/lms/djangoapps/instructor/views/tools.py +++ b/lms/djangoapps/instructor/views/tools.py @@ -159,10 +159,40 @@ def title_or_url(node): return title +def get_extended_due(course, unit, student): + """ + Get the extended due date out of a student's state for a particular unit. + """ + student_module = StudentModule.objects.get( + student_id=student.id, + course_id=course.id, + module_state_key=unit.location + ) + + state = json.loads(student_module.state) + extended = state.get('extended_due', None) + if extended: + return DATE_FIELD.from_json(extended) + + def set_due_date_extension(course, unit, student, due_date): """ - Sets a due date extension. + Sets a due date extension. Raises DashboardError if the unit or extended + due date is invalid. """ + if due_date: + # Check that the new due date is valid: + original_due_date = getattr(unit, 'due', None) + + if not original_due_date: + raise DashboardError(_("Unit {0} has no due date to extend.").format(unit.location)) + if due_date < original_due_date: + raise DashboardError(_("An extended due date must be later than the original due date.")) + else: + # We are deleting a due date extension. Check that it exists: + if not get_extended_due(course, unit, student): + raise DashboardError(_("No due date extension is set for that student and unit.")) + def set_due_date(node): """ Recursively set the due date on a node and all of its children.