feat: only email users about their goal in the morning in their timezone (#28922)
This commit is contained in:
committed by
GitHub
parent
e2dc24af3a
commit
449d5c7a2d
@@ -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})
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)),
|
||||
],
|
||||
),
|
||||
]
|
||||
@@ -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"
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user