From 7fd643faa40932dd114640e885bf8f9cc5d2d27d Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Mon, 30 Oct 2017 16:16:46 -0400 Subject: [PATCH] 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