Merge pull request #30999 from open-craft/agrendalath/invalidate_cache_on_due_date_extension

fix: regenerate the cache on due date extensions
This commit is contained in:
Pooja Kulkarni
2022-11-24 11:10:05 -05:00
committed by GitHub
3 changed files with 46 additions and 7 deletions

View File

@@ -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()

View File

@@ -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):
"""

View File

@@ -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):
"""