diff --git a/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py b/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py index 32a052fb0dd0fafee41f5f3d808f408333524e85..2ac84ced8ab6d2ed7e4eb77caa60126b5e8fda65 100644 --- a/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py +++ b/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.11.25 on 2019-10-31 14:49 +# Generated by Django 1.11.25 on 2019-11-01 15:56 from __future__ import unicode_literals from django.db import migrations, models @@ -17,7 +17,7 @@ class Migration(migrations.Migration): name='FBEEnrollmentExclusion', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('enrollment', models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, to='student.CourseEnrollment')), + ('enrollment', models.OneToOneField(on_delete=django.db.models.deletion.DO_NOTHING, to='student.CourseEnrollment')), ], ), ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index f8e56282e2b3e35f870e0d59576616c00e242ab2..44352ab77c15d347bb2613da69bda7fbd5a7132a 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -2052,7 +2052,7 @@ class FBEEnrollmentExclusion(models.Model): .. no_pii: """ - enrollment = models.ForeignKey( + enrollment = models.OneToOneField( CourseEnrollment, on_delete=models.DO_NOTHING, ) diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index b3cc3cb846a40a60369e7f22035526a51097b44a..a9a6f5f707fe7dd3b0965b368b0b49c028cd0cdf 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -244,7 +244,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 40 + QUERY_COUNT = 36 TEST_DATA = { # (providers, course_width, enable_ccx, view_as_ccx): ( # # of sql queries to default, @@ -273,7 +273,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 40 + QUERY_COUNT = 36 TEST_DATA = { ('no_overrides', 1, True, False): (QUERY_COUNT, 3), diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 3030836f2b187a12b740d7ee8d1862489a7ef16e..17cd089b58b2cca586ad9137a80d7edda5a57365 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -225,8 +225,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 193), - (ModuleStoreEnum.Type.split, 4, 185), + (ModuleStoreEnum.Type.mongo, 10, 186), + (ModuleStoreEnum.Type.split, 4, 180), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1459,8 +1459,8 @@ class ProgressPageTests(ProgressPageBaseTests): self.assertContains(resp, u"Download Your Certificate") @ddt.data( - (True, 60), - (False, 59) + (True, 56), + (False, 55) ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1473,8 +1473,8 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 68, 48), - (True, 59, 43) + (False, 64, 44), + (True, 55, 39) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index 04115859b1dd825e8c06e549c08b1bf134c8ed37..a22bb2c73fb66f3e9d64270c464255013b001fff 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -404,8 +404,8 @@ class ViewsQueryCountTestCase( return inner @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 4, 42), - (ModuleStoreEnum.Type.split, 3, 13, 42), + (ModuleStoreEnum.Type.mongo, 3, 4, 41), + (ModuleStoreEnum.Type.split, 3, 13, 41), ) @ddt.unpack @count_queries @@ -413,8 +413,8 @@ class ViewsQueryCountTestCase( self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 38), - (ModuleStoreEnum.Type.split, 3, 10, 38), + (ModuleStoreEnum.Type.mongo, 3, 3, 37), + (ModuleStoreEnum.Type.split, 3, 10, 37), ) @ddt.unpack @count_queries diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index b0452f05ba79a1f006387b29bc026017751d272d..d477150742df0bf470719aa92d83ec0a581aad45 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -464,18 +464,18 @@ 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, 5, 2, 25, 10), - (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 25, 10), + (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 24, 9), + (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 24, 9), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, False, 1, 3, 3, 25, 10), - (ModuleStoreEnum.Type.split, False, 50, 3, 3, 25, 10), + (ModuleStoreEnum.Type.split, False, 1, 3, 3, 24, 9), + (ModuleStoreEnum.Type.split, False, 50, 3, 3, 24, 9), # Enabling Enterprise integration should have no effect on the number of mongo queries made. - (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 25, 10), - (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 25, 10), + (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 24, 9), + (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 24, 9), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, True, 1, 3, 3, 25, 10), - (ModuleStoreEnum.Type.split, True, 50, 3, 3, 25, 10), + (ModuleStoreEnum.Type.split, True, 1, 3, 3, 24, 9), + (ModuleStoreEnum.Type.split, True, 50, 3, 3, 24, 9), ) @ddt.unpack def test_number_of_mongo_queries( diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 8fb163369f79a189c601312423416667c74df966..b4d5ff87378d60002b1cd594ab98403bcde00ba9 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -97,35 +97,35 @@ class TestCourseGradeFactory(GradeTestBase): [self.sequence.display_name, self.sequence2.display_name] ) - with self.assertNumQueries(5), mock_get_score(1, 2): + with self.assertNumQueries(4), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - num_queries = 48 + num_queries = 47 with self.assertNumQueries(num_queries), mock_get_score(1, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(6): + with self.assertNumQueries(5): _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 - num_queries = 10 + num_queries = 9 with self.assertNumQueries(num_queries), mock_get_score(1, 4): grade_factory.update(self.request.user, self.course, force_update_subsections=False) - with self.assertNumQueries(6): + with self.assertNumQueries(5): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - num_queries = 27 + num_queries = 26 with self.assertNumQueries(num_queries), mock_get_score(2, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(6): + with self.assertNumQueries(5): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 31 + num_queries = 30 with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(6): + with self.assertNumQueries(5): _assert_read(expected_pass=False, expected_percent=0.0) # updated to grade of 0.0 @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) diff --git a/openedx/core/djangoapps/config_model_utils/utils.py b/openedx/core/djangoapps/config_model_utils/utils.py index e6f23cc9345de6cc787f7a0661d5e6341d603544..50a986b1092ee3cfc5d0672a5960a80ce3e4c92e 100644 --- a/openedx/core/djangoapps/config_model_utils/utils.py +++ b/openedx/core/djangoapps/config_model_utils/utils.py @@ -23,7 +23,10 @@ def is_in_holdback(user, enrollment): pass if enrollment is not None: - if FBEEnrollmentExclusion.objects.filter(enrollment=enrollment).exists(): - return True + try: + if enrollment.fbeenrollmentexclusion: + return True + except FBEEnrollmentExclusion.DoesNotExist: + pass return in_holdback diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index f35787eb16a68031a6c65127a6d8f68250b653a0..1d4b3399e551e4ae4d7248805caa3c718f33298b 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -121,6 +121,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): schedules = Schedule.objects.select_related( 'enrollment__user__profile', 'enrollment__course', + 'enrollment__fbeenrollmentexclusion', ).filter( Q(enrollment__course__end__isnull=True) | Q( enrollment__course__end__gte=self.current_datetime diff --git a/openedx/features/content_type_gating/models.py b/openedx/features/content_type_gating/models.py index 476e7e7827268a6e9fa41cddd8d2aa55bb544515..cd7bd1336f609377874062b1db1b3aa193f8474b 100644 --- a/openedx/features/content_type_gating/models.py +++ b/openedx/features/content_type_gating/models.py @@ -86,7 +86,7 @@ class ContentTypeGatingConfig(StackedConfigurationModel): return False @classmethod - def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None, user_partition=None): + def enabled_for_enrollment(cls, user=None, course_key=None, user_partition=None): """ Return whether Content Type Gating is enabled for this enrollment. @@ -95,27 +95,15 @@ class ContentTypeGatingConfig(StackedConfigurationModel): such as the org, site, or globally), and if the configuration is specified to be ``enabled_as_of`` before the enrollment was created. - Only one of enrollment and (user, course_key) may be specified at a time. - Arguments: enrollment: The enrollment being queried. user: The user being queried. course_key: The CourseKey of the course being queried. """ - if enrollment is not None and (user is not None or course_key is not None): - raise ValueError('Specify enrollment or user/course_key, but not both') - - if enrollment is None and (user is None or course_key is None): + if user is None or course_key is None: raise ValueError('Both user and course_key must be specified if no enrollment is provided') - if enrollment is None and user is None and course_key is None: - raise ValueError('At least one of enrollment or user and course_key must be specified') - - if course_key is None: - course_key = enrollment.course_id - - if enrollment is None: - enrollment = CourseEnrollment.get_enrollment(user, course_key) + enrollment = CourseEnrollment.get_enrollment(user, course_key, ['fbeenrollmentexclusion']) if user is None and enrollment is not None: user = enrollment.user diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py index 325cf31b1d5b4b4276d8f4f33f584eadfa2b8c46..902762aae02a865aea0fb42ac46e307191f1f41f 100644 --- a/openedx/features/content_type_gating/tests/test_models.py +++ b/openedx/features/content_type_gating/tests/test_models.py @@ -33,18 +33,15 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): super(TestContentTypeGatingConfig, self).setUp() @ddt.data( - (True, True, True), - (True, True, False), - (True, False, True), - (True, False, False), - (False, False, True), - (False, False, False), + (True, True), + (True, False), + (False, True), + (False, False), ) @ddt.unpack def test_enabled_for_enrollment( self, already_enrolled, - pass_enrollment, enrolled_before_enabled, ): @@ -69,22 +66,14 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): else: existing_enrollment = None - if pass_enrollment: - enrollment = existing_enrollment - user = None - course_key = None - else: - enrollment = None - user = self.user - course_key = self.course_overview.id + enrollment = None + user = self.user + course_key = self.course_overview.id query_count = 8 - if not already_enrolled and pass_enrollment or not pass_enrollment and already_enrolled: - query_count = 9 with self.assertNumQueries(query_count): enabled = ContentTypeGatingConfig.enabled_for_enrollment( - enrollment=enrollment, user=user, course_key=course_key, ) @@ -92,11 +81,11 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): def test_enabled_for_enrollment_failure(self): with self.assertRaises(ValueError): - ContentTypeGatingConfig.enabled_for_enrollment(None, None, None) + ContentTypeGatingConfig.enabled_for_enrollment(None, None) with self.assertRaises(ValueError): - ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='enrollment'), Mock(name='user'), None) + ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='user'), None) with self.assertRaises(ValueError): - ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='enrollment'), None, Mock(name='course_key')) + ContentTypeGatingConfig.enabled_for_enrollment(None, Mock(name='course_key')) @ddt.data(True, False) def test_enabled_for_course( diff --git a/openedx/features/course_duration_limits/management/__init__.py b/openedx/features/course_duration_limits/management/__init__.py deleted file mode 100644 index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000 diff --git a/openedx/features/course_duration_limits/management/commands/__init__.py b/openedx/features/course_duration_limits/management/commands/__init__.py deleted file mode 100644 index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000 diff --git a/openedx/features/course_duration_limits/management/commands/cdl_setup_models_to_send_test_emails.py b/openedx/features/course_duration_limits/management/commands/cdl_setup_models_to_send_test_emails.py deleted file mode 100644 index f78964de5b4dc442fdf5f25969034f4a1f30af5c..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/management/commands/cdl_setup_models_to_send_test_emails.py +++ /dev/null @@ -1,57 +0,0 @@ -""" -A managment command that can be used to set up Schedules with various configurations for testing. -""" - -from __future__ import absolute_import - -import datetime -from textwrap import dedent - -import factory -import pytz -from django.contrib.sites.models import Site -from django.core.management.base import BaseCommand - -from course_modes.models import CourseMode -from course_modes.tests.factories import CourseModeFactory -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK, CourseFactory - - -class CourseDurationLimitExpirySchedule(ScheduleFactory): - """ - A ScheduleFactory that creates a Schedule set up for Course Duration Limit expiry - """ - start = factory.Faker('date_time_between', start_date='-21d', end_date='-21d', tzinfo=pytz.UTC) - - -class Command(BaseCommand): - """ - A management command that generates schedule objects for all expected course duration limit - email types, so that it is easy to generate test emails of all available types. - """ - help = dedent(__doc__).strip() - - def handle(self, *args, **options): - courses = modulestore().get_courses() - - # Find the largest auto-generated course, and pick the next sequence id to generate the next - # course with. - max_org_sequence_id = max([0] + [int(course.org[4:]) for course in courses if course.org.startswith('org.')]) - - XMODULE_FACTORY_LOCK.enable() - CourseFactory.reset_sequence(max_org_sequence_id + 1, force=True) - course = CourseFactory.create( - start=datetime.datetime.today() - datetime.timedelta(days=30), - end=datetime.datetime.today() + datetime.timedelta(days=30), - number=factory.Sequence('schedules_test_course_{}'.format), - display_name=factory.Sequence(u'Schedules Test Course {}'.format), - ) - XMODULE_FACTORY_LOCK.disable() - course_overview = CourseOverview.load_from_module_store(course.id) - CourseModeFactory.create(course_id=course_overview.id, mode_slug=CourseMode.AUDIT) - CourseModeFactory.create(course_id=course_overview.id, mode_slug=CourseMode.VERIFIED) - CourseDurationLimitExpirySchedule.create_batch(20, enrollment__course=course_overview) - ScheduleConfigFactory.create(site=Site.objects.get(name='example.com')) diff --git a/openedx/features/course_duration_limits/management/commands/send_access_expiry_reminder.py b/openedx/features/course_duration_limits/management/commands/send_access_expiry_reminder.py deleted file mode 100644 index 354bb188fa7d1c38922155a36671bead4ca93790..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/management/commands/send_access_expiry_reminder.py +++ /dev/null @@ -1,26 +0,0 @@ -""" -./manage.py lms send_access_expiry_reminder <domain> - -Send out reminder emails for any students who will lose access to course content in 7 days. -""" -from __future__ import absolute_import - -from textwrap import dedent - -from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand -from openedx.features.course_duration_limits.tasks import CourseDurationLimitExpiryReminder - -from ... import resolvers - - -class Command(SendEmailBaseCommand): - """ - Send out reminder emails for any students who will lose access - to course content in 7 days. - - ./manage.py lms send_access_expiry_reminder <domain> - """ - help = dedent(__doc__).strip() - async_send_task = CourseDurationLimitExpiryReminder - log_prefix = resolvers.EXPIRY_REMINDER_LOG_PREFIX - offsets = (7,) # Days until Course Duration Limit expiry diff --git a/openedx/features/course_duration_limits/message_types.py b/openedx/features/course_duration_limits/message_types.py deleted file mode 100644 index 9cc8c73c57549c7a40c362a65d619553ee77f9c5..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/message_types.py +++ /dev/null @@ -1,15 +0,0 @@ -""" -Message types used for ACE communication by the course_duration_limits app. -""" -from __future__ import absolute_import - -import logging - -from openedx.core.djangoapps.ace_common.message import BaseMessageType -from openedx.core.djangoapps.schedules.config import DEBUG_MESSAGE_WAFFLE_FLAG - - -class ExpiryReminder(BaseMessageType): - def __init__(self, *args, **kwargs): - super(ExpiryReminder, self).__init__(*args, **kwargs) - self.log_level = logging.DEBUG if DEBUG_MESSAGE_WAFFLE_FLAG.is_enabled() else None diff --git a/openedx/features/course_duration_limits/models.py b/openedx/features/course_duration_limits/models.py index 0cdf052d124453f86be09396297b60e092d7333b..a19a52f9332ba9770b17f9ec98f5f5540b0a24f2 100644 --- a/openedx/features/course_duration_limits/models.py +++ b/openedx/features/course_duration_limits/models.py @@ -80,7 +80,7 @@ class CourseDurationLimitConfig(StackedConfigurationModel): return False @classmethod - def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None): + def enabled_for_enrollment(cls, user=None, course_key=None): """ Return whether Course Duration Limits are enabled for this enrollment. @@ -97,20 +97,10 @@ class CourseDurationLimitConfig(StackedConfigurationModel): course_key: The CourseKey of the course being queried. """ - if enrollment is not None and (user is not None or course_key is not None): - raise ValueError('Specify enrollment or user/course_key, but not both') - - if enrollment is None and (user is None or course_key is None): + if user is None or course_key is None: raise ValueError('Both user and course_key must be specified if no enrollment is provided') - if enrollment is None and user is None and course_key is None: - raise ValueError('At least one of enrollment or user and course_key must be specified') - - if course_key is None: - course_key = enrollment.course_id - - if enrollment is None: - enrollment = CourseEnrollment.get_enrollment(user, course_key) + enrollment = CourseEnrollment.get_enrollment(user, course_key, ['fbeenrollmentexclusion']) if user is None and enrollment is not None: user = enrollment.user diff --git a/openedx/features/course_duration_limits/resolvers.py b/openedx/features/course_duration_limits/resolvers.py deleted file mode 100644 index 9d3dfc68edefb1ee45320a2fba55fe21b93041ae..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/resolvers.py +++ /dev/null @@ -1,186 +0,0 @@ -""" -Resolvers used to find users for course_duration_limit message -""" - -from __future__ import absolute_import - -import logging -from datetime import datetime, timedelta - -from django.contrib.staticfiles.templatetags.staticfiles import static -from django.db.models import Q -from django.utils.timesince import timeuntil -from django.utils.translation import ugettext as _ -from eventtracking import tracker - -from course_modes.models import CourseMode -from lms.djangoapps.courseware.date_summary import verified_upgrade_deadline_link -from lms.djangoapps.experiments.stable_bucketing import stable_bucketing_hash_group -from openedx.core.djangoapps.catalog.utils import get_course_run_details -from openedx.core.djangoapps.schedules.resolvers import ( - BinnedSchedulesBaseResolver, - InvalidContextError, - _get_trackable_course_home_url -) -from track import segment - -from .access import MAX_DURATION, MIN_DURATION, get_user_course_expiration_date -from .models import CourseDurationLimitConfig - -LOG = logging.getLogger(__name__) - -DEFAULT_NUM_BINS = 24 -EXPIRY_REMINDER_NUM_BINS = 1 -EXPIRY_REMINDER_LOG_PREFIX = 'FBE Expiry Reminder' - - -class ExpiryReminderResolver(BinnedSchedulesBaseResolver): - """ - Send a message to all users whose course duration limit expiration date - is at ``self.current_date`` + ``day_offset``. - """ - log_prefix = EXPIRY_REMINDER_LOG_PREFIX - schedule_date_field = 'start' - num_bins = EXPIRY_REMINDER_NUM_BINS - - def __init__( - self, - async_send_task, - site, - course_key, - target_datetime, - day_offset, - override_recipient_email=None - ): - access_duration = MIN_DURATION - discovery_course_details = get_course_run_details(course_key, ['weeks_to_complete']) - expected_weeks = discovery_course_details.get('weeks_to_complete') - if expected_weeks: - access_duration = timedelta(weeks=expected_weeks) - - access_duration = max(MIN_DURATION, min(MAX_DURATION, access_duration)) - - self.course_key = course_key - - super(ExpiryReminderResolver, self).__init__( - async_send_task, - site, - target_datetime - access_duration, - day_offset - access_duration.days, - 0, - override_recipient_email, - ) - - # TODO: This isn't named well, given the purpose we're using it for. That's ok for now, - # this is just a test. - @property - def experience_filter(self): - return Q(enrollment__course_id=self.course_key, enrollment__mode=CourseMode.AUDIT) - - def get_template_context(self, user, user_schedules): - course_id_strs = [] - course_links = [] - first_valid_upsell_context = None - first_schedule = None - first_expiration_date = None - - self.log_info(u"Found %s schedules for %s", len(user_schedules), user.username) - - for schedule in user_schedules: - upsell_context = _get_upsell_information_for_schedule(user, schedule) - if not upsell_context['show_upsell']: - self.log_info(u"No upsell available for %r", schedule.enrollment) - continue - - if not CourseDurationLimitConfig.enabled_for_enrollment(enrollment=schedule.enrollment): - self.log_info(u"course duration limits not enabled for %r", schedule.enrollment) - continue - - expiration_date = get_user_course_expiration_date(user, schedule.enrollment.course) - if expiration_date is None: - self.log_info(u"No course expiration date for %r", schedule.enrollment.course) - continue - - if first_valid_upsell_context is None: - first_schedule = schedule - first_valid_upsell_context = upsell_context - first_expiration_date = expiration_date - course_id_str = str(schedule.enrollment.course_id) - course_id_strs.append(course_id_str) - course_links.append({ - 'url': _get_trackable_course_home_url(schedule.enrollment.course_id), - 'name': schedule.enrollment.course.display_name - }) - - if first_schedule is None: - self.log_info(u'No courses eligible for upgrade for user %s.', user.username) - raise InvalidContextError() - - # Experiment code: Skip users who are in the control bucket - hash_bucket = stable_bucketing_hash_group('fbe_access_expiry_reminder', 2, user.username) - properties = { - 'site': self.site.domain, # pylint: disable=no-member - 'app_label': 'course_duration_limits', - 'nonInteraction': 1, - 'bucket': hash_bucket, - 'experiment': 'REVMI-95', - } - course_ids = course_id_strs - properties['num_courses'] = len(course_ids) - if course_ids: - properties['course_ids'] = course_ids[:10] - properties['primary_course_id'] = course_ids[0] - - tracking_context = { - 'host': self.site.domain, # pylint: disable=no-member - 'path': '/', # make up a value, in order to allow the host to be passed along. - } - # I wonder if the user of this event should be the recipient, as they are not the ones - # who took an action. Rather, the system is acting, and they are the object. - # Admittedly that may be what 'nonInteraction' is meant to address. But sessionization may - # get confused by these events if they're attributed in this way, because there's no way for - # this event to get context that would match with what the user might be doing at the moment. - # But the events do show up in GA being joined up with existing sessions (i.e. within a half - # hour in the past), so they don't always break sessions. Not sure what happens after these. - # We can put the recipient_user_id into the properties, and then export as a custom dimension. - with tracker.get_tracker().context('course_duration_limits', tracking_context): - segment.track( - user_id=user.id, - event_name='edx.bi.experiment.user.bucketed', - properties=properties, - ) - if hash_bucket == 0: - raise InvalidContextError() - - context = { - 'course_links': course_links, - 'first_course_name': first_schedule.enrollment.course.display_name, - 'cert_image': static('course_experience/images/verified-cert.png'), - 'course_ids': course_id_strs, - 'first_course_expiration_date': first_expiration_date.strftime(_(u"%b. %d, %Y")), - 'time_until_expiration': timeuntil(first_expiration_date.date(), now=datetime.utcnow().date()) - } - context.update(first_valid_upsell_context) - return context - - -def _get_verified_upgrade_link(user, schedule): - enrollment = schedule.enrollment - if enrollment.is_active: - return verified_upgrade_deadline_link(user, enrollment.course) - - -def _get_upsell_information_for_schedule(user, schedule): - """ - Return upsell variables for inclusion in a message template being sent to this user. - """ - template_context = {} - - verified_upgrade_link = _get_verified_upgrade_link(user, schedule) - has_verified_upgrade_link = verified_upgrade_link is not None - - if has_verified_upgrade_link: - template_context['upsell_link'] = verified_upgrade_link - - template_context['show_upsell'] = has_verified_upgrade_link - return template_context diff --git a/openedx/features/course_duration_limits/tasks.py b/openedx/features/course_duration_limits/tasks.py deleted file mode 100644 index 446428eafadbe97ef62cdab72cb48831af052799..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/tasks.py +++ /dev/null @@ -1,174 +0,0 @@ -""" -Tasks requiring asynchronous handling for course_duration_limits -""" - -from __future__ import absolute_import - -import datetime -import logging - -import six -import waffle -from celery import task -from celery_utils.logged_task import LoggedTask -from django.conf import settings -from django.contrib.auth.models import User -from django.contrib.sites.models import Site -from edx_ace import ace -from edx_ace.message import Message -from edx_ace.utils.date import deserialize, serialize -from edx_django_utils.monitoring import set_custom_metric -from opaque_keys.edx.keys import CourseKey - -from openedx.core.djangoapps.schedules.resolvers import _get_datetime_beginning_of_day -from openedx.core.djangoapps.schedules.tasks import _annonate_send_task_for_monitoring, _track_message_sent -from openedx.core.lib.celery.task_utils import emulate_http_request - -from . import message_types, resolvers -from .models import CourseDurationLimitConfig - -LOG = logging.getLogger(__name__) -ROUTING_KEY = getattr(settings, 'ACE_ROUTING_KEY', None) - - -class CourseDurationLimitMessageBaseTask(LoggedTask): - """ - Base class for top-level Schedule tasks that create subtasks - for each Bin. - """ - ignore_result = True - routing_key = ROUTING_KEY - num_bins = resolvers.DEFAULT_NUM_BINS - enqueue_config_var = None # define in subclass - log_prefix = None - resolver = None # define in subclass - async_send_task = None # define in subclass - - @classmethod - def log_debug(cls, message, *args, **kwargs): - """ - Wrapper around LOG.debug that prefixes the message. - """ - LOG.debug(cls.log_prefix + ': ' + message, *args, **kwargs) - - @classmethod - def log_info(cls, message, *args, **kwargs): - """ - Wrapper around LOG.info that prefixes the message. - """ - LOG.info(cls.log_prefix + ': ' + message, *args, **kwargs) - - @classmethod - def enqueue(cls, site, current_date, day_offset, override_recipient_email=None): - current_date = _get_datetime_beginning_of_day(current_date) - - for course_key, config in CourseDurationLimitConfig.all_current_course_configs().items(): - if not config['enabled'][0]: - cls.log_info(u'Course duration limits disabled for course_key %s, skipping', course_key) - continue - - # enqueue_enabled, _ = config[cls.enqueue_config_var] - # TODO: Switch over to a model where enqueing is based in CourseDurationLimitConfig - enqueue_enabled = waffle.switch_is_active('course_duration_limits.enqueue_enabled') - - if not enqueue_enabled: - cls.log_info(u'Message queuing disabled for course_key %s', course_key) - continue - - target_date = current_date + datetime.timedelta(days=day_offset) - task_args = ( - site.id, - six.text_type(course_key), - serialize(target_date), - day_offset, - override_recipient_email, - ) - cls().apply_async( - task_args, - retry=False, - ) - - def run( # pylint: disable=arguments-differ - self, site_id, course_key_str, target_day_str, day_offset, override_recipient_email=None, - ): - try: - site = Site.objects.select_related('configuration').get(id=site_id) - with emulate_http_request(site=site): - msg_type = self.make_message_type(day_offset) - _annotate_for_monitoring(msg_type, course_key_str, target_day_str, day_offset) - return self.resolver( # pylint: disable=not-callable - self.async_send_task, - site, - CourseKey.from_string(course_key_str), - deserialize(target_day_str), - day_offset, - override_recipient_email=override_recipient_email, - ).send(msg_type) - except Exception: # pylint: disable=broad-except - LOG.exception("Task failed") - - def make_message_type(self, day_offset): - raise NotImplementedError - - -@task(base=LoggedTask, ignore_result=True, routing_key=ROUTING_KEY) -def _expiry_reminder_schedule_send(site_id, msg_str): - _schedule_send( - msg_str, - site_id, - 'deliver_expiry_reminder', - resolvers.EXPIRY_REMINDER_LOG_PREFIX, - ) - - -class CourseDurationLimitExpiryReminder(CourseDurationLimitMessageBaseTask): - """ - Task to send out a reminder that a users access to course content is expiring soon. - """ - num_bins = resolvers.EXPIRY_REMINDER_NUM_BINS - enqueue_config_var = 'enqueue_expiry_reminder' - log_prefix = resolvers.EXPIRY_REMINDER_LOG_PREFIX - resolver = resolvers.ExpiryReminderResolver - async_send_task = _expiry_reminder_schedule_send - - def make_message_type(self, day_offset): - return message_types.ExpiryReminder() - - -def _schedule_send(msg_str, site_id, delivery_config_var, log_prefix): - site = Site.objects.select_related('configuration').get(pk=site_id) - if _is_delivery_enabled(site, delivery_config_var, log_prefix): - msg = Message.from_string(msg_str) - - user = User.objects.get(username=msg.recipient.username) # pylint: disable=no-member - with emulate_http_request(site=site, user=user): - _annonate_send_task_for_monitoring(msg) - LOG.debug(u'%s: Sending message = %s', log_prefix, msg_str) - ace.send(msg) - _track_message_sent(site, user, msg) - - -def _is_delivery_enabled(site, delivery_config_var, log_prefix): # pylint: disable=unused-argument - # Experiment TODO: when this is going to prod, switch over to a config-model backed solution - #if getattr(CourseDurationLimitConfig.current(site=site), delivery_config_var, False): - if waffle.switch_is_active('course_duration_limits.delivery_enabled'): - return True - else: - LOG.info(u'%s: Message delivery disabled for site %s', log_prefix, site.domain) - return False - - -def _annotate_for_monitoring(message_type, course_key, target_day_str, day_offset): - """ - Set custom metrics in monitoring to make it easier to identify what messages are being sent and why. - """ - # This identifies the type of message being sent, for example: schedules.recurring_nudge3. - set_custom_metric('message_name', '{0}.{1}'.format(message_type.app_label, message_type.name)) - # The domain name of the site we are sending the message for. - set_custom_metric('course_key', course_key) - # The date we are processing data for. - set_custom_metric('target_day', target_day_str) - # The number of days relative to the current date to process data for. - set_custom_metric('day_offset', day_offset) - # A unique identifier for this batch of messages being sent. - set_custom_metric('send_uuid', message_type.uuid) diff --git a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/body.html b/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/body.html deleted file mode 100644 index 900044b8fabf6fe936aaa581e0f6265d58e7802a..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/body.html +++ /dev/null @@ -1,47 +0,0 @@ -{% extends 'ace_common/edx_ace/common/base_body.html' %} -{% load i18n %} -{% load ace %} - -{% block preview_text %} -{% blocktrans trimmed %} -We hope you have enjoyed {{first_course_name}}! You lose all access to this course in {{time_until_expiration}}. -{% endblocktrans %} -{% endblock %} - -{% block content %} -<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation"> - <tr> - <td> - <p> - {% blocktrans trimmed %} - We hope you have enjoyed {{first_course_name}}! You lose all access to this course, including your progress, on - {{ first_course_expiration_date }} ({{time_until_expiration}}). - {% endblocktrans %} - </p> - <p> - {% blocktrans trimmed %} - Upgrade now to get unlimited access and for the chance to earn a verified certificate. - {% endblocktrans %} - </p> - - {% if show_upsell %} - <p> - <a href="{% with_link_tracking upsell_link %}" style=" - color: #1e8142; - text-decoration: none; - border-radius: 4px; - -webkit-border-radius: 4px; - -moz-border-radius: 4px; - background-color: #FFFFFF; - border: 3px solid #1e8142; - display: inline-block; - padding: 8px 65px; - "> - <font color="#1e8142"><b>{% trans "Upgrade Now" %}</b></font> - </a> - </p> - {% endif %} - </td> - </tr> -</table> -{% endblock %} diff --git a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/body.txt b/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/body.txt deleted file mode 100644 index d6839e070a295d66238d9a0b7fb86f7e3d18d7db..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/body.txt +++ /dev/null @@ -1,13 +0,0 @@ -{% autoescape off %} -{% load i18n %} -{% load ace %} - -{% blocktrans trimmed %} -We hope you have enjoyed {{first_course_name}}! You lose all access to this course, including your progress, on {{ first_course_expiration_date }} ({{time_until_expiration}}). - -Upgrade now to get unlimited access and for the chance to earn a verified certificate. -{% endblocktrans %} -{% if show_upsell %} -{% trans "Upgrade Now" %} <{% with_link_tracking upsell_link %}> -{% endif %} -{% endautoescape %} diff --git a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/from_name.txt b/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/from_name.txt deleted file mode 100644 index 039cc99c9a5771bf78bd32d9664ed559489598f6..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/from_name.txt +++ /dev/null @@ -1,3 +0,0 @@ -{% autoescape off %} -{{ first_course_name }} -{% endautoescape %} diff --git a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/head.html b/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/head.html deleted file mode 100644 index 366ada7ad92e41ef3842688d49b34f2b2525a7a7..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/head.html +++ /dev/null @@ -1 +0,0 @@ -{% extends 'ace_common/edx_ace/common/base_head.html' %} diff --git a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/subject.txt b/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/subject.txt deleted file mode 100644 index 6e3dccbcd225e71403919d0e704e37d0592b943a..0000000000000000000000000000000000000000 --- a/openedx/features/course_duration_limits/templates/course_duration_limits/edx_ace/expiryreminder/email/subject.txt +++ /dev/null @@ -1,5 +0,0 @@ -{% autoescape off %} -{% load i18n %} - -{% blocktrans trimmed %}Upgrade to keep your access to {{first_course_name}}{% endblocktrans %} -{% endautoescape %} diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py index e6b2e16e315c6bdea697fd19dd91367f990ab63f..18eaafb45e8cc43379b3cf04ff3114ddbbeccb9c 100644 --- a/openedx/features/course_duration_limits/tests/test_models.py +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -39,18 +39,15 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): super(TestCourseDurationLimitConfig, self).setUp() @ddt.data( - (True, True, True), - (True, True, False), - (True, False, True), - (True, False, False), - (False, False, True), - (False, False, False), + (True, True), + (True, False), + (False, True), + (False, False), ) @ddt.unpack def test_enabled_for_enrollment( self, already_enrolled, - pass_enrollment, enrolled_before_enabled, ): @@ -75,22 +72,13 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): else: existing_enrollment = None - if pass_enrollment: - enrollment = existing_enrollment - user = None - course_key = None - else: - enrollment = None - user = self.user - course_key = self.course_overview.id + user = self.user + course_key = self.course_overview.id - query_count = 8 - if pass_enrollment and already_enrolled or not pass_enrollment and not already_enrolled: - query_count = 7 + query_count = 7 with self.assertNumQueries(query_count): enabled = CourseDurationLimitConfig.enabled_for_enrollment( - enrollment=enrollment, user=user, course_key=course_key, ) @@ -98,16 +86,14 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): def test_enabled_for_enrollment_failure(self): with self.assertRaises(ValueError): - CourseDurationLimitConfig.enabled_for_enrollment(None, None, None) + CourseDurationLimitConfig.enabled_for_enrollment(None, None) with self.assertRaises(ValueError): CourseDurationLimitConfig.enabled_for_enrollment( - Mock(name='enrollment'), Mock(name='user'), None ) with self.assertRaises(ValueError): CourseDurationLimitConfig.enabled_for_enrollment( - Mock(name='enrollment'), None, Mock(name='course_key') ) diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 5d3597a74f823bae5b3cdaa6da46cc6f08c5cef1..9f71e21641839ace52cd2e03d458d68b62ddc53d 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -220,7 +220,7 @@ class TestCourseHomePage(CourseHomePageTestCase): # Fetch the view and verify the query counts # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(106, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(97, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index d3862e8bad13637b047f4815501167abb7a98dad..e56705cfa1871378a0ef33fd83aae17d23e22a5b 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -134,7 +134,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(59, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(56, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url)