From e5c8acb60984e0b9c88915d423b3dc8f4e3d8d9f Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Fri, 20 Oct 2017 12:07:31 -0400 Subject: [PATCH 1/4] Allow communication experiences to be customized per-learner --- openedx/core/djangoapps/schedules/admin.py | 5 +++++ .../migrations/0006_scheduleexperience.py | 22 +++++++++++++++++++ openedx/core/djangoapps/schedules/models.py | 18 +++++++++++++++ .../core/djangoapps/schedules/resolvers.py | 11 ++++++++-- 4 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py diff --git a/openedx/core/djangoapps/schedules/admin.py b/openedx/core/djangoapps/schedules/admin.py index d1485e519e..e6cb1bfc12 100644 --- a/openedx/core/djangoapps/schedules/admin.py +++ b/openedx/core/djangoapps/schedules/admin.py @@ -4,12 +4,17 @@ from django.utils.translation import ugettext_lazy as _ from . import models +class ScheduleExperienceAdminInline(admin.StackedInline): + model = models.ScheduleExperience + + @admin.register(models.Schedule) class ScheduleAdmin(admin.ModelAdmin): list_display = ('username', 'course_id', 'active', 'start', 'upgrade_deadline') raw_id_fields = ('enrollment',) readonly_fields = ('modified',) search_fields = ('enrollment__user__username', 'enrollment__course_id',) + inlines = (ScheduleExperienceAdminInline,) def username(self, obj): return obj.enrollment.user.username diff --git a/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py b/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py new file mode 100644 index 0000000000..4ffca66d46 --- /dev/null +++ b/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('schedules', '0005_auto_20171010_1722'), + ] + + operations = [ + migrations.CreateModel( + name='ScheduleExperience', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('experience_type', models.IntegerField(default=0, choices=[(0, b'Recurring Nudge and Upgrade Reminder'), (1, b'Course Updates')])), + ('schedule', models.OneToOneField(related_name='experience', to='schedules.Schedule')), + ], + ), + ] diff --git a/openedx/core/djangoapps/schedules/models.py b/openedx/core/djangoapps/schedules/models.py index 7248e98754..eff9fabd2d 100644 --- a/openedx/core/djangoapps/schedules/models.py +++ b/openedx/core/djangoapps/schedules/models.py @@ -6,6 +6,13 @@ from model_utils.models import TimeStampedModel from config_models.models import ConfigurationModel +EXPERIENCE_TYPES = ( + (0, 'Recurring Nudge and Upgrade Reminder'), + (1, 'Course Updates'), +) +DEFAULT_EXPERIENCE_TYPE = EXPERIENCE_TYPES[0][0] + + class Schedule(TimeStampedModel): enrollment = models.OneToOneField('student.CourseEnrollment', null=False) active = models.BooleanField( @@ -23,6 +30,12 @@ class Schedule(TimeStampedModel): help_text=_('Deadline by which the learner must upgrade to a verified seat') ) + def get_experience_type(self): + if (hasattr(self, 'experience')): + return self.experience.experience_type + else: + return DEFAULT_EXPERIENCE_TYPE + class Meta(object): verbose_name = _('Schedule') verbose_name_plural = _('Schedules') @@ -39,3 +52,8 @@ class ScheduleConfig(ConfigurationModel): deliver_upgrade_reminder = models.BooleanField(default=False) enqueue_course_update = models.BooleanField(default=False) deliver_course_update = models.BooleanField(default=False) + + +class ScheduleExperience(models.Model): + schedule = models.OneToOneField(Schedule, related_name='experience') + experience_type = models.IntegerField(choices=EXPERIENCE_TYPES, default=DEFAULT_EXPERIENCE_TYPE) diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index d2da8f31da..4770ef32ec 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -18,7 +18,7 @@ from courseware.date_summary import verified_upgrade_deadline_link, verified_upg from openedx.core.djangoapps.monitoring_utils import function_trace, set_custom_metric from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist -from openedx.core.djangoapps.schedules.models import Schedule +from openedx.core.djangoapps.schedules.models import DEFAULT_EXPERIENCE_TYPE, EXPERIENCE_TYPES, Schedule from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin from openedx.core.djangoapps.schedules.template_context import ( absolute_url, @@ -64,6 +64,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): relative to. For example, if this resolver finds schedules that started 7 days ago this variable should be set to "start". num_bins -- the int number of bins to split the users into + experience_type -- the string name for the experience type that users will be filtered to """ async_send_task = attr.ib() site = attr.ib() @@ -74,6 +75,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): schedule_date_field = None num_bins = DEFAULT_NUM_BINS + experience_type = DEFAULT_EXPERIENCE_TYPE def __attrs_post_init__(self): # TODO: in the next refactor of this task, pass in current_datetime instead of reproducing it here @@ -123,10 +125,14 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): 'enrollment__user__profile', 'enrollment__course', ).prefetch_related( - 'enrollment__course__modes' + 'enrollment__course__modes', + 'experience', ).filter( Q(enrollment__course__end__isnull=True) | Q( enrollment__course__end__gte=self.current_datetime), + Q(experience__isnull=True) | Q(experience__experience_type=self.experience_type) + if self.experience_type == DEFAULT_EXPERIENCE_TYPE else + Q(experience__isnull=False) & Q(experience__experience_type=self.experience_type), enrollment__user__in=users, enrollment__is_active=True, **schedule_day_equals_target_day_filter @@ -333,6 +339,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): log_prefix = 'Course Update' schedule_date_field = 'start' num_bins = COURSE_UPDATE_NUM_BINS + experience_type = EXPERIENCE_TYPES[1][0] def schedules_for_bin(self): week_num = abs(self.day_offset) / 7 From 95d1e5c25e254101ccac2ff6d7a1dad5b2e3e435 Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Fri, 27 Oct 2017 08:41:31 -0400 Subject: [PATCH 2/4] use a left outer join for experience types --- .../commands/tests/send_email_base.py | 4 +- .../commands/tests/test_experiences.py | 59 +++++++++++++++++++ .../migrations/0006_scheduleexperience.py | 2 +- openedx/core/djangoapps/schedules/models.py | 22 +++---- .../core/djangoapps/schedules/resolvers.py | 23 +++++--- .../djangoapps/schedules/tests/factories.py | 8 +++ 6 files changed, 96 insertions(+), 22 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index db773e482d..a7eee64467 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -19,8 +19,8 @@ from openedx.core.djangoapps.schedules import resolvers, tasks from openedx.core.djangoapps.schedules.resolvers import _get_datetime_beginning_of_day from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES +from openedx.core.djangolib.testing.utils import FilteredQueryCountMixin, CacheIsolationTestCase from student.tests.factories import UserFactory -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase SITE_QUERY = 2 # django_site, site_configuration_siteconfiguration @@ -56,7 +56,7 @@ LOG = logging.getLogger(__name__) @ddt.ddt @freeze_time('2017-08-01 00:00:00', tz_offset=0, tick=True) -class ScheduleSendEmailTestBase(SharedModuleStoreTestCase): +class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase): __test__ = False diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py new file mode 100644 index 0000000000..d026bf24dc --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py @@ -0,0 +1,59 @@ +from collections import namedtuple + +import datetime +import ddt +import pytz +from edx_ace.utils.date import serialize +from freezegun import freeze_time +from mock import patch + +from courseware.models import DynamicUpgradeDeadlineConfiguration +from openedx.core.djangoapps.schedules import tasks +from openedx.core.djangoapps.schedules.models import ScheduleExperience +from openedx.core.djangoapps.schedules.resolvers import _get_datetime_beginning_of_day +from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory, ScheduleConfigFactory +from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory, SiteConfigurationFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms, FilteredQueryCountMixin, CacheIsolationTestCase + + +@ddt.ddt +@skip_unless_lms +@freeze_time('2017-08-01 00:00:00', tz_offset=0, tick=True) +class TestExperiences(FilteredQueryCountMixin, CacheIsolationTestCase): + + ENABLED_CACHES = ['default'] + + ExperienceTest = namedtuple('ExperienceTest', 'experience offset email_sent') + + def setUp(self): + super(TestExperiences, self).setUp() + + site = SiteFactory.create() + self.site_config = SiteConfigurationFactory.create(site=site) + ScheduleConfigFactory.create(site=self.site_config.site) + + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + + @ddt.data( + ExperienceTest(experience=ScheduleExperience.DEFAULT, offset=-3, email_sent=True), + ExperienceTest(experience=ScheduleExperience.DEFAULT, offset=-10, email_sent=True), + ExperienceTest(experience=ScheduleExperience.COURSE_UPDATES, offset=-3, email_sent=True), + ExperienceTest(experience=ScheduleExperience.COURSE_UPDATES, offset=-10, email_sent=False), + ) + @patch.object(tasks, 'ace') + def test_experience_type_exclusion(self, test_config, mock_ace): + current_day = _get_datetime_beginning_of_day(datetime.datetime.now(pytz.UTC)) + target_day = current_day + datetime.timedelta(days=test_config.offset) + + schedule = ScheduleFactory.create( + start=target_day, + enrollment__course__self_paced=True, + experience__experience_type=test_config.experience, + ) + + tasks.ScheduleRecurringNudge.apply(kwargs=dict( + site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=test_config.offset, + bin_num=(schedule.enrollment.user.id % tasks.ScheduleRecurringNudge.num_bins), + )) + + self.assertEqual(mock_ace.send.called, test_config.email_sent) diff --git a/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py b/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py index 4ffca66d46..df0b41412b 100644 --- a/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py +++ b/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py @@ -15,7 +15,7 @@ class Migration(migrations.Migration): name='ScheduleExperience', fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('experience_type', models.IntegerField(default=0, choices=[(0, b'Recurring Nudge and Upgrade Reminder'), (1, b'Course Updates')])), + ('experience_type', models.PositiveSmallIntegerField(default=0, choices=[(0, b'Recurring Nudge and Upgrade Reminder'), (1, b'Course Updates')])), ('schedule', models.OneToOneField(related_name='experience', to='schedules.Schedule')), ], ), diff --git a/openedx/core/djangoapps/schedules/models.py b/openedx/core/djangoapps/schedules/models.py index eff9fabd2d..adf6cbd454 100644 --- a/openedx/core/djangoapps/schedules/models.py +++ b/openedx/core/djangoapps/schedules/models.py @@ -6,13 +6,6 @@ from model_utils.models import TimeStampedModel from config_models.models import ConfigurationModel -EXPERIENCE_TYPES = ( - (0, 'Recurring Nudge and Upgrade Reminder'), - (1, 'Course Updates'), -) -DEFAULT_EXPERIENCE_TYPE = EXPERIENCE_TYPES[0][0] - - class Schedule(TimeStampedModel): enrollment = models.OneToOneField('student.CourseEnrollment', null=False) active = models.BooleanField( @@ -31,10 +24,10 @@ class Schedule(TimeStampedModel): ) def get_experience_type(self): - if (hasattr(self, 'experience')): + try: return self.experience.experience_type - else: - return DEFAULT_EXPERIENCE_TYPE + except ScheduleExperience.DoesNotExist: + return ScheduleExperience.DEFAULT class Meta(object): verbose_name = _('Schedule') @@ -55,5 +48,12 @@ class ScheduleConfig(ConfigurationModel): class ScheduleExperience(models.Model): + DEFAULT = 0 + COURSE_UPDATES = 1 + EXPERIENCES = ( + (DEFAULT, 'Recurring Nudge and Upgrade Reminder'), + (COURSE_UPDATES, 'Course Updates') + ) + schedule = models.OneToOneField(Schedule, related_name='experience') - experience_type = models.IntegerField(choices=EXPERIENCE_TYPES, default=DEFAULT_EXPERIENCE_TYPE) + experience_type = models.PositiveSmallIntegerField(choices=EXPERIENCES, default=DEFAULT) diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 4770ef32ec..05ba2b4b6b 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -18,7 +18,7 @@ from courseware.date_summary import verified_upgrade_deadline_link, verified_upg from openedx.core.djangoapps.monitoring_utils import function_trace, set_custom_metric from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist -from openedx.core.djangoapps.schedules.models import DEFAULT_EXPERIENCE_TYPE, EXPERIENCE_TYPES, Schedule +from openedx.core.djangoapps.schedules.models import Schedule, ScheduleExperience from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin from openedx.core.djangoapps.schedules.template_context import ( absolute_url, @@ -64,7 +64,9 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): relative to. For example, if this resolver finds schedules that started 7 days ago this variable should be set to "start". num_bins -- the int number of bins to split the users into - experience_type -- the string name for the experience type that users will be filtered to + experience_filter -- a queryset filter used to select only the users who should be getting this message as part + of their experience. This defaults to users without a specified experience type and those + in the "recurring nudges and upgrade reminder" experience. """ async_send_task = attr.ib() site = attr.ib() @@ -75,7 +77,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): schedule_date_field = None num_bins = DEFAULT_NUM_BINS - experience_type = DEFAULT_EXPERIENCE_TYPE + experience_filter = Q(experience__experience_type=ScheduleExperience.DEFAULT) | Q(experience__isnull=True) def __attrs_post_init__(self): # TODO: in the next refactor of this task, pass in current_datetime instead of reproducing it here @@ -126,13 +128,10 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): 'enrollment__course', ).prefetch_related( 'enrollment__course__modes', - 'experience', ).filter( Q(enrollment__course__end__isnull=True) | Q( enrollment__course__end__gte=self.current_datetime), - Q(experience__isnull=True) | Q(experience__experience_type=self.experience_type) - if self.experience_type == DEFAULT_EXPERIENCE_TYPE else - Q(experience__isnull=False) & Q(experience__experience_type=self.experience_type), + self.experience_filter, enrollment__user__in=users, enrollment__is_active=True, **schedule_day_equals_target_day_filter @@ -238,6 +237,14 @@ class RecurringNudgeResolver(BinnedSchedulesBaseResolver): schedule_date_field = 'start' num_bins = RECURRING_NUDGE_NUM_BINS + @property + def experience_filter(self): + if self.day_offset == -3: + experiences = [ScheduleExperience.DEFAULT, ScheduleExperience.COURSE_UPDATES] + return Q(experience__experience_type__in=experiences) | Q(experience__isnull=True) + else: + return Q(experience__experience_type=ScheduleExperience.DEFAULT) | Q(experience__isnull=True) + def get_template_context(self, user, user_schedules): first_schedule = user_schedules[0] context = { @@ -339,7 +346,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): log_prefix = 'Course Update' schedule_date_field = 'start' num_bins = COURSE_UPDATE_NUM_BINS - experience_type = EXPERIENCE_TYPES[1][0] + experience_filter = Q(experience__experience_type=ScheduleExperience.COURSE_UPDATES) def schedules_for_bin(self): week_num = abs(self.day_offset) / 7 diff --git a/openedx/core/djangoapps/schedules/tests/factories.py b/openedx/core/djangoapps/schedules/tests/factories.py index affc67cc3b..af6a5875c9 100644 --- a/openedx/core/djangoapps/schedules/tests/factories.py +++ b/openedx/core/djangoapps/schedules/tests/factories.py @@ -6,6 +6,13 @@ from student.tests.factories import CourseEnrollmentFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory +class ScheduleExperienceFactory(factory.DjangoModelFactory): + class Meta(object): + model = models.ScheduleExperience + + experience_type = models.ScheduleExperience.DEFAULT + + class ScheduleFactory(factory.DjangoModelFactory): class Meta(object): model = models.Schedule @@ -13,6 +20,7 @@ class ScheduleFactory(factory.DjangoModelFactory): start = factory.Faker('future_datetime', tzinfo=pytz.UTC) upgrade_deadline = factory.Faker('future_datetime', tzinfo=pytz.UTC) enrollment = factory.SubFactory(CourseEnrollmentFactory) + experience = factory.RelatedFactory(ScheduleExperienceFactory, 'schedule') class ScheduleConfigFactory(factory.DjangoModelFactory): From 57bf89c8e222ca9088e31d59ff9f757e9932991b Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Mon, 30 Oct 2017 13:57:38 -0400 Subject: [PATCH 3/4] Create schedule experience on schedule creation --- openedx/core/djangoapps/schedules/signals.py | 17 ++++++++++++++--- .../djangoapps/schedules/tests/test_signals.py | 12 +++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index 1af925f22e..45b5927c7f 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -12,6 +12,9 @@ from courseware.models import ( OrgDynamicUpgradeDeadlineConfiguration ) from edx_ace.utils import date +from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist +from openedx.core.djangoapps.schedules.models import ScheduleExperience +from openedx.core.djangoapps.schedules.resolvers import get_week_highlights from openedx.core.djangoapps.signals.signals import COURSE_START_DATE_CHANGED from openedx.core.djangoapps.theming.helpers import get_current_site from student.models import CourseEnrollment @@ -53,14 +56,22 @@ def create_schedule(sender, **kwargs): upgrade_deadline = _calculate_upgrade_deadline(enrollment.course_id, content_availability_date) - Schedule.objects.create( + schedule = Schedule.objects.create( enrollment=enrollment, start=content_availability_date, upgrade_deadline=upgrade_deadline ) - log.debug('Schedules: created a new schedule starting at %s with an upgrade deadline of %s', - content_availability_date, upgrade_deadline) + try: + get_week_highlights(enrollment.course_id, 1) + experience_type = ScheduleExperience.COURSE_UPDATES + except CourseUpdateDoesNotExist: + experience_type = ScheduleExperience.DEFAULT + + ScheduleExperience(schedule=schedule, experience_type=experience_type).save() + + log.debug('Schedules: created a new schedule starting at %s with an upgrade deadline of %s and experience type: %s', + content_availability_date, upgrade_deadline, ScheduleExperience.EXPERIENCES[experience_type][1]) @receiver(COURSE_START_DATE_CHANGED, dispatch_uid="update_schedules_on_course_start_changed") diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index 1bf0048c5e..98b6f48f67 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -6,6 +6,7 @@ from pytz import utc from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory from courseware.models import DynamicUpgradeDeadlineConfiguration +from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangoapps.schedules.signals import CREATE_SCHEDULE_WAFFLE_FLAG from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag @@ -23,11 +24,12 @@ from ..tests.factories import ScheduleConfigFactory @skip_unless_lms class CreateScheduleTests(SharedModuleStoreTestCase): - def assert_schedule_created(self): + def assert_schedule_created(self, experience_type=ScheduleExperience.DEFAULT): course = _create_course_run(self_paced=True) enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) self.assertIsNotNone(enrollment.schedule) self.assertIsNone(enrollment.schedule.upgrade_deadline) + self.assertEquals(enrollment.schedule.experience.experience_type, experience_type) def assert_schedule_not_created(self): course = _create_course_run(self_paced=True) @@ -78,6 +80,14 @@ class CreateScheduleTests(SharedModuleStoreTestCase): with self.assertRaises(Schedule.DoesNotExist): enrollment.schedule + @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) + @patch('openedx.core.djangoapps.schedules.signals.get_week_highlights') + def test_create_schedule_course_updates_experience(self, mock_get_week_highlights, mock_get_current_site): + site = SiteFactory.create() + mock_get_week_highlights.return_value = True + mock_get_current_site.return_value = site + self.assert_schedule_created(experience_type=ScheduleExperience.COURSE_UPDATES) + @ddt.ddt @skip_unless_lms From 7fd643faa40932dd114640e885bf8f9cc5d2d27d Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Mon, 30 Oct 2017 16:16:46 -0400 Subject: [PATCH 4/4] Add tests for experience types, ensure courses have a verified mode --- common/djangoapps/student/models.py | 24 +-- common/djangoapps/student/tests/factories.py | 32 +++- .../student/tests/test_certificates.py | 2 + .../certificates/tests/test_webview_views.py | 6 +- lms/djangoapps/courseware/date_summary.py | 15 +- .../tests/test_view_authentication.py | 1 + lms/djangoapps/courseware/tests/test_views.py | 7 +- lms/djangoapps/discussion/tests/test_views.py | 8 +- .../commands/tests/send_email_base.py | 140 +++++++++++------- .../commands/tests/test_experiences.py | 59 -------- .../commands/tests/test_send_course_update.py | 19 ++- .../tests/test_send_recurring_nudge.py | 17 ++- .../tests/test_send_upgrade_reminder.py | 37 +++-- .../management/commands/tests/upsell_base.py | 7 +- openedx/core/djangoapps/schedules/models.py | 13 +- .../core/djangoapps/schedules/resolvers.py | 13 +- openedx/core/djangoapps/schedules/signals.py | 6 +- .../djangoapps/schedules/tests/factories.py | 2 +- .../schedules/tests/test_signals.py | 19 ++- 19 files changed, 246 insertions(+), 181 deletions(-) delete mode 100644 openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index ab269f49b8..2ef11c0dbc 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1686,9 +1686,13 @@ class CourseEnrollment(models.Model): """ if not self._course_overview: try: - self._course_overview = CourseOverview.get_from_id(self.course_id) - except (CourseOverview.DoesNotExist, IOError): - self._course_overview = None + self._course_overview = self.course + except CourseOverview.DoesNotExist: + log.info('Course Overviews: unable to find course overview for enrollment, loading from modulestore.') + try: + self._course_overview = CourseOverview.get_from_id(self.course_id) + except (CourseOverview.DoesNotExist, IOError): + self._course_overview = None return self._course_overview @cached_property @@ -1717,7 +1721,14 @@ class CourseEnrollment(models.Model): # When course modes expire they aren't found any more and None would be returned. # Replicate that behavior here by returning None if the personalized deadline is in the past. if datetime.now(UTC) >= self.dynamic_upgrade_deadline: + log.debug('Schedules: Returning None since dynamic upgrade deadline has already passed.') return None + + if self.verified_mode is None: + log.debug('Schedules: Returning None for dynamic upgrade deadline since the course does not have a ' + 'verified mode.') + return None + return self.dynamic_upgrade_deadline return self.course_upgrade_deadline @@ -1733,12 +1744,7 @@ class CourseEnrollment(models.Model): Returns: datetime|None """ - try: - course_overview = self.course - except CourseOverview.DoesNotExist: - course_overview = self.course_overview - - if not course_overview.self_paced: + if not self.course_overview.self_paced: return None if not DynamicUpgradeDeadlineConfiguration.is_enabled(): diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index 7099da2976..4dd23216b9 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -12,6 +12,8 @@ from opaque_keys.edx.keys import CourseKey from pytz import UTC from course_modes.models import CourseMode +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from student.models import ( CourseAccessRole, CourseEnrollment, @@ -126,9 +128,33 @@ class CourseEnrollmentFactory(DjangoModelFactory): model = CourseEnrollment user = factory.SubFactory(UserFactory) - course = factory.SubFactory( - 'openedx.core.djangoapps.content.course_overviews.tests.factories.CourseOverviewFactory', - ) + + @classmethod + def _create(cls, model_class, *args, **kwargs): + manager = cls._get_manager(model_class) + course_kwargs = {} + for key in kwargs.keys(): + if key.startswith('course__'): + course_kwargs[key.split('__')[1]] = kwargs.pop(key) + + if 'course' not in kwargs: + course_id = kwargs.get('course_id') + course_overview = None + if course_id is not None: + if isinstance(course_id, basestring): + course_id = CourseKey.from_string(course_id) + course_kwargs.setdefault('id', course_id) + + try: + course_overview = CourseOverview.get_from_id(course_id) + except CourseOverview.DoesNotExist: + pass + + if course_overview is None: + course_overview = CourseOverviewFactory(**course_kwargs) + kwargs['course'] = course_overview + + return manager.create(*args, **kwargs) class CourseAccessRoleFactory(DjangoModelFactory): diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index 3906f0b356..47749dea3e 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -111,6 +111,8 @@ class CertificateDashboardMessageDisplayTest(CertificateDisplayTestBase): Tests the certificates messages for a course in the dashboard. """ + ENABLED_SIGNALS = ['course_published'] + @classmethod def setUpClass(cls): super(CertificateDashboardMessageDisplayTest, cls).setUpClass() diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 899af116ac..59fdd905ae 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -12,8 +12,9 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.test.client import Client, RequestFactory from django.test.utils import override_settings + from util.date_utils import strftime_localized -from mock import Mock, patch +from mock import patch from nose.plugins.attrib import attr from certificates.api import get_certificate_url @@ -73,6 +74,9 @@ class CommonCertificatesTestCase(ModuleStoreTestCase): """ Common setUp and utility methods for Certificate tests """ + + ENABLED_SIGNALS = ['course_published'] + def setUp(self): super(CommonCertificatesTestCase, self).setUp() self.client = Client() diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index 1bf76a47ba..0a541754f5 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -405,16 +405,19 @@ def verified_upgrade_deadline_link(user, course=None, course_id=None): ecommerce_service = EcommerceService() if ecommerce_service.is_enabled(user): - if course is not None and isinstance(course, CourseOverview): - course_mode = course.modes.get(mode_slug=CourseMode.VERIFIED) + course_mode = CourseMode.verified_mode_for_course(course_id) + if course_mode is not None: + return ecommerce_service.get_checkout_page_url(course_mode.sku) else: - course_mode = CourseMode.objects.get( - course_id=course_id, mode_slug=CourseMode.VERIFIED - ) - return ecommerce_service.get_checkout_page_url(course_mode.sku) + raise CourseModeNotFoundException('Cannot generate a verified upgrade link without a valid verified mode' + ' for course {}'.format(unicode(course_id))) return reverse('verify_student_upgrade_and_verify', args=(course_id,)) +class CourseModeNotFoundException(Exception): + pass + + def verified_upgrade_link_is_valid(enrollment=None): """ Return whether this enrollment can be upgraded. diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py index 5d8195ad53..1c2cc27469 100644 --- a/lms/djangoapps/courseware/tests/test_view_authentication.py +++ b/lms/djangoapps/courseware/tests/test_view_authentication.py @@ -29,6 +29,7 @@ class TestViewAuth(EnterpriseTestConsentRequired, ModuleStoreTestCase, LoginEnro """ ACCOUNT_INFO = [('view@test.com', 'foo'), ('view2@test.com', 'foo')] + ENABLED_SIGNALS = ['course_published'] @staticmethod def _reverse_urls(names, course): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index c115db93b9..341ebc3d23 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -807,7 +807,12 @@ class ViewsTestCase(ModuleStoreTestCase): CourseModeFactory.create(mode_slug=CourseMode.VERIFIED, course_id=course) # Enroll user in the course - enrollment = CourseEnrollmentFactory(course_id=course, user=self.user, mode=CourseMode.AUDIT) + # Don't use the CourseEnrollmentFactory since it ensures a CourseOverview is available + enrollment = CourseEnrollment.objects.create( + course_id=course, + user=self.user, + mode=CourseMode.AUDIT, + ) self.assertEqual(enrollment.course_overview, None) diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 65faab5325..c2b113e2b9 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -403,15 +403,15 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): # course is outside the context manager that is verifying the number of queries, # and with split mongo, that method ends up querying disabled_xblocks (which is then # cached and hence not queried as part of call_single_thread). - (ModuleStoreEnum.Type.mongo, False, 1, 6, 4, 17, 4), - (ModuleStoreEnum.Type.mongo, False, 50, 6, 4, 17, 4), + (ModuleStoreEnum.Type.mongo, False, 1, 6, 4, 16, 4), + (ModuleStoreEnum.Type.mongo, False, 50, 6, 4, 16, 4), # split mongo: 3 queries, regardless of thread response size. (ModuleStoreEnum.Type.split, False, 1, 3, 3, 16, 4), (ModuleStoreEnum.Type.split, False, 50, 3, 3, 16, 4), # Enabling Enterprise integration should have no effect on the number of mongo queries made. - (ModuleStoreEnum.Type.mongo, True, 1, 6, 4, 17, 4), - (ModuleStoreEnum.Type.mongo, True, 50, 6, 4, 17, 4), + (ModuleStoreEnum.Type.mongo, True, 1, 6, 4, 16, 4), + (ModuleStoreEnum.Type.mongo, True, 50, 6, 4, 16, 4), # split mongo: 3 queries, regardless of thread response size. (ModuleStoreEnum.Type.split, True, 1, 3, 3, 16, 4), (ModuleStoreEnum.Type.split, True, 50, 3, 3, 16, 4), diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index a7eee64467..b98f65c7d6 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -1,3 +1,4 @@ +from collections import namedtuple, defaultdict from copy import deepcopy import datetime import ddt @@ -9,6 +10,9 @@ from freezegun import freeze_time from mock import Mock, patch import pytz +from commerce.models import CommerceConfiguration +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory from courseware.models import DynamicUpgradeDeadlineConfiguration from edx_ace.channel import ChannelType from edx_ace.utils.date import serialize @@ -20,10 +24,12 @@ from openedx.core.djangoapps.schedules.resolvers import _get_datetime_beginning_ from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.core.djangolib.testing.utils import FilteredQueryCountMixin, CacheIsolationTestCase +from student.models import CourseEnrollment from student.tests.factories import UserFactory -SITE_QUERY = 2 # django_site, site_configuration_siteconfiguration +SITE_QUERY = 1 # django_site +SITE_CONFIG_QUERY = 1 # site_configuration_siteconfiguration SCHEDULES_QUERY = 1 # schedules_schedule COURSE_MODES_QUERY = 1 # course_modes_coursemode @@ -33,18 +39,14 @@ ORG_DEADLINE_QUERY = 1 # courseware_orgdynamicupgradedeadlineconfiguration COURSE_DEADLINE_QUERY = 1 # courseware_coursedynamicupgradedeadlineconfiguration COMMERCE_CONFIG_QUERY = 1 # commerce_commerceconfiguration -NUM_QUERIES_NO_MATCHING_SCHEDULES = ( +NUM_QUERIES_SITE_SCHEDULES = ( SITE_QUERY + + SITE_CONFIG_QUERY + SCHEDULES_QUERY ) -NUM_QUERIES_WITH_MATCHES = ( - NUM_QUERIES_NO_MATCHING_SCHEDULES + - COURSE_MODES_QUERY -) - NUM_QUERIES_FIRST_MATCH = ( - NUM_QUERIES_WITH_MATCHES + NUM_QUERIES_SITE_SCHEDULES + GLOBAL_DEADLINE_QUERY + ORG_DEADLINE_QUERY + COURSE_DEADLINE_QUERY @@ -54,6 +56,9 @@ NUM_QUERIES_FIRST_MATCH = ( LOG = logging.getLogger(__name__) +ExperienceTest = namedtuple('ExperienceTest', 'experience offset email_sent') + + @ddt.ddt @freeze_time('2017-08-01 00:00:00', tz_offset=0, tick=True) class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase): @@ -73,6 +78,9 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) ScheduleConfigFactory.create(site=self.site_config.site) DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + CommerceConfiguration.objects.create(checkout_on_ecommerce_service=True) + + self._courses_with_verified_modes = set() def _calculate_bin_for_user(self, user): return user.id % self.task.num_bins @@ -92,6 +100,24 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) templates_override[0]['OPTIONS']['string_if_invalid'] = "TEMPLATE WARNING - MISSING VARIABLE [%s]" return templates_override + def _schedule_factory(self, offset=None, **factory_kwargs): + _, _, target_day, upgrade_deadline = self._get_dates(offset=offset) + factory_kwargs.setdefault('start', target_day) + factory_kwargs.setdefault('upgrade_deadline', upgrade_deadline) + factory_kwargs.setdefault('enrollment__course__self_paced', True) + if hasattr(self, 'experience_type'): + factory_kwargs.setdefault('experience__experience_type', self.experience_type) + schedule = ScheduleFactory(**factory_kwargs) + course_id = schedule.enrollment.course_id + if course_id not in self._courses_with_verified_modes: + CourseModeFactory( + course_id=course_id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=30), + ) + self._courses_with_verified_modes.add(course_id) + return schedule + def test_command_task_binding(self): self.assertEqual(self.command.async_send_task, self.task) @@ -130,11 +156,7 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) with patch.object(self.task, 'async_send_task') as mock_schedule_send: current_day, offset, target_day, upgrade_deadline = self._get_dates() schedules = [ - ScheduleFactory.create( - start=target_day, - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, - ) for _ in range(schedule_count) + self._schedule_factory() for _ in range(schedule_count) ] bins_in_use = frozenset((self._calculate_bin_for_user(s.enrollment.user)) for s in schedules) @@ -142,18 +164,17 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) target_day_str = serialize(target_day) for b in range(self.task.num_bins): - LOG.debug('Running bin %d', b) - expected_queries = NUM_QUERIES_NO_MATCHING_SCHEDULES + LOG.debug('Checking bin %d', b) + expected_queries = NUM_QUERIES_SITE_SCHEDULES if b in bins_in_use: if is_first_match: expected_queries = ( # Since this is the first match, we need to cache all of the config models, so we run a # query for each of those... NUM_QUERIES_FIRST_MATCH + + COURSE_MODES_QUERY # to cache the course modes for this course ) is_first_match = False - else: - expected_queries = NUM_QUERIES_WITH_MATCHES with self.assertNumQueries(expected_queries, table_blacklist=WAFFLE_TABLES): self.task.apply(kwargs=dict( @@ -171,13 +192,12 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) def test_no_course_overview(self): current_day, offset, target_day, upgrade_deadline = self._get_dates() - schedule = ScheduleFactory.create( - start=target_day, - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, + # Don't use CourseEnrollmentFactory since it creates a course overview + enrollment = CourseEnrollment.objects.create( + course_id=CourseKey.from_string('edX/toy/Not_2012_Fall'), + user=UserFactory.create(), ) - schedule.enrollment.course_id = CourseKey.from_string('edX/toy/Not_2012_Fall') - schedule.enrollment.save() + schedule = self._schedule_factory(enrollment=enrollment) with patch.object(self.task, 'async_send_task') as mock_schedule_send: for b in range(self.task.num_bins): @@ -249,25 +269,16 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) user2 = UserFactory.create(id=self.task.num_bins * 2) current_day, offset, target_day, upgrade_deadline = self._get_dates() - ScheduleFactory.create( - upgrade_deadline=upgrade_deadline, - start=target_day, + self._schedule_factory( enrollment__course__org=filtered_org, - enrollment__course__self_paced=True, enrollment__user=user1, ) - ScheduleFactory.create( - upgrade_deadline=upgrade_deadline, - start=target_day, + self._schedule_factory( enrollment__course__org=unfiltered_org, - enrollment__course__self_paced=True, enrollment__user=user1, ) - ScheduleFactory.create( - upgrade_deadline=upgrade_deadline, - start=target_day, + self._schedule_factory( enrollment__course__org=unfiltered_org, - enrollment__course__self_paced=True, enrollment__user=user2, ) @@ -284,17 +295,12 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) user1 = UserFactory.create(id=self.task.num_bins) current_day, offset, target_day, upgrade_deadline = self._get_dates() - schedule = ScheduleFactory.create( - start=target_day, - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, - enrollment__user=user1, - ) - - schedule.enrollment.course.start = current_day - datetime.timedelta(days=30) end_date_offset = -2 if has_course_ended else 2 - schedule.enrollment.course.end = current_day + datetime.timedelta(days=end_date_offset) - schedule.enrollment.course.save() + self._schedule_factory( + enrollment__user=user1, + enrollment__course__start=current_day - datetime.timedelta(days=30), + enrollment__course__end=current_day + datetime.timedelta(days=end_date_offset) + ) with patch.object(self.task, 'async_send_task') as mock_schedule_send: self.task.apply(kwargs=dict( @@ -312,15 +318,14 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) current_day, offset, target_day, upgrade_deadline = self._get_dates() num_courses = 3 for course_index in range(num_courses): - ScheduleFactory.create( - start=target_day, - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, + self._schedule_factory( enrollment__user=user, enrollment__course__id=CourseKey.from_string('edX/toy/course{}'.format(course_index)) ) - additional_course_queries = num_courses - 1 if self.queries_deadline_for_each_course else 0 + # 2 queries per course, one for the course opt out and one for the course modes + # one query for course modes for the first schedule if we aren't checking the deadline for each course + additional_course_queries = (num_courses * 2) - 1 if self.queries_deadline_for_each_course else 1 expected_query_count = NUM_QUERIES_FIRST_MATCH + additional_course_queries with self.assertNumQueries(expected_query_count, table_blacklist=WAFFLE_TABLES): with patch.object(self.task, 'async_send_task') as mock_schedule_send: @@ -333,7 +338,9 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) self.assertEqual(mock_schedule_send.apply_async.call_count, expected_call_count) self.assertFalse(mock_ace.send.called) - @ddt.data(1, 10, 100) + @ddt.data( + 1, 10 + ) def test_templates(self, message_count): for offset in self.expected_offsets: self._assert_template_for_offset(offset, message_count) @@ -344,10 +351,8 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) user = UserFactory.create() for course_index in range(message_count): - ScheduleFactory.create( - start=target_day, - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, + self._schedule_factory( + offset=offset, enrollment__user=user, enrollment__course__id=CourseKey.from_string('edX/toy/course{}'.format(course_index)) ) @@ -366,7 +371,10 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) num_expected_queries = NUM_QUERIES_FIRST_MATCH if self.queries_deadline_for_each_course: - num_expected_queries += (message_count - 1) + # one query per course for opt-out and one for course modes + num_expected_queries += (message_count * 2) - 1 + else: + num_expected_queries += 1 with self.assertNumQueries(num_expected_queries, table_blacklist=WAFFLE_TABLES): self.task.apply(kwargs=dict( @@ -385,3 +393,23 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) self.assertNotIn("TEMPLATE WARNING", template) self.assertNotIn("{{", template) self.assertNotIn("}}", template) + + def _check_if_email_sent_for_experience(self, test_config): + current_day, offset, target_day, _ = self._get_dates(offset=test_config.offset) + + kwargs = { + 'offset': offset + } + if test_config.experience is None: + kwargs['experience'] = None + else: + kwargs['experience__experience_type'] = test_config.experience + schedule = self._schedule_factory(**kwargs) + + with patch.object(tasks, 'ace') as mock_ace: + self.task.apply(kwargs=dict( + site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, + bin_num=self._calculate_bin_for_user(schedule.enrollment.user), + )) + + self.assertEqual(mock_ace.send.called, test_config.email_sent) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py deleted file mode 100644 index d026bf24dc..0000000000 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_experiences.py +++ /dev/null @@ -1,59 +0,0 @@ -from collections import namedtuple - -import datetime -import ddt -import pytz -from edx_ace.utils.date import serialize -from freezegun import freeze_time -from mock import patch - -from courseware.models import DynamicUpgradeDeadlineConfiguration -from openedx.core.djangoapps.schedules import tasks -from openedx.core.djangoapps.schedules.models import ScheduleExperience -from openedx.core.djangoapps.schedules.resolvers import _get_datetime_beginning_of_day -from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory, ScheduleConfigFactory -from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory, SiteConfigurationFactory -from openedx.core.djangolib.testing.utils import skip_unless_lms, FilteredQueryCountMixin, CacheIsolationTestCase - - -@ddt.ddt -@skip_unless_lms -@freeze_time('2017-08-01 00:00:00', tz_offset=0, tick=True) -class TestExperiences(FilteredQueryCountMixin, CacheIsolationTestCase): - - ENABLED_CACHES = ['default'] - - ExperienceTest = namedtuple('ExperienceTest', 'experience offset email_sent') - - def setUp(self): - super(TestExperiences, self).setUp() - - site = SiteFactory.create() - self.site_config = SiteConfigurationFactory.create(site=site) - ScheduleConfigFactory.create(site=self.site_config.site) - - DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) - - @ddt.data( - ExperienceTest(experience=ScheduleExperience.DEFAULT, offset=-3, email_sent=True), - ExperienceTest(experience=ScheduleExperience.DEFAULT, offset=-10, email_sent=True), - ExperienceTest(experience=ScheduleExperience.COURSE_UPDATES, offset=-3, email_sent=True), - ExperienceTest(experience=ScheduleExperience.COURSE_UPDATES, offset=-10, email_sent=False), - ) - @patch.object(tasks, 'ace') - def test_experience_type_exclusion(self, test_config, mock_ace): - current_day = _get_datetime_beginning_of_day(datetime.datetime.now(pytz.UTC)) - target_day = current_day + datetime.timedelta(days=test_config.offset) - - schedule = ScheduleFactory.create( - start=target_day, - enrollment__course__self_paced=True, - experience__experience_type=test_config.experience, - ) - - tasks.ScheduleRecurringNudge.apply(kwargs=dict( - site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=test_config.offset, - bin_num=(schedule.enrollment.user.id % tasks.ScheduleRecurringNudge.num_bins), - )) - - self.assertEqual(mock_ace.send.called, test_config.email_sent) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py index 88ac415810..b4f3a5cd64 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_course_update.py @@ -1,3 +1,4 @@ +import ddt from mock import patch from unittest import skipUnless @@ -5,11 +6,16 @@ from django.conf import settings from openedx.core.djangoapps.schedules import resolvers, tasks from openedx.core.djangoapps.schedules.management.commands import send_course_update as nudge -from openedx.core.djangoapps.schedules.management.commands.tests.send_email_base import ScheduleSendEmailTestBase +from openedx.core.djangoapps.schedules.management.commands.tests.send_email_base import ( + ScheduleSendEmailTestBase, + ExperienceTest +) from openedx.core.djangoapps.schedules.management.commands.tests.upsell_base import ScheduleUpsellTestMixin +from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangolib.testing.utils import skip_unless_lms +@ddt.ddt @skip_unless_lms @skipUnless( 'openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, @@ -25,7 +31,8 @@ class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestBase): command = nudge.Command deliver_config = 'deliver_course_update' enqueue_config = 'enqueue_course_update' - expected_offsets = xrange(-7, -77, -7) + expected_offsets = range(-7, -77, -7) + experience_type = ScheduleExperience.EXPERIENCES.course_updates queries_deadline_for_each_course = True @@ -35,3 +42,11 @@ class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestBase): mock_highlights = patcher.start() mock_highlights.return_value = ['Highlight {}'.format(num + 1) for num in range(3)] self.addCleanup(patcher.stop) + + @ddt.data( + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.default, offset=expected_offsets[0], email_sent=False), + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.course_updates, offset=expected_offsets[0], email_sent=True), + ExperienceTest(experience=None, offset=expected_offsets[0], email_sent=False), + ) + def test_schedule_in_different_experience(self, test_config): + self._check_if_email_sent_for_experience(test_config) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py index 044c4d97d7..6a152cb24d 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py @@ -1,14 +1,18 @@ from unittest import skipUnless +import ddt from django.conf import settings 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.management.commands.tests.send_email_base import ScheduleSendEmailTestBase +from openedx.core.djangoapps.schedules.management.commands.tests.send_email_base import ScheduleSendEmailTestBase, \ + ExperienceTest from openedx.core.djangoapps.schedules.management.commands.tests.upsell_base import ScheduleUpsellTestMixin +from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangolib.testing.utils import skip_unless_lms +@ddt.ddt @skip_unless_lms @skipUnless( 'openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, @@ -27,3 +31,14 @@ class TestSendRecurringNudge(ScheduleUpsellTestMixin, ScheduleSendEmailTestBase) expected_offsets = (-3, -10) consolidates_emails_for_learner = True + + @ddt.data( + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.default, offset=-3, email_sent=True), + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.default, offset=-10, email_sent=True), + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.course_updates, offset=-3, email_sent=True), + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.course_updates, offset=-10, email_sent=False), + ExperienceTest(experience=None, offset=-3, email_sent=True), + ExperienceTest(experience=None, offset=-10, email_sent=True), + ) + def test_nudge_experience(self, test_config): + self._check_if_email_sent_for_experience(test_config) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py index d323874e2a..6c00d3a97d 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py @@ -11,8 +11,9 @@ from opaque_keys.edx.locator import CourseLocator from course_modes.models import CourseMode from openedx.core.djangoapps.schedules import resolvers, tasks from openedx.core.djangoapps.schedules.management.commands import send_upgrade_reminder as reminder -from openedx.core.djangoapps.schedules.management.commands.tests.send_email_base import ScheduleSendEmailTestBase -from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory +from openedx.core.djangoapps.schedules.management.commands.tests.send_email_base import ScheduleSendEmailTestBase, \ + ExperienceTest +from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory @@ -41,18 +42,14 @@ class TestUpgradeReminder(ScheduleSendEmailTestBase): @ddt.data(True, False) @patch.object(tasks, 'ace') def test_verified_learner(self, is_verified, mock_ace): - user = UserFactory.create(id=self.task.num_bins) current_day, offset, target_day, upgrade_deadline = self._get_dates() - ScheduleFactory.create( - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, - enrollment__user=user, + schedule = self._schedule_factory( enrollment__mode=CourseMode.VERIFIED if is_verified else CourseMode.AUDIT, ) self.task.apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, - bin_num=self._calculate_bin_for_user(user), + bin_num=self._calculate_bin_for_user(schedule.enrollment.user), )) self.assertEqual(mock_ace.send.called, not is_verified) @@ -62,10 +59,8 @@ class TestUpgradeReminder(ScheduleSendEmailTestBase): user = UserFactory.create() schedules = [ - ScheduleFactory.create( - upgrade_deadline=upgrade_deadline, + self._schedule_factory( enrollment__user=user, - enrollment__course__self_paced=True, enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(i)), enrollment__mode=CourseMode.VERIFIED if i in (0, 3) else CourseMode.AUDIT, ) @@ -88,3 +83,23 @@ class TestUpgradeReminder(ScheduleSendEmailTestBase): message.context['course_ids'], [str(schedules[i].enrollment.course.id) for i in (1, 2, 4)] ) + + @patch.object(tasks, 'ace') + def test_course_without_verified_mode(self, mock_ace): + current_day, offset, target_day, upgrade_deadline = self._get_dates() + schedule = self._schedule_factory() + schedule.enrollment.course.modes.filter(mode_slug=CourseMode.VERIFIED).delete() + + self.task.apply(kwargs=dict( + site_id=self.site_config.site.id, target_day_str=serialize(target_day), day_offset=offset, + bin_num=self._calculate_bin_for_user(schedule.enrollment.user), + )) + self.assertEqual(mock_ace.send.called, False) + + @ddt.data( + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.default, offset=expected_offsets[0], email_sent=True), + ExperienceTest(experience=ScheduleExperience.EXPERIENCES.course_updates, offset=expected_offsets[0], email_sent=False), + ExperienceTest(experience=None, offset=expected_offsets[0], email_sent=True), + ) + def test_upgrade_reminder_experience(self, test_config): + self._check_if_email_sent_for_experience(test_config) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py index ade7ce9fd3..229227142a 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/upsell_base.py @@ -8,7 +8,6 @@ from edx_ace.utils.date import serialize from edx_ace.message import Message from courseware.models import DynamicUpgradeDeadlineConfiguration -from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory @ddt.ddt @@ -34,10 +33,8 @@ class ScheduleUpsellTestMixin(object): if testcase.set_deadline: upgrade_deadline = current_day + datetime.timedelta(days=testcase.deadline_offset) - schedule = ScheduleFactory.create( - start=target_day, - upgrade_deadline=upgrade_deadline, - enrollment__course__self_paced=True, + schedule = self._schedule_factory( + upgrade_deadline=upgrade_deadline ) sent_messages = [] diff --git a/openedx/core/djangoapps/schedules/models.py b/openedx/core/djangoapps/schedules/models.py index adf6cbd454..b60ba955fc 100644 --- a/openedx/core/djangoapps/schedules/models.py +++ b/openedx/core/djangoapps/schedules/models.py @@ -1,6 +1,7 @@ from django.db import models from django.utils.translation import ugettext_lazy as _ from django.contrib.sites.models import Site +from model_utils import Choices from model_utils.models import TimeStampedModel from config_models.models import ConfigurationModel @@ -27,7 +28,7 @@ class Schedule(TimeStampedModel): try: return self.experience.experience_type except ScheduleExperience.DoesNotExist: - return ScheduleExperience.DEFAULT + return ScheduleExperience.EXPERIENCES.default class Meta(object): verbose_name = _('Schedule') @@ -48,12 +49,10 @@ class ScheduleConfig(ConfigurationModel): class ScheduleExperience(models.Model): - DEFAULT = 0 - COURSE_UPDATES = 1 - EXPERIENCES = ( - (DEFAULT, 'Recurring Nudge and Upgrade Reminder'), - (COURSE_UPDATES, 'Course Updates') + EXPERIENCES = Choices( + (0, 'default', 'Recurring Nudge and Upgrade Reminder'), + (1, 'course_updates', 'Course Updates') ) schedule = models.OneToOneField(Schedule, related_name='experience') - experience_type = models.PositiveSmallIntegerField(choices=EXPERIENCES, default=DEFAULT) + experience_type = models.PositiveSmallIntegerField(choices=EXPERIENCES, default=EXPERIENCES.default) diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 05ba2b4b6b..d3fcf91534 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -77,7 +77,8 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): schedule_date_field = None num_bins = DEFAULT_NUM_BINS - experience_filter = Q(experience__experience_type=ScheduleExperience.DEFAULT) | Q(experience__isnull=True) + experience_filter = (Q(experience__experience_type=ScheduleExperience.EXPERIENCES.default) + | Q(experience__isnull=True)) def __attrs_post_init__(self): # TODO: in the next refactor of this task, pass in current_datetime instead of reproducing it here @@ -126,8 +127,6 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): schedules = Schedule.objects.select_related( 'enrollment__user__profile', 'enrollment__course', - ).prefetch_related( - 'enrollment__course__modes', ).filter( Q(enrollment__course__end__isnull=True) | Q( enrollment__course__end__gte=self.current_datetime), @@ -148,6 +147,8 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): # This will run the query and cache all of the results in memory. num_schedules = len(schedules) + LOG.debug('Number of schedules = %d', num_schedules) + # This should give us a sense of the volume of data being processed by each task. set_custom_metric('num_schedules', num_schedules) @@ -240,10 +241,10 @@ class RecurringNudgeResolver(BinnedSchedulesBaseResolver): @property def experience_filter(self): if self.day_offset == -3: - experiences = [ScheduleExperience.DEFAULT, ScheduleExperience.COURSE_UPDATES] + experiences = [ScheduleExperience.EXPERIENCES.default, ScheduleExperience.EXPERIENCES.course_updates] return Q(experience__experience_type__in=experiences) | Q(experience__isnull=True) else: - return Q(experience__experience_type=ScheduleExperience.DEFAULT) | Q(experience__isnull=True) + return Q(experience__experience_type=ScheduleExperience.EXPERIENCES.default) | Q(experience__isnull=True) def get_template_context(self, user, user_schedules): first_schedule = user_schedules[0] @@ -346,7 +347,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): log_prefix = 'Course Update' schedule_date_field = 'start' num_bins = COURSE_UPDATE_NUM_BINS - experience_filter = Q(experience__experience_type=ScheduleExperience.COURSE_UPDATES) + experience_filter = Q(experience__experience_type=ScheduleExperience.EXPERIENCES.course_updates) def schedules_for_bin(self): week_num = abs(self.day_offset) / 7 diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index 45b5927c7f..9fba0ae534 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -64,14 +64,14 @@ def create_schedule(sender, **kwargs): try: get_week_highlights(enrollment.course_id, 1) - experience_type = ScheduleExperience.COURSE_UPDATES + experience_type = ScheduleExperience.EXPERIENCES.course_updates except CourseUpdateDoesNotExist: - experience_type = ScheduleExperience.DEFAULT + experience_type = ScheduleExperience.EXPERIENCES.default ScheduleExperience(schedule=schedule, experience_type=experience_type).save() log.debug('Schedules: created a new schedule starting at %s with an upgrade deadline of %s and experience type: %s', - content_availability_date, upgrade_deadline, ScheduleExperience.EXPERIENCES[experience_type][1]) + content_availability_date, upgrade_deadline, ScheduleExperience.EXPERIENCES[experience_type]) @receiver(COURSE_START_DATE_CHANGED, dispatch_uid="update_schedules_on_course_start_changed") diff --git a/openedx/core/djangoapps/schedules/tests/factories.py b/openedx/core/djangoapps/schedules/tests/factories.py index af6a5875c9..4b54c712f2 100644 --- a/openedx/core/djangoapps/schedules/tests/factories.py +++ b/openedx/core/djangoapps/schedules/tests/factories.py @@ -10,7 +10,7 @@ class ScheduleExperienceFactory(factory.DjangoModelFactory): class Meta(object): model = models.ScheduleExperience - experience_type = models.ScheduleExperience.DEFAULT + experience_type = models.ScheduleExperience.EXPERIENCES.default class ScheduleFactory(factory.DjangoModelFactory): diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index 98b6f48f67..f89bd8a508 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -6,6 +6,7 @@ from pytz import utc 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.models import CourseOverview from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangoapps.schedules.signals import CREATE_SCHEDULE_WAFFLE_FLAG from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory @@ -24,16 +25,22 @@ from ..tests.factories import ScheduleConfigFactory @skip_unless_lms class CreateScheduleTests(SharedModuleStoreTestCase): - def assert_schedule_created(self, experience_type=ScheduleExperience.DEFAULT): + def assert_schedule_created(self, experience_type=ScheduleExperience.EXPERIENCES.default): course = _create_course_run(self_paced=True) - enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) + enrollment = CourseEnrollmentFactory( + course_id=course.id, + mode=CourseMode.AUDIT, + ) self.assertIsNotNone(enrollment.schedule) self.assertIsNone(enrollment.schedule.upgrade_deadline) self.assertEquals(enrollment.schedule.experience.experience_type, experience_type) def assert_schedule_not_created(self): course = _create_course_run(self_paced=True) - enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) + enrollment = CourseEnrollmentFactory( + course_id=course.id, + mode=CourseMode.AUDIT, + ) with self.assertRaises(Schedule.DoesNotExist): enrollment.schedule @@ -86,7 +93,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): site = SiteFactory.create() mock_get_week_highlights.return_value = True mock_get_current_site.return_value = site - self.assert_schedule_created(experience_type=ScheduleExperience.COURSE_UPDATES) + self.assert_schedule_created(experience_type=ScheduleExperience.EXPERIENCES.course_updates) @ddt.ddt @@ -114,7 +121,7 @@ class UpdateScheduleTests(SharedModuleStoreTestCase): course = _create_course_run(self_paced=True, start_day_offset=5) # course starts in future enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) - self.assert_schedule_dates(enrollment.schedule, enrollment.course_overview.start) + self.assert_schedule_dates(enrollment.schedule, enrollment.course.start) course.start = course.start + datetime.timedelta(days=3) # new course start changes to another future date self.store.update_item(course, ModuleStoreEnum.UserID.test) @@ -138,7 +145,7 @@ class UpdateScheduleTests(SharedModuleStoreTestCase): course = _create_course_run(self_paced=True, start_day_offset=5) # course starts in future enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) - previous_start = enrollment.course_overview.start + previous_start = enrollment.course.start self.assert_schedule_dates(enrollment.schedule, previous_start) course.start = course.start + datetime.timedelta(days=-10) # new course start changes to a past date