From 95d1e5c25e254101ccac2ff6d7a1dad5b2e3e435 Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Fri, 27 Oct 2017 08:41:31 -0400 Subject: [PATCH] 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):