chore: add tests for new goal reminder email management command
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
|
||||
from django.contrib import admin
|
||||
|
||||
from lms.djangoapps.course_goals.models import CourseGoal, UserActivity
|
||||
from lms.djangoapps.course_goals.models import CourseGoal, CourseGoalReminderStatus, UserActivity
|
||||
|
||||
|
||||
@admin.register(CourseGoal)
|
||||
@@ -17,6 +17,23 @@ class CourseGoalAdmin(admin.ModelAdmin):
|
||||
search_fields = ('user__username', 'course_key')
|
||||
|
||||
|
||||
@admin.register(CourseGoalReminderStatus)
|
||||
class CourseGoalReminderStatusAdmin(admin.ModelAdmin):
|
||||
"""Admin for CourseGoalReminderStatus"""
|
||||
list_display = ('id',
|
||||
'goal_user',
|
||||
'goal_course_key',
|
||||
'email_reminder_sent')
|
||||
raw_id_fields = ('goal',)
|
||||
search_fields = ('goal__user__username', 'goal__course_key')
|
||||
|
||||
def goal_user(self, obj):
|
||||
return obj.goal.user.username
|
||||
|
||||
def goal_course_key(self, obj):
|
||||
return obj.goal.course_key
|
||||
|
||||
|
||||
@admin.register(UserActivity)
|
||||
class UserActivityAdmin(admin.ModelAdmin):
|
||||
"""Admin for UserActivity"""
|
||||
|
||||
@@ -2,27 +2,36 @@
|
||||
Command to trigger sending reminder emails for learners to achieve their Course Goals
|
||||
"""
|
||||
from datetime import date, timedelta
|
||||
import logging
|
||||
|
||||
from django.contrib.sites.models import Site
|
||||
from django.core.management.base import BaseCommand
|
||||
|
||||
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.course_goals.models import CourseGoal, UserActivity
|
||||
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.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.lib.celery.task_utils import emulate_http_request
|
||||
from openedx.features.course_duration_limits.access import get_user_course_expiration_date
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
MONDAY_WEEKDAY = 0
|
||||
SUNDAY_WEEKDAY = 6
|
||||
|
||||
|
||||
def ace_send():
|
||||
"""Just here as a mock hook for tests - drop this once we fix AA-909"""
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
"""
|
||||
Example usage:
|
||||
$ ./manage.py lms goal_reminder_email
|
||||
"""
|
||||
help = ''
|
||||
help = 'Send emails to users that are in danger of missing their course goals for the week'
|
||||
|
||||
def handle(self, *args, **options):
|
||||
"""
|
||||
@@ -38,55 +47,67 @@ class Command(BaseCommand):
|
||||
# Monday is the start of when we consider user's activity towards counting towards their weekly
|
||||
# goal. As such, we use Mondays to clear out the email reminders sent from the previous week.
|
||||
if today.weekday() == MONDAY_WEEKDAY:
|
||||
CourseGoal.objects.filter(email_reminder_sent=True).update(email_reminder_sent=False)
|
||||
CourseGoalReminderStatus.objects.filter(email_reminder_sent=True).update(email_reminder_sent=False)
|
||||
log.info('Cleared all reminder statuses')
|
||||
return
|
||||
|
||||
course_goals = CourseGoal.objects.filter(days_per_week__gt=0, subscribed_to_reminders=True, email_reminder_sent=False)
|
||||
course_goals = CourseGoal.objects.filter(
|
||||
days_per_week__gt=0, subscribed_to_reminders=True,
|
||||
).exclude(
|
||||
reminder_status__email_reminder_sent=True,
|
||||
)
|
||||
all_goal_course_keys = course_goals.values_list('course_key', flat=True).distinct()
|
||||
# Exclude all courses whose end dates are earlier than Sunday so we don't send an email about hitting
|
||||
# a course goal when it may not even be possible.
|
||||
# NOTE TO MT: Should we maybe also use this list and set all of the course goals in this to be unsubscribed?
|
||||
courses_to_exclude = CourseOverview.objects.filter(
|
||||
id__in=all_goal_course_keys, end__date__lt=sunday_date
|
||||
id__in=all_goal_course_keys, end__date__lte=sunday_date
|
||||
).values_list('id', flat=True)
|
||||
|
||||
count = 0
|
||||
course_goals = course_goals.exclude(course_key__in=courses_to_exclude).select_related('user').order_by('user')
|
||||
for goal in course_goals:
|
||||
if not COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(goal.course_key):
|
||||
continue
|
||||
with emulate_http_request(site=Site.objects.get_current()): # emulate a request for waffle's benefit
|
||||
for goal in course_goals:
|
||||
if self.handle_goal(goal, today, sunday_date, monday_date):
|
||||
count += 1
|
||||
|
||||
enrollment = CourseEnrollment.objects.prefetch_related('course_overview').get(user=goal.user, course__id=goal.course_key)
|
||||
# If you're not actively enrolled in the course or your enrollment was this week
|
||||
if not enrollment.is_active or enrollment.created.date() >= monday_date:
|
||||
continue
|
||||
log.info(f'Sent {count} emails')
|
||||
|
||||
audit_access_expiration_date = get_user_course_expiration_date(user, enrollment.course_overview)
|
||||
# If an audit user's access expires this week, exclude them from the email since they may not
|
||||
# be able to hit their goal anyway
|
||||
if audit_access_expiration_date and audit_access_expiration_date.date() < sunday_date:
|
||||
# NOTE TO MT: Is this overkill? If they upgrade, I think they'd need to reselect it, but if they
|
||||
# don't, then we at least stop doing this for them for the rest of time.
|
||||
goal.update(subscribed_to_reminders=False)
|
||||
continue
|
||||
@staticmethod
|
||||
def handle_goal(goal, today, sunday_date, monday_date):
|
||||
"""Sends an email reminder for a single CourseGoal, if it passes all our checks"""
|
||||
if not COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(goal.course_key):
|
||||
return False
|
||||
|
||||
# NOTE TO MT: I had a thread with Aperture Eng (Justin Hynes) about which function to use here. This was the recommended one
|
||||
cert = get_certificate_for_user_id(goal.user, goal.course_key)
|
||||
# If a user has a downloadable certificate, we will consider them as having completed
|
||||
# the course and opt them out of receiving emails
|
||||
if cert and cert.status == CertificateStatuses.downloadable:
|
||||
# NOTE TO MT: Is this overkill? If their cert status changes, they would remain unsubscribed.
|
||||
# Also just not sure if it's bad for us to unsubscribe them anyway. Could be a weird thing to
|
||||
# come back to. I leave to your discretion.
|
||||
goal.update(subscribed_to_reminders=False)
|
||||
continue
|
||||
enrollment = CourseEnrollment.get_enrollment(goal.user, goal.course_key, select_related=['course'])
|
||||
# If you're not actively enrolled in the course or your enrollment was this week
|
||||
if not enrollment or not enrollment.is_active or enrollment.created.date() >= monday_date:
|
||||
return False
|
||||
|
||||
# Check the number of days left to successfully hit their goal
|
||||
week_activity_count = UserActivity.objects.filter(user=goal.user, course_key=goal.course_key, date__gte=monday_date).count()
|
||||
required_days_left = goal.days_per_week - week_activity_count
|
||||
# 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
|
||||
if required_days_left == days_left_in_week:
|
||||
# TODO: hook up email https://openedx.atlassian.net/browse/AA-909
|
||||
# ace.send(msg)
|
||||
goal.update(email_reminder_sent=True)
|
||||
audit_access_expiration_date = get_user_course_expiration_date(goal.user, enrollment.course_overview)
|
||||
# If an audit user's access expires this week, exclude them from the email since they may not
|
||||
# be able to hit their goal anyway
|
||||
if audit_access_expiration_date and audit_access_expiration_date.date() <= sunday_date:
|
||||
return False
|
||||
|
||||
cert = get_certificate_for_user_id(goal.user, goal.course_key)
|
||||
# If a user has a downloadable certificate, we will consider them as having completed
|
||||
# the course and opt them out of receiving emails
|
||||
if cert and cert.status == CertificateStatuses.downloadable:
|
||||
return False
|
||||
|
||||
# Check the number of days left to successfully hit their goal
|
||||
week_activity_count = UserActivity.objects.filter(
|
||||
user=goal.user, course_key=goal.course_key, date__gte=monday_date,
|
||||
).count()
|
||||
required_days_left = goal.days_per_week - week_activity_count
|
||||
# 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
|
||||
if required_days_left == days_left_in_week:
|
||||
# TODO: hook up email AA-909
|
||||
# ace.send(msg)
|
||||
ace_send() # temporary for tests, drop with AA-909 and just mock ace.send directly
|
||||
CourseGoalReminderStatus.objects.update_or_create(goal=goal, defaults={'email_reminder_sent': True})
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
"""Tests for the goal_reminder_email command"""
|
||||
|
||||
from datetime import datetime
|
||||
from pytz import UTC
|
||||
from unittest import mock
|
||||
|
||||
import ddt
|
||||
from django.core.management import call_command
|
||||
from django.test import TestCase
|
||||
from edx_toggles.toggles.testutils import override_waffle_flag
|
||||
from freezegun import freeze_time
|
||||
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory
|
||||
from lms.djangoapps.course_goals.models import CourseGoalReminderStatus
|
||||
from lms.djangoapps.course_goals.tests.factories import (
|
||||
CourseGoalFactory, CourseGoalReminderStatusFactory, UserActivityFactory,
|
||||
)
|
||||
from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS
|
||||
from lms.djangoapps.certificates.data import CertificateStatuses
|
||||
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
|
||||
# Some constants just for clarity of tests (assuming week starts on a Monday, as March 2021 used below does)
|
||||
MONDAY = 0
|
||||
TUESDAY = 1
|
||||
WEDNESDAY = 2
|
||||
THURSDAY = 3
|
||||
FRIDAY = 4
|
||||
SATURDAY = 5
|
||||
SUNDAY = 6
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@skip_unless_lms
|
||||
@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True)
|
||||
class TestGoalReminderEmailCommand(TestCase):
|
||||
"""
|
||||
Test goal_reminder_email management command.
|
||||
|
||||
A lot of these methods will hardcode references to March 2021. This is just a convenient anchor point for us
|
||||
because it started on a Monday. Calls to the management command will freeze time so it's during March.
|
||||
"""
|
||||
def make_valid_goal(self, **kwargs):
|
||||
"""Creates a goal that will cause an email to be sent as the goal is valid but has been missed"""
|
||||
kwargs.setdefault('days_per_week', 6)
|
||||
kwargs.setdefault('subscribed_to_reminders', True)
|
||||
kwargs.setdefault('overview__start', datetime(2021, 1, 1, tzinfo=UTC))
|
||||
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
|
||||
CourseEnrollmentFactory(user=goal.user, course_id=goal.course_key)
|
||||
|
||||
return goal
|
||||
|
||||
def call_command(self, day=TUESDAY, expect_sent=None, expect_send_count=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.ace_send') as mock_send:
|
||||
with freeze_time(f'2021-03-0{day + 1}'): # March 2021 starts on a Monday
|
||||
call_command('goal_reminder_email')
|
||||
|
||||
if expect_sent is not None:
|
||||
assert CourseGoalReminderStatus.objects.filter(email_reminder_sent=True).exists() == expect_sent
|
||||
|
||||
if expect_send_count is None:
|
||||
expect_send_count = 1 if expect_sent else 0
|
||||
assert mock_send.call_count == expect_send_count
|
||||
|
||||
def test_happy_path(self):
|
||||
"""Confirm that with default arguments, our test methods send an email"""
|
||||
# A lot of our "negative" tests below assume that these methods called with these arguments will give you a
|
||||
# working "email sent" state. And then tweak one thing to confirm it didn't send.
|
||||
# So, if you change this method at all, go also change the "failure case" tests below to match.
|
||||
self.make_valid_goal()
|
||||
self.call_command(expect_sent=True)
|
||||
|
||||
def test_clear_all_on_monday(self):
|
||||
"""Verify that we reset all email tracking on Monday"""
|
||||
CourseGoalReminderStatusFactory(email_reminder_sent=True)
|
||||
CourseGoalReminderStatusFactory(email_reminder_sent=False)
|
||||
self.call_command(MONDAY)
|
||||
assert CourseGoalReminderStatus.objects.filter(email_reminder_sent=False).count() == 2
|
||||
assert CourseGoalReminderStatus.objects.filter(email_reminder_sent=True).count() == 0
|
||||
|
||||
@ddt.data(
|
||||
(4, 5, SATURDAY, False), # Already made target
|
||||
(2, 0, FRIDAY, False), # Just before end of week cutoff
|
||||
(2, 0, SATURDAY, True), # Just after end of week cutoff
|
||||
(2, 0, SUNDAY, False), # Day after end of week cutoff
|
||||
(2, 1, SATURDAY, False), # With some activity in the bag already, that cutoff moves up
|
||||
(2, 1, SUNDAY, True), # ...to Sunday
|
||||
(7, 3, WEDNESDAY, False), # Same as above, but with some more interesting numbers
|
||||
(7, 3, THURSDAY, True), # ditto (after cutoff)
|
||||
(7, 3, FRIDAY, False), # ditto (day after)
|
||||
(7, 0, MONDAY, False), # We never send on Mondays, only clear
|
||||
# Here are some unrealistic edge cases - just want to make sure we don't blow up with an exception
|
||||
(9, 1, TUESDAY, False),
|
||||
(9, 9, TUESDAY, False),
|
||||
(1, 9, TUESDAY, False),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_will_send_on_right_day(self, days_per_week, days_of_activity, current_day, expect_sent):
|
||||
"""Verify that via the normal conditions, we either send or not based on the days of activity"""
|
||||
goal = self.make_valid_goal(days_per_week=days_per_week)
|
||||
for day in range(days_of_activity):
|
||||
UserActivityFactory(user=goal.user, course_key=goal.course_key, date=datetime(2021, 3, day + 1, tzinfo=UTC))
|
||||
|
||||
self.call_command(day=current_day, expect_sent=expect_sent)
|
||||
|
||||
def test_feature_disabled(self):
|
||||
self.make_valid_goal()
|
||||
with override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=False):
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
def test_never_enrolled(self):
|
||||
self.make_valid_goal()
|
||||
CourseEnrollment.objects.all().delete()
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
def test_inactive_enrollment(self):
|
||||
self.make_valid_goal()
|
||||
CourseEnrollment.objects.update(is_active=False)
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
def test_recent_enrollment(self):
|
||||
self.make_valid_goal()
|
||||
CourseEnrollment.objects.update(created=datetime(2021, 3, 1, tzinfo=UTC))
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
@ddt.data(
|
||||
(datetime(2021, 3, 8, tzinfo=UTC), True),
|
||||
(datetime(2021, 3, 7, tzinfo=UTC), False),
|
||||
)
|
||||
@ddt.unpack
|
||||
@mock.patch('lms.djangoapps.course_goals.management.commands.goal_reminder_email.get_user_course_expiration_date')
|
||||
def test_access_expired(self, expiration_date, expect_sent, mock_get_expiration_date):
|
||||
self.make_valid_goal()
|
||||
mock_get_expiration_date.return_value = expiration_date # awkward to set up, so just mock it
|
||||
self.call_command(expect_sent=expect_sent)
|
||||
|
||||
def test_cert_already_generated(self):
|
||||
goal = self.make_valid_goal()
|
||||
GeneratedCertificateFactory(user=goal.user, course_id=goal.course_key, status=CertificateStatuses.downloadable)
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
def test_unsubscribed(self):
|
||||
self.make_valid_goal(subscribed_to_reminders=False)
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
def test_no_double_sends(self):
|
||||
self.make_valid_goal()
|
||||
self.call_command(expect_sent=True, expect_send_count=1)
|
||||
self.call_command(expect_sent=True, expect_send_count=0)
|
||||
|
||||
def test_no_days_per_week(self):
|
||||
self.make_valid_goal(days_per_week=0)
|
||||
self.call_command(expect_sent=False)
|
||||
|
||||
@ddt.data(
|
||||
datetime(2021, 2, 1, tzinfo=UTC), # very over and done with
|
||||
datetime(2021, 3, 7, tzinfo=UTC), # ending this Sunday
|
||||
)
|
||||
def test_old_course(self, end):
|
||||
self.make_valid_goal(overview__end=end)
|
||||
self.call_command(expect_sent=False)
|
||||
@@ -0,0 +1,30 @@
|
||||
# Generated by Django 2.2.24 on 2021-09-20 16:42
|
||||
|
||||
from django.db import migrations, models
|
||||
import django.db.models.deletion
|
||||
import django.utils.timezone
|
||||
import model_utils.fields
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('course_goals', '0007_set_unsubscribe_token_default'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.CreateModel(
|
||||
name='CourseGoalReminderStatus',
|
||||
fields=[
|
||||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
|
||||
('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')),
|
||||
('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')),
|
||||
('email_reminder_sent', models.BooleanField(default=False, help_text='Tracks if the email reminder to complete the Course Goal has been sent this week.')),
|
||||
('goal', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='reminder_status', to='course_goals.CourseGoal')),
|
||||
],
|
||||
options={
|
||||
'abstract': False,
|
||||
'verbose_name_plural': 'Course goal reminder statuses',
|
||||
},
|
||||
),
|
||||
]
|
||||
@@ -8,6 +8,7 @@ from django.contrib.auth import get_user_model
|
||||
from django.db import models
|
||||
from django.utils.translation import ugettext_lazy as _
|
||||
from model_utils import Choices
|
||||
from model_utils.models import TimeStampedModel
|
||||
from opaque_keys.edx.django.models import CourseKeyField
|
||||
from simple_history.models import HistoricalRecords
|
||||
|
||||
@@ -36,9 +37,6 @@ class CourseGoal(models.Model):
|
||||
course_key = CourseKeyField(max_length=255, db_index=True)
|
||||
# The goal a user has set for the number of days they want to learn per week
|
||||
days_per_week = models.PositiveIntegerField(default=0)
|
||||
# email_reminder_sent = models.BooleanField(
|
||||
# default=False, help_text='Tracks if the email reminder to complete the Course Goal has been sent this week.'
|
||||
# )
|
||||
|
||||
# Controls whether a user will receive emails reminding them to stay on track with their learning goal
|
||||
subscribed_to_reminders = models.BooleanField(default=False)
|
||||
@@ -67,6 +65,21 @@ class CourseGoal(models.Model):
|
||||
super().save(**kwargs)
|
||||
|
||||
|
||||
class CourseGoalReminderStatus(TimeStampedModel):
|
||||
"""
|
||||
Tracks whether we've sent a reminder about a particular goal this week.
|
||||
|
||||
See the management command goal_reminder_email for more detail about how this is used.
|
||||
"""
|
||||
class Meta:
|
||||
verbose_name_plural = "Course goal reminder statuses"
|
||||
|
||||
goal = models.OneToOneField(CourseGoal, on_delete=models.CASCADE, related_name='reminder_status')
|
||||
email_reminder_sent = models.BooleanField(
|
||||
default=False, help_text='Tracks if the email reminder to complete the Course Goal has been sent this week.'
|
||||
)
|
||||
|
||||
|
||||
class UserActivity(models.Model):
|
||||
"""
|
||||
Tracks the date a user performs an activity in a course for goal purposes.
|
||||
|
||||
36
lms/djangoapps/course_goals/tests/factories.py
Normal file
36
lms/djangoapps/course_goals/tests/factories.py
Normal file
@@ -0,0 +1,36 @@
|
||||
"""Provides factories for course goals."""
|
||||
|
||||
import factory
|
||||
from factory.django import DjangoModelFactory
|
||||
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from lms.djangoapps.course_goals.models import CourseGoal, CourseGoalReminderStatus, UserActivity
|
||||
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
|
||||
|
||||
|
||||
class CourseGoalFactory(DjangoModelFactory):
|
||||
"""Factory for CourseGoal, which will make user and course for you"""
|
||||
class Meta:
|
||||
model = CourseGoal
|
||||
|
||||
class Params:
|
||||
overview = factory.SubFactory(CourseOverviewFactory)
|
||||
|
||||
user = factory.SubFactory(UserFactory)
|
||||
course_key = factory.SelfAttribute('overview.id')
|
||||
|
||||
|
||||
class CourseGoalReminderStatusFactory(DjangoModelFactory):
|
||||
"""Factory for CourseGoalReminderStatus"""
|
||||
class Meta:
|
||||
model = CourseGoalReminderStatus
|
||||
|
||||
goal = factory.SubFactory(CourseGoalFactory)
|
||||
|
||||
|
||||
class UserActivityFactory(DjangoModelFactory):
|
||||
"""Factory for UserActivity"""
|
||||
class Meta:
|
||||
model = UserActivity
|
||||
|
||||
user = factory.SubFactory(UserFactory)
|
||||
Reference in New Issue
Block a user