From 77e490f0578cbaa5a4c2e6110b848cceef30962b Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 9 Jul 2020 09:31:31 -0400 Subject: [PATCH] ARCHBOM-1305: remove deprecated flag_undefined_default (#24426) This is the final step in removing the deprecated flag_undefined_default as explained by the following ADR: https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst Notes: * All uses of flag_undefined_default=False were always supposed to have been no-ops. * All uses of flag_undefined_default=True that are removed in this PR have been replaced by migrations in past PRs. * The temporary metric temp_flag_default_used id no longer reporting any data. ARCHBOM-1305 --- cms/djangoapps/contentstore/config/waffle.py | 3 - cms/djangoapps/contentstore/views/videos.py | 1 - lms/djangoapps/course_api/blocks/toggles.py | 1 - lms/djangoapps/experiments/flags.py | 2 +- lms/djangoapps/experiments/views_custom.py | 1 - lms/djangoapps/grades/config/waffle.py | 4 - lms/djangoapps/grades/tests/test_tasks.py | 20 +-- lms/djangoapps/instructor/toggles.py | 1 - .../instructor_task/config/waffle.py | 2 - lms/djangoapps/verify_student/toggles.py | 1 - .../core/djangoapps/models/config/waffle.py | 1 - openedx/core/djangoapps/schedules/config.py | 2 - .../djangoapps/user_authn/views/register.py | 1 - .../video_pipeline/config/waffle.py | 2 - .../core/djangoapps/waffle_utils/__init__.py | 116 ++++++++---------- .../0001-refactor-waffle-flag-default.rst | 2 + .../waffle_utils/tests/test_init.py | 46 ++++--- .../features/course_experience/__init__.py | 2 +- openedx/features/discounts/applicability.py | 1 - 19 files changed, 89 insertions(+), 120 deletions(-) diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index 021020c1f2..ef9ed250ad 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -30,18 +30,15 @@ def waffle_flags(): ENABLE_PROCTORING_PROVIDER_OVERRIDES = CourseWaffleFlag( waffle_namespace=waffle_flags(), flag_name=u'enable_proctoring_provider_overrides', - flag_undefined_default=False ) # TODO: After removing this flag, add a migration to remove waffle flag in a follow-up deployment. ENABLE_CHECKLISTS_QUALITY = CourseWaffleFlag( waffle_namespace=waffle_flags(), flag_name=u'enable_checklists_quality', - flag_undefined_default=True ) SHOW_REVIEW_RULES_FLAG = CourseWaffleFlag( waffle_namespace=waffle_flags(), flag_name=u'show_review_rules', - flag_undefined_default=False ) diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index d2486c6a30..5717323da5 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -81,7 +81,6 @@ WAFFLE_STUDIO_FLAG_NAMESPACE = WaffleFlagNamespace(name=u'studio') ENABLE_VIDEO_UPLOAD_PAGINATION = CourseWaffleFlag( waffle_namespace=WAFFLE_STUDIO_FLAG_NAMESPACE, flag_name=u'enable_video_upload_pagination', - flag_undefined_default=False ) # Default expiration, in seconds, of one-time URLs used for uploading videos. KEY_EXPIRATION_IN_SECONDS = 86400 diff --git a/lms/djangoapps/course_api/blocks/toggles.py b/lms/djangoapps/course_api/blocks/toggles.py index 7335dd3faf..c4c1b6fe87 100644 --- a/lms/djangoapps/course_api/blocks/toggles.py +++ b/lms/djangoapps/course_api/blocks/toggles.py @@ -23,5 +23,4 @@ COURSE_BLOCKS_API_NAMESPACE = WaffleFlagNamespace(name=u'course_blocks_api') HIDE_ACCESS_DENIALS_FLAG = WaffleFlag( waffle_namespace=COURSE_BLOCKS_API_NAMESPACE, flag_name=u'hide_access_denials', - flag_undefined_default=False ) diff --git a/lms/djangoapps/experiments/flags.py b/lms/djangoapps/experiments/flags.py index 7b5a5ee923..2c6cd5ad16 100644 --- a/lms/djangoapps/experiments/flags.py +++ b/lms/djangoapps/experiments/flags.py @@ -61,7 +61,7 @@ class ExperimentWaffleFlag(CourseWaffleFlag): self.num_buckets = num_buckets self.experiment_id = experiment_id self.bucket_flags = [ - CourseWaffleFlag(waffle_namespace, '{}.{}'.format(flag_name, bucket), flag_undefined_default=False) + CourseWaffleFlag(waffle_namespace, '{}.{}'.format(flag_name, bucket)) for bucket in range(num_buckets) ] diff --git a/lms/djangoapps/experiments/views_custom.py b/lms/djangoapps/experiments/views_custom.py index 23f4ede863..1068e28152 100644 --- a/lms/djangoapps/experiments/views_custom.py +++ b/lms/djangoapps/experiments/views_custom.py @@ -45,7 +45,6 @@ from track import segment MOBILE_UPSELL_FLAG = WaffleFlag( waffle_namespace=WaffleFlagNamespace(name=u'experiments'), flag_name=u'mobile_upsell_rev934', - flag_undefined_default=False ) MOBILE_UPSELL_EXPERIMENT = 'mobile_upsell_experiment' diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py index e23e6f7571..2aa6dff21c 100644 --- a/lms/djangoapps/grades/config/waffle.py +++ b/lms/djangoapps/grades/config/waffle.py @@ -38,25 +38,21 @@ def waffle_flags(): REJECTED_EXAM_OVERRIDES_GRADE: CourseWaffleFlag( namespace, REJECTED_EXAM_OVERRIDES_GRADE, - flag_undefined_default=True, ), # TODO: After removing this flag, add a migration to remove waffle flag in a follow-up deployment. ENFORCE_FREEZE_GRADE_AFTER_COURSE_END: CourseWaffleFlag( namespace, ENFORCE_FREEZE_GRADE_AFTER_COURSE_END, - flag_undefined_default=True, ), # Have this course override flag so we can selectively turn off the gradebook for courses. # TODO: After removing this flag, add a migration to remove waffle flag in a follow-up deployment. WRITABLE_GRADEBOOK: CourseWaffleFlag( namespace, WRITABLE_GRADEBOOK, - flag_undefined_default=True, ), BULK_MANAGEMENT: CourseWaffleFlag( namespace, BULK_MANAGEMENT, - flag_undefined_default=False, ), } diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 316be35a76..c836f1e33e 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,10 +164,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 37, True), - (ModuleStoreEnum.Type.mongo, 1, 37, False), - (ModuleStoreEnum.Type.split, 3, 37, True), - (ModuleStoreEnum.Type.split, 3, 37, False), + (ModuleStoreEnum.Type.mongo, 1, 36, True), + (ModuleStoreEnum.Type.mongo, 1, 36, False), + (ModuleStoreEnum.Type.split, 3, 36, True), + (ModuleStoreEnum.Type.split, 3, 36, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 37), - (ModuleStoreEnum.Type.split, 3, 37), + (ModuleStoreEnum.Type.mongo, 1, 36), + (ModuleStoreEnum.Type.split, 3, 36), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -225,8 +225,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 20), - (ModuleStoreEnum.Type.split, 3, 20), + (ModuleStoreEnum.Type.mongo, 1, 19), + (ModuleStoreEnum.Type.split, 3, 19), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -240,8 +240,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, 38), - (ModuleStoreEnum.Type.split, 3, 38), + (ModuleStoreEnum.Type.mongo, 1, 37), + (ModuleStoreEnum.Type.split, 3, 37), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor/toggles.py b/lms/djangoapps/instructor/toggles.py index 3242b12fb9..a35264e10e 100644 --- a/lms/djangoapps/instructor/toggles.py +++ b/lms/djangoapps/instructor/toggles.py @@ -22,7 +22,6 @@ WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='instructor') OPTIMISED_IS_SMALL_COURSE = WaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, flag_name='optimised_is_small_course', - flag_undefined_default=False ) diff --git a/lms/djangoapps/instructor_task/config/waffle.py b/lms/djangoapps/instructor_task/config/waffle.py index 134ece8952..25cdae96ef 100644 --- a/lms/djangoapps/instructor_task/config/waffle.py +++ b/lms/djangoapps/instructor_task/config/waffle.py @@ -25,12 +25,10 @@ def waffle_flags(): GENERATE_PROBLEM_GRADE_REPORT_VERIFIED_ONLY: CourseWaffleFlag( waffle_namespace=INSTRUCTOR_TASK_WAFFLE_FLAG_NAMESPACE, flag_name=GENERATE_PROBLEM_GRADE_REPORT_VERIFIED_ONLY, - flag_undefined_default=False, ), GENERATE_COURSE_GRADE_REPORT_VERIFIED_ONLY: CourseWaffleFlag( waffle_namespace=INSTRUCTOR_TASK_WAFFLE_FLAG_NAMESPACE, flag_name=GENERATE_COURSE_GRADE_REPORT_VERIFIED_ONLY, - flag_undefined_default=False, ), } diff --git a/lms/djangoapps/verify_student/toggles.py b/lms/djangoapps/verify_student/toggles.py index 74f5a42b55..c2fb3fd58d 100644 --- a/lms/djangoapps/verify_student/toggles.py +++ b/lms/djangoapps/verify_student/toggles.py @@ -22,7 +22,6 @@ WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='verify_student') USE_NEW_EMAIL_TEMPLATES = WaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, flag_name='use_new_email_templates', - flag_undefined_default=False ) diff --git a/openedx/core/djangoapps/models/config/waffle.py b/openedx/core/djangoapps/models/config/waffle.py index 7db160b43b..a5f53163ec 100644 --- a/openedx/core/djangoapps/models/config/waffle.py +++ b/openedx/core/djangoapps/models/config/waffle.py @@ -26,7 +26,6 @@ def waffle_flags(): COURSE_DETAIL_UPDATE_CERTIFICATE_DATE: CourseWaffleFlag( waffle_namespace=COURSE_DETAIL_WAFFLE_NAMESPACE, flag_name=COURSE_DETAIL_UPDATE_CERTIFICATE_DATE, - flag_undefined_default=False, ) } diff --git a/openedx/core/djangoapps/schedules/config.py b/openedx/core/djangoapps/schedules/config.py index 98d39a81ee..506e080fcd 100644 --- a/openedx/core/djangoapps/schedules/config.py +++ b/openedx/core/djangoapps/schedules/config.py @@ -14,13 +14,11 @@ WAFFLE_SWITCH_NAMESPACE = WaffleSwitchNamespace(name=u'schedules') CREATE_SCHEDULE_WAFFLE_FLAG = CourseWaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, flag_name=u'create_schedules_for_course', - flag_undefined_default=False ) COURSE_UPDATE_WAFFLE_FLAG = CourseWaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, flag_name=u'send_updates_for_course', - flag_undefined_default=False ) DEBUG_MESSAGE_WAFFLE_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, u'enable_debugging') diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index d9105f4702..bfc454f303 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -109,7 +109,6 @@ REGISTER_USER = Signal(providing_args=["user", "registration"]) REGISTRATION_FAILURE_LOGGING_FLAG = WaffleFlag( waffle_namespace=WaffleFlagNamespace(name=u'registration'), flag_name=u'enable_failure_logging', - flag_undefined_default=False ) diff --git a/openedx/core/djangoapps/video_pipeline/config/waffle.py b/openedx/core/djangoapps/video_pipeline/config/waffle.py index 78e9bd5a95..3ad2400c6d 100644 --- a/openedx/core/djangoapps/video_pipeline/config/waffle.py +++ b/openedx/core/djangoapps/video_pipeline/config/waffle.py @@ -27,11 +27,9 @@ def waffle_flags(): ENABLE_DEVSTACK_VIDEO_UPLOADS: WaffleFlag( waffle_namespace=namespace, flag_name=ENABLE_DEVSTACK_VIDEO_UPLOADS, - flag_undefined_default=False ), ENABLE_VEM_PIPELINE: CourseWaffleFlag( waffle_namespace=namespace, flag_name=ENABLE_VEM_PIPELINE, - flag_undefined_default=False ) } diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 0710ebeabc..567ec2daf0 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -226,7 +226,7 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): """ return self._get_request_cache().setdefault('flags', {}) - def is_flag_active(self, flag_name, check_before_waffle_callback=None, flag_undefined_default=None): + def is_flag_active(self, flag_name, check_before_waffle_callback=None): """ Returns and caches whether the provided flag is active. @@ -239,6 +239,10 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): Important: Caching for the check_before_waffle_callback must be handled by the callback itself. + Note: A waffle flag's default is False if not defined. If you think you + need the default to be True, see the module docstring for + alternatives. + Arguments: flag_name (String): The name of the flag to check. check_before_waffle_callback (function): (Optional) A function that @@ -246,62 +250,55 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): check_before_waffle_callback(namespaced_flag_name) returns True or False, it is returned. If it returns None, then waffle is used. - DEPRECATED flag_undefined_default (Boolean): A default value to be - returned if the waffle flag is to be checked, but doesn't exist. - See module docstring for alternative. + + """ + # validate arguments + namespaced_flag_name = self._namespaced_name(flag_name) + + if check_before_waffle_callback: + value = check_before_waffle_callback(namespaced_flag_name) + if value is not None: + # Do not cache value for the callback, because the key might be different. + # The callback needs to handle its own caching if it wants it. + self._set_waffle_flag_metric(namespaced_flag_name, value) + return value + + value = self._cached_flags.get(namespaced_flag_name) + if value is not None: + self._set_waffle_flag_metric(namespaced_flag_name, value) + return value + + request = crum.get_current_request() + if not request: + log.warning(u"%sFlag '%s' accessed without a request", self.log_prefix, namespaced_flag_name) + # Return the Flag's Everyone value if not in a request context. + # Note: this skips the cache as the value might be different + # in a normal request context. This case seems to occur when + # a page redirects to a 404, or for celery workers. + value = self._is_flag_active_for_everyone(namespaced_flag_name) + self._set_waffle_flag_metric(namespaced_flag_name, value) + set_custom_metric('warn_flag_no_request_return_value', value) + return value + + value = flag_is_active(request, namespaced_flag_name) + self._cached_flags[namespaced_flag_name] = value + + self._set_waffle_flag_metric(namespaced_flag_name, value) + return value + + def _is_flag_active_for_everyone(self, namespaced_flag_name): + """ + Returns True if the waffle flag is configured as active for Everyone, + False otherwise. """ # Import is placed here to avoid model import at project startup. from waffle.models import Flag - if flag_undefined_default: - warnings.warn( - # NOTE: This will be removed once ARCHBOM-132, currently in-progress, is complete. - 'flag_undefined_default has been deprecated. For existing uses this is already actively being fixed.', - DeprecationWarning - ) - - # validate arguments - namespaced_flag_name = self._namespaced_name(flag_name) - value = None - if check_before_waffle_callback: - value = check_before_waffle_callback(namespaced_flag_name) - - if value is None: - # Do not get cached value for the callback, because the key might be different. - # The callback needs to handle its own caching if it wants it. - value = self._cached_flags.get(namespaced_flag_name) - if value is None: - is_flag_active_for_everyone = False - if flag_undefined_default is not None: - # determine if the flag is undefined in waffle - try: - waffle_flag = Flag.objects.get(name=namespaced_flag_name) - is_flag_active_for_everyone = (waffle_flag.everyone is True) - except Flag.DoesNotExist: - if flag_undefined_default: - # This metric will go away once this has been fully retired with ARCHBOM-132. - # Also, even though the value will only track the last flag, that should be enough. - set_custom_metric('temp_flag_default_used', namespaced_flag_name) - value = flag_undefined_default - - if value is None: - request = crum.get_current_request() - if request: - value = flag_is_active(request, namespaced_flag_name) - else: - log.warning(u"%sFlag '%s' accessed without a request", self.log_prefix, namespaced_flag_name) - # Return the Flag's Everyone value if not in a request context. - # Note: this skips the cache as the value might be different - # in a normal request context. This case seems to occur when - # a page redirects to a 404, or in Celery workers. - self._set_waffle_flag_metric(namespaced_flag_name, is_flag_active_for_everyone) - set_custom_metric('warn_flag_no_request_return_value', is_flag_active_for_everyone) - return is_flag_active_for_everyone - - self._cached_flags[namespaced_flag_name] = value - - self._set_waffle_flag_metric(namespaced_flag_name, value) - return value + try: + waffle_flag = Flag.objects.get(name=namespaced_flag_name) + return (waffle_flag.everyone is True) + except Flag.DoesNotExist: + return False def _set_waffle_flag_metric(self, name, value): """ @@ -378,16 +375,14 @@ class WaffleFlag(object): Represents a single waffle flag, using a cached waffle namespace. """ - def __init__(self, waffle_namespace, flag_name, flag_undefined_default=None): + def __init__(self, waffle_namespace, flag_name): """ Initializes the waffle flag instance. Arguments: waffle_namespace (WaffleFlagNamespace | String): Namespace for this flag. flag_name (String): The name of the flag (without namespacing). - DEPRECATED flag_undefined_default (Boolean): A default value to be returned - if the waffle flag is to be checked, but doesn't exist. See module - docstring for alternative. + """ if isinstance(waffle_namespace, six.string_types): waffle_namespace = WaffleFlagNamespace(name=waffle_namespace) @@ -395,7 +390,6 @@ class WaffleFlag(object): self.waffle_namespace = waffle_namespace self.waffle_namespace = waffle_namespace self.flag_name = flag_name - self.flag_undefined_default = flag_undefined_default @property def namespaced_flag_name(self): @@ -408,10 +402,7 @@ class WaffleFlag(object): """ Returns whether or not the flag is enabled. """ - return self.waffle_namespace.is_flag_active( - self.flag_name, - flag_undefined_default=self.flag_undefined_default - ) + return self.waffle_namespace.is_flag_active(self.flag_name) @contextmanager def override(self, active=True): @@ -476,7 +467,6 @@ class CourseWaffleFlag(WaffleFlag): return self.waffle_namespace.is_flag_active( self.flag_name, check_before_waffle_callback=self._get_course_override_callback(course_key), - flag_undefined_default=self.flag_undefined_default ) def is_enabled_without_course_context(self): diff --git a/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst b/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst index d38ce65273..694e7133f5 100644 --- a/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst +++ b/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst @@ -30,6 +30,8 @@ Consequences * We will need to implement one of the alternate solutions for each flag currently using ``flag_undefined_default=True``, and then finally remove the deprecated ``flag_undefined_default`` argument. +UPDATE: This work has been completed and ``flag_undefined_default`` has been removed. + Rejected Alternatives ===================== diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index f29b16e40f..1ce931f592 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -94,19 +94,13 @@ class TestCourseWaffleFlag(TestCase): @override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME]) @patch('openedx.core.djangoapps.waffle_utils.set_custom_metric') - @ddt.data( - {'flag_undefined_default': None, 'result': False}, - {'flag_undefined_default': False, 'result': False}, - {'flag_undefined_default': True, 'result': True}, - ) - def test_undefined_waffle_flag(self, data, mock_set_custom_metric): + def test_undefined_waffle_flag(self, mock_set_custom_metric): """ - Test flag with various defaults provided for undefined waffle flags. + Test flag with undefined waffle flag. """ test_course_flag = CourseWaffleFlag( self.TEST_NAMESPACE, self.FLAG_NAME, - flag_undefined_default=data['flag_undefined_default'] ) with patch( @@ -119,8 +113,8 @@ class TestCourseWaffleFlag(TestCase): return_value=WaffleFlagCourseOverrideModel.ALL_CHOICES.unset ): # check twice to test that the result is properly cached - self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result']) - self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result']) + self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) + self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) # result is cached, so override check should happen once WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( self.NAMESPACED_FLAG_NAME, @@ -129,26 +123,31 @@ class TestCourseWaffleFlag(TestCase): self._assert_waffle_flag_metric( mock_set_custom_metric, - expected_flag_value=str(data['result']), - flag_undefined_default=data['flag_undefined_default'], + expected_flag_value=str(False), ) - @ddt.data( - {'flag_undefined_default': None, 'result': False}, - {'flag_undefined_default': False, 'result': False}, - {'flag_undefined_default': True, 'result': True}, - ) - def test_without_request(self, data): + def test_without_request_and_undefined_waffle(self): """ - Test the flag behavior when outside a request context. + Test the flag behavior when outside a request context and waffle data undefined. """ crum.set_current_request(None) test_course_flag = CourseWaffleFlag( self.TEST_NAMESPACE, self.FLAG_NAME, - flag_undefined_default=data['flag_undefined_default'] ) - self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result']) + self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) + + def test_without_request_and_everyone_active_waffle(self): + """ + Test the flag behavior when outside a request context and waffle active for everyone. + """ + crum.set_current_request(None) + test_course_flag = CourseWaffleFlag( + self.TEST_NAMESPACE, + self.FLAG_NAME, + ) + with override_flag(self.NAMESPACED_FLAG_NAME, active=True): + self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), True) @ddt.data( {'expected_count': 0, 'waffle_flag_metric_setting': None}, @@ -169,13 +168,12 @@ class TestCourseWaffleFlag(TestCase): self.assertEqual(mock_set_custom_metric.call_count, data['expected_count']) - def _assert_waffle_flag_metric(self, mock_set_custom_metric, expected_flag_value=None, flag_undefined_default=None): + def _assert_waffle_flag_metric(self, mock_set_custom_metric, expected_flag_value=None): if expected_flag_value: expected_flag_name = 'flag_{}'.format(self.NAMESPACED_FLAG_NAME) expected_calls = [call(expected_flag_name, expected_flag_value)] mock_set_custom_metric.assert_has_calls(expected_calls) - expected_call_count = 2 if flag_undefined_default else 1 - self.assertEqual(mock_set_custom_metric.call_count, expected_call_count) + self.assertEqual(mock_set_custom_metric.call_count, 1) else: self.assertEqual(mock_set_custom_metric.call_count, 0) diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index 4ebf33e98f..90fdd7f476 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -22,7 +22,7 @@ class DefaultTrueWaffleFlagNamespace(WaffleFlagNamespace): and refactor/fix any tests that shouldn't be removed. """ - def is_flag_active(self, flag_name, check_before_waffle_callback=None, flag_undefined_default=None): + def is_flag_active(self, flag_name, check_before_waffle_callback=None): """ Overrides is_flag_active, and returns and caches whether the provided flag is active. diff --git a/openedx/features/discounts/applicability.py b/openedx/features/discounts/applicability.py index a63adddd85..d06518bbae 100644 --- a/openedx/features/discounts/applicability.py +++ b/openedx/features/discounts/applicability.py @@ -40,7 +40,6 @@ from track import segment DISCOUNT_APPLICABILITY_FLAG = WaffleFlag( waffle_namespace=WaffleFlagNamespace(name=u'discounts'), flag_name=u'enable_discounting', - flag_undefined_default=False ) DISCOUNT_APPLICABILITY_HOLDBACK = 'first_purchase_discount_holdback'