Allow omission of course_key in call to CourseWaffleFlag.is_enabled

This effectively evaluates the flag outside of the context of a course.
This was previously available through `.is_enabled_without_course_context`,
which has been removed in favor of simply `is_enabled()`.

This was done to make the CourseWaffleFlag interface more uniform with
that of WaffleFlag and ExperimentWaffleFlag and eliminate unecessary
branching when handling CourseWaffleFlags.
This commit is contained in:
Kyle McCormick
2020-08-07 15:25:22 -04:00
committed by Kyle McCormick
parent b05948153a
commit 662388c7db
3 changed files with 12 additions and 41 deletions

View File

@@ -184,11 +184,7 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
# First check if forced into a particular bucket, using our subordinate bucket flags.
# If not, calculate their default bucket using a consistent hash function.
for i, bucket_flag in enumerate(self.bucket_flags):
forced_into_bucket = (
bucket_flag.is_enabled(course_key) if course_key
else bucket_flag.is_enabled_without_course_context()
)
if forced_into_bucket:
if bucket_flag.is_enabled(course_key):
bucket = i
break
else:
@@ -224,17 +220,12 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
def is_enabled(self, course_key=None):
return self.get_bucket(course_key) != 0
def is_enabled_without_course_context(self):
return self.is_enabled()
def is_experiment_on(self, course_key=None):
"""
Return whether the overall experiment flag is enabled for this user.
This disregards `.bucket_flags`.
"""
if course_key is None:
return super().is_enabled_without_course_context()
return super().is_enabled(course_key)
@contextmanager

View File

@@ -137,11 +137,9 @@ class ExperimentWaffleFlagTests(SharedModuleStoreTestCase):
def test_is_enabled(self):
with patch('experiments.flags.ExperimentWaffleFlag.get_bucket', return_value=1):
self.assertEqual(self.flag.is_enabled_without_course_context(), True)
self.assertEqual(self.flag.is_enabled(self.key), True)
self.assertEqual(self.flag.is_enabled(), True)
with patch('experiments.flags.ExperimentWaffleFlag.get_bucket', return_value=0):
self.assertEqual(self.flag.is_enabled_without_course_context(), False)
self.assertEqual(self.flag.is_enabled(self.key), False)
self.assertEqual(self.flag.is_enabled(), False)

View File

@@ -456,38 +456,20 @@ class CourseWaffleFlag(WaffleFlag):
return course_override_callback
def _is_enabled(self, course_key=None):
"""
Returns whether or not the flag is enabled without error checking.
Arguments:
course_key (CourseKey): The course to check for override before
checking waffle.
"""
return self.waffle_namespace.is_flag_active(
self.flag_name,
check_before_waffle_callback=self._get_course_override_callback(course_key),
)
def is_enabled_without_course_context(self):
"""
Returns whether or not the flag is enabled outside the context of a given course.
This should only be used when a course waffle flag must be used outside of a course.
If this is intended for use with a simple global setting, use simple waffle flag instead.
"""
return self._is_enabled()
def is_enabled(self, course_key):
def is_enabled(self, course_key=None): # pylint: disable=arguments-differ
"""
Returns whether or not the flag is enabled within the context of a given course.
Arguments:
course_key (CourseKey): The course to check for override before
checking waffle.
course_key (Optional[CourseKey]): The course to check for override before
checking waffle. If omitted, check whether the flag is enabled
outside the context of any course.
"""
# validate arguments
assert issubclass(type(course_key), CourseKey), u"The course_key '{}' must be a CourseKey.".format(
str(course_key)
if course_key:
assert isinstance(course_key, CourseKey), (
"Provided course_key '{}' is not instance of CourseKey.".format(course_key)
)
return self.waffle_namespace.is_flag_active(
self.flag_name,
check_before_waffle_callback=self._get_course_override_callback(course_key),
)
return self._is_enabled(course_key)