Skip to content

E
edx-platform
Commit 1dc9b119 by Tyler Hallada Committed by GitHub
Options

Merge pull request #16209 from edx/thallada/dont-send-emails-after-course-end 

Don't send RET emails after course end
parents e507dac8 1c8fcf21
Showing with 143 additions and 88 deletions
openedx/core/djangoapps/content/course_overviews/tests/factories.py
import json
import factory
from django.utils.timezone import get_current_timezone
from factory.django import DjangoModelFactory
from ..models import CourseOverview
......@@ -14,7 +15,8 @@ class CourseOverviewFactory(DjangoModelFactory):
version = CourseOverview.VERSION
pre_requisite_courses = []
start = factory.Faker('past_datetime')
start = factory.Faker('past_datetime', tzinfo=get_current_timezone())
end = factory.Faker('future_datetime', tzinfo=get_current_timezone())
org = 'edX'
@factory.lazy_attribute
......
openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py
......@@ -18,6 +18,7 @@ from opaque_keys.edx.locator import CourseLocator
from course_modes.models import CourseMode
from course_modes.tests.factories import CourseModeFactory
from courseware.models import DynamicUpgradeDeadlineConfiguration
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.schedules import resolvers, tasks
from openedx.core.djangoapps.schedules.management.commands import send_recurring_nudge as nudge
from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory
......@@ -61,9 +62,9 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
@patch.object(nudge.Command, 'resolver_class')
def test_handle(self, mock_resolver):
test_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
test_day = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
nudge.Command().handle(date='2017-08-01', site_domain_name=self.site_config.site.domain)
mock_resolver.assert_called_with(self.site_config.site, test_time)
mock_resolver.assert_called_with(self.site_config.site, test_day)
for day in (-3, -10):
mock_resolver().send.assert_any_call(day, None)
......@@ -71,16 +72,16 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
@patch.object(tasks, 'ace')
@patch.object(resolvers.ScheduleStartResolver, 'async_send_task')
def test_resolver_send(self, mock_schedule_bin, mock_ace):
current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
nudge.ScheduleStartResolver(self.site_config.site, current_time).send(-3)
test_time = current_time + datetime.timedelta(days=-3)
current_day = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
nudge.ScheduleStartResolver(self.site_config.site, current_day).send(-3)
test_day = current_day + datetime.timedelta(days=-3)
self.assertFalse(mock_schedule_bin.called)
mock_schedule_bin.apply_async.assert_any_call(
(self.site_config.site.id, serialize(test_time), -3, 0, [], True, None),
(self.site_config.site.id, serialize(test_day), -3, 0, [], True, None),
retry=False,
)
mock_schedule_bin.apply_async.assert_any_call(
(self.site_config.site.id, serialize(test_time), -3, tasks.RECURRING_NUDGE_NUM_BINS - 1, [], True, None),
(self.site_config.site.id, serialize(test_day), -3, tasks.RECURRING_NUDGE_NUM_BINS - 1, [], True, None),
retry=False,
)
self.assertFalse(mock_ace.send.called)
......@@ -98,8 +99,8 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
bins_in_use = frozenset((s.enrollment.user.id % tasks.RECURRING_NUDGE_NUM_BINS) for s in schedules)
test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC)
test_time_str = serialize(test_time)
test_datetime = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC)
test_datetime_str = serialize(test_datetime)
for b in range(tasks.RECURRING_NUDGE_NUM_BINS):
expected_queries = NUM_QUERIES_NO_MATCHING_SCHEDULES
if b in bins_in_use:
......@@ -108,7 +109,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
with self.assertNumQueries(expected_queries, table_blacklist=WAFFLE_TABLES):
tasks.recurring_nudge_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b,
self.site_config.site.id, target_day_str=test_datetime_str, day_offset=-3, bin_num=b,
org_list=[schedules[0].enrollment.course.org],
)
self.assertEqual(mock_schedule_send.apply_async.call_count, schedule_count)
......@@ -123,12 +124,12 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
schedule.enrollment.course_id = CourseKey.from_string('edX/toy/Not_2012_Fall')
schedule.enrollment.save()
test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC)
test_time_str = serialize(test_time)
test_datetime = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC)
test_datetime_str = serialize(test_datetime)
for b in range(tasks.RECURRING_NUDGE_NUM_BINS):
with self.assertNumQueries(NUM_QUERIES_NO_MATCHING_SCHEDULES, table_blacklist=WAFFLE_TABLES):
tasks.recurring_nudge_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b,
self.site_config.site.id, target_day_str=test_datetime_str, day_offset=-3, bin_num=b,
org_list=[schedule.enrollment.course.org],
)
......@@ -140,6 +141,27 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
# is null.
self.assertEqual(mock_schedule_send.apply_async.call_count, 0)
@patch.object(tasks, '_recurring_nudge_schedule_send')
def test_send_after_course_end(self, mock_schedule_send):
user1 = UserFactory.create(id=tasks.RECURRING_NUDGE_NUM_BINS)
schedule = ScheduleFactory.create(
start=datetime.datetime(2017, 8, 3, 20, 34, 30, tzinfo=pytz.UTC),
enrollment__user=user1,
)
schedule.enrollment.course = CourseOverviewFactory()
schedule.enrollment.course.end = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=1)
test_datetime = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC)
test_datetime_str = serialize(test_datetime)
tasks.recurring_nudge_schedule_bin.apply_async(
self.site_config.site.id, target_day_str=test_datetime_str, day_offset=-3, bin_num=0,
org_list=[schedule.enrollment.course.org],
)
self.assertFalse(mock_schedule_send.apply_async.called)
@patch.object(tasks, 'ace')
def test_delivery_disabled(self, mock_ace):
ScheduleConfigFactory.create(site=self.site_config.site, deliver_recurring_nudge=False)
......@@ -153,8 +175,8 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
def test_enqueue_disabled(self, mock_schedule_bin, mock_ace):
ScheduleConfigFactory.create(site=self.site_config.site, enqueue_recurring_nudge=False)
current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
nudge.ScheduleStartResolver(self.site_config.site, current_time).send(3)
current_datetime = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
nudge.ScheduleStartResolver(self.site_config.site, current_datetime).send(3)
self.assertFalse(mock_schedule_bin.called)
self.assertFalse(mock_schedule_bin.apply_async.called)
self.assertFalse(mock_ace.send.called)
......@@ -196,11 +218,11 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
enrollment__user=user2,
)
test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC)
test_time_str = serialize(test_time)
test_datetime = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC)
test_datetime_str = serialize(test_datetime)
with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES):
tasks.recurring_nudge_schedule_bin(
limited_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=0,
limited_config.site.id, target_day_str=test_datetime_str, day_offset=-3, bin_num=0,
org_list=org_list, exclude_orgs=exclude_orgs,
)
......@@ -220,11 +242,11 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
for course_num in (1, 2, 3)
]
test_time = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC)
test_time_str = serialize(test_time)
test_datetime = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC)
test_datetime_str = serialize(test_datetime)
with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES):
tasks.recurring_nudge_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=-3,
self.site_config.site.id, target_day_str=test_datetime_str, day_offset=-3,
bin_num=user.id % tasks.RECURRING_NUDGE_NUM_BINS,
org_list=[schedules[0].enrollment.course.org],
)
......@@ -245,8 +267,8 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
for course_num in range(message_count)
]
test_time = datetime.datetime(2017, 8, 3, 19, tzinfo=pytz.UTC)
test_time_str = serialize(test_time)
test_datetime = datetime.datetime(2017, 8, 3, 19, tzinfo=pytz.UTC)
test_datetime_str = serialize(test_datetime)
patch_policies(self, [StubPolicy([ChannelType.PUSH])])
mock_channel = Mock(
......@@ -263,7 +285,7 @@ class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase):
with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES):
tasks.recurring_nudge_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=day,
self.site_config.site.id, target_day_str=test_datetime_str, day_offset=day,
bin_num=self._calculate_bin_for_user(user), org_list=[schedules[0].enrollment.course.org],
)
......
openedx/core/djangoapps/schedules/tasks.py
......@@ -9,7 +9,7 @@ from django.contrib.sites.models import Site
from django.contrib.staticfiles.templatetags.staticfiles import static
from django.core.exceptions import ValidationError
from django.core.urlresolvers import reverse
from django.db.models import F, Min
from django.db.models import F, Min, Q
from django.db.utils import DatabaseError
from django.utils.formats import dateformat, get_format
import pytz
......@@ -90,12 +90,15 @@ def _recurring_nudge_schedule_send(site_id, msg_str):
def recurring_nudge_schedule_bin(
site_id, target_day_str, day_offset, bin_num, org_list, exclude_orgs=False, override_recipient_email=None,
):
target_day = deserialize(target_day_str)
target_datetime = deserialize(target_day_str)
# TODO: in the next refactor of this task, pass in current_datetime instead of reproducing it here
current_datetime = target_datetime - datetime.timedelta(days=day_offset)
msg_type = RecurringNudge(abs(day_offset))
for (user, language, context) in _recurring_nudge_schedules_for_bin(
Site.objects.get(id=site_id),
target_day,
current_datetime,
target_datetime,
bin_num,
org_list,
exclude_orgs
......@@ -111,11 +114,11 @@ def recurring_nudge_schedule_bin(
_recurring_nudge_schedule_send.apply_async((site_id, str(msg)), retry=False)
def _recurring_nudge_schedules_for_bin(site, target_day, bin_num, org_list, exclude_orgs=False):
beginning_of_day = target_day.replace(hour=0, minute=0, second=0)
def _recurring_nudge_schedules_for_bin(site, current_datetime, target_datetime, bin_num, org_list, exclude_orgs=False):
schedules = get_schedules_with_target_date_by_bin_and_orgs(
schedule_date_field='start',
target_date=beginning_of_day,
current_datetime=current_datetime,
target_datetime=target_datetime,
bin_num=bin_num,
num_bins=RECURRING_NUDGE_NUM_BINS,
org_list=org_list,
......@@ -154,12 +157,15 @@ class UpgradeReminder(ScheduleMessageType):
def upgrade_reminder_schedule_bin(
site_id, target_day_str, day_offset, bin_num, org_list, exclude_orgs=False, override_recipient_email=None,
):
target_day = deserialize(target_day_str)
target_datetime = deserialize(target_day_str)
# TODO: in the next refactor of this task, pass in current_datetime instead of reproducing it here
current_datetime = target_datetime - datetime.timedelta(days=day_offset)
msg_type = UpgradeReminder()
for (user, language, context) in _upgrade_reminder_schedules_for_bin(
Site.objects.get(id=site_id),
target_day,
current_datetime,
target_datetime,
bin_num,
org_list,
exclude_orgs
......@@ -185,12 +191,11 @@ def _upgrade_reminder_schedule_send(site_id, msg_str):
ace.send(msg)
def _upgrade_reminder_schedules_for_bin(site, target_day, bin_num, org_list, exclude_orgs=False):
beginning_of_day = target_day.replace(hour=0, minute=0, second=0)
def _upgrade_reminder_schedules_for_bin(site, current_datetime, target_datetime, bin_num, org_list, exclude_orgs=False):
schedules = get_schedules_with_target_date_by_bin_and_orgs(
schedule_date_field='upgrade_deadline',
target_date=beginning_of_day,
current_datetime=current_datetime,
target_datetime=target_datetime,
bin_num=bin_num,
num_bins=RECURRING_NUDGE_NUM_BINS,
org_list=org_list,
......@@ -235,12 +240,15 @@ class CourseUpdate(ScheduleMessageType):
def course_update_schedule_bin(
site_id, target_day_str, day_offset, bin_num, org_list, exclude_orgs=False, override_recipient_email=None,
):
target_day = deserialize(target_day_str)
target_datetime = deserialize(target_day_str)
# TODO: in the next refactor of this task, pass in current_datetime instead of reproducing it here
current_datetime = target_datetime - datetime.timedelta(days=day_offset)
msg_type = CourseUpdate()
for (user, language, context) in _course_update_schedules_for_bin(
Site.objects.get(id=site_id),
target_day,
current_datetime,
target_datetime,
day_offset,
bin_num,
org_list,
......@@ -267,12 +275,13 @@ def _course_update_schedule_send(site_id, msg_str):
ace.send(msg)
def _course_update_schedules_for_bin(site, target_day, day_offset, bin_num, org_list, exclude_orgs=False):
def _course_update_schedules_for_bin(site, current_datetime, target_datetime, day_offset, bin_num, org_list,
exclude_orgs=False):
week_num = abs(day_offset) / 7
beginning_of_day = target_day.replace(hour=0, minute=0, second=0)
schedules = get_schedules_with_target_date_by_bin_and_orgs(
schedule_date_field='start',
target_date=beginning_of_day,
current_datetime=current_datetime,
target_datetime=target_datetime,
bin_num=bin_num,
num_bins=COURSE_UPDATE_NUM_BINS,
org_list=org_list,
......@@ -317,37 +326,40 @@ def get_course_week_summary(course_id, week_num):
raise CourseUpdateDoesNotExist()
def get_schedules_with_target_date_by_bin_and_orgs(schedule_date_field, target_date, bin_num, num_bins=DEFAULT_NUM_BINS,
org_list=None, exclude_orgs=False, order_by='enrollment__user__id'):
def get_schedules_with_target_date_by_bin_and_orgs(schedule_date_field, current_datetime, target_datetime, bin_num,
num_bins=DEFAULT_NUM_BINS, org_list=None, exclude_orgs=False,
order_by='enrollment__user__id'):
"""
Returns Schedules with the target_date, related to Users whose id matches the bin_num, and filtered by org_list.
Arguments:
schedule_date_field -- string field name to query on the User's Schedule model
target_date -- datetime day (with zeroed-out time) that the User's Schedule's schedule_date_field value should fall
under
current_datetime -- datetime that will be used as "right now" in the query
target_datetime -- datetime that the User's Schedule's schedule_date_field value should fall under
bin_num -- int for selecting the bin of Users whose id % num_bins == bin_num
num_bin -- int specifying the number of bins to separate the Users into (default: DEFAULT_NUM_BINS)
org_list -- list of course_org names (strings) that the returned Schedules must or must not be in (default: None)
exclude_orgs -- boolean indicating whether the returned Schedules should exclude (True) the course_orgs in org_list
or strictly include (False) them (default: False)
order_by -- string for field to sort the resulting Schedules by
"""
schedule_date_equals_target_date_filter = {
'courseenrollment__schedule__{}__gte'.format(schedule_date_field): target_date,
'courseenrollment__schedule__{}__lt'.format(schedule_date_field): target_date + datetime.timedelta(days=1),
target_day = _get_datetime_beginning_of_day(target_datetime)
schedule_day_equals_target_day_filter = {
'courseenrollment__schedule__{}__gte'.format(schedule_date_field): target_day,
'courseenrollment__schedule__{}__lt'.format(schedule_date_field): target_day + datetime.timedelta(days=1),
}
users = User.objects.filter(
courseenrollment__is_active=True,
**schedule_date_equals_target_date_filter
**schedule_day_equals_target_day_filter
).annotate(
id_mod=F('id') % num_bins
).filter(
id_mod=bin_num
)
schedule_date_equals_target_date_filter = {
'{}__gte'.format(schedule_date_field): target_date,
'{}__lt'.format(schedule_date_field): target_date + datetime.timedelta(days=1),
schedule_day_equals_target_day_filter = {
'{}__gte'.format(schedule_date_field): target_day,
'{}__lt'.format(schedule_date_field): target_day + datetime.timedelta(days=1),
}
schedules = Schedule.objects.select_related(
'enrollment__user__profile',
......@@ -355,9 +367,10 @@ def get_schedules_with_target_date_by_bin_and_orgs(schedule_date_field, target_d
).prefetch_related(
'enrollment__course__modes'
).filter(
Q(enrollment__course__end__isnull=True) | Q(enrollment__course__end__gte=current_datetime),
enrollment__user__in=users,
enrollment__is_active=True,
**schedule_date_equals_target_date_filter
**schedule_day_equals_target_day_filter
).order_by(order_by)
if org_list is not None:
......@@ -399,3 +412,10 @@ def _get_link_to_purchase_verified_certificate(a_user, a_schedule):
return None
return verified_upgrade_deadline_link(a_user, enrollment.course)
def _get_datetime_beginning_of_day(dt):
"""
Truncates hours, minutes, seconds, and microseconds to zero on given datetime.
"""
return dt.replace(hour=0, minute=0, second=0, microsecond=0)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment