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/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/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index db773e482d..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 @@ -19,11 +23,13 @@ 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.models import CourseEnrollment from student.tests.factories import UserFactory -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -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,9 +56,12 @@ 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(SharedModuleStoreTestCase): +class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase): __test__ = False @@ -73,6 +78,9 @@ class ScheduleSendEmailTestBase(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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(SharedModuleStoreTestCase): 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_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/migrations/0006_scheduleexperience.py b/openedx/core/djangoapps/schedules/migrations/0006_scheduleexperience.py new file mode 100644 index 0000000000..df0b41412b --- /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.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 7248e98754..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 @@ -23,6 +24,12 @@ class Schedule(TimeStampedModel): help_text=_('Deadline by which the learner must upgrade to a verified seat') ) + def get_experience_type(self): + try: + return self.experience.experience_type + except ScheduleExperience.DoesNotExist: + return ScheduleExperience.EXPERIENCES.default + class Meta(object): verbose_name = _('Schedule') verbose_name_plural = _('Schedules') @@ -39,3 +46,13 @@ 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): + 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=EXPERIENCES.default) diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index d2da8f31da..d3fcf91534 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 Schedule, ScheduleExperience from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin from openedx.core.djangoapps.schedules.template_context import ( absolute_url, @@ -64,6 +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_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() @@ -74,6 +77,8 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): schedule_date_field = None num_bins = DEFAULT_NUM_BINS + 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 @@ -122,11 +127,10 @@ 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), + self.experience_filter, enrollment__user__in=users, enrollment__is_active=True, **schedule_day_equals_target_day_filter @@ -143,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) @@ -232,6 +238,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.EXPERIENCES.default, ScheduleExperience.EXPERIENCES.course_updates] + return Q(experience__experience_type__in=experiences) | Q(experience__isnull=True) + else: + 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] context = { @@ -333,6 +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.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 1af925f22e..9fba0ae534 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.EXPERIENCES.course_updates + except CourseUpdateDoesNotExist: + 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]) @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 affc67cc3b..4b54c712f2 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.EXPERIENCES.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): diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index 1bf0048c5e..f89bd8a508 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -6,6 +6,8 @@ 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 from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag @@ -23,15 +25,22 @@ from ..tests.factories import ScheduleConfigFactory @skip_unless_lms class CreateScheduleTests(SharedModuleStoreTestCase): - def assert_schedule_created(self): + 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 @@ -78,6 +87,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.EXPERIENCES.course_updates) + @ddt.ddt @skip_unless_lms @@ -104,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) @@ -128,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