diff --git a/cms/envs/common.py b/cms/envs/common.py index 74f7e33cb8..96287aae80 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1010,6 +1010,9 @@ INSTALLED_APPS = ( # Customized celery tasks, including persisting failed tasks so they can # be retried 'celery_utils', + + # Waffle related utilities + 'openedx.core.djangoapps.waffle_utils', ) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 593931f79e..e388487c32 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -2267,7 +2267,7 @@ def auto_auth(request): elif course_id: # Redirect to the course homepage (in LMS) or outline page (in Studio) try: - redirect_url = reverse(course_home_url_name(request), kwargs={'course_id': course_id}) + redirect_url = reverse(course_home_url_name(course_key), kwargs={'course_id': course_id}) except NoReverseMatch: redirect_url = reverse('course_handler', kwargs={'course_key_string': course_id}) else: diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 0fba797f97..339a10b853 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -231,18 +231,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): (25, 1), + ('no_overrides', 2, True, False): (25, 1), + ('no_overrides', 3, True, False): (25, 1), + ('ccx', 1, True, False): (25, 1), + ('ccx', 2, True, False): (25, 1), + ('ccx', 3, True, False): (25, 1), + ('no_overrides', 1, False, False): (25, 1), + ('no_overrides', 2, False, False): (25, 1), + ('no_overrides', 3, False, False): (25, 1), + ('ccx', 1, False, False): (25, 1), + ('ccx', 2, False, False): (25, 1), + ('ccx', 3, False, False): (25, 1), } @@ -254,19 +254,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): (25, 3), + ('no_overrides', 2, True, False): (25, 3), + ('no_overrides', 3, True, False): (25, 3), + ('ccx', 1, True, False): (25, 3), + ('ccx', 2, True, False): (25, 3), + ('ccx', 3, True, False): (25, 3), + ('ccx', 1, True, True): (26, 3), + ('ccx', 2, True, True): (26, 3), + ('ccx', 3, True, True): (26, 3), + ('no_overrides', 1, False, False): (25, 3), + ('no_overrides', 2, False, False): (25, 3), + ('no_overrides', 3, False, False): (25, 3), + ('ccx', 1, False, False): (25, 3), + ('ccx', 2, False, False): (25, 3), + ('ccx', 3, False, False): (25, 3), } diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index b889653fd4..049d6a0aa6 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -2,15 +2,14 @@ This module is essentially a broker to xmodule/tabs.py -- it was originally introduced to perform some LMS-specific tab display gymnastics for the Entrance Exams feature """ -import waffle - from django.conf import settings from django.utils.translation import ugettext as _, ugettext_noop from courseware.access import has_access from courseware.entrance_exams import user_can_skip_entrance_exam from openedx.core.lib.course_tabs import CourseTabPluginManager -from openedx.features.course_experience import default_course_url_name, UNIFIED_COURSE_EXPERIENCE_FLAG +from openedx.features.course_experience import default_course_url_name +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG from request_cache.middleware import RequestCache from student.models import CourseEnrollment from xmodule.tabs import CourseTab, CourseTabList, key_checker, link_reverse_func @@ -66,8 +65,7 @@ class CourseInfoTab(CourseTab): """ The "Home" tab is not shown for the new unified course experience. """ - request = RequestCache.get_current_request() - return not waffle.flag_is_active(request, UNIFIED_COURSE_EXPERIENCE_FLAG) + return not UNIFIED_COURSE_TAB_FLAG.is_enabled(course.id) class SyllabusTab(EnrolledTab): diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index d4619d882d..f8f17420b1 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -7,8 +7,6 @@ from django.core.urlresolvers import reverse from freezegun import freeze_time from nose.plugins.attrib import attr from pytz import utc -from waffle.testutils import override_flag -from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG from commerce.models import CommerceConfiguration from course_modes.tests.factories import CourseModeFactory @@ -24,6 +22,8 @@ from courseware.date_summary import ( ) from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.user_api.preferences.api import set_user_preference +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG from student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.verify_student.models import VerificationDeadline from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory @@ -194,7 +194,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): 'info', 'openedx.course_experience.course_home', ) - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_todays_date_no_timezone(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() @@ -218,7 +218,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): 'info', 'openedx.course_experience.course_home', ) - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_todays_date_timezone(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() @@ -249,7 +249,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): 'info', 'openedx.course_experience.course_home', ) - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_start_date_render(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() @@ -267,7 +267,7 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): 'info', 'openedx.course_experience.course_home', ) - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_start_date_render_time_zone(self, url_name): with freeze_time('2015-01-02'): self.setup_course_and_user() diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 58bc1908e8..28a33222f5 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -2,8 +2,6 @@ Test cases for tabs. """ -from waffle.testutils import override_flag - from django.core.urlresolvers import reverse from django.http import Http404 from mock import MagicMock, Mock, patch @@ -18,7 +16,8 @@ from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.factories import InstructorFactory, StaffFactory from courseware.views.views import get_static_tab_fragment, StaticCourseTabView from openedx.core.djangolib.testing.utils import get_mock_request -from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG from student.models import CourseEnrollment from student.tests.factories import UserFactory from util.milestones_helpers import ( @@ -776,12 +775,13 @@ class CourseInfoTabTestCase(TabTestCase): self.user = self.create_mock_user() self.request = get_mock_request(self.user) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=False) def test_default_tab(self): # Verify that the course info tab is the first tab tabs = get_course_tab_list(self.request, self.course) self.assertEqual(tabs[0].type, 'course_info') - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_default_tab_for_new_course_experience(self): # Verify that the unified course experience hides the course info tab tabs = get_course_tab_list(self.request, self.course) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 24c35a79a0..48a7432f2e 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -210,8 +210,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 142), - (ModuleStoreEnum.Type.split, 4, 142), + (ModuleStoreEnum.Type.mongo, 10, 143), + (ModuleStoreEnum.Type.split, 4, 143), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1420,12 +1420,12 @@ class ProgressPageTests(ProgressPageBaseTests): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(40), check_mongo_calls(1): + with self.assertNumQueries(41), check_mongo_calls(1): self._get_progress_page() @ddt.data( - (False, 40, 26), - (True, 33, 22) + (False, 41, 27), + (True, 34, 23) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 423e211626..6bc59e3077 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -86,9 +86,9 @@ from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.features.course_experience import ( - UNIFIED_COURSE_EXPERIENCE_FLAG, + UNIFIED_COURSE_TAB_FLAG, UNIFIED_COURSE_VIEW_FLAG, - course_home_url_name + course_home_url_name, ) from openedx.features.course_experience.views.course_dates import CourseDatesFragmentView from openedx.features.enterprise_support.api import data_sharing_consent_required @@ -263,11 +263,12 @@ def course_info(request, course_id): return url return None - # If the unified course experience is enabled, redirect to the "Course" tab - if waffle.flag_is_active(request, UNIFIED_COURSE_EXPERIENCE_FLAG): - return redirect(reverse(course_home_url_name(request), args=[course_id])) - course_key = CourseKey.from_string(course_id) + + # If the unified course experience is enabled, redirect to the "Course" tab + if UNIFIED_COURSE_TAB_FLAG.is_enabled(course_key): + return redirect(reverse(course_home_url_name(course_key), args=[course_id])) + with modulestore().bulk_operations(course_key): course = get_course_by_id(course_key, depth=2) access_response = has_access(request.user, 'load', course, course_key) @@ -669,7 +670,7 @@ def course_about(request, course_id): modes = CourseMode.modes_for_course_dict(course_key) if configuration_helpers.get_value('ENABLE_MKTG_SITE', settings.FEATURES.get('ENABLE_MKTG_SITE', False)): - return redirect(reverse(course_home_url_name(request), args=[unicode(course.id)])) + return redirect(reverse(course_home_url_name(course.id), args=[unicode(course.id)])) registered = registered_for_course(course, request.user) @@ -677,7 +678,7 @@ def course_about(request, course_id): studio_url = get_studio_url(course, 'settings/details') if has_access(request.user, 'load', course): - course_target = reverse(course_home_url_name(request), args=[course.id.to_deprecated_string()]) + course_target = reverse(course_home_url_name(course.id), args=[course.id.to_deprecated_string()]) else: course_target = reverse('about_course', args=[course.id.to_deprecated_string()]) @@ -1241,7 +1242,7 @@ def course_survey(request, course_id): course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) - redirect_url = reverse(course_home_url_name(request), args=[course_id]) + redirect_url = reverse(course_home_url_name(course.id), args=[course_id]) # if there is no Survey associated with this course, # then redirect to the course instead diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py index c7ea272c40..54c9b5b3b8 100644 --- a/lms/djangoapps/grades/config/waffle.py +++ b/lms/djangoapps/grades/config/waffle.py @@ -2,7 +2,7 @@ This module contains various configuration settings via waffle switches for the Grades app. """ -from openedx.core.djangolib.waffle_utils import WaffleSwitchPlus +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace # Namespace @@ -18,4 +18,4 @@ def waffle(): """ Returns the namespaced, cached, audited Waffle class for Grades. """ - return WaffleSwitchPlus(namespace=WAFFLE_NAMESPACE, log_prefix=u'Grades: ') + return WaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix=u'Grades: ') diff --git a/lms/envs/common.py b/lms/envs/common.py index 7b4640aa17..973cac74c6 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2211,6 +2211,9 @@ INSTALLED_APPS = ( # Unusual migrations 'database_fixups', + # Waffle related utilities + 'openedx.core.djangoapps.waffle_utils', + # Features 'openedx.features.course_bookmarks', 'openedx.features.course_experience', diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 53e178f89c..542fe920ed 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -56,7 +56,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ % endif
- <% course_target = reverse(course_home_url_name(), args=[unicode(course_overview.id)]) %> + <% course_target = reverse(course_home_url_name(course_overview.id), args=[unicode(course_overview.id)]) %>

${_('Course details')}

% if not reg_code_already_redeemed: %if redemption_success: - <% course_url = reverse(course_home_url_name(), args=[course.id.to_deprecated_string()]) %> + <% course_url = reverse(course_home_url_name(course.id), args=[course.id.to_deprecated_string()]) %> ${_("View Course")} %elif not registered_for_course:
diff --git a/lms/tests.py b/lms/tests.py index 1e275e7acc..c70d482bd0 100644 --- a/lms/tests.py +++ b/lms/tests.py @@ -55,6 +55,6 @@ class HelpModalTests(ModuleStoreTestCase): Simple test to make sure that you don't get a 500 error when the modal is enabled. """ - url = reverse(course_home_url_name(), args=[self.course.id.to_deprecated_string()]) + url = reverse(course_home_url_name(self.course.id), args=[self.course.id.to_deprecated_string()]) resp = self.client.get(url) self.assertEqual(resp.status_code, 200) diff --git a/openedx/core/djangoapps/content/block_structure/config/__init__.py b/openedx/core/djangoapps/content/block_structure/config/__init__.py index 246d3a24ae..fdcf284dcc 100644 --- a/openedx/core/djangoapps/content/block_structure/config/__init__.py +++ b/openedx/core/djangoapps/content/block_structure/config/__init__.py @@ -2,7 +2,7 @@ This module contains various configuration settings via waffle switches for the Block Structure framework. """ -from openedx.core.djangolib.waffle_utils import WaffleSwitchPlus +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace from request_cache.middleware import request_cached from .models import BlockStructureConfiguration @@ -22,7 +22,7 @@ def waffle(): """ Returns the namespaced and cached Waffle class for BlockStructures. """ - return WaffleSwitchPlus(namespace=WAFFLE_NAMESPACE, log_prefix=u'BlockStructure: ') + return WaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix=u'BlockStructure: ') @request_cached diff --git a/openedx/core/djangoapps/monitoring_utils/middleware.py b/openedx/core/djangoapps/monitoring_utils/middleware.py index b2a7a9cfdd..bfa864d9af 100644 --- a/openedx/core/djangoapps/monitoring_utils/middleware.py +++ b/openedx/core/djangoapps/monitoring_utils/middleware.py @@ -19,7 +19,7 @@ except ImportError: import psutil import request_cache -from openedx.core.djangolib.waffle_utils import WaffleSwitchPlus +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace REQUEST_CACHE_KEY = 'monitoring_custom_metrics' @@ -163,4 +163,4 @@ class MonitoringMemoryMiddleware(object): """ Returns whether this middleware is enabled. """ - return WaffleSwitchPlus(namespace=WAFFLE_NAMESPACE).is_enabled(u'enable_memory_middleware') + return WaffleSwitchNamespace(name=WAFFLE_NAMESPACE).is_enabled(u'enable_memory_middleware') diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py new file mode 100644 index 0000000000..94cee7de4d --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -0,0 +1,298 @@ +""" +Utilities for waffle. + +Includes namespacing, caching, and course overrides for waffle flags. + +Usage: + +For Waffle Flags, first set up the namespace, and then create flags using the +namespace. For example: + + WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='course_experience') + + HIDE_SEARCH_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, 'hide_search') + UNIFIED_COURSE_TAB_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'unified_course_tab') + +You can check these flags in code using the following: + + HIDE_SEARCH_FLAG.is_enabled() + UNIFIED_COURSE_TAB_FLAG.is_enabled(course_key) + +To test these WaffleFlags, see testutils.py. + +In the above examples, you will use Django Admin "waffle" section to configure +for a flag named: course_experience.unified_course_tab + +You could also use the Django Admin "waffle_utils" section to configure a course +override for this same flag (e.g. course_experience.unified_course_tab). + +For Waffle Switches, first set up the namespace, and then create the flag name. +For example: + + WAFFLE_SWITCHES = WaffleSwitchNamespace(name=WAFFLE_NAMESPACE) + + ESTIMATE_FIRST_ATTEMPTED = 'estimate_first_attempted' + +You can then use the switch as follows: + + WAFFLE_SWITCHES.is_enabled(waffle.ESTIMATE_FIRST_ATTEMPTED) + +To test WaffleSwitchNamespace, use the provided context managers. For example: + + with WAFFLE_SWITCHES.override(waffle.ESTIMATE_FIRST_ATTEMPTED, active=True): + ... + +""" +from abc import ABCMeta +from contextlib import contextmanager +import logging +from waffle.testutils import override_switch as waffle_override_switch +from waffle import flag_is_active, switch_is_active + +from opaque_keys.edx.keys import CourseKey +from request_cache import get_request, get_cache as get_request_cache + +from .models import WaffleFlagCourseOverrideModel + +log = logging.getLogger(__name__) + + +class WaffleNamespace(object): + """ + A base class for a request cached namespace for waffle flags/switches. + + An instance of this class represents a single namespace + (e.g. "course_experience"), and can be used to work with a set of + flags or switches that will all share this namespace. + + """ + __metaclass__ = ABCMeta + + def __init__(self, name, log_prefix=None): + """ + Initializes the waffle namespace instance. + + Arguments: + name (String): Namespace string appended to start of all waffle + flags and switches (e.g. "grades") + log_prefix (String): Optional string to be appended to log messages + (e.g. "Grades: "). Defaults to ''. + + """ + assert name, "The name is required." + self.name = name + self.log_prefix = log_prefix if log_prefix else '' + + def _namespaced_name(self, setting_name): + """ + Returns the namespaced name of the waffle switch/flag. + + For example, the namespaced name of a waffle switch/flag would be: + my_namespace.my_setting_name + + Arguments: + setting_name (String): The name of the flag or switch. + + """ + return u'{}.{}'.format(self.name, setting_name) + + @staticmethod + def _get_request_cache(): + """ + Returns a request cache shared by all instances of this class. + """ + return get_request_cache('WaffleNamespace') + + +class WaffleSwitchNamespace(WaffleNamespace): + """ + Provides a single namespace for a set of waffle switches. + + All namespaced switch values are stored in a single request cache containing + all switches for all namespaces. + + """ + def is_enabled(self, switch_name): + """ + Returns and caches whether the given waffle switch is enabled. + """ + namespaced_switch_name = self._namespaced_name(switch_name) + value = self._cached_switches.get(namespaced_switch_name) + if value is None: + value = switch_is_active(namespaced_switch_name) + self._cached_switches[namespaced_switch_name] = value + return value + + @contextmanager + def override(self, switch_name, active=True): + """ + Overrides the active value for the given switch for the duration of this + contextmanager. + Note: The value is overridden in the request cache AND in the model. + """ + previous_active = self.is_enabled(switch_name) + try: + self.override_for_request(switch_name, active) + with self.override_in_model(switch_name, active): + yield + finally: + self.override_for_request(switch_name, previous_active) + + def override_for_request(self, switch_name, active=True): + """ + Overrides the active value for the given switch for the remainder of + this request (as this is not a context manager). + Note: The value is overridden in the request cache, not in the model. + """ + namespaced_switch_name = self._namespaced_name(switch_name) + self._cached_switches[namespaced_switch_name] = active + log.info(u"%sSwitch '%s' set to %s for request.", self.log_prefix, namespaced_switch_name, active) + + @contextmanager + def override_in_model(self, switch_name, active=True): + """ + Overrides the active value for the given switch for the duration of this + contextmanager. + Note: The value is overridden in the model, not the request cache. + """ + namespaced_switch_name = self._namespaced_name(switch_name) + with waffle_override_switch(namespaced_switch_name, active): + log.info(u"%sSwitch '%s' set to %s in model.", self.log_prefix, namespaced_switch_name, active) + yield + + @property + def _cached_switches(self): + """ + Returns a dictionary of all namespaced switches in the request cache. + """ + return self._get_request_cache().setdefault('switches', {}) + + +class WaffleFlagNamespace(WaffleNamespace): + """ + Provides a single namespace for a set of waffle flags. + + All namespaced flag values are stored in a single request cache containing + all flags for all namespaces. + + """ + __metaclass__ = ABCMeta + + @property + def _cached_flags(self): + """ + Returns a dictionary of all namespaced flags in the request cache. + """ + return self._get_request_cache().setdefault('flags', {}) + + def is_flag_active(self, flag_name, check_before_waffle_callback=None): + """ + Returns and caches whether the provided flag is active. + + If the flag value is already cached in the request, it is returned. + If check_before_waffle_callback is supplied, it is called before + checking waffle. + If check_before_waffle_callback returns None, or if it is not supplied, + then waffle is used to check the flag. + + Arguments: + flag_name (String): The name of the flag to check. + check_before_waffle_callback (function): (Optional) A function that + will be checked before continuing on to waffle. If + check_before_waffle_callback(namespaced_flag_name) returns True + or False, it is cached and returned. If it returns None, then + waffle is used. + + """ + # validate arguments + namespaced_flag_name = self._namespaced_name(flag_name) + value = self._cached_flags.get(namespaced_flag_name) + + if value is None: + if check_before_waffle_callback: + value = check_before_waffle_callback(namespaced_flag_name) + + if value is None: + value = flag_is_active(get_request(), namespaced_flag_name) + + self._cached_flags[namespaced_flag_name] = value + return value + + +class WaffleFlag(object): + """ + Represents a single waffle flag, using a cached waffle namespace. + """ + + def __init__(self, waffle_namespace, flag_name): + """ + Initializes the waffle flag instance. + + Arguments: + waffle_namespace (WaffleFlagNamespace): Provides a cached namespace + for this flag. + flag_name (String): The name of the flag (without namespacing). + + """ + self.waffle_namespace = waffle_namespace + self.flag_name = flag_name + + def is_enabled(self): + """ + Returns whether or not the flag is enabled. + """ + return self.waffle_namespace.is_flag_active(self.flag_name) + + +class CourseWaffleFlag(WaffleFlag): + """ + Represents a single waffle flag that can be forced on/off for a course. + + Uses a cached waffle namespace. + + """ + + def _get_course_override_callback(self, course_id): + """ + Returns a function to use as the check_before_waffle_callback. + + Arguments: + course_id (CourseKey): The course to check for override before + checking waffle. + + """ + def course_override_callback(namespaced_flag_name): + """ + Returns True/False if the flag was forced on or off for the provided + course. Returns None if the flag was not overridden. + + Arguments: + namespaced_flag_name (String): A namespaced version of the flag + to check. + + """ + force_override = WaffleFlagCourseOverrideModel.override_value(namespaced_flag_name, course_id) + + if force_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.on: + return True + if force_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.off: + return False + return None + return course_override_callback + + def is_enabled(self, course_id=None): + """ + Returns whether or not the flag is enabled. + + Arguments: + course_id (CourseKey): The course to check for override before + checking waffle. + + """ + # validate arguments + assert issubclass(type(course_id), CourseKey), "The course_id '{}' must be a CourseKey.".format(str(course_id)) + + return self.waffle_namespace.is_flag_active( + self.flag_name, + check_before_waffle_callback=self._get_course_override_callback(course_id) + ) diff --git a/openedx/core/djangoapps/waffle_utils/admin.py b/openedx/core/djangoapps/waffle_utils/admin.py new file mode 100644 index 0000000000..4fd20e6a1d --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/admin.py @@ -0,0 +1,29 @@ +""" +Django admin page for waffle utils models +""" +from django.contrib import admin + +from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin + + +from .forms import WaffleFlagCourseOverrideAdminForm +from .models import WaffleFlagCourseOverrideModel + + +class WaffleFlagCourseOverrideAdmin(KeyedConfigurationModelAdmin): + """ + Admin for course override of waffle flags. + + Includes search by course_id and waffle_flag. + + """ + form = WaffleFlagCourseOverrideAdminForm + search_fields = ['waffle_flag', 'course_id'] + fieldsets = ( + (None, { + 'fields': ('waffle_flag', 'course_id', 'override_choice', 'enabled'), + 'description': 'Enter a valid course id and an existing waffle flag. The waffle flag name is not validated.' + }), + ) + +admin.site.register(WaffleFlagCourseOverrideModel, WaffleFlagCourseOverrideAdmin) diff --git a/openedx/core/djangoapps/waffle_utils/forms.py b/openedx/core/djangoapps/waffle_utils/forms.py new file mode 100644 index 0000000000..21c69cf924 --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/forms.py @@ -0,0 +1,35 @@ +""" +Defines a form for providing validation of subsection grade templates. +""" +from django import forms + +from openedx.core.lib.courses import clean_course_id + +from .models import WaffleFlagCourseOverrideModel + + +class WaffleFlagCourseOverrideAdminForm(forms.ModelForm): + """ + Input form for course override of waffle flags, allowing us to verify data. + """ + class Meta(object): + model = WaffleFlagCourseOverrideModel + fields = '__all__' + + def clean_course_id(self): + """ + Validate the course id + """ + return clean_course_id(self) + + def clean_waffle_flag(self): + """ + Validate the waffle flag is an existing flag. + """ + cleaned_flag = self.cleaned_data['waffle_flag'] + + if not cleaned_flag: + msg = u'Waffle flag must be supplied.' + raise forms.ValidationError(msg) + + return cleaned_flag.strip() diff --git a/openedx/core/djangoapps/waffle_utils/migrations/0001_initial.py b/openedx/core/djangoapps/waffle_utils/migrations/0001_initial.py new file mode 100644 index 0000000000..bfe400cfaa --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/migrations/0001_initial.py @@ -0,0 +1,34 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import openedx.core.djangoapps.xmodule_django.models + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='WaffleFlagCourseOverrideModel', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('waffle_flag', models.CharField(max_length=255, db_index=True)), + ('course_id', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True)), + ('override_choice', models.CharField(default=b'on', max_length=3, choices=[(b'on', 'Force On'), (b'off', 'Force Off')])), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + options={ + 'verbose_name': 'Waffle flag course override', + 'verbose_name_plural': 'Waffle flag course overrides', + }, + ), + ] diff --git a/openedx/core/djangoapps/waffle_utils/migrations/__init__.py b/openedx/core/djangoapps/waffle_utils/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/waffle_utils/models.py b/openedx/core/djangoapps/waffle_utils/models.py new file mode 100644 index 0000000000..95de5100a3 --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/models.py @@ -0,0 +1,61 @@ +""" +Models for configuring waffle utils. +""" +from django.db.models import CharField +from django.utils.translation import ugettext_lazy as _ +from model_utils import Choices + +from config_models.models import ConfigurationModel +from openedx.core.djangoapps.xmodule_django.models import CourseKeyField +from request_cache.middleware import request_cached + + +class WaffleFlagCourseOverrideModel(ConfigurationModel): + """ + Used to force a waffle flag on or off for a course. + """ + OVERRIDE_CHOICES = Choices(('on', _('Force On')), ('off', _('Force Off'))) + ALL_CHOICES = OVERRIDE_CHOICES + Choices('unset') + + KEY_FIELDS = ('waffle_flag', 'course_id') + + # The course that these features are attached to. + waffle_flag = CharField(max_length=255, db_index=True) + course_id = CourseKeyField(max_length=255, db_index=True) + override_choice = CharField(choices=OVERRIDE_CHOICES, default=OVERRIDE_CHOICES.on, max_length=3) + + @classmethod + @request_cached + def override_value(cls, waffle_flag, course_id): + """ + Returns whether the waffle flag was overridden (on or off) for the + course, or is unset. + + Arguments: + waffle_flag (String): The name of the flag. + course_id (CourseKey): The course id for which the flag may have + been overridden. + + If the current config is not set or disabled for this waffle flag and + course id, returns ALL_CHOICES.unset. + Otherwise, returns ALL_CHOICES.on or ALL_CHOICES.off as configured for + the override_choice. + + """ + if not course_id or not waffle_flag: + return cls.ALL_CHOICES.unset + + effective = cls.objects.filter(waffle_flag=waffle_flag, course_id=course_id).order_by('-change_date').first() + if effective and effective.enabled: + return effective.override_choice + return cls.ALL_CHOICES.unset + + class Meta(object): + app_label = "waffle_utils" + verbose_name = 'Waffle flag course override' + verbose_name_plural = 'Waffle flag course overrides' + + def __unicode__(self): + enabled_label = "Enabled" if self.enabled else "Not Enabled" + # pylint: disable=no-member + return u"Course '{}': Persistent Grades {}".format(self.course_id.to_deprecated_string(), enabled_label) diff --git a/openedx/core/djangoapps/waffle_utils/tests/__init__.py b/openedx/core/djangoapps/waffle_utils/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py new file mode 100644 index 0000000000..bd9fcb47df --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -0,0 +1,52 @@ +""" +Tests for waffle utils features. +""" +import ddt +from django.test import TestCase +from mock import patch +from opaque_keys.edx.keys import CourseKey +from waffle.testutils import override_flag + +from request_cache.middleware import RequestCache + +from .. import CourseWaffleFlag, WaffleFlagNamespace +from ..models import WaffleFlagCourseOverrideModel + + +@ddt.ddt +class TestCourseWaffleFlag(TestCase): + """ + Tests the CourseWaffleFlag. + """ + + NAMESPACE_NAME = "test_namespace" + FLAG_NAME = "test_flag" + NAMESPACED_FLAG_NAME = NAMESPACE_NAME + "." + FLAG_NAME + + TEST_COURSE_KEY = CourseKey.from_string("edX/DemoX/Demo_Course") + TEST_NAMESPACE = WaffleFlagNamespace(NAMESPACE_NAME) + TEST_COURSE_FLAG = CourseWaffleFlag(TEST_NAMESPACE, FLAG_NAME) + + @ddt.data( + {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.on, 'waffle_enabled': False, 'result': True}, + {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.off, 'waffle_enabled': True, 'result': False}, + {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, 'waffle_enabled': True, 'result': True}, + {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, 'waffle_enabled': False, 'result': False}, + ) + def test_course_waffle_flag(self, data): + """ + Tests various combinations of a flag being set in waffle and overridden + for a course. + """ + RequestCache.clear_request_cache() + + with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']): + with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']): + # check twice to test that the result is properly cached + self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result']) + self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result']) + # result is cached, so override check should happen once + WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( + self.NAMESPACED_FLAG_NAME, + self.TEST_COURSE_KEY + ) diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_models.py b/openedx/core/djangoapps/waffle_utils/tests/test_models.py new file mode 100644 index 0000000000..980a4dc082 --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/tests/test_models.py @@ -0,0 +1,51 @@ +""" +Tests for waffle utils models. +""" +from ddt import data, ddt, unpack +from django.test import TestCase +from opaque_keys.edx.keys import CourseKey + +from request_cache.middleware import RequestCache + +from ..models import WaffleFlagCourseOverrideModel + + +@ddt +class WaffleFlagCourseOverrideTests(TestCase): + """ + Tests for the waffle flag course override model. + """ + + WAFFLE_TEST_NAME = "waffle_test_course_override" + TEST_COURSE_KEY = CourseKey.from_string("edX/DemoX/Demo_Course") + OVERRIDE_CHOICES = WaffleFlagCourseOverrideModel.ALL_CHOICES + + # Data format: ( is_enabled, override_choice, expected_result ) + @data((True, OVERRIDE_CHOICES.on, OVERRIDE_CHOICES.on), + (True, OVERRIDE_CHOICES.off, OVERRIDE_CHOICES.off), + (False, OVERRIDE_CHOICES.on, OVERRIDE_CHOICES.unset)) + @unpack + def test_setting_override(self, is_enabled, override_choice, expected_result): + RequestCache.clear_request_cache() + self.set_waffle_course_override(override_choice, is_enabled) + override_value = WaffleFlagCourseOverrideModel.override_value( + self.WAFFLE_TEST_NAME, self.TEST_COURSE_KEY + ) + self.assertEqual(override_value, expected_result) + + def test_setting_override_multiple_times(self): + RequestCache.clear_request_cache() + self.set_waffle_course_override(self.OVERRIDE_CHOICES.on) + self.set_waffle_course_override(self.OVERRIDE_CHOICES.off) + override_value = WaffleFlagCourseOverrideModel.override_value( + self.WAFFLE_TEST_NAME, self.TEST_COURSE_KEY + ) + self.assertEqual(override_value, self.OVERRIDE_CHOICES.off) + + def set_waffle_course_override(self, override_choice, is_enabled=True): + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag=self.WAFFLE_TEST_NAME, + override_choice=override_choice, + enabled=is_enabled, + course_id=self.TEST_COURSE_KEY + ) diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_testutils.py b/openedx/core/djangoapps/waffle_utils/tests/test_testutils.py new file mode 100644 index 0000000000..e04ae6ed2d --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/tests/test_testutils.py @@ -0,0 +1,57 @@ +""" +Tests for waffle utils test utilities. +""" +from django.test import TestCase +from opaque_keys.edx.keys import CourseKey + +from request_cache.middleware import RequestCache + +from .. import CourseWaffleFlag, WaffleFlagNamespace +from ..testutils import override_waffle_flag + + +class OverrideWaffleFlagTests(TestCase): + """ + Tests for the override_waffle_flag decorator. + """ + + NAMESPACE_NAME = "test_namespace" + FLAG_NAME = "test_flag" + NAMESPACED_FLAG_NAME = NAMESPACE_NAME + "." + FLAG_NAME + + TEST_COURSE_KEY = CourseKey.from_string("edX/DemoX/Demo_Course") + TEST_NAMESPACE = WaffleFlagNamespace(NAMESPACE_NAME) + TEST_COURSE_FLAG = CourseWaffleFlag(TEST_NAMESPACE, FLAG_NAME) + + def setUp(self): + super(OverrideWaffleFlagTests, self).setUp() + RequestCache.clear_request_cache() + + @override_waffle_flag(TEST_COURSE_FLAG, True) + def check_is_enabled_with_decorator(self): + # test flag while overridden with decorator + self.assertTrue(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY)) + + def test_override_waffle_flag_pre_cached(self): + # checks and caches the is_enabled value + self.assertFalse(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY)) + flag_cache = self.TEST_COURSE_FLAG.waffle_namespace._cached_flags + self.assertIn(self.NAMESPACED_FLAG_NAME, flag_cache) + + # test flag while overridden with decorator + self.check_is_enabled_with_decorator() + + # test cached flag is restored + self.assertIn(self.NAMESPACED_FLAG_NAME, flag_cache) + self.assertEquals(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), False) + + def test_override_waffle_flag_not_pre_cached(self): + # check that the flag is not yet cached + flag_cache = self.TEST_COURSE_FLAG.waffle_namespace._cached_flags + self.assertNotIn(self.NAMESPACED_FLAG_NAME, flag_cache) + + # test flag while overridden with decorator + self.check_is_enabled_with_decorator() + + # test cache is removed when no longer using decorator/context manager + self.assertNotIn(self.NAMESPACED_FLAG_NAME, flag_cache) diff --git a/openedx/core/djangoapps/waffle_utils/testutils.py b/openedx/core/djangoapps/waffle_utils/testutils.py new file mode 100644 index 0000000000..ac8bd90c3a --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/testutils.py @@ -0,0 +1,60 @@ +""" +Test utilities for waffle utilities. +""" + +from functools import wraps + +from waffle.testutils import override_flag + + +def override_waffle_flag(flag, active): + """ + To be used as a decorator for a test function to override a namespaced + waffle flag. + + flag (WaffleFlag): The namespaced cached waffle flag. + active (Boolean): The value to which the flag will be set. + + Example usage: + + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) + + """ + + def real_decorator(function): + """ + Actual decorator function. + """ + + @wraps(function) + def wrapper(*args, **kwargs): + """ + Provides the actual override functionality of the decorator. + + Saves the previous cached value of the flag and restores it (if it + was set), after overriding it. + + """ + waffle_namespace = flag.waffle_namespace + namespaced_flag_name = waffle_namespace._namespaced_name(flag.flag_name) + + # save previous value and whether it existed in the cache + cached_value_existed = namespaced_flag_name in waffle_namespace._cached_flags + if cached_value_existed: + previous_value = waffle_namespace._cached_flags[namespaced_flag_name] + + # set new value + waffle_namespace._cached_flags[namespaced_flag_name] = active + + with override_flag(namespaced_flag_name, active): + # call wrapped function + function(*args, **kwargs) + + # restore value + if cached_value_existed: + waffle_namespace._cached_flags[namespaced_flag_name] = previous_value + elif namespaced_flag_name in waffle_namespace._cached_flags: + del waffle_namespace._cached_flags[namespaced_flag_name] + return wrapper + + return real_decorator diff --git a/openedx/core/djangolib/waffle_utils.py b/openedx/core/djangolib/waffle_utils.py deleted file mode 100644 index 893e3b107b..0000000000 --- a/openedx/core/djangolib/waffle_utils.py +++ /dev/null @@ -1,108 +0,0 @@ -""" -Utilities for waffle usage. -""" -import logging -from abc import ABCMeta -from contextlib import contextmanager - -from waffle import switch_is_active -from waffle.testutils import override_switch as waffle_override_switch - -from request_cache import get_cache as get_request_cache - -log = logging.getLogger(__name__) - - -class WafflePlus(object): - """ - Waffle helper class that provides native support for - namespacing waffle settings and caching within a request. - """ - __metaclass__ = ABCMeta - - def __init__(self, namespace, log_prefix=None): - self.namespace = namespace - self.log_prefix = log_prefix - - def _namespaced_setting_name(self, setting_name): - """ - Returns the namespaced name of the waffle switch/flag. - """ - assert self.namespace is not None - return u'{}.{}'.format(self.namespace, setting_name) - - @staticmethod - def _get_request_cache(): - """ - Returns the request cache used by WafflePlus classes. - """ - return get_request_cache('WafflePlus') - - -class WaffleSwitchPlus(WafflePlus): - """ - Waffle Switch helper class that provides native support for - namespacing waffle switches and caching within a request. - """ - def is_enabled(self, switch_name): - """ - Returns and caches whether the given waffle switch is enabled. - """ - namespaced_switch_name = self._namespaced_setting_name(switch_name) - value = self._cached_switches.get(namespaced_switch_name) - if value is None: - value = switch_is_active(namespaced_switch_name) - self._cached_switches[namespaced_switch_name] = value - return value - - @contextmanager - def override(self, switch_name, active=True): - """ - Overrides the active value for the given switch for the duration of this - contextmanager. - Note: The value is overridden in the request cache AND in the model. - """ - previous_active = self.is_enabled(switch_name) - try: - self.override_for_request(switch_name, active) - with self.override_in_model(switch_name, active): - yield - finally: - self.override_for_request(switch_name, previous_active) - - def override_for_request(self, switch_name, active=True): - """ - Overrides the active value for the given switch for the remainder of - this request (as this is not a context manager). - Note: The value is overridden in the request cache, not in the model. - """ - namespaced_switch_name = self._namespaced_setting_name(switch_name) - self._cached_switches[namespaced_switch_name] = active - log.info(u"%sSwitch '%s' set to %s for request.", self.log_prefix, namespaced_switch_name, active) - - @contextmanager - def override_in_model(self, switch_name, active=True): - """ - Overrides the active value for the given switch for the duration of this - contextmanager. - Note: The value is overridden in the model, not the request cache. - """ - namespaced_switch_name = self._namespaced_setting_name(switch_name) - with waffle_override_switch(namespaced_switch_name, active): - log.info(u"%sSwitch '%s' set to %s in model.", self.log_prefix, namespaced_switch_name, active) - yield - - @property - def _cached_switches(self): - """ - Returns cached active values of all switches in this namespace. - """ - return self._all_cached_switches.setdefault(self.namespace, {}) - - @property - def _all_cached_switches(self): - """ - Returns dictionary of all switches in the request cache, - keyed by namespace. - """ - return self._get_request_cache().setdefault('switches', {}) diff --git a/openedx/core/lib/courses.py b/openedx/core/lib/courses.py index 193eb53764..abf109eb89 100644 --- a/openedx/core/lib/courses.py +++ b/openedx/core/lib/courses.py @@ -1,11 +1,15 @@ """ Common utility functions related to courses. """ +from django import forms from django.conf import settings +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import CourseKey from xmodule.assetstore.assetmgr import AssetManager from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import modulestore def course_image_url(course, image_key='course_image'): @@ -43,3 +47,37 @@ def create_course_image_thumbnail(course, dimensions): _content, thumb_loc = contentstore().generate_thumbnail(course_image, dimensions=dimensions) return StaticContent.serialize_asset_key_with_slash(thumb_loc) + + +def clean_course_id(model_form, is_required=True): + """ + Cleans and validates a course_id for use with a Django ModelForm. + + Arguments: + model_form (form.ModelForm): The form that has a course_id. + is_required (Boolean): Default True. When True, validates that the + course_id is not empty. In all cases, when course_id is supplied, + validates that it is a valid course. + + Returns: + (CourseKey) The cleaned and validated course_id as a CourseKey. + + NOTE: This should ultimately replace all copies of "def clean_course_id". + + """ + cleaned_id = model_form.cleaned_data["course_id"] + + if not cleaned_id and not is_required: + return None + + try: + course_key = CourseKey.from_string(cleaned_id) + except InvalidKeyError: + msg = u'Course id invalid. Entered course id was: "{0}."'.format(cleaned_id) + raise forms.ValidationError(msg) + + if not modulestore().has_course(course_key): + msg = u'Course not found. Entered course id was: "{0}". '.format(course_key.to_deprecated_string()) + raise forms.ValidationError(msg) + + return course_key diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index eb20680596..89aba3e6d8 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -1,18 +1,22 @@ """ Unified course experience settings and helper methods. """ - import waffle +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlagNamespace from request_cache.middleware import RequestCache -# Waffle flag to enable a single unified "Course" tab. -UNIFIED_COURSE_EXPERIENCE_FLAG = 'unified_course_experience' - -# Waffle flag to enable the full screen course content view -# along with a unified course home page. +# Waffle flag to enable the full screen course content view along with a unified +# course home page. +# NOTE: This is the only legacy flag that does not use the namespace. UNIFIED_COURSE_VIEW_FLAG = 'unified_course_view' +# Namespace for course experience waffle flags. +WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='course_experience') + +# Waffle flag to enable a single unified "Course" tab. +UNIFIED_COURSE_TAB_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'unified_course_tab') + def default_course_url_name(request=None): """ @@ -24,11 +28,16 @@ def default_course_url_name(request=None): return 'courseware' -def course_home_url_name(request=None): +def course_home_url_name(course_key): """ Returns the course home page's URL name for the current user. + + Arguments: + course_key (CourseKey): The course key for which the home url is being + requested. + """ - if waffle.flag_is_active(request or RequestCache.get_current_request(), UNIFIED_COURSE_EXPERIENCE_FLAG): + if UNIFIED_COURSE_TAB_FLAG.is_enabled(course_key): return 'openedx.course_experience.course_home' else: return 'info' diff --git a/openedx/features/course_experience/templates/course_experience/course-home-fragment.html b/openedx/features/course_experience/templates/course_experience/course-home-fragment.html index f2fa73142c..ae76038715 100644 --- a/openedx/features/course_experience/templates/course_experience/course-home-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-home-fragment.html @@ -5,7 +5,6 @@ <%! import json -import waffle from django.conf import settings from django.utils.translation import ugettext as _ @@ -15,7 +14,7 @@ from django.core.urlresolvers import reverse from django_comment_client.permissions import has_permission from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG %> <%block name="content"> @@ -58,7 +57,7 @@ from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG
- % if welcome_message_fragment and waffle.flag_is_active(request, UNIFIED_COURSE_EXPERIENCE_FLAG): + % if welcome_message_fragment and UNIFIED_COURSE_TAB_FLAG.is_enabled(course.id):
${HTML(welcome_message_fragment.body_html())}
@@ -76,7 +75,7 @@ from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG ${_("Bookmarks")} - % if waffle.flag_is_active(request, UNIFIED_COURSE_EXPERIENCE_FLAG): + % if UNIFIED_COURSE_TAB_FLAG.is_enabled(course.id):
  • diff --git a/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html b/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html index 993b996acf..17b0c284b2 100644 --- a/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-updates-fragment.html @@ -4,17 +4,9 @@ <%namespace name='static' file='../static_content.html'/> <%! -import json - -from django.conf import settings from django.utils.translation import ugettext as _ -from django.template.defaultfilters import escapejs -from django.core.urlresolvers import reverse -from django_comment_client.permissions import has_permission -from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG %> <%block name="content"> 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 3795c3b19f..1c6f934f50 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -2,18 +2,16 @@ Tests for the course home page. """ -from waffle.testutils import override_flag - from django.core.urlresolvers import reverse +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG from student.models import CourseEnrollment from student.tests.factories import UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls -from openedx.features.course_experience import UNIFIED_COURSE_EXPERIENCE_FLAG - from .test_course_updates import create_course_update, remove_course_updates TEST_PASSWORD = 'test' @@ -71,22 +69,13 @@ class TestCourseHomePage(SharedModuleStoreTestCase): remove_course_updates(self.course) super(TestCourseHomePage, self).tearDown() - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) - def test_unified_page(self): - """ - Verify the rendering of the unified page. - """ - url = course_home_url(self.course) - response = self.client.get(url) - self.assertContains(response, '

    Test Course

    ') - - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=True) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True) def test_welcome_message_when_unified(self): url = course_home_url(self.course) response = self.client.get(url) self.assertContains(response, TEST_WELCOME_MESSAGE, status_code=200) - @override_flag(UNIFIED_COURSE_EXPERIENCE_FLAG, active=False) + @override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=False) def test_welcome_message_when_not_unified(self): url = course_home_url(self.course) response = self.client.get(url) @@ -100,7 +89,7 @@ class TestCourseHomePage(SharedModuleStoreTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(43): + with self.assertNumQueries(42): with check_mongo_calls(5): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 1ce91a1719..b6471b274c 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -124,7 +124,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): course_updates_url(self.course) # Fetch the view and verify that the query counts haven't changed - with self.assertNumQueries(32): + with self.assertNumQueries(33): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url)