From 49e664cbd7ea8ad42dcebf7021be0a11c68f0fc5 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 15 Sep 2022 14:27:56 +0200 Subject: [PATCH] fix: regenerate the cache on due date extensions --- lms/djangoapps/instructor/tests/test_api.py | 15 ++++++---- lms/djangoapps/instructor/tests/test_tools.py | 28 ++++++++++++++++++- lms/djangoapps/instructor/views/tools.py | 10 +++++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index a01959d92e..3f88482073 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -3916,13 +3916,13 @@ def get_extended_due(course, unit, user): return None -def get_date_for_block(course, unit, user): +def get_date_for_block(course, unit, user, use_cached=False): """ Gets the due date for the given user on the given unit (overridden or original). Returns `None` if there is no date set. - (Differs from edx-when's get_date_for_block only in that we skip the cache. + Differs from edx-when's get_date_for_block only in that we skip the cache when `use_cached` is `False` (default). """ - return get_dates_for_course(course.id, user=user, use_cached=False).get((unit.location, 'due'), None) + return get_dates_for_course(course.id, user=user, use_cached=use_cached).get((unit.location, 'due'), None) class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @@ -4014,14 +4014,16 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): def test_change_due_date(self): url = reverse('change_due_date', kwargs={'course_id': str(self.course.id)}) + due_date = datetime.datetime(2013, 12, 30, tzinfo=UTC) response = self.client.post(url, { 'student': self.user1.username, 'url': str(self.week1.location), 'due_datetime': '12/30/2013 00:00' }) assert response.status_code == 200, response.content - assert datetime.datetime(2013, 12, 30, 0, 0, tzinfo=UTC) ==\ - get_extended_due(self.course, self.week1, self.user1) + assert get_extended_due(self.course, self.week1, self.user1) == due_date + # This operation regenerates the cache, so we can use cached results from edx-when. + assert get_date_for_block(self.course, self.week1, self.user1, use_cached=True) == due_date def test_change_to_invalid_due_date(self): url = reverse('change_due_date', kwargs={'course_id': str(self.course.id)}) @@ -4074,7 +4076,8 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): 'url': str(self.week3.location), }) self.assertContains(response, 'Successfully reset due date for student') - assert get_date_for_block(self.course, self.week3, self.user1) == original_due + # This operation regenerates the cache, so we can use cached results from edx-when. + assert get_date_for_block(self.course, self.week3, self.user1, use_cached=True) == original_due 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 cffd7edb47..07bd1c33b5 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -12,8 +12,9 @@ import pytest from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import MultipleObjectsReturned from django.test import TestCase -from edx_when.api import set_dates_for_course +from edx_when.api import get_dates_for_course, set_dates_for_course from edx_when.field_data import DateLookupFieldData +from mock.mock import MagicMock, patch from opaque_keys.edx.keys import CourseKey from pytz import UTC from xmodule.fields import Date @@ -284,6 +285,31 @@ class TestSetDueDateExtension(ModuleStoreTestCase): with pytest.raises(tools.DashboardError): tools.set_due_date_extension(self.course, self.week3, user, extended) + @patch('edx_when.api.get_dates_for_course', wraps=get_dates_for_course) + def test_set_due_date_extension_cache_invalidation(self, mock_method: MagicMock): + """ + Tests that the course dates are reloaded once they are overridden. + """ + extended_hw = datetime.datetime(2013, 10, 25, 0, 0, tzinfo=UTC) + tools.set_due_date_extension(self.course, self.assignment, self.user, extended_hw) + + assert mock_method.call_count == 2 + mock_method.assert_called_with(self.course.id, user=self.user, published_version=None, use_cached=False) + + @patch('edx_when.api.get_dates_for_course', wraps=get_dates_for_course) + def test_set_due_date_extension_cache_invalidation_with_version(self, mock_method: MagicMock): + """ + Tests that the course dates are reloaded for both versioned and non-versioned scenarios, to provide + a unified experience across the platform. + """ + self.course.course_version = 'test_version' + extended_hw = datetime.datetime(2013, 10, 25, 0, 0, tzinfo=UTC) + tools.set_due_date_extension(self.course, self.assignment, self.user, extended_hw) + + assert mock_method.call_count == 3 + mock_method.assert_any_call(self.course.id, user=self.user, published_version='test_version', use_cached=False) + mock_method.assert_any_call(self.course.id, user=self.user, use_cached=False) + class TestDataDumps(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py index 06fdad5865..f6ad49dbc5 100644 --- a/lms/djangoapps/instructor/views/tools.py +++ b/lms/djangoapps/instructor/views/tools.py @@ -204,6 +204,16 @@ def set_due_date_extension(course, unit, student, due_date, actor=None, reason=' else: api.set_date_for_block(course.id, block.location, 'due', None, user=student, reason=reason, actor=actor) + # edx-proctoring is checking cached course dates, so the overrides made above will not be enforced until the + # TieredCache is reloaded. This can lead to situations when a student's extension is revoked, but they can still + # complete an exam for some time. The instructors don't have a way of checking whether the cache has been + # regenerated because the Instructor Dashboard simply lists all extensions. Therefore, to avoid having a confusing + # user experience, we want trigger cache regeneration after changing the due date. + api.get_dates_for_course(course.id, user=student, published_version=version, use_cached=False) + if version: + # edx-proctoring is not using the course version while checking its dates. + api.get_dates_for_course(course.id, user=student, use_cached=False) + def dump_module_extensions(course, unit): """