From 684d6e36696f1f8e2235d8754538084049b79b4a Mon Sep 17 00:00:00 2001
From: Bill Filler <bfiller@edx.org>
Date: Mon, 17 Dec 2018 12:34:35 -0500
Subject: [PATCH] check if user is in holdback before showing new track
 selection

---
 common/djangoapps/course_modes/views.py       |  5 +++-
 .../course_api/blocks/tests/test_api.py       | 12 ++++----
 .../courseware/tests/test_course_info.py      |  4 +--
 lms/djangoapps/grades/tests/test_tasks.py     | 20 ++++++-------
 .../features/content_type_gating/models.py    | 29 ++++++++++---------
 .../content_type_gating/tests/test_models.py  |  2 +-
 6 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py
index ae2be04a7ad..e7351a1da25 100644
--- a/common/djangoapps/course_modes/views.py
+++ b/common/djangoapps/course_modes/views.py
@@ -188,7 +188,10 @@ class ChooseModeView(View):
             "error": error,
             "responsive": True,
             "nav_hidden": True,
-            "content_gating_enabled": ContentTypeGatingConfig.enabled_for_course(course_key=course_key),
+            "content_gating_enabled": ContentTypeGatingConfig.enabled_for_enrollment(
+                user=request.user,
+                course_key=course_key
+            ),
         }
         context.update(
             get_experiment_user_metadata_context(
diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py
index 08df6b37af7..99e56062892 100644
--- a/lms/djangoapps/course_api/blocks/tests/test_api.py
+++ b/lms/djangoapps/course_api/blocks/tests/test_api.py
@@ -162,7 +162,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
             self._get_blocks(
                 course,
                 expected_mongo_queries=0,
-                expected_sql_queries=8 if with_storage_backing else 7,
+                expected_sql_queries=9 if with_storage_backing else 8,
             )
 
     @ddt.data(
@@ -179,9 +179,9 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
             clear_course_from_cache(course.id)
 
             if with_storage_backing:
-                num_sql_queries = 18
+                num_sql_queries = 19
             else:
-                num_sql_queries = 8
+                num_sql_queries = 9
 
             self._get_blocks(
                 course,
@@ -211,7 +211,7 @@ class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase
             self._get_blocks(
                 course,
                 expected_mongo_queries=0,
-                expected_sql_queries=9 if with_storage_backing else 8,
+                expected_sql_queries=10 if with_storage_backing else 9,
             )
 
     @ddt.data(
@@ -228,9 +228,9 @@ class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase
             clear_course_from_cache(course.id)
 
             if with_storage_backing:
-                num_sql_queries = 19
+                num_sql_queries = 20
             else:
-                num_sql_queries = 9
+                num_sql_queries = 10
 
             self._get_blocks(
                 course,
diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py
index e7749d4e573..28668435758 100644
--- a/lms/djangoapps/courseware/tests/test_course_info.py
+++ b/lms/djangoapps/courseware/tests/test_course_info.py
@@ -436,8 +436,8 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest
 
     def test_num_queries_instructor_paced(self):
         # TODO: decrease query count as part of REVO-28
-        self.fetch_course_info_with_queries(self.instructor_paced_course, 42, 3)
+        self.fetch_course_info_with_queries(self.instructor_paced_course, 43, 3)
 
     def test_num_queries_self_paced(self):
         # TODO: decrease query count as part of REVO-28
-        self.fetch_course_info_with_queries(self.self_paced_course, 42, 3)
+        self.fetch_course_info_with_queries(self.self_paced_course, 43, 3)
diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py
index 824526dff81..888857d8eda 100644
--- a/lms/djangoapps/grades/tests/test_tasks.py
+++ b/lms/djangoapps/grades/tests/test_tasks.py
@@ -176,10 +176,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEquals(mock_block_structure_create.call_count, 1)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 33, True),
-        (ModuleStoreEnum.Type.mongo, 1, 33, False),
-        (ModuleStoreEnum.Type.split, 3, 33, True),
-        (ModuleStoreEnum.Type.split, 3, 33, False),
+        (ModuleStoreEnum.Type.mongo, 1, 34, True),
+        (ModuleStoreEnum.Type.mongo, 1, 34, False),
+        (ModuleStoreEnum.Type.split, 3, 34, True),
+        (ModuleStoreEnum.Type.split, 3, 34, False),
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -191,8 +191,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                     self._apply_recalculate_subsection_grade()
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 33),
-        (ModuleStoreEnum.Type.split, 3, 33),
+        (ModuleStoreEnum.Type.mongo, 1, 34),
+        (ModuleStoreEnum.Type.split, 3, 34),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -237,8 +237,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
         )
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 17),
-        (ModuleStoreEnum.Type.split, 3, 17),
+        (ModuleStoreEnum.Type.mongo, 1, 18),
+        (ModuleStoreEnum.Type.split, 3, 18),
     )
     @ddt.unpack
     def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
@@ -252,8 +252,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 34),
-        (ModuleStoreEnum.Type.split, 3, 34),
+        (ModuleStoreEnum.Type.mongo, 1, 35),
+        (ModuleStoreEnum.Type.split, 3, 35),
     )
     @ddt.unpack
     def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
diff --git a/openedx/features/content_type_gating/models.py b/openedx/features/content_type_gating/models.py
index f1e36e3c958..b359e3ab2c9 100644
--- a/openedx/features/content_type_gating/models.py
+++ b/openedx/features/content_type_gating/models.py
@@ -108,25 +108,26 @@ class ContentTypeGatingConfig(StackedConfigurationModel):
             if user_variable_represents_correct_user and has_staff_roles(user, course_key):
                 return False
 
+        # check if user is in holdback
+        is_in_holdback = False
+        if user and user.is_authenticated and (user_variable_represents_correct_user):
+            try:
+                holdback_value = ExperimentData.objects.get(
+                    user=user,
+                    experiment_id=EXPERIMENT_ID,
+                    key=EXPERIMENT_DATA_HOLDBACK_KEY,
+                ).value
+                is_in_holdback = holdback_value == 'True'
+            except ExperimentData.DoesNotExist:
+                pass
+        if is_in_holdback:
+            return False
+
         # enrollment might be None if the user isn't enrolled. In that case,
         # return enablement as if the user enrolled today
         if enrollment is None:
             return cls.enabled_for_course(course_key=course_key, target_datetime=timezone.now())
         else:
-            # TODO: clean up as part of REV-100
-            is_in_holdback = False
-            if user and (user_variable_represents_correct_user):
-                try:
-                    holdback_value = ExperimentData.objects.get(
-                        user=user,
-                        experiment_id=EXPERIMENT_ID,
-                        key=EXPERIMENT_DATA_HOLDBACK_KEY,
-                    ).value
-                    is_in_holdback = holdback_value == 'True'
-                except ExperimentData.DoesNotExist:
-                    pass
-            if is_in_holdback:
-                return False
             current_config = cls.current(course_key=enrollment.course_id)
             return current_config.enabled_as_of_datetime(target_datetime=enrollment.created)
 
diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py
index c1fee5517d1..e51a16b5298 100644
--- a/openedx/features/content_type_gating/tests/test_models.py
+++ b/openedx/features/content_type_gating/tests/test_models.py
@@ -74,7 +74,7 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase):
             course_key = self.course_overview.id
 
         query_count = 8
-        if not pass_enrollment and already_enrolled:
+        if not already_enrolled or not pass_enrollment and already_enrolled:
             query_count = 9
 
         with self.assertNumQueries(query_count):
-- 
GitLab