From c2774083ea5f3f9a1eefb349f0179f22489a6dc6 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Tue, 29 Oct 2019 11:29:03 -0400 Subject: [PATCH] create table that excludes enrollments from FBE --- .../migrations/0024_fbeenrollmentexclusion.py | 23 +++++++++++++++++++ common/djangoapps/student/models.py | 16 +++++++++++++ .../tests/test_field_override_performance.py | 4 ++-- lms/djangoapps/courseware/tests/test_views.py | 12 +++++----- .../django_comment_client/base/tests.py | 8 +++---- lms/djangoapps/discussion/tests/test_views.py | 16 ++++++------- .../grades/tests/test_course_grade_factory.py | 18 +++++++-------- .../djangoapps/config_model_utils/utils.py | 7 +++++- .../features/content_type_gating/models.py | 2 +- .../content_type_gating/tests/test_models.py | 6 ++--- .../features/course_duration_limits/models.py | 2 +- .../tests/test_models.py | 6 ++--- .../tests/views/test_course_home.py | 2 +- .../tests/views/test_course_updates.py | 2 +- 14 files changed, 84 insertions(+), 40 deletions(-) create mode 100644 common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py diff --git a/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py b/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py new file mode 100644 index 0000000000..32a052fb0d --- /dev/null +++ b/common/djangoapps/student/migrations/0024_fbeenrollmentexclusion.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.25 on 2019-10-31 14:49 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0023_bulkunenrollconfiguration'), + ] + + operations = [ + migrations.CreateModel( + 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')), + ], + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index a3ad3b184a..f8e56282e2 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -2045,6 +2045,22 @@ class CourseEnrollment(models.Model): cache[(user_id, course_key)] = enrollment_state +@python_2_unicode_compatible +class FBEEnrollmentExclusion(models.Model): + """ + Disable FBE for enrollments in this table. + + .. no_pii: + """ + enrollment = models.ForeignKey( + CourseEnrollment, + on_delete=models.DO_NOTHING, + ) + + def __str__(self): + return "[FBEEnrollmentExclusion] %s" % (self.enrollment,) + + @receiver(models.signals.post_save, sender=CourseEnrollment) @receiver(models.signals.post_delete, sender=CourseEnrollment) def invalidate_enrollment_mode_cache(sender, instance, **kwargs): # pylint: disable=unused-argument, invalid-name diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index a9a6f5f707..b3cc3cb846 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 = 36 + QUERY_COUNT = 40 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 = 36 + QUERY_COUNT = 40 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 17cd089b58..3030836f2b 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, 186), - (ModuleStoreEnum.Type.split, 4, 180), + (ModuleStoreEnum.Type.mongo, 10, 193), + (ModuleStoreEnum.Type.split, 4, 185), ) @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, 56), - (False, 55) + (True, 60), + (False, 59) ) @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, 64, 44), - (True, 55, 39) + (False, 68, 48), + (True, 59, 43) ) @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 a22bb2c73f..04115859b1 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, 41), - (ModuleStoreEnum.Type.split, 3, 13, 41), + (ModuleStoreEnum.Type.mongo, 3, 4, 42), + (ModuleStoreEnum.Type.split, 3, 13, 42), ) @ddt.unpack @count_queries @@ -413,8 +413,8 @@ class ViewsQueryCountTestCase( self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 37), - (ModuleStoreEnum.Type.split, 3, 10, 37), + (ModuleStoreEnum.Type.mongo, 3, 3, 38), + (ModuleStoreEnum.Type.split, 3, 10, 38), ) @ddt.unpack @count_queries diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index d477150742..b0452f05ba 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, 24, 9), - (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 24, 9), + (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 25, 10), + (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 25, 10), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, False, 1, 3, 3, 24, 9), - (ModuleStoreEnum.Type.split, False, 50, 3, 3, 24, 9), + (ModuleStoreEnum.Type.split, False, 1, 3, 3, 25, 10), + (ModuleStoreEnum.Type.split, False, 50, 3, 3, 25, 10), # Enabling Enterprise integration should have no effect on the number of mongo queries made. - (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 24, 9), - (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 24, 9), + (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 25, 10), + (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 25, 10), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, True, 1, 3, 3, 24, 9), - (ModuleStoreEnum.Type.split, True, 50, 3, 3, 24, 9), + (ModuleStoreEnum.Type.split, True, 1, 3, 3, 25, 10), + (ModuleStoreEnum.Type.split, True, 50, 3, 3, 25, 10), ) @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 b4d5ff8737..8fb163369f 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(4), mock_get_score(1, 2): + with self.assertNumQueries(5), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - num_queries = 47 + num_queries = 48 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(5): + with self.assertNumQueries(6): _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 - num_queries = 9 + num_queries = 10 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(5): + with self.assertNumQueries(6): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - num_queries = 26 + num_queries = 27 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(5): + with self.assertNumQueries(6): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 30 + num_queries = 31 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(5): + with self.assertNumQueries(6): _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 8d2bf1b860..e6f23cc934 100644 --- a/openedx/core/djangoapps/config_model_utils/utils.py +++ b/openedx/core/djangoapps/config_model_utils/utils.py @@ -3,9 +3,10 @@ from __future__ import absolute_import from experiments.models import ExperimentData from openedx.features.course_duration_limits.config import EXPERIMENT_DATA_HOLDBACK_KEY, EXPERIMENT_ID +from student.models import FBEEnrollmentExclusion -def is_in_holdback(user): +def is_in_holdback(user, enrollment): """ Return true if given user is in holdback expermiment """ @@ -21,4 +22,8 @@ def is_in_holdback(user): except ExperimentData.DoesNotExist: pass + if enrollment is not None: + if FBEEnrollmentExclusion.objects.filter(enrollment=enrollment).exists(): + return True + return in_holdback diff --git a/openedx/features/content_type_gating/models.py b/openedx/features/content_type_gating/models.py index b3093f0450..476e7e7827 100644 --- a/openedx/features/content_type_gating/models.py +++ b/openedx/features/content_type_gating/models.py @@ -134,7 +134,7 @@ class ContentTypeGatingConfig(StackedConfigurationModel): return False # check if user is in holdback - if user_variable_represents_correct_user and is_in_holdback(user): + if user_variable_represents_correct_user and is_in_holdback(user, enrollment): return False if not correct_modes_for_fbe(course_key, 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 e7cb035d6b..325cf31b1d 100644 --- a/openedx/features/content_type_gating/tests/test_models.py +++ b/openedx/features/content_type_gating/tests/test_models.py @@ -78,9 +78,9 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): user = self.user course_key = self.course_overview.id - query_count = 7 - if not already_enrolled or not pass_enrollment and already_enrolled: - query_count = 8 + 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( diff --git a/openedx/features/course_duration_limits/models.py b/openedx/features/course_duration_limits/models.py index 9feede4249..0cdf052d12 100644 --- a/openedx/features/course_duration_limits/models.py +++ b/openedx/features/course_duration_limits/models.py @@ -128,7 +128,7 @@ class CourseDurationLimitConfig(StackedConfigurationModel): student_masquerade = is_masquerading_as_specific_student(user, course_key) # check if user is in holdback - if (no_masquerade or student_masquerade) and is_in_holdback(user): + if (no_masquerade or student_masquerade) and is_in_holdback(user, enrollment): return False not_student_masquerade = is_masquerading and not student_masquerade diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py index 1ec9898b0a..e6b2e16e31 100644 --- a/openedx/features/course_duration_limits/tests/test_models.py +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -84,9 +84,9 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): user = self.user course_key = self.course_overview.id - query_count = 7 - if pass_enrollment and already_enrolled: - query_count = 6 + query_count = 8 + if pass_enrollment and already_enrolled or not pass_enrollment and not already_enrolled: + query_count = 7 with self.assertNumQueries(query_count): enabled = CourseDurationLimitConfig.enabled_for_enrollment( 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 9f71e21641..5d3597a74f 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(97, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(106, 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 e56705cfa1..d3862e8bad 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(56, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(59, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url)