diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 2c5c6c82f0..d8a706fc4f 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -9,13 +9,15 @@ from django.conf import settings from django.urls import reverse from django.utils.translation import ugettext as _ from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import LibraryLocator from pytz import UTC from six import text_type from django_comment_common.models import assign_default_role from django_comment_common.utils import seed_permissions_roles from openedx.core.djangoapps.site_configuration.models import SiteConfiguration -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_STUDIO_UI_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student import auth from student.models import CourseEnrollment from student.roles import CourseInstructorRole, CourseStaffRole @@ -458,7 +460,12 @@ def get_visibility_partition_info(xblock, course=None): if len(partition["groups"]) > 1 or any(group["selected"] for group in partition["groups"]): selectable_partitions.append(partition) - if CONTENT_TYPE_GATING_STUDIO_UI_FLAG.is_enabled(): + flag_enabled = CONTENT_TYPE_GATING_FLAG.is_enabled() + course_key = xblock.scope_ids.usage_id.course_key + is_library = isinstance(course_key, LibraryLocator) + if not is_library and ( + flag_enabled or ContentTypeGatingConfig.current(course_key=course_key).studio_override_enabled + ): selectable_partitions += get_user_partition_info(xblock, schemes=["content_type_gate"], course=course) # Now add the cohort user partitions. diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 15a8a36528..ae2be04a7a 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -30,7 +30,7 @@ from openedx.core.djangoapps.catalog.utils import get_currency_data from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.programs.utils import ProgramDataExtender, ProgramProgressMeter from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from student.models import CourseEnrollment from util.db import outer_atomic from xmodule.modulestore.django import modulestore @@ -188,7 +188,7 @@ class ChooseModeView(View): "error": error, "responsive": True, "nav_hidden": True, - "content_gating_enabled": CONTENT_TYPE_GATING_FLAG.is_enabled(), + "content_gating_enabled": ContentTypeGatingConfig.enabled_for_course(course_key=course_key), } context.update( get_experiment_user_metadata_context( diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index a17b864e3b..95c90111d3 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1309,7 +1309,7 @@ class CourseEnrollment(models.Model): return enrollment @classmethod - def get_enrollment(cls, user, course_key): + def get_enrollment(cls, user, course_key, select_related=None): """Returns a CourseEnrollment object. Args: @@ -1324,7 +1324,10 @@ class CourseEnrollment(models.Model): if user.is_anonymous: return None try: - return cls.objects.get( + query = cls.objects + if select_related is not None: + query = query.select_related(*select_related) + return query.get( user=user, course_id=course_key ) diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index b88613a187..8c64d0cdc6 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -5,7 +5,7 @@ import itertools import json import re import unittest -from datetime import timedelta +from datetime import timedelta, date import ddt from completion.test_utils import submit_completions_for_testing, CompletionWaffleTestMixin @@ -31,7 +31,7 @@ from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from openedx.core.djangoapps.user_authn.cookies import _get_user_info_cookie_data from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from student.helpers import DISABLE_UNENROLL_CERT_STATES from student.models import CourseEnrollment, UserProfile from student.signals import REFUND_ORDER @@ -723,13 +723,13 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, ) @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_content_gating_course_card_changes(self): """ When a course is expired, the links on the course card should be removed. Links will be removed from the course title, course image and button (View Course/Resume Course). The course card should have an access expired message. """ + CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) self.override_waffle_switch(True) course = CourseFactory.create(start=self.THREE_YEARS_AGO) diff --git a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py index 6db47039d8..a1c136aee0 100644 --- a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py +++ b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py @@ -3,7 +3,8 @@ Test the partitions and partitions service """ -from unittest import TestCase +from datetime import date +from django.test import TestCase from mock import Mock, patch from opaque_keys.edx.locator import CourseLocator @@ -15,6 +16,7 @@ from xmodule.partitions.partitions import ( from xmodule.partitions.partitions_service import ( PartitionService, get_all_partitions_for_course, FEATURES ) +from openedx.features.content_type_gating.models import ContentTypeGatingConfig class TestGroup(TestCase): @@ -435,17 +437,11 @@ class PartitionServiceBaseClass(PartitionTestCase): def setUp(self): super(PartitionServiceBaseClass, self).setUp() - content_gating_flag_patcher = patch( - 'openedx.features.content_type_gating.partitions.CONTENT_TYPE_GATING_FLAG.is_enabled', - return_value=True, - ).start() - self.addCleanup(content_gating_flag_patcher.stop) - content_gating_ui_flag_patcher = patch( - 'openedx.features.content_type_gating.partitions.CONTENT_TYPE_GATING_STUDIO_UI_FLAG.is_enabled', - return_value=True, - ).start() - self.addCleanup(content_gating_ui_flag_patcher.stop) - + ContentTypeGatingConfig.objects.create( + enabled=True, + enabled_as_of=date(2018, 1, 1), + studio_override_enabled=True + ) self.course = Mock(id=CourseLocator('org_0', 'course_0', 'run_0')) self.partition_service = self._create_service("ma") diff --git a/common/lib/xmodule/xmodule/tests/test_split_test_module.py b/common/lib/xmodule/xmodule/tests/test_split_test_module.py index c9706d9f9e..cc211a92e5 100644 --- a/common/lib/xmodule/xmodule/tests/test_split_test_module.py +++ b/common/lib/xmodule/xmodule/tests/test_split_test_module.py @@ -129,16 +129,12 @@ class SplitTestModuleLMSTest(SplitTestModuleTest): def setUp(self): super(SplitTestModuleLMSTest, self).setUp() + content_gating_flag_patcher = patch( - 'openedx.features.content_type_gating.partitions.CONTENT_TYPE_GATING_FLAG.is_enabled', - return_value=False, + 'openedx.features.content_type_gating.partitions.ContentTypeGatingConfig.current', + return_value=Mock(enabled=False, studio_override_enabled=False), ).start() self.addCleanup(content_gating_flag_patcher.stop) - content_gating_ui_flag_patcher = patch( - 'openedx.features.content_type_gating.partitions.CONTENT_TYPE_GATING_STUDIO_UI_FLAG.is_enabled', - return_value=False, - ).start() - self.addCleanup(content_gating_ui_flag_patcher.stop) @ddt.data((0, 'split_test_cond0'), (1, 'split_test_cond1')) @ddt.unpack diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 3343931283..b5de22f460 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -3,7 +3,7 @@ Performance tests for field overrides. """ import itertools -from datetime import datetime +from datetime import datetime, date import ddt import mock @@ -22,7 +22,7 @@ from lms.djangoapps.ccx.tests.factories import CcxFactory from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from pytz import UTC from student.models import CourseEnrollment from student.tests.factories import UserFactory @@ -198,11 +198,15 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT XBLOCK_FIELD_DATA_WRAPPERS=[], MODULESTORE_FIELD_OVERRIDE_PROVIDERS=[], ) - @mock.patch.object(CONTENT_TYPE_GATING_FLAG, 'is_enabled', return_value=True) - def test_field_overrides(self, overrides, course_width, enable_ccx, view_as_ccx, _mock_flag): + def test_field_overrides(self, overrides, course_width, enable_ccx, view_as_ccx): """ Test without any field overrides. """ + ContentTypeGatingConfig.objects.create( + enabled=True, + enabled_as_of=date(2018, 1, 1), + ) + providers = { 'no_overrides': (), 'ccx': ('ccx.overrides.CustomCoursesForEdxOverrideProvider',) @@ -240,18 +244,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (24, 1), - ('no_overrides', 2, True, False): (24, 1), - ('no_overrides', 3, True, False): (24, 1), - ('ccx', 1, True, False): (24, 1), - ('ccx', 2, True, False): (24, 1), - ('ccx', 3, True, False): (24, 1), - ('no_overrides', 1, False, False): (24, 1), - ('no_overrides', 2, False, False): (24, 1), - ('no_overrides', 3, False, False): (24, 1), - ('ccx', 1, False, False): (24, 1), - ('ccx', 2, False, False): (24, 1), - ('ccx', 3, False, False): (24, 1), + ('no_overrides', 1, True, False): (26, 1), + ('no_overrides', 2, True, False): (26, 1), + ('no_overrides', 3, True, False): (26, 1), + ('ccx', 1, True, False): (26, 1), + ('ccx', 2, True, False): (26, 1), + ('ccx', 3, True, False): (26, 1), + ('no_overrides', 1, False, False): (26, 1), + ('no_overrides', 2, False, False): (26, 1), + ('no_overrides', 3, False, False): (26, 1), + ('ccx', 1, False, False): (26, 1), + ('ccx', 2, False, False): (26, 1), + ('ccx', 3, False, False): (26, 1), } @@ -263,19 +267,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (24, 3), - ('no_overrides', 2, True, False): (24, 3), - ('no_overrides', 3, True, False): (24, 3), - ('ccx', 1, True, False): (24, 3), - ('ccx', 2, True, False): (24, 3), - ('ccx', 3, True, False): (24, 3), - ('ccx', 1, True, True): (25, 3), - ('ccx', 2, True, True): (25, 3), - ('ccx', 3, True, True): (25, 3), - ('no_overrides', 1, False, False): (24, 3), - ('no_overrides', 2, False, False): (24, 3), - ('no_overrides', 3, False, False): (24, 3), - ('ccx', 1, False, False): (24, 3), - ('ccx', 2, False, False): (24, 3), - ('ccx', 3, False, False): (24, 3), + ('no_overrides', 1, True, False): (26, 3), + ('no_overrides', 2, True, False): (26, 3), + ('no_overrides', 3, True, False): (26, 3), + ('ccx', 1, True, False): (26, 3), + ('ccx', 2, True, False): (26, 3), + ('ccx', 3, True, False): (26, 3), + ('ccx', 1, True, True): (27, 3), + ('ccx', 2, True, True): (27, 3), + ('ccx', 3, True, True): (27, 3), + ('no_overrides', 1, False, False): (26, 3), + ('no_overrides', 2, False, False): (26, 3), + ('no_overrides', 3, False, False): (26, 3), + ('ccx', 1, False, False): (26, 3), + ('ccx', 2, False, False): (26, 3), + ('ccx', 3, False, False): (26, 3), } diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 1c03f26d74..14eb1992b8 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=6 if with_storage_backing else 5, + expected_sql_queries=7 if with_storage_backing else 6, ) @ddt.data( @@ -179,9 +179,9 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): clear_course_from_cache(course.id) if with_storage_backing: - num_sql_queries = 16 + num_sql_queries = 17 else: - num_sql_queries = 6 + num_sql_queries = 7 self._get_blocks( course, @@ -211,7 +211,7 @@ class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=7 if with_storage_backing else 6, + expected_sql_queries=8 if with_storage_backing else 7, ) @ddt.data( @@ -228,9 +228,9 @@ class TestQueryCountsWithIndividualOverrideProvider(TestGetBlocksQueryCountsBase clear_course_from_cache(course.id) if with_storage_backing: - num_sql_queries = 17 + num_sql_queries = 18 else: - num_sql_queries = 7 + num_sql_queries = 8 self._get_blocks( course, diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index 27b161354e..e7139b64b1 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -7,7 +7,6 @@ from django.conf import settings from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers from openedx.features.content_type_gating.block_transformers import ContentTypeGateTransformer -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from .transformers import ( library_content, @@ -41,22 +40,13 @@ def get_course_block_access_transformers(user): which the block structure is to be transformed. """ - if CONTENT_TYPE_GATING_FLAG.is_enabled(): - # [REV/Revisit] remove this duplicated code when flag is removed - course_block_access_transformers = [ - library_content.ContentLibraryTransformer(), - start_date.StartDateTransformer(), - ContentTypeGateTransformer(), - user_partitions.UserPartitionTransformer(), - visibility.VisibilityTransformer(), - ] - else: - course_block_access_transformers = [ - library_content.ContentLibraryTransformer(), - start_date.StartDateTransformer(), - user_partitions.UserPartitionTransformer(), - visibility.VisibilityTransformer(), - ] + course_block_access_transformers = [ + library_content.ContentLibraryTransformer(), + start_date.StartDateTransformer(), + ContentTypeGateTransformer(), + user_partitions.UserPartitionTransformer(), + visibility.VisibilityTransformer(), + ] if has_individual_student_override_provider(): course_block_access_transformers += [load_override_data.OverrideDataTransformer(user)] diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 6c43c1511c..376f167e54 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -45,7 +45,6 @@ from mobile_api.models import IgnoreMobileAvailableFlagConfig from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.external_auth.models import ExternalAuthMap from openedx.features.course_duration_limits.access import check_course_expired -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student import auth from student.models import CourseEnrollmentAllowed from student.roles import ( @@ -361,7 +360,7 @@ def _has_access_course(user, action, courselike): else: return view_with_prereqs - if CONTENT_TYPE_GATING_FLAG.is_enabled(): + if CourseDurationLimitConfig.enabled_for_enrollment(user=user, course_key=courselike.id): has_not_expired = check_course_expired(user, courselike) if not has_not_expired: staff_access = _has_staff_access_to_descriptor(user, courselike, courselike.id) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index f5662c2e6a..127c76be62 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -31,7 +31,7 @@ from lms.djangoapps.ccx.models import CustomCourseForEdX from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag from openedx.core.lib.tests import attr -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from student.models import CourseEnrollment from student.roles import CourseCcxCoachRole, CourseStaffRole from student.tests.factories import ( @@ -827,8 +827,9 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): ) @ddt.unpack @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_course_catalog_access_num_queries(self, user_attr_name, action, course_attr_name): + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime.date(2018, 1, 1)) + course = getattr(self, course_attr_name) # get a fresh user object that won't have any cached role information @@ -839,16 +840,23 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): user = User.objects.get(id=user.id) if user_attr_name == 'user_staff' and action == 'see_exists': - # checks staff role - num_queries = 1 + # always checks staff role, and if the course has started, check the duration configuration + if course_attr_name == 'course_started': + num_queries = 4 + else: + num_queries = 1 elif user_attr_name == 'user_normal' and action == 'see_exists': if course_attr_name == 'course_started': - num_queries = 1 + num_queries = 4 else: # checks staff role and enrollment data num_queries = 2 else: - num_queries = 0 + # if the course has started, check the duration configuration + if action == 'see_exists' and course_attr_name == 'course_started': + num_queries = 3 + else: + num_queries = 0 course_overview = CourseOverview.get_from_id(course.id) with self.assertNumQueries(num_queries, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index f031c57868..18bbd16a81 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -2,6 +2,7 @@ """ Test the course_info xblock """ +from datetime import date import ddt import mock from django.conf import settings @@ -15,7 +16,7 @@ from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag from openedx.core.lib.tests import attr -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired from pyquery import PyQuery as pq @@ -417,6 +418,8 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest def setUp(self): super(SelfPacedCourseInfoTestCase, self).setUp() + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) + self.setup_user() def fetch_course_info_with_queries(self, course, sql_queries, mongo_queries): @@ -431,10 +434,8 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest resp = self.client.get(url) self.assertEqual(resp.status_code, 200) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) - def gitest_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 31, 3) + def test_num_queries_instructor_paced(self): + self.fetch_course_info_with_queries(self.instructor_paced_course, 38, 3) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 31, 3) + self.fetch_course_info_with_queries(self.self_paced_course, 38, 3) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index e6bcb6808a..da935fd4e7 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -5,7 +5,7 @@ Tests courseware views.py import itertools import json import unittest -from datetime import datetime, timedelta +from datetime import datetime, timedelta, date from HTMLParser import HTMLParser from urllib import quote, urlencode from uuid import uuid4 @@ -65,9 +65,12 @@ from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.lib.gating import api as gating_api from openedx.core.lib.tests import attr from openedx.core.lib.url_utils import quote_slashes -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG -from openedx.features.course_experience import COURSE_OUTLINE_PAGE_FLAG, UNIFIED_COURSE_TAB_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired +from openedx.features.course_experience import ( + COURSE_OUTLINE_PAGE_FLAG, + UNIFIED_COURSE_TAB_FLAG, +) from student.models import CourseEnrollment from student.tests.factories import TEST_PASSWORD, AdminFactory, CourseEnrollmentFactory, UserFactory from util.tests.test_date_utils import fake_pgettext, fake_ugettext @@ -206,13 +209,13 @@ class IndexQueryTestCase(ModuleStoreTestCase): CREATE_USER = False NUM_PROBLEMS = 20 - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( (ModuleStoreEnum.Type.mongo, 10, 162), - (ModuleStoreEnum.Type.split, 4, 158), + (ModuleStoreEnum.Type.split, 4, 160), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) with self.store.default_store(store_type): course = CourseFactory.create() with self.store.bulk_operations(course.id): @@ -1432,26 +1435,26 @@ class ProgressPageTests(ProgressPageBaseTests): resp = self._get_progress_page() self.assertContains(resp, u"Download Your Certificate") - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( - (True, 40), - (False, 39) + (True, 46), + (False, 45) ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): """Test that query counts remain the same for self-paced and instructor-paced courses.""" + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) self.setup_course(self_paced=self_paced) with self.assertNumQueries(query_count, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): self._get_progress_page() - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 47, 30), - (True, 39, 26) + (False, 53, 33), + (True, 45, 29) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) self.setup_course() with grades_waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=enable_waffle): with self.assertNumQueries( @@ -2703,12 +2706,12 @@ class TestIndexViewWithCourseDurationLimits(ModuleStoreTestCase): CourseEnrollmentFactory(user=self.user, course_id=self.course.id) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_index_with_course_duration_limits(self): """ Test that the courseware contains the course expiration banner when course_duration_limits are enabled. """ + CourseDurationLimitConfig.objects.create(enabled=True) self.assertTrue(self.client.login(username=self.user.username, password='test')) response = self.client.get( reverse( @@ -2723,12 +2726,12 @@ class TestIndexViewWithCourseDurationLimits(ModuleStoreTestCase): bannerText = get_expiration_banner_text(self.user, self.course) self.assertContains(response, bannerText, html=True) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, False) def test_index_without_course_duration_limits(self): """ Test that the courseware does not contain the course expiration banner when course_duration_limits are disabled. """ + CourseDurationLimitConfig.objects.create(enabled=False) self.assertTrue(self.client.login(username=self.user.username, password='test')) response = self.client.get( reverse( diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 04c09a77f6..0e22f02d2e 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -1,6 +1,6 @@ import json import logging -from datetime import datetime +from datetime import datetime, date import ddt from django.urls import reverse @@ -46,7 +46,7 @@ from openedx.core.djangoapps.course_groups.tests.helpers import config_course_co from openedx.core.djangoapps.course_groups.tests.test_views import CohortViewsTestCase from openedx.core.djangoapps.util.testing import ContentGroupTestCase from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired from student.roles import CourseStaffRole, UserBasedRole from student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -424,7 +424,6 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): def setUp(self): super(SingleThreadQueryCountTestCase, self).setUp() - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( # Old mongo with cache. There is an additional SQL query for old mongo # because the first time that disabled_xblocks is queried is in call_single_thread, @@ -432,18 +431,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, 19, 7), - (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 19, 7), + (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 21, 6), + (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 21, 6), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, False, 1, 3, 3, 19, 7), - (ModuleStoreEnum.Type.split, False, 50, 3, 3, 19, 7), + (ModuleStoreEnum.Type.split, False, 1, 3, 3, 21, 6), + (ModuleStoreEnum.Type.split, False, 50, 3, 3, 21, 6), # Enabling Enterprise integration should have no effect on the number of mongo queries made. - (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 19, 7), - (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 19, 7), + (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 21, 6), + (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 21, 6), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, True, 1, 3, 3, 19, 7), - (ModuleStoreEnum.Type.split, True, 50, 3, 3, 19, 7), + (ModuleStoreEnum.Type.split, True, 1, 3, 3, 21, 6), + (ModuleStoreEnum.Type.split, True, 50, 3, 3, 21, 6), ) @ddt.unpack def test_number_of_mongo_queries( @@ -457,6 +456,7 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): num_cached_sql_queries, mock_request ): + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) with modulestore().default_store(default_store): course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': 'dummy_discussion_id'}}) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index b6a8f235b9..a60b37edf1 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -39,7 +39,6 @@ from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag from openedx.core.lib.tests import attr -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student.roles import CourseStaffRole, UserBasedRole from student.tests.factories import CourseAccessRoleFactory, CourseEnrollmentFactory, UserFactory from util.testing import UrlResetMixin @@ -403,20 +402,18 @@ class ViewsQueryCountTestCase( func(self, *args, **kwargs) return inner - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 4, 38), - (ModuleStoreEnum.Type.split, 3, 13, 38), + (ModuleStoreEnum.Type.mongo, 3, 4, 39), + (ModuleStoreEnum.Type.split, 3, 13, 39), ) @ddt.unpack @count_queries def test_create_thread(self, mock_request): self.create_thread_helper(mock_request) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 34), - (ModuleStoreEnum.Type.split, 3, 10, 34), + (ModuleStoreEnum.Type.mongo, 3, 3, 35), + (ModuleStoreEnum.Type.split, 3, 10, 35), ) @ddt.unpack @count_queries diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 6aa15af834..b365dd4d4f 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -92,35 +92,35 @@ class TestCourseGradeFactory(GradeTestBase): [self.sequence.display_name, self.sequence2.display_name] ) - with self.assertNumQueries(2), mock_get_score(1, 2): + with self.assertNumQueries(3), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - num_queries = 41 + num_queries = 42 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(2): + with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 - num_queries = 6 + num_queries = 7 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(2): + with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - num_queries = 20 + num_queries = 21 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(2): + with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 23 + num_queries = 24 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(2): + with self.assertNumQueries(3): _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}) @@ -311,7 +311,7 @@ class TestGradeIteration(SharedModuleStoreTestCase): else mock_course_grade.return_value for student in self.students ] - with self.assertNumQueries(6): + with self.assertNumQueries(8): all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) self.assertEqual( {student: text_type(all_errors[student]) for student in all_errors}, diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 5400afa90c..18cf2da5f6 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, 31, True), - (ModuleStoreEnum.Type.mongo, 1, 31, False), - (ModuleStoreEnum.Type.split, 3, 31, True), - (ModuleStoreEnum.Type.split, 3, 31, False), + (ModuleStoreEnum.Type.mongo, 1, 32, True), + (ModuleStoreEnum.Type.mongo, 1, 32, False), + (ModuleStoreEnum.Type.split, 3, 32, True), + (ModuleStoreEnum.Type.split, 3, 32, 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, 31), - (ModuleStoreEnum.Type.split, 3, 31), + (ModuleStoreEnum.Type.mongo, 1, 32), + (ModuleStoreEnum.Type.split, 3, 32), ) @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, 15), - (ModuleStoreEnum.Type.split, 3, 15), + (ModuleStoreEnum.Type.mongo, 1, 16), + (ModuleStoreEnum.Type.split, 3, 16), ) @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, 32), - (ModuleStoreEnum.Type.split, 3, 32), + (ModuleStoreEnum.Type.mongo, 1, 33), + (ModuleStoreEnum.Type.split, 3, 33), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index de07b194e4..63b6448761 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -413,7 +413,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): RequestCache.clear_all_namespaces() - expected_query_count = 45 + expected_query_count = 47 with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(mongo_count): with self.assertNumQueries(expected_query_count): diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index dd16ade8e8..1f4989861f 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -29,8 +29,7 @@ from mobile_api.utils import API_V05, API_V1 from openedx.core.lib.courses import course_image_url from openedx.core.lib.tests import attr from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory -from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from student.models import CourseEnrollment from student.tests.factories import CourseEnrollmentFactory from util.milestones_helpers import set_prerequisite_courses @@ -289,12 +288,12 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest (API_V1, False, 1), ) @ddt.unpack - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_enrollment_with_gating(self, api_version, expired, num_courses_returned): ''' Test that expired courses are only returned in v1 of API when waffle flag enabled, and un-expired courses always returned ''' + CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=datetime.date(2018, 1, 1)) courses = self._get_enrollment_data(api_version, expired) self._assert_enrollment_results(api_version, courses, num_courses_returned) @@ -305,12 +304,12 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest (API_V1, False, 1), ) @ddt.unpack - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, False) def test_enrollment_no_gating(self, api_version, expired, num_courses_returned): ''' Test that expired and non-expired courses returned if waffle flag is disabled regarless of version of API ''' + CourseDurationLimitConfig.objects.create(enabled=False) courses = self._get_enrollment_data(api_version, expired) self._assert_enrollment_results(api_version, courses, num_courses_returned) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 1898d68dc2..8417528895 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -22,7 +22,6 @@ from experiments.models import ExperimentData, ExperimentKeyValue from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED from mobile_api.utils import API_V05 from openedx.features.course_duration_limits.access import check_course_expired -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student.models import CourseEnrollment, User from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError diff --git a/openedx/core/djangoapps/config_model_utils/admin.py b/openedx/core/djangoapps/config_model_utils/admin.py new file mode 100644 index 0000000000..9d4b7e530f --- /dev/null +++ b/openedx/core/djangoapps/config_model_utils/admin.py @@ -0,0 +1,42 @@ +""" +Convenience classes for defining StackedConfigModel Admin pages. +""" + +from django import forms + +from opaque_keys.edx.keys import CourseKey + +from config_models.admin import ConfigurationModelAdmin + + +class CourseOverviewField(forms.ModelChoiceField): + def to_python(self, value): + if value in self.empty_values: + return None + return super(CourseOverviewField, self).to_python(CourseKey.from_string(value)) + + +class StackedConfigModelAdminForm(forms.ModelForm): + class Meta: + field_classes = { + 'course': CourseOverviewField + } + + +class StackedConfigModelAdmin(ConfigurationModelAdmin): + """ + A specialized ConfigurationModel ModelAdmin for StackedConfigModels. + """ + form = StackedConfigModelAdminForm + + def get_fields(self, request, obj=None): + fields = super(StackedConfigModelAdmin, self).get_fields(request, obj) + return list(self.model.KEY_FIELDS) + [field for field in fields if field not in self.model.KEY_FIELDS] + + def get_displayable_field_names(self): + """ + Return all field names, excluding reverse foreign key relationships. + """ + names = super(StackedConfigModelAdmin, self).get_displayable_field_names() + fixed_names = ['id', 'change_date', 'changed_by'] + list(self.model.KEY_FIELDS) + return fixed_names + [name for name in names if name not in fixed_names] diff --git a/openedx/core/djangoapps/config_model_utils/models.py b/openedx/core/djangoapps/config_model_utils/models.py index d8d1ab797a..eb3e850388 100644 --- a/openedx/core/djangoapps/config_model_utils/models.py +++ b/openedx/core/djangoapps/config_model_utils/models.py @@ -11,13 +11,14 @@ from collections import namedtuple from django.conf import settings from django.db import models +from django.db.models import Q, F from django.contrib.sites.models import Site from django.contrib.sites.requests import RequestSite from django.core.exceptions import ValidationError from django.utils.translation import ugettext_lazy as _ import crum -from config_models.models import ConfigurationModel +from config_models.models import ConfigurationModel, cache from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -34,43 +35,17 @@ class StackedConfigurationModel(ConfigurationModel): STACKABLE_FIELDS = ('enabled',) enabled = models.NullBooleanField(default=None, verbose_name=_("Enabled")) - site = models.ForeignKey(Site, on_delete=models.CASCADE, null=True) - org = models.CharField(max_length=255, db_index=True, null=True) + site = models.ForeignKey(Site, on_delete=models.CASCADE, null=True, blank=True) + org = models.CharField(max_length=255, db_index=True, null=True, blank=True) course = models.ForeignKey( CourseOverview, on_delete=models.DO_NOTHING, null=True, + blank=True, ) @classmethod - def attribute_tuple(cls): - """ - Returns a namedtuple with all attributes that can be overridden on this config model. - - For example, if MyStackedConfig.STACKABLE_FIELDS = ('enabled', 'enabled_as_of', 'studio_enabled'), - then: - - # These lines are the same - MyStackedConfig.attribute_tuple() - namedtuple('MyStackedConfigValues', ('enabled', 'enabled_as_of', 'studio_enabled')) - - # attribute_tuple() behavior - MyStackedConfigValues = MyStackedConfig.attribute_tuple() - MyStackedConfigValues(True, '10/1/18', False).enabled # True - MyStackedConfigValues(True, '10/1/18', False).enabled_as_of # '10/1/18' - MyStackedConfigValues(True, '10/1/18', False).studio_enabled # False - """ - if hasattr(cls, '_attribute_tuple'): - return cls._attribute_tuple - - cls._attribute_tuple = namedtuple( - '{}Values'.format(cls.__name__), - cls.STACKABLE_FIELDS - ) - return cls._attribute_tuple - - @classmethod - def current(cls, site=None, org=None, course=None): # pylint: disable=arguments-differ + def current(cls, site=None, org=None, course_key=None): # pylint: disable=arguments-differ """ Return the current overridden configuration at the specified level. @@ -112,12 +87,18 @@ class StackedConfigurationModel(ConfigurationModel): specified down to the level of the supplied argument (or global values if no arguments are supplied). """ + cache_key_name = cls.cache_key_name(site, org, course_key=course_key) + cached = cache.get(cache_key_name) + + if cached is not None: + return cached + # Raise an error if more than one of site/org/course are specified simultaneously. - if len([arg for arg in [site, org, course] if arg is not None]) > 1: + if len([arg for arg in [site, org, course_key] if arg is not None]) > 1: raise ValueError("Only one of site, org, and course can be specified") - if org is None and course is not None: - org = cls._org_from_course(course) + if org is None and course_key is not None: + org = cls._org_from_course_key(course_key) if site is None and org is not None: site = cls._site_from_org(org) @@ -130,40 +111,62 @@ class StackedConfigurationModel(ConfigurationModel): values = field_defaults.copy() - global_current = super(StackedConfigurationModel, cls).current(None, None, None) - for field in stackable_fields: - values[field.name] = field.value_from_object(global_current) + global_override_q = Q(site=None, org=None, course_id=None) + site_override_q = Q(site=site, org=None, course_id=None) + org_override_q = Q(site=None, org=org, course_id=None) + course_override_q = Q(site=None, org=None, course_id=course_key) - def _override_fields_with_level(level_config): + overrides = cls.objects.current_set().filter( + global_override_q | + site_override_q | + org_override_q | + course_override_q + ).order_by( + # Sort nulls first, and in reverse specificity order + # so that the overrides are in the order of general to specific. + # + # Site | Org | Course + # -------------------- + # Null | Null | Null + # site | Null | Null + # Null | org | Null + # Null | Null | Course + F('course').desc(nulls_first=True), + F('org').desc(nulls_first=True), + F('site').desc(nulls_first=True), + ) + + for override in overrides: for field in stackable_fields: - value = field.value_from_object(level_config) + value = field.value_from_object(override) if value != field_defaults[field.name]: values[field.name] = value - if site is not None: - _override_fields_with_level( - super(StackedConfigurationModel, cls).current(site, None, None) - ) - - if org is not None: - _override_fields_with_level( - super(StackedConfigurationModel, cls).current(None, org, None) - ) - - if course is not None: - _override_fields_with_level( - super(StackedConfigurationModel, cls).current(None, None, course) - ) - - return cls.attribute_tuple()(**values) + current = cls(**values) + cache.set(cache_key_name, current, cls.cache_timeout) + return current @classmethod - def _org_from_course(cls, course_key): + def cache_key_name(cls, site, org, course=None, course_key=None): # pylint: disable=arguments-differ + if course is not None and course_key is not None: + raise ValueError("Only one of course and course_key can be specified at a time") + if course is not None: + course_key = course + + if site is None: + site_id = None + else: + site_id = site.id + + return super(StackedConfigurationModel, cls).cache_key_name(site_id, org, course_key) + + @classmethod + def _org_from_course_key(cls, course_key): return course_key.org @classmethod def _site_from_org(cls, org): - configuration = SiteConfiguration.get_configuration_for_org(org) + configuration = SiteConfiguration.get_configuration_for_org(org, select_related=['site']) if configuration is None: try: return Site.objects.get(id=settings.SITE_ID) diff --git a/openedx/core/djangoapps/site_configuration/models.py b/openedx/core/djangoapps/site_configuration/models.py index 9fce936698..ff14711155 100644 --- a/openedx/core/djangoapps/site_configuration/models.py +++ b/openedx/core/djangoapps/site_configuration/models.py @@ -61,15 +61,19 @@ class SiteConfiguration(models.Model): return default @classmethod - def get_configuration_for_org(cls, org): + def get_configuration_for_org(cls, org, select_related=None): """ This returns a SiteConfiguration object which has an org_filter that matches the supplied org Args: org (str): Org to use to filter SiteConfigurations + select_related (list or None): A list of values to pass as arguments to select_related """ - for configuration in cls.objects.filter(values__contains=org, enabled=True).all(): + query = cls.objects.filter(values__contains=org, enabled=True).all() + if select_related is not None: + query = query.select_related(*select_related) + for configuration in query: course_org_filter = configuration.get_value('course_org_filter', []) # The value of 'course_org_filter' can be configured as a string representing # a single organization or a list of strings representing multiple organizations. diff --git a/openedx/features/content_type_gating/admin.py b/openedx/features/content_type_gating/admin.py new file mode 100644 index 0000000000..bb27e9aaea --- /dev/null +++ b/openedx/features/content_type_gating/admin.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +""" +Django Admin pages for ContentTypeGatingConfig. +""" + +from __future__ import unicode_literals + +from django.contrib import admin +from django.utils.translation import ugettext_lazy as _ + +from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin +from .models import ContentTypeGatingConfig + + +class ContentTypeGatingConfigAdmin(StackedConfigModelAdmin): + fieldsets = ( + ('Context', { + 'fields': ('site', 'org', 'course'), + 'description': _( + 'These define the context to enable course duration limits on. ' + 'If no values are set, then the configuration applies globally. ' + 'If a single value is set, then the configuration applies to all courses ' + 'within that context. At most one value can be set at a time.
' + 'If multiple contexts apply to a course (for example, if configuration ' + 'is specified for the course specifically, and for the org that the course ' + 'is in, then the more specific context overrides the more general context.' + ), + }), + ('Configuration', { + 'fields': ('enabled', 'enabled_as_of', 'studio_override_enabled'), + 'description': _( + 'If any of these values is left empty or "Unknown", then their value ' + 'at runtime will be retrieved from the next most specific context that applies. ' + 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' + 'course will be Enabled only if the org that it is in is Enabled.' + ), + }) + ) + +admin.site.register(ContentTypeGatingConfig, ContentTypeGatingConfigAdmin) diff --git a/openedx/features/content_type_gating/block_transformers.py b/openedx/features/content_type_gating/block_transformers.py index b4e5df133f..daf1f3f964 100644 --- a/openedx/features/content_type_gating/block_transformers.py +++ b/openedx/features/content_type_gating/block_transformers.py @@ -8,6 +8,7 @@ from openedx.core.djangoapps.content.block_structure.transformer import ( BlockStructureTransformer, ) from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID +from openedx.features.content_type_gating.models import ContentTypeGatingConfig class ContentTypeGateTransformer(BlockStructureTransformer): @@ -35,6 +36,12 @@ class ContentTypeGateTransformer(BlockStructureTransformer): block_structure.request_xblock_fields('group_access', 'graded', 'has_score', 'weight') def transform(self, usage_info, block_structure): + if not ContentTypeGatingConfig.enabled_for_enrollment( + user=usage_info.user, + course_key=usage_info.course_key, + ): + return + for block_key in block_structure.topological_traversal(): graded = block_structure.get_xblock_field(block_key, 'graded') has_score = block_structure.get_xblock_field(block_key, 'has_score') diff --git a/openedx/features/content_type_gating/field_override.py b/openedx/features/content_type_gating/field_override.py index 42d22a811a..76b01b4fae 100644 --- a/openedx/features/content_type_gating/field_override.py +++ b/openedx/features/content_type_gating/field_override.py @@ -6,9 +6,7 @@ from django.conf import settings from lms.djangoapps.courseware.field_overrides import FieldOverrideProvider from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID -from openedx.features.course_duration_limits.config import ( - CONTENT_TYPE_GATING_FLAG, -) +from openedx.features.content_type_gating.models import ContentTypeGatingConfig class ContentTypeGatingFieldOverride(FieldOverrideProvider): @@ -18,9 +16,6 @@ class ContentTypeGatingFieldOverride(FieldOverrideProvider): graded content to only be accessible to the Full Access group """ def get(self, block, name, default): - if not CONTENT_TYPE_GATING_FLAG.is_enabled(): - return default - if name != 'group_access': return default @@ -61,4 +56,4 @@ class ContentTypeGatingFieldOverride(FieldOverrideProvider): @classmethod def enabled_for(cls, course): """This simple override provider is always enabled""" - return True + return ContentTypeGatingConfig.enabled_for_course(course_key=course.scope_ids.usage_id.course_key) diff --git a/openedx/features/content_type_gating/migrations/0001_initial.py b/openedx/features/content_type_gating/migrations/0001_initial.py new file mode 100644 index 0000000000..ab0e3f13df --- /dev/null +++ b/openedx/features/content_type_gating/migrations/0001_initial.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2018-11-08 19:43 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('course_overviews', '0014_courseoverview_certificate_available_date'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('sites', '0002_alter_domain_unique'), + ] + + operations = [ + migrations.CreateModel( + name='ContentTypeGatingConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.NullBooleanField(default=None, verbose_name='Enabled')), + ('org', models.CharField(blank=True, db_index=True, max_length=255, null=True)), + ('enabled_as_of', models.DateField(blank=True, default=None, null=True, verbose_name='Enabled As Of')), + ('studio_override_enabled', models.NullBooleanField(default=None, verbose_name='Studio Override Enabled')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ('course', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='course_overviews.CourseOverview')), + ('site', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='sites.Site')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/openedx/features/content_type_gating/migrations/0002_auto_20181119_0959.py b/openedx/features/content_type_gating/migrations/0002_auto_20181119_0959.py new file mode 100644 index 0000000000..a71bec3f61 --- /dev/null +++ b/openedx/features/content_type_gating/migrations/0002_auto_20181119_0959.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2018-11-19 14:59 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_type_gating', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='contenttypegatingconfig', + name='enabled_as_of', + field=models.DateField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date (UTC) will be affected.', null=True, verbose_name='Enabled As Of'), + ), + migrations.AlterField( + model_name='contenttypegatingconfig', + name='studio_override_enabled', + field=models.NullBooleanField(default=None, help_text='Allow Feature Based Enrollment visibility to be overriden on a per-component basis in Studio.', verbose_name='Studio Override Enabled'), + ), + ] diff --git a/openedx/features/content_type_gating/models.py b/openedx/features/content_type_gating/models.py new file mode 100644 index 0000000000..f69d1fc0cc --- /dev/null +++ b/openedx/features/content_type_gating/models.py @@ -0,0 +1,132 @@ +""" +Content Type Gating Configuration Models +""" + +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from datetime import datetime +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.encoding import python_2_unicode_compatible +from django.utils.translation import ugettext_lazy as _ + +from student.models import CourseEnrollment +from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG + + +@python_2_unicode_compatible +class ContentTypeGatingConfig(StackedConfigurationModel): + """ + A ConfigurationModel used to manage configuration for Content Type Gating (Feature Based Enrollments). + """ + + STACKABLE_FIELDS = ('enabled', 'enabled_as_of', 'studio_override_enabled') + + enabled_as_of = models.DateField( + default=None, + null=True, + verbose_name=_('Enabled As Of'), + blank=True, + help_text=_( + 'If the configuration is Enabled, then all enrollments ' + 'created after this date (UTC) will be affected.' + ) + ) + studio_override_enabled = models.NullBooleanField( + default=None, + verbose_name=_('Studio Override Enabled'), + blank=True, + help_text=_( + 'Allow Feature Based Enrollment visibility to be overriden ' + 'on a per-component basis in Studio.' + ) + ) + + @classmethod + def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None): + """ + Return whether Content Type Gating is enabled for this enrollment. + + Content Type Gating is enabled for an enrollment if it is enabled for + the course being enrolled in (either specifically, or via a containing context, + 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 CONTENT_TYPE_GATING_FLAG.is_enabled(): + return True + + 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): + 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 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_date=datetime.utcnow().date()) + else: + current_config = cls.current(course_key=enrollment.course_id) + return current_config.enabled_as_of_date(target_date=enrollment.created.date()) + + @classmethod + def enabled_for_course(cls, course_key, target_date=None): + """ + Return whether Content Type Gating is enabled for this course as of a particular date. + + Content Type Gating is enabled for a course on a date if it is enabled either specifically, + or via a containing context, such as the org, site, or globally, and if the configuration + is specified to be ``enabled_as_of`` before ``target_date``. + + Only one of enrollment and (user, course_key) may be specified at a time. + + Arguments: + course_key: The CourseKey of the course being queried. + target_date: The date to checked enablement as of. Defaults to the current date. + """ + if CONTENT_TYPE_GATING_FLAG.is_enabled(): + return True + + if target_date is None: + target_date = datetime.utcnow().date() + + current_config = cls.current(course_key=course_key) + return current_config.enabled_as_of_date(target_date=target_date) + + def clean(self): + if self.enabled and self.enabled_as_of is None: + raise ValidationError({'enabled_as_of': _('enabled_as_of must be set when enabled is True')}) + + def enabled_as_of_date(self, target_date): + """ + Return whether this Content Type Gating configuration context is enabled as of a date. + + Arguments: + target_date (:class:`datetime.date`): The date that ``enabled_as_of`` must be equal to or before + """ + if CONTENT_TYPE_GATING_FLAG.is_enabled(): + return True + + # Explicitly cast this to bool, so that when self.enabled is None the method doesn't return None + return bool(self.enabled and self.enabled_as_of <= target_date) + + def __str__(self): + return "ContentTypeGatingConfig(enabled={!r}, enabled_as_of={!r}, studio_override_enabled={!r})" diff --git a/openedx/features/content_type_gating/partitions.py b/openedx/features/content_type_gating/partitions.py index 7be8236ad5..e1a464d9bc 100644 --- a/openedx/features/content_type_gating/partitions.py +++ b/openedx/features/content_type_gating/partitions.py @@ -23,10 +23,7 @@ from lms.djangoapps.courseware.masquerade import ( ) from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError from openedx.core.lib.mobile_utils import is_request_from_mobile_app -from openedx.features.course_duration_limits.config import ( - CONTENT_TYPE_GATING_FLAG, - CONTENT_TYPE_GATING_STUDIO_UI_FLAG, -) +from openedx.features.content_type_gating.models import ContentTypeGatingConfig LOG = logging.getLogger(__name__) @@ -46,7 +43,7 @@ def create_content_gating_partition(course): Create and return the Content Gating user partition. """ - if not (CONTENT_TYPE_GATING_FLAG.is_enabled() or CONTENT_TYPE_GATING_STUDIO_UI_FLAG.is_enabled()): + if not ContentTypeGatingConfig.enabled_for_course(course_key=course.id): return None try: @@ -144,7 +141,7 @@ class ContentTypeGatingPartitionScheme(object): # For now, treat everyone as a Full-access user, until we have the rest of the # feature gating logic in place. - if not CONTENT_TYPE_GATING_FLAG.is_enabled(): + if not ContentTypeGatingConfig.enabled_for_enrollment(user=user, course_key=course_key): return cls.FULL_ACCESS # If CONTENT_TYPE_GATING is enabled use the following logic to determine whether a user should have FULL_ACCESS diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 93fa0c0702..51a9f41829 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -2,6 +2,7 @@ Test audit user's access to various content based on content-gating features. """ +from datetime import date import ddt from django.conf import settings from django.test.client import RequestFactory @@ -14,7 +15,7 @@ from lms.djangoapps.courseware.module_render import load_single_xblock from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.lib.url_utils import quote_slashes from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from student.roles import CourseInstructorRole, CourseStaffRole from student.tests.factories import ( AdminFactory, @@ -28,7 +29,6 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @ddt.ddt -@override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @override_settings(FIELD_OVERRIDE_PROVIDERS=( 'openedx.features.content_type_gating.field_override.ContentTypeGatingFieldOverride', )) @@ -176,6 +176,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): course_id=self.courses['audit_only']['course'].id, mode='audit' ) + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) @classmethod def _create_course(cls, run, display_name, modes, component_types): @@ -240,7 +241,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): } @patch("crum.get_current_request") - def _assert_block_is_gated(self, mock_get_current_request, block, is_gated, user_id, course_id): + def _assert_block_is_gated(self, mock_get_current_request, block, is_gated, user_id, course): """ Asserts that a block in a specific course is gated for a specific user @@ -258,9 +259,9 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): vertical_xblock = load_single_xblock( request=fake_request, user_id=user_id, - course_id=unicode(course_id), + course_id=unicode(course.id), usage_key_string=unicode(self.blocks_dict['vertical'].scope_ids.usage_id), - course=None + course=course ) runtime = vertical_xblock.runtime @@ -290,13 +291,13 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): self._assert_block_is_gated( block=self.blocks_dict[prob_type], user_id=self.users['audit'].id, - course_id=self.course.id, + course=self.course, is_gated=is_gated ) self._assert_block_is_gated( block=self.blocks_dict[prob_type], user_id=self.users['verified'].id, - course_id=self.course.id, + course=self.course, is_gated=False ) @@ -310,7 +311,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): self._assert_block_is_gated( block=block, user_id=self.audit_user.id, - course_id=self.course.id, + course=self.course, is_gated=is_gated ) @@ -344,7 +345,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): self._assert_block_is_gated( block=self.courses[course]['blocks'][component_type], user_id=self.users[user_track].id, - course_id=self.courses[course]['course'].id, + course=self.courses[course]['course'], is_gated=is_gated, ) @@ -393,6 +394,6 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): self._assert_block_is_gated( block=self.blocks_dict['problem'], user_id=course_team_member.id, - course_id=self.course.id, + course=self.course, is_gated=False ) diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py new file mode 100644 index 0000000000..c8e56d55a2 --- /dev/null +++ b/openedx/features/content_type_gating/tests/test_models.py @@ -0,0 +1,304 @@ +import ddt +from datetime import timedelta, date, datetime, time +import itertools +from mock import Mock + +from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from openedx.features.content_type_gating.models import ContentTypeGatingConfig +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from student.tests.factories import CourseEnrollmentFactory, UserFactory + + +@ddt.ddt +class TestContentTypeGatingConfig(CacheIsolationTestCase): + + ENABLED_CACHES = ['default'] + + def setUp(self): + self.course_overview = CourseOverviewFactory.create() + self.user = UserFactory.create() + super(TestContentTypeGatingConfig, self).setUp() + + @ddt.data( + (True, True, True), + (True, True, False), + (True, False, True), + (True, False, False), + (False, False, True), + (False, False, False), + ) + @ddt.unpack + def test_enabled_for_enrollment( + self, + already_enrolled, + pass_enrollment, + enrolled_before_enabled, + ): + + # Tweak the day to enable the config so that it is either before + # or after today (which is when the enrollment will be created) + if enrolled_before_enabled: + enabled_as_of = date.today() + timedelta(days=1) + else: + enabled_as_of = date.today() - timedelta(days=1) + + config = ContentTypeGatingConfig.objects.create( + enabled=True, + course=self.course_overview, + enabled_as_of=enabled_as_of, + ) + + if already_enrolled: + existing_enrollment = CourseEnrollmentFactory.create( + user=self.user, + course=self.course_overview, + ) + 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 + + if pass_enrollment: + query_count = 4 + else: + query_count = 5 + + with self.assertNumQueries(query_count): + enabled = ContentTypeGatingConfig.enabled_for_enrollment( + enrollment=enrollment, + user=user, + course_key=course_key, + ) + self.assertEqual(not enrolled_before_enabled, enabled) + + def test_enabled_for_enrollment_failure(self): + with self.assertRaises(ValueError): + ContentTypeGatingConfig.enabled_for_enrollment(None, None, None) + with self.assertRaises(ValueError): + ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='enrollment'), Mock(name='user'), None) + with self.assertRaises(ValueError): + ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='enrollment'), None, Mock(name='course_key')) + + @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) + def test_enabled_for_enrollment_flag_override(self): + self.assertTrue(ContentTypeGatingConfig.enabled_for_enrollment(None, None, None)) + self.assertTrue(ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='enrollment'), Mock(name='user'), None)) + self.assertTrue(ContentTypeGatingConfig.enabled_for_enrollment(Mock(name='enrollment'), None, Mock(name='course_key'))) + + @ddt.data(True, False) + def test_enabled_for_course( + self, + before_enabled, + ): + config = ContentTypeGatingConfig.objects.create( + enabled=True, + course=self.course_overview, + enabled_as_of=date(2018, 1, 1), + ) + + # Tweak the day to check for course enablement so it is either + # before or after when the configuration was enabled + if before_enabled: + target_date = config.enabled_as_of - timedelta(days=1) + else: + target_date = config.enabled_as_of + timedelta(days=1) + + course_key = self.course_overview.id + + self.assertEqual( + not before_enabled, + ContentTypeGatingConfig.enabled_for_course( + course_key=course_key, + target_date=target_date, + ) + ) + + @ddt.data( + # Generate all combinations of setting each configuration level to True/False/None + *itertools.product(*[(True, False, None)] * 4) + ) + @ddt.unpack + def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting): + """ + Test that the stacked configuration overrides happen in the correct order and priority. + + This is tested by exhaustively setting each combination of contexts, and validating that only + the lowest level context that is set to not-None is applied. + """ + # Add a bunch of configuration outside the contexts that are being tested, to make sure + # there are no leaks of configuration across contexts + non_test_course_enabled = CourseOverviewFactory.create(org='non-test-org-enabled') + non_test_course_disabled = CourseOverviewFactory.create(org='non-test-org-disabled') + non_test_site_cfg_enabled = SiteConfigurationFactory.create(values={'course_org_filter': non_test_course_enabled.org}) + non_test_site_cfg_disabled = SiteConfigurationFactory.create(values={'course_org_filter': non_test_course_disabled.org}) + + ContentTypeGatingConfig.objects.create(course=non_test_course_enabled, enabled=True, enabled_as_of=date(2018, 1, 1)) + ContentTypeGatingConfig.objects.create(course=non_test_course_disabled, enabled=False) + ContentTypeGatingConfig.objects.create(org=non_test_course_enabled.org, enabled=True, enabled_as_of=date(2018, 1, 1)) + ContentTypeGatingConfig.objects.create(org=non_test_course_disabled.org, enabled=False) + ContentTypeGatingConfig.objects.create(site=non_test_site_cfg_enabled.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + ContentTypeGatingConfig.objects.create(site=non_test_site_cfg_disabled.site, enabled=False) + + # Set up test objects + test_course = CourseOverviewFactory.create(org='test-org') + test_site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': test_course.org}) + + ContentTypeGatingConfig.objects.create(enabled=global_setting, enabled_as_of=date(2018, 1, 1)) + ContentTypeGatingConfig.objects.create(course=test_course, enabled=course_setting, enabled_as_of=date(2018, 1, 1)) + ContentTypeGatingConfig.objects.create(org=test_course.org, enabled=org_setting, enabled_as_of=date(2018, 1, 1)) + ContentTypeGatingConfig.objects.create(site=test_site_cfg.site, enabled=site_setting, enabled_as_of=date(2018, 1, 1)) + + all_settings = [global_setting, site_setting, org_setting, course_setting] + expected_global_setting = self._resolve_settings([global_setting]) + expected_site_setting = self._resolve_settings([global_setting, site_setting]) + expected_org_setting = self._resolve_settings([global_setting, site_setting, org_setting]) + expected_course_setting = self._resolve_settings([global_setting, site_setting, org_setting, course_setting]) + + self.assertEqual(expected_global_setting, ContentTypeGatingConfig.current().enabled) + self.assertEqual(expected_site_setting, ContentTypeGatingConfig.current(site=test_site_cfg.site).enabled) + self.assertEqual(expected_org_setting, ContentTypeGatingConfig.current(org=test_course.org).enabled) + self.assertEqual(expected_course_setting, ContentTypeGatingConfig.current(course_key=test_course.id).enabled) + + def test_caching_global(self): + global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the global value is not retrieved from cache after save + with self.assertNumQueries(1): + self.assertTrue(ContentTypeGatingConfig.current().enabled) + + # Check that the global value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(ContentTypeGatingConfig.current().enabled) + + global_config.enabled = False + global_config.save() + + # Check that the global value in cache was deleted on save + with self.assertNumQueries(1): + self.assertFalse(ContentTypeGatingConfig.current().enabled) + + def test_caching_site(self): + site_cfg = SiteConfigurationFactory() + site_config = ContentTypeGatingConfig(site=site_cfg.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + site_config.save() + + # Check that the site value is not retrieved from cache after save + with self.assertNumQueries(1): + self.assertTrue(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) + + # Check that the site value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) + + site_config.enabled = False + site_config.save() + + # Check that the site value in cache was deleted on save + with self.assertNumQueries(1): + self.assertFalse(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) + + global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the site value is not updated in cache by changing the global value + with self.assertNumQueries(0): + self.assertFalse(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) + + def test_caching_org(self): + course = CourseOverviewFactory.create(org='test-org') + site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': course.org}) + org_config = ContentTypeGatingConfig(org=course.org, enabled=True, enabled_as_of=date(2018, 1, 1)) + org_config.save() + + # Check that the org value is not retrieved from cache after save + with self.assertNumQueries(2): + self.assertTrue(ContentTypeGatingConfig.current(org=course.org).enabled) + + # Check that the org value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(ContentTypeGatingConfig.current(org=course.org).enabled) + + org_config.enabled = False + org_config.save() + + # Check that the org value in cache was deleted on save + with self.assertNumQueries(2): + self.assertFalse(ContentTypeGatingConfig.current(org=course.org).enabled) + + global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the org value is not updated in cache by changing the global value + with self.assertNumQueries(0): + self.assertFalse(ContentTypeGatingConfig.current(org=course.org).enabled) + + site_config = ContentTypeGatingConfig(site=site_cfg.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + site_config.save() + + # Check that the org value is not updated in cache by changing the site value + with self.assertNumQueries(0): + self.assertFalse(ContentTypeGatingConfig.current(org=course.org).enabled) + + def test_caching_course(self): + course = CourseOverviewFactory.create(org='test-org') + site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': course.org}) + course_config = ContentTypeGatingConfig(course=course, enabled=True, enabled_as_of=date(2018, 1, 1)) + course_config.save() + + # Check that the org value is not retrieved from cache after save + with self.assertNumQueries(2): + self.assertTrue(ContentTypeGatingConfig.current(course_key=course.id).enabled) + + # Check that the org value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(ContentTypeGatingConfig.current(course_key=course.id).enabled) + + course_config.enabled = False + course_config.save() + + # Check that the org value in cache was deleted on save + with self.assertNumQueries(2): + self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) + + global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the org value is not updated in cache by changing the global value + with self.assertNumQueries(0): + self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) + + site_config = ContentTypeGatingConfig(site=site_cfg.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + site_config.save() + + # Check that the org value is not updated in cache by changing the site value + with self.assertNumQueries(0): + self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) + + org_config = ContentTypeGatingConfig(org=course.org, enabled=True, enabled_as_of=date(2018, 1, 1)) + org_config.save() + + # Check that the org value is not updated in cache by changing the site value + with self.assertNumQueries(0): + self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) + + def _resolve_settings(self, settings): + if all(setting is None for setting in settings): + return None + + return [ + setting + for setting + in settings + if setting is not None + ][-1] diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index ecaee92806..e19c8bfb63 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -17,7 +17,7 @@ from openedx.core.djangoapps.catalog.utils import get_course_run_details from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangolib.markup import HTML -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig MIN_DURATION = timedelta(weeks=4) MAX_DURATION = timedelta(weeks=12) @@ -100,7 +100,7 @@ def register_course_expired_message(request, course): """ Add a banner notifying the user of the user course expiration date if it exists. """ - if CONTENT_TYPE_GATING_FLAG.is_enabled(): + if CourseDurationLimitConfig.enabled_for_enrollment(user=request.user, course_key=course.id): expiration_date = get_user_course_expiration_date(request.user, course) if expiration_date: upgrade_message = _('Your access to this course expires on {expiration_date}. \ diff --git a/openedx/features/course_duration_limits/admin.py b/openedx/features/course_duration_limits/admin.py new file mode 100644 index 0000000000..1686c5bb38 --- /dev/null +++ b/openedx/features/course_duration_limits/admin.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +""" +Django Admin pages for CourseDurationLimitConfig. +""" + +from __future__ import unicode_literals + +from django.contrib import admin +from django.utils.translation import ugettext_lazy as _ + +from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin +from .models import CourseDurationLimitConfig + + +class CourseDurationLimitConfigAdmin(StackedConfigModelAdmin): + fieldsets = ( + ('Context', { + 'fields': ('site', 'org', 'course'), + 'description': _( + 'These define the context to enable course duration limits on. ' + 'If no values are set, then the configuration applies globally. ' + 'If a single value is set, then the configuration applies to all courses ' + 'within that context. At most one value can be set at a time.
' + 'If multiple contexts apply to a course (for example, if configuration ' + 'is specified for the course specifically, and for the org that the course ' + 'is in, then the more specific context overrides the more general context.' + ), + }), + ('Configuration', { + 'fields': ('enabled', 'enabled_as_of'), + 'description': _( + 'If any of these values is left empty or "Unknown", then their value ' + 'at runtime will be retrieved from the next most specific context that applies. ' + 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' + 'course will be Enabled only if the org that it is in is Enabled.' + ), + }) + ) + +admin.site.register(CourseDurationLimitConfig, CourseDurationLimitConfigAdmin) diff --git a/openedx/features/course_duration_limits/config.py b/openedx/features/course_duration_limits/config.py index 23836c1133..eff4f675e7 100644 --- a/openedx/features/course_duration_limits/config.py +++ b/openedx/features/course_duration_limits/config.py @@ -11,9 +11,3 @@ CONTENT_TYPE_GATING_FLAG = WaffleFlag( flag_name=u'debug', flag_undefined_default=False ) - -CONTENT_TYPE_GATING_STUDIO_UI_FLAG = WaffleFlag( - waffle_namespace=WAFFLE_FLAG_NAMESPACE, - flag_name=u'studio_ui', - flag_undefined_default=False -) diff --git a/openedx/features/course_duration_limits/migrations/0001_initial.py b/openedx/features/course_duration_limits/migrations/0001_initial.py new file mode 100644 index 0000000000..85bfedf590 --- /dev/null +++ b/openedx/features/course_duration_limits/migrations/0001_initial.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2018-11-08 19:43 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('course_overviews', '0014_courseoverview_certificate_available_date'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('sites', '0002_alter_domain_unique'), + ] + + operations = [ + migrations.CreateModel( + name='CourseDurationLimitConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.NullBooleanField(default=None, verbose_name='Enabled')), + ('org', models.CharField(blank=True, db_index=True, max_length=255, null=True)), + ('enabled_as_of', models.DateField(blank=True, default=None, null=True, verbose_name='Enabled As Of')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ('course', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='course_overviews.CourseOverview')), + ('site', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='sites.Site')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/openedx/features/course_duration_limits/migrations/0002_auto_20181119_0959.py b/openedx/features/course_duration_limits/migrations/0002_auto_20181119_0959.py new file mode 100644 index 0000000000..32468ce598 --- /dev/null +++ b/openedx/features/course_duration_limits/migrations/0002_auto_20181119_0959.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2018-11-19 14:59 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_duration_limits', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='coursedurationlimitconfig', + name='enabled_as_of', + field=models.DateField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date (UTC) will be affected.', null=True, verbose_name='Enabled As Of'), + ), + ] diff --git a/openedx/features/course_duration_limits/migrations/__init__.py b/openedx/features/course_duration_limits/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/course_duration_limits/models.py b/openedx/features/course_duration_limits/models.py new file mode 100644 index 0000000000..5364b96b19 --- /dev/null +++ b/openedx/features/course_duration_limits/models.py @@ -0,0 +1,123 @@ +""" +Course Duration Limit Configuration Models +""" + +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from datetime import datetime +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.encoding import python_2_unicode_compatible +from django.utils.translation import ugettext_lazy as _ + +from student.models import CourseEnrollment +from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG + + +@python_2_unicode_compatible +class CourseDurationLimitConfig(StackedConfigurationModel): + """ + Configuration to manage the Course Duration Limit facility. + """ + + STACKABLE_FIELDS = ('enabled', 'enabled_as_of') + + enabled_as_of = models.DateField( + default=None, + null=True, + verbose_name=_('Enabled As Of'), + blank=True, + help_text=_( + 'If the configuration is Enabled, then all enrollments ' + 'created after this date (UTC) will be affected.' + ) + ) + + @classmethod + def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None): + """ + Return whether Course Duration Limits are enabled for this enrollment. + + Course Duration Limits are enabled for an enrollment if they are enabled for + the course being enrolled in (either specifically, or via a containing context, + 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 CONTENT_TYPE_GATING_FLAG.is_enabled(): + return True + + 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): + 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 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_date=datetime.utcnow().date()) + else: + current_config = cls.current(course_key=enrollment.course_id) + return current_config.enabled_as_of_date(target_date=enrollment.created.date()) + + @classmethod + def enabled_for_course(cls, course_key, target_date=None): + """ + Return whether Course Duration Limits are enabled for this course as of a particular date. + + Course Duration Limits are enabled for a course on a date if they are enabled either specifically, + or via a containing context, such as the org, site, or globally, and if the configuration + is specified to be ``enabled_as_of`` before ``target_date``. + + Only one of enrollment and (user, course_key) may be specified at a time. + + Arguments: + course_key: The CourseKey of the course being queried. + target_date: The date to checked enablement as of. Defaults to the current date. + """ + if CONTENT_TYPE_GATING_FLAG.is_enabled(): + return True + + if target_date is None: + target_date = datetime.utcnow().date() + + current_config = cls.current(course_key=course_key) + return current_config.enabled_as_of_date(target_date=target_date) + + def clean(self): + if self.enabled and self.enabled_as_of is None: + raise ValidationError({'enabled_as_of': _('enabled_as_of must be set when enabled is True')}) + + def enabled_as_of_date(self, target_date): + """ + Return whether this Course Duration Limit configuration context is enabled as of a date. + + Arguments: + target_date (:class:`datetime.date`): The date that ``enabled_as_of`` must be equal to or before + """ + if CONTENT_TYPE_GATING_FLAG.is_enabled(): + return True + + # Explicitly cast this to bool, so that when self.enabled is None the method doesn't return None + return bool(self.enabled and self.enabled_as_of <= target_date) + + def __str__(self): + return "CourseDurationLimits(enabled={!r}, enabled_as_of={!r})" diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py new file mode 100644 index 0000000000..1d38736bf8 --- /dev/null +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -0,0 +1,335 @@ +""" +Tests of CourseDurationLimitConfig. +""" + +from datetime import timedelta, date +import itertools + +import ddt +from mock import Mock + +from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig +from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from student.tests.factories import CourseEnrollmentFactory, UserFactory + + +@ddt.ddt +class TestCourseDurationLimitConfig(CacheIsolationTestCase): + """ + Tests of CourseDurationLimitConfig + """ + + ENABLED_CACHES = ['default'] + + def setUp(self): + self.course_overview = CourseOverviewFactory.create() + self.user = UserFactory.create() + super(TestCourseDurationLimitConfig, self).setUp() + + @ddt.data( + (True, True, True), + (True, True, False), + (True, False, True), + (True, False, False), + (False, False, True), + (False, False, False), + ) + @ddt.unpack + def test_enabled_for_enrollment( + self, + already_enrolled, + pass_enrollment, + enrolled_before_enabled, + ): + + # Tweak the day to enable the config so that it is either before + # or after today (which is when the enrollment will be created) + if enrolled_before_enabled: + enabled_as_of = date.today() + timedelta(days=1) + else: + enabled_as_of = date.today() - timedelta(days=1) + + CourseDurationLimitConfig.objects.create( + enabled=True, + course=self.course_overview, + enabled_as_of=enabled_as_of, + ) + + if already_enrolled: + existing_enrollment = CourseEnrollmentFactory.create( + user=self.user, + course=self.course_overview, + ) + 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 + + if pass_enrollment: + query_count = 4 + else: + query_count = 5 + + with self.assertNumQueries(query_count): + enabled = CourseDurationLimitConfig.enabled_for_enrollment( + enrollment=enrollment, + user=user, + course_key=course_key, + ) + self.assertEqual(not enrolled_before_enabled, enabled) + + def test_enabled_for_enrollment_failure(self): + with self.assertRaises(ValueError): + CourseDurationLimitConfig.enabled_for_enrollment(None, 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') + ) + + @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) + def test_enabled_for_enrollment_flag_override(self): + self.assertTrue(CourseDurationLimitConfig.enabled_for_enrollment( + None, + None, + None + )) + self.assertTrue(CourseDurationLimitConfig.enabled_for_enrollment( + Mock(name='enrollment'), + Mock(name='user'), + None + )) + self.assertTrue(CourseDurationLimitConfig.enabled_for_enrollment( + Mock(name='enrollment'), + None, + Mock(name='course_key') + )) + + @ddt.data(True, False) + def test_enabled_for_course( + self, + before_enabled, + ): + config = CourseDurationLimitConfig.objects.create( + enabled=True, + course=self.course_overview, + enabled_as_of=date.today(), + ) + + # Tweak the day to check for course enablement so it is either + # before or after when the configuration was enabled + if before_enabled: + target_date = config.enabled_as_of - timedelta(days=1) + else: + target_date = config.enabled_as_of + timedelta(days=1) + + course_key = self.course_overview.id + + self.assertEqual( + not before_enabled, + CourseDurationLimitConfig.enabled_for_course( + course_key=course_key, + target_date=target_date, + ) + ) + + @ddt.data( + # Generate all combinations of setting each configuration level to True/False/None + *itertools.product(*[(True, False, None)] * 4) + ) + @ddt.unpack + def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting): + """ + Test that the stacked configuration overrides happen in the correct order and priority. + + This is tested by exhaustively setting each combination of contexts, and validating that only + the lowest level context that is set to not-None is applied. + """ + # Add a bunch of configuration outside the contexts that are being tested, to make sure + # there are no leaks of configuration across contexts + non_test_course_enabled = CourseOverviewFactory.create(org='non-test-org-enabled') + non_test_course_disabled = CourseOverviewFactory.create(org='non-test-org-disabled') + non_test_site_cfg_enabled = SiteConfigurationFactory.create( + values={'course_org_filter': non_test_course_enabled.org} + ) + non_test_site_cfg_disabled = SiteConfigurationFactory.create( + values={'course_org_filter': non_test_course_disabled.org} + ) + + CourseDurationLimitConfig.objects.create(course=non_test_course_enabled, enabled=True) + CourseDurationLimitConfig.objects.create(course=non_test_course_disabled, enabled=False) + CourseDurationLimitConfig.objects.create(org=non_test_course_enabled.org, enabled=True) + CourseDurationLimitConfig.objects.create(org=non_test_course_disabled.org, enabled=False) + CourseDurationLimitConfig.objects.create(site=non_test_site_cfg_enabled.site, enabled=True) + CourseDurationLimitConfig.objects.create(site=non_test_site_cfg_disabled.site, enabled=False) + + # Set up test objects + test_course = CourseOverviewFactory.create(org='test-org') + test_site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': test_course.org}) + + CourseDurationLimitConfig.objects.create(enabled=global_setting) + CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting) + CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting) + CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting) + + expected_global_setting = self._resolve_settings([global_setting]) + expected_site_setting = self._resolve_settings([global_setting, site_setting]) + expected_org_setting = self._resolve_settings([global_setting, site_setting, org_setting]) + expected_course_setting = self._resolve_settings([global_setting, site_setting, org_setting, course_setting]) + + self.assertEqual(expected_global_setting, CourseDurationLimitConfig.current().enabled) + self.assertEqual(expected_site_setting, CourseDurationLimitConfig.current(site=test_site_cfg.site).enabled) + self.assertEqual(expected_org_setting, CourseDurationLimitConfig.current(org=test_course.org).enabled) + self.assertEqual(expected_course_setting, CourseDurationLimitConfig.current(course_key=test_course.id).enabled) + + def test_caching_global(self): + global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the global value is not retrieved from cache after save + with self.assertNumQueries(1): + self.assertTrue(CourseDurationLimitConfig.current().enabled) + + # Check that the global value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(CourseDurationLimitConfig.current().enabled) + + global_config.enabled = False + global_config.save() + + # Check that the global value in cache was deleted on save + with self.assertNumQueries(1): + self.assertFalse(CourseDurationLimitConfig.current().enabled) + + def test_caching_site(self): + site_cfg = SiteConfigurationFactory() + site_config = CourseDurationLimitConfig(site=site_cfg.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + site_config.save() + + # Check that the site value is not retrieved from cache after save + with self.assertNumQueries(1): + self.assertTrue(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) + + # Check that the site value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) + + site_config.enabled = False + site_config.save() + + # Check that the site value in cache was deleted on save + with self.assertNumQueries(1): + self.assertFalse(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) + + global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the site value is not updated in cache by changing the global value + with self.assertNumQueries(0): + self.assertFalse(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) + + def test_caching_org(self): + course = CourseOverviewFactory.create(org='test-org') + site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': course.org}) + org_config = CourseDurationLimitConfig(org=course.org, enabled=True, enabled_as_of=date(2018, 1, 1)) + org_config.save() + + # Check that the org value is not retrieved from cache after save + with self.assertNumQueries(2): + self.assertTrue(CourseDurationLimitConfig.current(org=course.org).enabled) + + # Check that the org value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(CourseDurationLimitConfig.current(org=course.org).enabled) + + org_config.enabled = False + org_config.save() + + # Check that the org value in cache was deleted on save + with self.assertNumQueries(2): + self.assertFalse(CourseDurationLimitConfig.current(org=course.org).enabled) + + global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the org value is not updated in cache by changing the global value + with self.assertNumQueries(0): + self.assertFalse(CourseDurationLimitConfig.current(org=course.org).enabled) + + site_config = CourseDurationLimitConfig(site=site_cfg.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + site_config.save() + + # Check that the org value is not updated in cache by changing the site value + with self.assertNumQueries(0): + self.assertFalse(CourseDurationLimitConfig.current(org=course.org).enabled) + + def test_caching_course(self): + course = CourseOverviewFactory.create(org='test-org') + site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': course.org}) + course_config = CourseDurationLimitConfig(course=course, enabled=True, enabled_as_of=date(2018, 1, 1)) + course_config.save() + + # Check that the org value is not retrieved from cache after save + with self.assertNumQueries(2): + self.assertTrue(CourseDurationLimitConfig.current(course_key=course.id).enabled) + + # Check that the org value can be retrieved from cache after read + with self.assertNumQueries(0): + self.assertTrue(CourseDurationLimitConfig.current(course_key=course.id).enabled) + + course_config.enabled = False + course_config.save() + + # Check that the org value in cache was deleted on save + with self.assertNumQueries(2): + self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) + + global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=date(2018, 1, 1)) + global_config.save() + + # Check that the org value is not updated in cache by changing the global value + with self.assertNumQueries(0): + self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) + + site_config = CourseDurationLimitConfig(site=site_cfg.site, enabled=True, enabled_as_of=date(2018, 1, 1)) + site_config.save() + + # Check that the org value is not updated in cache by changing the site value + with self.assertNumQueries(0): + self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) + + org_config = CourseDurationLimitConfig(org=course.org, enabled=True, enabled_as_of=date(2018, 1, 1)) + org_config.save() + + # Check that the org value is not updated in cache by changing the site value + with self.assertNumQueries(0): + self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) + + def _resolve_settings(self, settings): + if all(setting is None for setting in settings): + return None + + return [ + setting + for setting + in settings + if setting is not None + ][-1] 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 6b27b676d7..565d18118a 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -2,7 +2,7 @@ """ Tests for the course home page. """ -from datetime import datetime, timedelta +from datetime import datetime, timedelta, date import ddt import mock @@ -25,7 +25,7 @@ from lms.djangoapps.course_goals.api import add_course_goal, remove_course_goal from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience import ( SHOW_REVIEWS_TOOL_FLAG, SHOW_UPGRADE_MSG_ON_COURSE_HOME, @@ -174,16 +174,16 @@ class TestCourseHomePage(CourseHomePageTestCase): response = self.client.get(url) self.assertContains(response, TEST_COURSE_UPDATES_TOOL, status_code=200) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_queries(self): """ Verify that the view's query count doesn't regress. """ + CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) # Pre-fetch the view to populate any caches course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(70, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(84, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) @@ -327,7 +327,6 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): ) self.assertRedirects(response, expected_url) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False}) def test_course_does_not_expire_for_different_roles(self): """ @@ -371,14 +370,14 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): "Should not expire access for user [{}]".format(user_description) ) - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) @mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False}) def test_expired_course(self): """ Ensure that a user accessing an expired course sees a redirect to the student dashboard, not a 404. - """ + CourseDurationLimitConfig.objects.create(enabled=True, enabled_as_of=date(2010, 1, 1)) + course = CourseFactory.create(start=THREE_YEARS_AGO) url = course_home_url(course) @@ -476,18 +475,28 @@ class TestCourseHomePageAccess(CourseHomePageTestCase): self.assertNotContains(response, TEST_COURSE_HOME_MESSAGE_PRE_START) # Verify that enrolled users are shown the course expiration banner if content gating is enabled - with override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True): - url = course_home_url(self.course) - response = self.client.get(url) - bannerText = get_expiration_banner_text(user, self.course) - self.assertContains(response, bannerText, html=True) + + # We use .save() explicitly here (rather than .objects.create) in order to force the + # cache to refresh. + config = CourseDurationLimitConfig( + course=CourseOverview.get_from_id(self.course.id), + enabled=True, + enabled_as_of=date(2018, 1, 1) + ) + config.save() + + url = course_home_url(self.course) + response = self.client.get(url) + bannerText = get_expiration_banner_text(user, self.course) + self.assertContains(response, bannerText, html=True) # Verify that enrolled users are not shown the course expiration banner if content gating is disabled - with override_waffle_flag(CONTENT_TYPE_GATING_FLAG, False): - url = course_home_url(self.course) - response = self.client.get(url) - bannerText = get_expiration_banner_text(user, self.course) - self.assertNotContains(response, bannerText, html=True) + config.enabled = False + config.save() + url = course_home_url(self.course) + response = self.client.get(url) + bannerText = get_expiration_banner_text(user, self.course) + self.assertNotContains(response, bannerText, html=True) # Verify that enrolled users are shown 'days until start' message before start date future_course = self.create_future_course() 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 c441483abf..b94e9649e1 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -1,10 +1,12 @@ """ Tests for the course updates page. """ +from datetime import date + from courseware.courses import get_course_info_usage_key from django.urls import reverse from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag -from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG +from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_experience.views.course_updates import STATUS_VISIBLE from student.models import CourseEnrollment from student.tests.factories import UserFactory @@ -119,15 +121,15 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): self.assertContains(response, 'First Message') self.assertContains(response, 'Second Message') - @override_waffle_flag(CONTENT_TYPE_GATING_FLAG, True) def test_queries(self): + ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=date(2018, 1, 1)) create_course_update(self.course, self.user, 'First Message') # Pre-fetch the view to populate any caches course_updates_url(self.course) # Fetch the view and verify that the query counts haven't changed - with self.assertNumQueries(44, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(46, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url) diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 840dc3f196..3b14b9f5f8 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -75,6 +75,8 @@ INSTALLED_APPS = ( 'openedx.core.djangoapps.content.block_structure.apps.BlockStructureConfig', 'openedx.core.djangoapps.catalog', 'openedx.core.djangoapps.self_paced', + 'openedx.features.content_type_gating', + 'openedx.features.course_duration_limits', 'milestones', 'celery_utils', 'waffle', @@ -91,6 +93,7 @@ MICROSITE_BACKEND = 'microsite_configuration.backends.filebased.FilebasedMicrosi MICROSITE_TEMPLATE_BACKEND = 'microsite_configuration.backends.filebased.FilebasedMicrositeTemplateBackend' SECRET_KEY = 'insecure-secret-key' +SITE_ID = 1 TRACK_MAX_EVENT = 50000