From 1482755bbd699c784866e12e7360bbbefbc24a0c Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Thu, 7 Jan 2021 15:56:00 -0500 Subject: [PATCH] Show discount deadline in a timezone-aware way Also, fix it and the access expiration deadline to not hardcode the date presentation in an American way. --- common/djangoapps/util/date_utils.py | 40 ++++++++++++++- .../djangoapps/util/tests/test_date_utils.py | 35 +++++++++++-- lms/djangoapps/courseware/tests/helpers.py | 17 ++----- .../features/course_duration_limits/access.py | 36 ++----------- .../tests/test_access.py | 2 +- .../tests/views/test_course_home.py | 50 ++----------------- openedx/features/discounts/applicability.py | 2 +- .../discounts/tests/test_applicability.py | 2 +- openedx/features/discounts/utils.py | 3 +- 9 files changed, 87 insertions(+), 100 deletions(-) diff --git a/common/djangoapps/util/date_utils.py b/common/djangoapps/util/date_utils.py index 0cbb56d08d..270b3d38be 100644 --- a/common/djangoapps/util/date_utils.py +++ b/common/djangoapps/util/date_utils.py @@ -6,9 +6,13 @@ Convenience methods for working with datetime objects import re from datetime import datetime, timedelta -from django.utils.translation import pgettext, ugettext +import crum +from django.utils.translation import get_language, pgettext, ugettext from pytz import UnknownTimeZoneError, timezone, utc +from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs +from openedx.core.djangolib.markup import HTML + def get_default_time_display(dtime): """ @@ -212,6 +216,40 @@ def strftime_localized(dtime, format): # pylint: disable=redefined-builtin return formatted_date +def strftime_localized_html(dtime, fmt): + """ + Returns an html string that can be further localized in browser by JS code + + For example, a user's default timezone preference is "whatever the browser default is" which must be queried via + Javascript. So we write out a UTC formatted date here, but tag it with enough information that dateutil_factory.js + can then later update the DOM to use the proper formatting. + + Arguments: + dtime (datetime): Datetime to format + fmt (str): One of the special enum values that strftime_localized accepts. Only SHORT_DATE supported now. + """ + locale_prefs = user_timezone_locale_prefs(crum.get_current_request()) + user_timezone = locale_prefs['user_timezone'] + language = get_language() + + # Here we map the enums for strftime_localized into the enums used by date-utils.js when localizing on JS side. + format_mapping = { + 'SHORT_DATE': 'shortDate', + } + assert fmt in format_mapping.keys(), 'format "%s" not yet supported in strftime_localized_html' % fmt + + date_html = '{formatted_date_localized}' + + return HTML(date_html).format( + language=language, + user_timezone=user_timezone, + format=format_mapping[fmt], + formatted_date=dtime.isoformat(), + formatted_date_localized=strftime_localized(dtime, fmt), + ) + + # In order to extract the strings below, we have to mark them with pgettext. # But we'll do the actual pgettext later, so use a no-op for now, and save the # real pgettext so we can assign it back to the global name later. diff --git a/common/djangoapps/util/tests/test_date_utils.py b/common/djangoapps/util/tests/test_date_utils.py index fa99ece333..5a448e01d9 100644 --- a/common/djangoapps/util/tests/test_date_utils.py +++ b/common/djangoapps/util/tests/test_date_utils.py @@ -8,11 +8,13 @@ import unittest from datetime import datetime, timedelta, tzinfo import ddt -import six +from markupsafe import Markup from mock import patch from pytz import utc -from common.djangoapps.util.date_utils import almost_same_datetime, get_default_time_display, get_time_display, strftime_localized +from common.djangoapps.util.date_utils import ( + almost_same_datetime, get_default_time_display, get_time_display, strftime_localized, strftime_localized_html +) def test_get_default_time_display(): @@ -134,9 +136,7 @@ class StrftimeLocalizedTest(unittest.TestCase): (fmt, expected) = fmt_expected dtime = datetime(2013, 2, 14, 16, 41, 17) self.assertEqual(expected, strftime_localized(dtime, fmt)) - # strftime doesn't like Unicode, so do the work in UTF8. - self.assertEqual(expected.encode('utf-8') if six.PY2 else expected, - dtime.strftime(fmt.encode('utf-8') if six.PY2 else fmt)) + self.assertEqual(expected, dtime.strftime(fmt)) @ddt.data( ("SHORT_DATE", "Feb 14, 2013"), @@ -211,3 +211,28 @@ class StrftimeLocalizedTest(unittest.TestCase): dtime = datetime(2013, 2, 14, 16, 41, 17) with self.assertRaises(ValueError): strftime_localized(dtime, fmt) + + +@ddt.ddt +class StrftimeLocalizedHtmlTest(unittest.TestCase): + """ + Tests for strftime_localized_html. + """ + @ddt.data( + None, + 'Africa/Casablanca', + ) + def test_happy_path(self, timezone): + dtime = datetime(2013, 2, 14, 16, 41, 17) + with patch('common.djangoapps.util.date_utils.user_timezone_locale_prefs', + return_value={'user_timezone': timezone}): + html = strftime_localized_html(dtime, 'SHORT_DATE') + self.assertIsInstance(html, Markup) + self.assertRegex(html, + 'Feb 14, 2013') + + def test_invalid_format_string(self): + dtime = datetime(2013, 2, 14, 16, 41, 17) + with self.assertRaisesRegex(AssertionError, 'format "NOPE" not yet supported in strftime_localized_html'): + strftime_localized_html(dtime, 'NOPE') diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 7b9e052600..9a4fbc65ef 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -27,10 +27,9 @@ from lms.djangoapps.courseware.masquerade import setup_masquerade from lms.djangoapps.lms_xblock.field_data import LmsFieldData from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.url_utils import quote_slashes -from openedx.features.course_duration_limits.access import EXPIRATION_DATE_FORMAT_STR from common.djangoapps.student.models import CourseEnrollment, Registration from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from common.djangoapps.util.date_utils import strftime_localized +from common.djangoapps.util.date_utils import strftime_localized_html from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -423,19 +422,9 @@ def get_expiration_banner_text(user, course, language='en'): 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.isoformat(), - formatted_date_localized=strftime_localized(expiration_date, EXPIRATION_DATE_FORMAT_STR) - ) + formatted_expiration_date = strftime_localized_html(expiration_date, 'SHORT_DATE') if upgrade_deadline: - formatted_upgrade_deadline = date_string.format( - language=language, - formatted_date=upgrade_deadline.isoformat(), - formatted_date_localized=strftime_localized(upgrade_deadline, EXPIRATION_DATE_FORMAT_STR) - ) + formatted_upgrade_deadline = strftime_localized_html(upgrade_deadline, 'SHORT_DATE') bannerText = u'Audit Access Expires {expiration_date}
\ You lose all access to this course, including your progress, on {expiration_date}.\ diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index b347dcbffc..31b9775eb1 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -4,19 +4,16 @@ Contains code related to computing content gating course duration limits and course access based on these limits. """ -import crum from django.utils import timezone -from django.utils.translation import get_language from django.utils.translation import ugettext as _ 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 common.djangoapps.util.date_utils import strftime_localized, strftime_localized_html 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.content.course_overviews.models import CourseOverview @@ -24,8 +21,6 @@ from openedx.core.djangoapps.course_date_signals.utils import get_expected_durat from openedx.core.djangolib.markup import HTML from openedx.features.course_duration_limits.models import CourseDurationLimitConfig -EXPIRATION_DATE_FORMAT_STR = '%b %-d, %Y' - class AuditExpiredError(AccessError): """ @@ -34,7 +29,7 @@ class AuditExpiredError(AccessError): def __init__(self, user, course, expiration_date): error_code = 'audit_expired' developer_message = 'User {} had access to {} until {}'.format(user, course, expiration_date) - expiration_date = strftime_localized(expiration_date, EXPIRATION_DATE_FORMAT_STR) + expiration_date = strftime_localized(expiration_date, 'SHORT_DATE') user_message = _('Access expired on {expiration_date}').format(expiration_date=expiration_date) try: course_name = course.display_name_with_default @@ -113,12 +108,6 @@ def check_course_expired(user, course): return ACCESS_GRANTED -def get_date_string(): - # Creating this method to allow unit testing an issue where this string was missing the unicode prefix - return '{formatted_date_localized}' - - def get_access_expiration_data(user, course): """ Create a dictionary of information about the access expiration for this user & course. @@ -165,14 +154,11 @@ def generate_course_expired_message(user, course): upgrade_deadline = expiration_data['upgrade_deadline'] upgrade_url = expiration_data['upgrade_url'] - user_timezone_locale = user_timezone_locale_prefs(crum.get_current_request()) - user_timezone = user_timezone_locale['user_timezone'] - if masquerading_expired_course: upgrade_message = _('This learner does not have access to this course. ' 'Their access expired on {expiration_date}.') return HTML(upgrade_message).format( - expiration_date=strftime_localized(expiration_date, EXPIRATION_DATE_FORMAT_STR) + expiration_date=strftime_localized_html(expiration_date, 'SHORT_DATE') ) else: expiration_message = _('{strong_open}Audit Access Expires {expiration_date}{strong_close}' @@ -188,21 +174,9 @@ def generate_course_expired_message(user, course): else: using_upgrade_messaging = False - language = get_language() - date_string = get_date_string() - formatted_expiration_date = date_string.format( - language=language, - user_timezone=user_timezone, - formatted_date=expiration_date.isoformat(), - formatted_date_localized=strftime_localized(expiration_date, EXPIRATION_DATE_FORMAT_STR) - ) + formatted_expiration_date = strftime_localized_html(expiration_date, 'SHORT_DATE') if using_upgrade_messaging: - formatted_upgrade_deadline = date_string.format( - language=language, - user_timezone=user_timezone, - formatted_date=upgrade_deadline.isoformat(), - formatted_date_localized=strftime_localized(upgrade_deadline, EXPIRATION_DATE_FORMAT_STR) - ) + formatted_upgrade_deadline = strftime_localized_html(upgrade_deadline, 'SHORT_DATE') return HTML(full_message).format( a_open=HTML('').format(upgrade_link=upgrade_url), diff --git a/openedx/features/course_duration_limits/tests/test_access.py b/openedx/features/course_duration_limits/tests/test_access.py index b7a9527739..d6139f9785 100644 --- a/openedx/features/course_duration_limits/tests/test_access.py +++ b/openedx/features/course_duration_limits/tests/test_access.py @@ -39,7 +39,7 @@ class TestAccess(CacheIsolationTestCase): def assertDateInMessage(self, date, message): # First, check that the formatted version is in there - self.assertIn(strftime_localized(date, '%b %-d, %Y'), message) + self.assertIn(strftime_localized(date, 'SHORT_DATE'), message) # But also that the machine-readable version is in there self.assertIn('data-datetime="%s"' % date.isoformat(), message) diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index c070e838ab..d0581da747 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -21,6 +21,7 @@ from waffle.testutils import override_flag from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory +from common.djangoapps.util.date_utils import strftime_localized_html from lms.djangoapps.experiments.models import ExperimentData from lms.djangoapps.commerce.models import CommerceConfiguration from lms.djangoapps.commerce.utils import EcommerceService @@ -37,7 +38,6 @@ from lms.djangoapps.courseware.tests.helpers import get_expiration_banner_text from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, @@ -414,19 +414,19 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): user = self.create_user_for_course(self.course, CourseUserType.ENROLLED) now_time = datetime.now(tz=UTC).strftime(u"%Y-%m-%d %H:%M:%S%z") ExperimentData.objects.create( - user=user, experiment_id=REV1008_EXPERIMENT_ID, key=str(self.course), value=now_time + user=user, experiment_id=REV1008_EXPERIMENT_ID, key=str(self.course.id), value=now_time ) self.client.login(username=user.username, password=self.TEST_PASSWORD) url = course_home_url(self.course) response = self.client.get(url) - discount_expiration_date = get_discount_expiration_date(user, self.course).strftime(u'%B %d') + expiration_date = strftime_localized_html(get_discount_expiration_date(user, self.course), 'SHORT_DATE') upgrade_link = verified_upgrade_deadline_link(user=user, course=self.course) bannerText = u'''
Upgrade by {discount_expiration_date} and save {percentage}% [{strikeout_price}]
Use code EDXWELCOME at checkout! Upgrade Now
'''.format( - discount_expiration_date=discount_expiration_date, + discount_expiration_date=expiration_date, percentage=percentage, strikeout_price=HTML(format_strikeout_price(user, self.course)[0]), upgrade_link=upgrade_link @@ -577,7 +577,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): response = self.client.get(url) - expiration_date = strftime_localized(course.start + timedelta(weeks=4) + timedelta(days=1), u'%b %-d, %Y') + expiration_date = strftime_localized(course.start + timedelta(weeks=4) + timedelta(days=1), 'SHORT_DATE') expected_params = QueryDict(mutable=True) course_name = CourseOverview.get_from_id(course.id).display_name_with_default expected_params['access_response_error'] = u'Access to {run} expired on {expiration_date}'.format( @@ -799,46 +799,6 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): bannerText = get_expiration_banner_text(self.staff_user, self.course) self.assertNotContains(response, bannerText, html=True) - @mock.patch("common.djangoapps.util.date_utils.strftime_localized") - @mock.patch("openedx.features.course_duration_limits.access.get_date_string") - def test_course_expiration_banner_with_unicode(self, mock_strftime_localized, mock_get_date_string): - """ - Ensure that switching to other languages that have unicode in their - date representations will not cause the course home page to 404. - """ - fake_unicode_start_time = u"üñîçø∂é_ßtå®t_tîµé" - mock_strftime_localized.return_value = fake_unicode_start_time - date_string = u'{formatted_date_localized}' - mock_get_date_string.return_value = date_string - - config = CourseDurationLimitConfig( - course=CourseOverview.get_from_id(self.course.id), - enabled=True, - enabled_as_of=datetime(2018, 1, 1, tzinfo=UTC) - ) - config.save() - url = course_home_url(self.course) - user = self.create_user_for_course(self.course, CourseUserType.UNENROLLED) - CourseEnrollment.enroll(user, self.course.id) - - language = 'eo' - DarkLangConfig( - released_languages=language, - changed_by=user, - enabled=True - ).save() - - response = self.client.get(url, HTTP_ACCEPT_LANGUAGE=language) - self.assertEqual(response.status_code, 200) - self.assertEqual(response['Content-Language'], language) - - # Check that if the string is incorrectly not marked as unicode we still get the error - with mock.patch("openedx.features.course_duration_limits.access.get_date_string", - return_value=date_string.encode('utf-8')): - response = self.client.get(url, HTTP_ACCEPT_LANGUAGE=language) - self.assertEqual(response.status_code, 500) - @override_waffle_flag(COURSE_PRE_START_ACCESS_FLAG, active=True) @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) def test_course_goals(self): diff --git a/openedx/features/discounts/applicability.py b/openedx/features/discounts/applicability.py index f979e46eeb..c681d811d0 100644 --- a/openedx/features/discounts/applicability.py +++ b/openedx/features/discounts/applicability.py @@ -64,7 +64,7 @@ def get_discount_expiration_date(user, course): time_limit_start = None try: - saw_banner = ExperimentData.objects.get(user=user, experiment_id=REV1008_EXPERIMENT_ID, key=str(course)) + saw_banner = ExperimentData.objects.get(user=user, experiment_id=REV1008_EXPERIMENT_ID, key=str(course.id)) time_limit_start = parse_datetime(saw_banner.value) except ExperimentData.DoesNotExist: return None diff --git a/openedx/features/discounts/tests/test_applicability.py b/openedx/features/discounts/tests/test_applicability.py index 7ba7eaf9ef..4b9ac22cbc 100644 --- a/openedx/features/discounts/tests/test_applicability.py +++ b/openedx/features/discounts/tests/test_applicability.py @@ -42,7 +42,7 @@ class TestApplicability(ModuleStoreTestCase): CourseModeFactory.create(course_id=self.course.id, mode_slug='verified') now_time = datetime.now(tz=pytz.UTC).strftime(u"%Y-%m-%d %H:%M:%S%z") ExperimentData.objects.create( - user=self.user, experiment_id=REV1008_EXPERIMENT_ID, key=str(self.course), value=now_time + user=self.user, experiment_id=REV1008_EXPERIMENT_ID, key=str(self.course.id), value=now_time ) holdback_patcher = patch( diff --git a/openedx/features/discounts/utils.py b/openedx/features/discounts/utils.py index f25e981c22..afd305b8a1 100644 --- a/openedx/features/discounts/utils.py +++ b/openedx/features/discounts/utils.py @@ -12,6 +12,7 @@ from edx_django_utils.cache import RequestCache from web_fragments.fragment import Fragment from common.djangoapps.course_modes.models import format_course_price, get_course_prices +from common.djangoapps.util.date_utils import strftime_localized_html from lms.djangoapps.experiments.models import ExperimentData from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -184,7 +185,7 @@ def generate_offer_html(user, course): '
' '' ), - discount_expiration_date=data['expiration_date'].strftime('%B %d'), + discount_expiration_date=strftime_localized_html(data['expiration_date'], 'SHORT_DATE'), percentage=data['percentage'], span_close=HTML(''), div_close=HTML('
'),