From e8d9a254bd4d41a0e39d9f225141dbdf70a7f2b3 Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Mon, 23 Nov 2020 12:21:13 -0500 Subject: [PATCH] AA-459: Respect user's timezone in access-expiration message Before, it would just always use UTC. --- common/djangoapps/util/date_utils.py | 3 +- lms/djangoapps/courseware/tests/helpers.py | 8 ++--- .../features/course_duration_limits/access.py | 31 ++++++++++--------- .../tests/test_access.py | 28 +++++++++++++---- 4 files changed, 43 insertions(+), 27 deletions(-) diff --git a/common/djangoapps/util/date_utils.py b/common/djangoapps/util/date_utils.py index 37f1955838..0cbb56d08d 100644 --- a/common/djangoapps/util/date_utils.py +++ b/common/djangoapps/util/date_utils.py @@ -6,7 +6,6 @@ Convenience methods for working with datetime objects import re from datetime import datetime, timedelta -import six from django.utils.translation import pgettext, ugettext from pytz import UnknownTimeZoneError, timezone, utc @@ -59,7 +58,7 @@ def get_time_display(dtime, format_string=None, coerce_tz=None): if dtime is None or format_string is None: return get_default_time_display(dtime) try: - return six.text_type(strftime_localized(dtime, format_string)) + return strftime_localized(dtime, format_string) except ValueError: return get_default_time_display(dtime) diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 9aecfc1947..7b9e052600 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -416,24 +416,24 @@ def get_expiration_banner_text(user, course, language='en'): Get text for banner that messages user course expiration date for different tests that depend on it. """ - expiration_date = now() + timedelta(weeks=4) upgrade_link = verified_upgrade_deadline_link(user=user, course=course) enrollment = CourseEnrollment.get_enrollment(user, course.id) + expiration_date = enrollment.created + timedelta(weeks=4) upgrade_deadline = enrollment.upgrade_deadline if upgrade_deadline is None or now() < upgrade_deadline: upgrade_deadline = enrollment.course_upgrade_deadline - date_string = u'{formatted_date_localized}' formatted_expiration_date = date_string.format( language=language, - formatted_date=expiration_date.strftime("%Y-%m-%d"), + formatted_date=expiration_date.isoformat(), formatted_date_localized=strftime_localized(expiration_date, EXPIRATION_DATE_FORMAT_STR) ) if upgrade_deadline: formatted_upgrade_deadline = date_string.format( language=language, - formatted_date=upgrade_deadline.strftime("%Y-%m-%d"), + formatted_date=upgrade_deadline.isoformat(), formatted_date_localized=strftime_localized(upgrade_deadline, EXPIRATION_DATE_FORMAT_STR) ) diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index fb8455b66d..3b35cb2cb8 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -4,10 +4,7 @@ Contains code related to computing content gating course duration limits and course access based on these limits. """ - -from datetime import timedelta - -import six +import crum from django.utils import timezone from django.utils.translation import get_language from django.utils.translation import ugettext as _ @@ -15,17 +12,17 @@ from edx_django_utils.cache import RequestCache from web_fragments.fragment import Fragment from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.util.date_utils import strftime_localized from lms.djangoapps.courseware.access_response import AccessError from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED +from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_specific_student -from openedx.core.djangoapps.catalog.utils import get_course_run_details from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_date_signals.utils import get_expected_duration from openedx.core.djangolib.markup import HTML from openedx.features.course_duration_limits.models import CourseDurationLimitConfig -from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.util.date_utils import strftime_localized EXPIRATION_DATE_FORMAT_STR = u'%b %-d, %Y' @@ -118,7 +115,7 @@ def check_course_expired(user, course): def get_date_string(): # Creating this method to allow unit testing an issue where this string was missing the unicode prefix - return u'{formatted_date_localized}' @@ -130,7 +127,11 @@ def generate_course_expired_message(user, course): if not expiration_date: return - if is_masquerading_as_specific_student(user, course.id) and timezone.now() > expiration_date: + user_timezone_locale = user_timezone_locale_prefs(crum.get_current_request()) + user_timezone = user_timezone_locale['user_timezone'] + + now = timezone.now() + if is_masquerading_as_specific_student(user, course.id) and now > expiration_date: upgrade_message = _('This learner does not have access to this course. ' u'Their access expired on {expiration_date}.') return HTML(upgrade_message).format( @@ -142,10 +143,8 @@ def generate_course_expired_message(user, course): return upgrade_deadline = enrollment.upgrade_deadline - now = timezone.now() - course_upgrade_deadline = enrollment.course_upgrade_deadline if (not upgrade_deadline) or (upgrade_deadline < now): - upgrade_deadline = course_upgrade_deadline + upgrade_deadline = enrollment.course_upgrade_deadline expiration_message = _(u'{strong_open}Audit Access Expires {expiration_date}{strong_close}' u'{line_break}You lose all access to this course, including your progress, on ' @@ -164,13 +163,15 @@ def generate_course_expired_message(user, course): date_string = get_date_string() formatted_expiration_date = date_string.format( language=language, - formatted_date=expiration_date.strftime("%Y-%m-%d"), + user_timezone=user_timezone, + formatted_date=expiration_date.isoformat(), formatted_date_localized=strftime_localized(expiration_date, EXPIRATION_DATE_FORMAT_STR) ) if using_upgrade_messaging: formatted_upgrade_deadline = date_string.format( language=language, - formatted_date=upgrade_deadline.strftime("%Y-%m-%d"), + user_timezone=user_timezone, + formatted_date=upgrade_deadline.isoformat(), formatted_date_localized=strftime_localized(upgrade_deadline, EXPIRATION_DATE_FORMAT_STR) ) @@ -254,7 +255,7 @@ def course_expiration_wrapper(user, block, view, frag, context): # pylint: disa # Course content must be escaped to render correctly due to the way the # way the XBlock rendering works. Transforming the safe markup to unicode # escapes correctly. - course_expiration_fragment.content = six.text_type(course_expiration_fragment.content) + course_expiration_fragment.content = str(course_expiration_fragment.content) course_expiration_fragment.add_content(frag.content) course_expiration_fragment.add_fragment_resources(frag) diff --git a/openedx/features/course_duration_limits/tests/test_access.py b/openedx/features/course_duration_limits/tests/test_access.py index b872f18f40..37d0f17dec 100644 --- a/openedx/features/course_duration_limits/tests/test_access.py +++ b/openedx/features/course_duration_limits/tests/test_access.py @@ -5,13 +5,17 @@ import itertools from datetime import datetime, timedelta import ddt +from crum import set_current_request +from django.test import RequestFactory from django.utils import timezone from pytz import UTC from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory +from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.courseware.models import DynamicUpgradeDeadlineConfiguration from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory +from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.features.course_duration_limits.access import ( generate_course_expired_message, @@ -32,6 +36,13 @@ class TestAccess(CacheIsolationTestCase): CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1, tzinfo=UTC)) DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + def assertDateInMessage(self, date, message): + # First, check that the formatted version is in there + self.assertIn(strftime_localized(date, '%b %-d, %Y'), message) + + # But also that the machine-readable version is in there + self.assertIn('data-datetime="%s"' % date.isoformat(), message) + @ddt.data( *itertools.product( itertools.product([None, -2, -1, 1, 2], repeat=2), @@ -42,6 +53,13 @@ class TestAccess(CacheIsolationTestCase): now = timezone.now() schedule_offset, course_offset = offsets + # Set a timezone and request, to test that the message looks at the user's setting + request = RequestFactory().get('/') + request.user = UserFactory() + set_current_request(request) + self.addCleanup(set_current_request, None) + set_user_preference(request.user, 'time_zone', 'Asia/Tokyo') + if schedule_offset is not None: schedule_upgrade_deadline = now + timedelta(days=schedule_offset) else: @@ -52,9 +70,6 @@ class TestAccess(CacheIsolationTestCase): else: course_upgrade_deadline = None - def format_date(date): - return strftime_localized(date, u'%b %-d, %Y') - enrollment = CourseEnrollmentFactory.create( course__start=datetime(2018, 1, 1, tzinfo=UTC), course__self_paced=True, @@ -78,16 +93,17 @@ class TestAccess(CacheIsolationTestCase): message = generate_course_expired_message(enrollment.user, enrollment.course) - self.assertIn(format_date(duration_limit_upgrade_deadline), message) + self.assertDateInMessage(duration_limit_upgrade_deadline, message) + self.assertIn('data-timezone="Asia/Tokyo"', message) soft_upgradeable = schedule_upgrade_deadline is not None and now < schedule_upgrade_deadline upgradeable = course_upgrade_deadline is None or now < course_upgrade_deadline has_upgrade_deadline = course_upgrade_deadline is not None if upgradeable and soft_upgradeable: - self.assertIn(format_date(schedule_upgrade_deadline), message) + self.assertDateInMessage(schedule_upgrade_deadline, message) elif upgradeable and has_upgrade_deadline: - self.assertIn(format_date(course_upgrade_deadline), message) + self.assertDateInMessage(course_upgrade_deadline, message) else: self.assertNotIn("Upgrade by", message)