From 449d5c7a2dfaa5617a2e139d33bb33d908cb0cca Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Wed, 6 Oct 2021 15:02:03 -0400 Subject: [PATCH] feat: only email users about their goal in the morning in their timezone (#28922) --- .../commands/goal_reminder_email.py | 10 ++++- .../tests/test_goal_reminder_email.py | 12 ++++-- .../courseware/context_processor.py | 39 ++++++++++++++++++- .../0016_lastseencoursewaretimezone.py | 24 ++++++++++++ lms/djangoapps/courseware/models.py | 15 +++++++ .../tests/test_context_processor.py | 27 +++++++++++-- .../core/djangoapps/courseware_api/views.py | 22 +++++++++++ 7 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 lms/djangoapps/courseware/migrations/0016_lastseencoursewaretimezone.py diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index e977d4305f..f8d45a7af5 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -1,7 +1,7 @@ """ Command to trigger sending reminder emails for learners to achieve their Course Goals """ -from datetime import date, timedelta +from datetime import date, datetime, timedelta import logging import six @@ -16,6 +16,7 @@ from edx_ace.recipient import Recipient from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.certificates.api import get_certificate_for_user_id from lms.djangoapps.certificates.data import CertificateStatuses +from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc from lms.djangoapps.course_goals.models import CourseGoal, CourseGoalReminderStatus, UserActivity from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from openedx.core.djangoapps.ace_common.template_context import get_base_template_context @@ -159,6 +160,13 @@ class Command(BaseCommand): # The weekdays are 0 indexed, but we want this to be 1 to match required_days_left. # Essentially, if today is Sunday, days_left_in_week should be 1 since they have Sunday to hit their goal. days_left_in_week = SUNDAY_WEEKDAY - today.weekday() + 1 + + # We want to email users in the morning of their timezone + user_timezone = get_user_timezone_or_last_seen_timezone_or_utc(goal.user) + now_in_users_timezone = datetime.now(user_timezone) + if not 9 <= now_in_users_timezone.hour < 12: + return False + if required_days_left == days_left_in_week: send_ace_message(goal) CourseGoalReminderStatus.objects.update_or_create(goal=goal, defaults={'email_reminder_sent': True}) diff --git a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py index 148703e909..2210a1b5d9 100644 --- a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py @@ -49,15 +49,15 @@ class TestGoalReminderEmailCommand(TestCase): kwargs.setdefault('overview__end', datetime(2021, 4, 1, tzinfo=UTC)) # Have it end in the future goal = CourseGoalFactory(**kwargs) - with freeze_time('2021-02-01'): # Create enrollment before March + with freeze_time('2021-02-01 10:00:00'): # Create enrollment before March CourseEnrollmentFactory(user=goal.user, course_id=goal.course_key) return goal - def call_command(self, day=TUESDAY, expect_sent=None, expect_send_count=None): + def call_command(self, day=TUESDAY, expect_sent=None, expect_send_count=None, time=None): """Calls the management command with a frozen time and optionally checks whether we sent an email""" with mock.patch('lms.djangoapps.course_goals.management.commands.goal_reminder_email.send_ace_message') as mock_send: # pylint: disable=line-too-long - with freeze_time(f'2021-03-0{day + 1}'): # March 2021 starts on a Monday + with freeze_time(time or f'2021-03-0{day + 1} 10:00:00'): # March 2021 starts on a Monday call_command('goal_reminder_email') if expect_sent is not None: @@ -108,6 +108,12 @@ class TestGoalReminderEmailCommand(TestCase): self.call_command(day=current_day, expect_sent=expect_sent) + def test_will_send_at_the_right_time(self): + """ We only send the emails from 9am-12pm in the user's time""" + self.make_valid_goal() + self.call_command(expect_sent=False, time='2021-03-02 8:00:00') + self.call_command(expect_sent=True, time='2021-03-02 10:00:00') + def test_feature_disabled(self): self.make_valid_goal() with override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=False): diff --git a/lms/djangoapps/courseware/context_processor.py b/lms/djangoapps/courseware/context_processor.py index 8444401db8..c6afdad46b 100644 --- a/lms/djangoapps/courseware/context_processor.py +++ b/lms/djangoapps/courseware/context_processor.py @@ -6,11 +6,15 @@ to the templates without having to append every view file. """ +from pytz import timezone +from edx_django_utils.cache import TieredCache +from lms.djangoapps.courseware.models import LastSeenCoursewareTimezone from openedx.core.djangoapps.user_api.errors import UserAPIInternalError, UserNotFound -from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences +from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, get_user_preferences from openedx.core.lib.cache_utils import get_cache + RETRIEVABLE_PREFERENCES = { 'user_timezone': 'time_zone', 'user_language': 'pref-lang' @@ -46,3 +50,36 @@ def user_timezone_locale_prefs(request): cached_value.update(user_prefs) return cached_value + + +def get_last_seen_courseware_timezone(user): + """ + The above method is for the timezone that is set on the user's account. + That timezone is often not set, so this field retrieves the browser timezone + from a recent courseware visit (updated daily) + """ + cache_key = 'browser_timezone_{}'.format(str(user.id)) + cached_value = TieredCache.get_cached_response(cache_key) + if not cached_value.is_found: + try: + LastSeenCoursewareTimezone.objects.get(user=user) + except LastSeenCoursewareTimezone.DoesNotExist: + return None + + else: + return cached_value.value + + +def get_user_timezone_or_last_seen_timezone_or_utc(user): + """ + Helper method for returning a reasonable timezone for a user. + This method returns the timezone in the user's account if that is set. + If that is not set, it returns a recent timezone that we have recorded from a user's visit to the courseware. + If that is not set, it returns UTC. + """ + user_timezone = ( + get_user_preference(user, 'time_zone') or + get_last_seen_courseware_timezone(user) or + 'UTC' + ) + return timezone(user_timezone) diff --git a/lms/djangoapps/courseware/migrations/0016_lastseencoursewaretimezone.py b/lms/djangoapps/courseware/migrations/0016_lastseencoursewaretimezone.py new file mode 100644 index 0000000000..6532207dd6 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0016_lastseencoursewaretimezone.py @@ -0,0 +1,24 @@ +# Generated by Django 2.2.24 on 2021-10-06 17:01 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('courseware', '0015_add_courseware_stats_index'), + ] + + operations = [ + migrations.CreateModel( + name='LastSeenCoursewareTimezone', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('last_seen_courseware_timezone', models.CharField(db_index=True, max_length=255)), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 9b86c33959..5ee97fb88a 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -485,3 +485,18 @@ class OrgDynamicUpgradeDeadlineConfiguration(OptOutDynamicUpgradeDeadlineMixin, default=False, help_text=_('Disable the dynamic upgrade deadline for this organization.') ) + + +class LastSeenCoursewareTimezone(models.Model): + """ + The timezone in the user's account is frequently not set. + This model stores a user's recent timezone that can be used as a fallback + + .. no_pii: + """ + + user = models.OneToOneField(User, on_delete=models.CASCADE) + last_seen_courseware_timezone = models.CharField(max_length=255, db_index=True) + + class Meta: + app_label = "courseware" diff --git a/lms/djangoapps/courseware/tests/test_context_processor.py b/lms/djangoapps/courseware/tests/test_context_processor.py index 07c9001ab4..3eb873765b 100644 --- a/lms/djangoapps/courseware/tests/test_context_processor.py +++ b/lms/djangoapps/courseware/tests/test_context_processor.py @@ -2,14 +2,18 @@ Unit tests for courseware context_processor """ - +from pytz import timezone from unittest.mock import Mock from django.contrib.auth.models import AnonymousUser -from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs +from lms.djangoapps.courseware.context_processor import ( + get_user_timezone_or_last_seen_timezone_or_utc, + user_timezone_locale_prefs, +) from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory class UserPrefContextProcessorUnitTest(ModuleStoreTestCase): @@ -20,7 +24,7 @@ class UserPrefContextProcessorUnitTest(ModuleStoreTestCase): def setUp(self): super().setUp() - self.user = UserFactory.create() + self.user = UserFactory.create(password='foo') self.request = Mock() self.request.user = self.user @@ -43,3 +47,20 @@ class UserPrefContextProcessorUnitTest(ModuleStoreTestCase): assert context['user_language'] is None assert context['user_timezone'] is not None assert context['user_timezone'] == 'Asia/Tokyo' + + def test_get_user_timezone_or_last_seen_timezone_or_utc(self): + # We default to UTC + course = CourseFactory() + time_zone = get_user_timezone_or_last_seen_timezone_or_utc(self.user) + assert time_zone == timezone('UTC') + + # We record the timezone when a user hits the courseware api + self.client.login(username=self.user.username, password='foo') + self.client.get(f'/api/courseware/course/{course.id}?browser_timezone=America/New_York') + time_zone = get_user_timezone_or_last_seen_timezone_or_utc(self.user) + assert time_zone == timezone('America/New_York') + + # If a user has their timezone set, then we use that setting + set_user_preference(self.user, 'time_zone', 'Asia/Tokyo') + time_zone = get_user_timezone_or_last_seen_timezone_or_utc(self.user) + assert time_zone == timezone('Asia/Tokyo') diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index d6dc8ebef8..2a708279b5 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -6,6 +6,7 @@ from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block from django.urls import reverse from django.utils.translation import ugettext as _ +from edx_django_utils.cache import TieredCache from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from opaque_keys import InvalidKeyError @@ -32,6 +33,7 @@ from lms.djangoapps.courseware.access_response import ( from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs from lms.djangoapps.courseware.courses import check_course_access from lms.djangoapps.courseware.masquerade import is_masquerading_as_specific_student, setup_masquerade +from lms.djangoapps.courseware.models import LastSeenCoursewareTimezone from lms.djangoapps.courseware.module_render import get_module_by_usage_id from lms.djangoapps.courseware.tabs import get_course_tab_list from lms.djangoapps.courseware.toggles import ( @@ -466,11 +468,28 @@ class CoursewareInformation(RetrieveAPIView): serializer_class = CourseInfoSerializer + def set_last_seen_courseware_timezone(self, user): + """ + The timezone in the user's account is frequently not set. + This method sets a user's recent timezone that can be used as a fallback + """ + cache_key = 'browser_timezone_{}'.format(str(user.id)) + browser_timezone = self.request.query_params.get('browser_timezone', None) + cached_value = TieredCache.get_cached_response(cache_key) + if not cached_value.is_found: + if browser_timezone: + TieredCache.set_all_tiers(cache_key, str(browser_timezone), 86400) # Refresh the cache daily + LastSeenCoursewareTimezone.objects.update_or_create( + user=user, + last_seen_courseware_timezone=browser_timezone, + ) + def get_object(self): """ Return the requested course object, if the user has appropriate permissions. """ + original_user = self.request.user if self.request.user.is_staff: username = self.request.GET.get('username', '') or self.request.user.username else: @@ -484,6 +503,9 @@ class CoursewareInformation(RetrieveAPIView): # Record course goals user activity for learning mfe courseware on web UserActivity.record_user_activity(self.request.user, course_key) + # Record a user's browser timezone + self.set_last_seen_courseware_timezone(original_user) + return overview def get_serializer_context(self):