From 216b99264a50011cb313f5b391abeae61870acee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Behmo?= <regis@behmo.com>
Date: Thu, 5 Nov 2020 09:53:47 +0100
Subject: [PATCH] Upgrade waffle classes to the new edx-toggles API
Waffle classes no longer have namespaces. All features are moved to the
WaffleFlag/WaffleSwitch classes.
Here we use the edx_toggles.toggles.__future__ API, which is available
in 1.2.0. This means that we don't have to upgrade edx-toggles. We
should remove the __future__ imports as soon as we upgrade to 2.0.0.
---
lms/djangoapps/experiments/flags.py | 35 ++++++++++++++++---
.../experiments/tests/test_flags.py | 9 +++++
openedx/core/djangoapps/schedules/config.py | 8 ++++-
.../core/djangoapps/waffle_utils/__init__.py | 9 +++--
openedx/core/djangoapps/waffle_utils/views.py | 6 ++--
.../tests/views/test_course_home.py | 6 ++--
.../tests/views/test_course_outline.py | 1 +
.../course_experience/views/course_outline.py | 4 +--
8 files changed, 58 insertions(+), 20 deletions(-)
diff --git a/lms/djangoapps/experiments/flags.py b/lms/djangoapps/experiments/flags.py
index 645be723626..4d9fd09c67c 100644
--- a/lms/djangoapps/experiments/flags.py
+++ b/lms/djangoapps/experiments/flags.py
@@ -4,7 +4,6 @@ Feature flag support for experiments
import datetime
import logging
-from contextlib import contextmanager
import dateutil
import pytz
@@ -62,6 +61,7 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
def test_my_experiment(self):
...
"""
+
def __init__(
self,
waffle_namespace,
@@ -81,6 +81,33 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
]
self.use_course_aware_bucketing = use_course_aware_bucketing
+ @property
+ def _app_label(self):
+ """
+ By convention, the app label associated to an experiment waffle flag is the dotted prefix of the flag name. For
+ example: if the flag name is "grades.my.experiment.waffle.flag", then the `_app_label` will be "grades".
+ This app label replaces what was formerly known as the waffle flag namespace.
+ """
+ return self._split_name[0]
+
+ @property
+ def _experiment_name(self):
+ """
+ By convention, the app label associated to an experiment waffle flag is the first dotted suffix of the flag
+ name. For example: if the flag name name is "grades.my.experiment.waffle.flag", then the `_experiment_name`
+ will be "my.experiment.waffle.flag".
+ """
+ return self._split_name[1]
+
+ @property
+ def _split_name(self):
+ """
+ Return the flag name prefix (before the first dot) and suffix. This raises a ValueError if the flag does not
+ contain a dot ".".
+ """
+ prefix, suffix = self.name.split(".", maxsplit=1)
+ return prefix, suffix
+
def _cache_bucket(self, key, value):
request_cache = RequestCache('experiments')
request_cache.set(key, value)
@@ -178,7 +205,7 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
# If we are using course-aware bucketing, then also append that course key
# to `bucketing_group_name`, such that users can be hashed into different
# buckets for different course-runs.
- experiment_name = bucketing_group_name = self.namespaced_flag_name
+ experiment_name = bucketing_group_name = self.name
if course_key:
experiment_name += ".{}".format(course_key)
if course_key and self.use_course_aware_bucketing:
@@ -225,8 +252,8 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
event_name='edx.bi.experiment.user.bucketed',
properties={
'site': request.site.domain,
- 'app_label': self.waffle_namespace.name,
- 'experiment': self.flag_name,
+ 'app_label': self._app_label,
+ 'experiment': self._experiment_name,
'course_id': str(course_key) if course_key else None,
'bucket': bucket,
'is_staff': user.is_staff,
diff --git a/lms/djangoapps/experiments/tests/test_flags.py b/lms/djangoapps/experiments/tests/test_flags.py
index 0540a65c8a7..425fd788e23 100644
--- a/lms/djangoapps/experiments/tests/test_flags.py
+++ b/lms/djangoapps/experiments/tests/test_flags.py
@@ -156,6 +156,15 @@ class ExperimentWaffleFlagTests(SharedModuleStoreTestCase):
self.assertEqual(self.flag.get_bucket(), expected_bucket)
self.assertEqual(self.flag.is_experiment_on(), active)
+ def test_app_label_experiment_name(self):
+ # pylint: disable=protected-access
+ self.assertEqual("experiments", self.flag._app_label)
+ self.assertEqual("test", self.flag._experiment_name)
+
+ flag = ExperimentWaffleFlag("namespace", "flag.name", __name__)
+ self.assertEqual("namespace", flag._app_label)
+ self.assertEqual("flag.name", flag._experiment_name)
+
class ExperimentWaffleFlagCourseAwarenessTest(SharedModuleStoreTestCase):
"""
diff --git a/openedx/core/djangoapps/schedules/config.py b/openedx/core/djangoapps/schedules/config.py
index 30571996ea0..82c31888100 100644
--- a/openedx/core/djangoapps/schedules/config.py
+++ b/openedx/core/djangoapps/schedules/config.py
@@ -3,7 +3,13 @@ Contains configuration for schedules app
"""
-from edx_toggles.toggles import LegacyWaffleFlag, LegacyWaffleFlagNamespace, LegacyWaffleSwitch, LegacyWaffleSwitchNamespace
+from edx_toggles.toggles import (
+ LegacyWaffleFlag,
+ LegacyWaffleFlagNamespace,
+ LegacyWaffleSwitch,
+ LegacyWaffleSwitchNamespace
+)
+
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='schedules')
diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py
index 2f03aea2701..09150e6ceea 100644
--- a/openedx/core/djangoapps/waffle_utils/__init__.py
+++ b/openedx/core/djangoapps/waffle_utils/__init__.py
@@ -131,7 +131,7 @@ class WaffleFlag(LegacyWaffleFlag):
yield
-class CourseWaffleFlag(BaseWaffleFlag):
+class CourseWaffleFlag(LegacyWaffleFlag):
"""
Represents a single waffle flag that can be forced on/off for a course. This class should be used instead of
WaffleFlag when in the context of a course.
@@ -165,13 +165,13 @@ class CourseWaffleFlag(BaseWaffleFlag):
from .models import WaffleFlagCourseOverrideModel
cache_key = "{}.{}".format(self.namespaced_flag_name, str(course_key))
- course_override = self._cached_flags.get(cache_key)
+ course_override = self.cached_flags().get(cache_key)
if course_override is None:
course_override = WaffleFlagCourseOverrideModel.override_value(
self.namespaced_flag_name, course_key
)
- self._cached_flags[cache_key] = course_override
+ self.cached_flags()[cache_key] = course_override
if course_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.on:
return True
@@ -196,7 +196,6 @@ class CourseWaffleFlag(BaseWaffleFlag):
)
is_enabled_for_course = self._get_course_override_value(course_key)
if is_enabled_for_course is not None:
- # pylint: disable=protected-access
- self.waffle_namespace._monitor_value(self.flag_name, is_enabled_for_course)
+ self.set_monitor_value(is_enabled_for_course)
return is_enabled_for_course
return super().is_enabled()
diff --git a/openedx/core/djangoapps/waffle_utils/views.py b/openedx/core/djangoapps/waffle_utils/views.py
index 8c409c2c5cb..0608cf4e33a 100644
--- a/openedx/core/djangoapps/waffle_utils/views.py
+++ b/openedx/core/djangoapps/waffle_utils/views.py
@@ -49,8 +49,7 @@ class ToggleStateView(views.APIView):
"""
waffle_switch_instances = WaffleSwitch.get_instances()
for switch_instance in waffle_switch_instances:
- switch_name = switch_instance.namespaced_switch_name
- switch = self._get_or_create_toggle_response(switches_dict, switch_name)
+ switch = self._get_or_create_toggle_response(switches_dict, switch_instance.name)
self._add_toggle_instance_details(switch, switch_instance)
def _add_waffle_switch_state(self, switches_dict):
@@ -98,8 +97,7 @@ class ToggleStateView(views.APIView):
"""
waffle_flag_instances = WaffleFlag.get_instances()
for flag_instance in waffle_flag_instances:
- flag_name = flag_instance.namespaced_flag_name
- flag = self._get_or_create_toggle_response(flags_dict, flag_name)
+ flag = self._get_or_create_toggle_response(flags_dict, flag_instance.name)
self._add_toggle_instance_details(flag, flag_instance)
def _add_toggle_instance_details(self, toggle, toggle_instance):
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 9c1bf73fb87..5bf97e406b4 100644
--- a/openedx/features/course_experience/tests/views/test_course_home.py
+++ b/openedx/features/course_experience/tests/views/test_course_home.py
@@ -951,9 +951,9 @@ class CourseHomeFragmentViewTests(ModuleStoreTestCase):
self.user = UserFactory()
self.client.login(username=self.user.username, password=TEST_PASSWORD)
- name = SHOW_UPGRADE_MSG_ON_COURSE_HOME.waffle_namespace._namespaced_name(
- SHOW_UPGRADE_MSG_ON_COURSE_HOME.flag_name)
- self.flag, __ = Flag.objects.update_or_create(name=name, defaults={'everyone': True})
+ self.flag, __ = Flag.objects.update_or_create(
+ name=SHOW_UPGRADE_MSG_ON_COURSE_HOME.name, defaults={'everyone': True}
+ )
def assert_upgrade_message_not_displayed(self):
response = self.client.get(self.url)
diff --git a/openedx/features/course_experience/tests/views/test_course_outline.py b/openedx/features/course_experience/tests/views/test_course_outline.py
index 8ac1fa5da19..28064fa4843 100644
--- a/openedx/features/course_experience/tests/views/test_course_outline.py
+++ b/openedx/features/course_experience/tests/views/test_course_outline.py
@@ -720,6 +720,7 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
switch_name = ENABLE_COMPLETION_TRACKING_SWITCH.name
switch, _ = Switch.objects.get_or_create(name=switch_name)
+ # pylint: disable=protected-access
self.assertEqual(switch.created, view._completion_data_collection_start())
switch.delete()
diff --git a/openedx/features/course_experience/views/course_outline.py b/openedx/features/course_experience/views/course_outline.py
index 80f0566bf4f..8fa266117f5 100644
--- a/openedx/features/course_experience/views/course_outline.py
+++ b/openedx/features/course_experience/views/course_outline.py
@@ -166,10 +166,8 @@ class CourseOutlineFragmentView(EdxFragmentView):
"""
Returns the date that the ENABLE_COMPLETION_TRACKING waffle switch was enabled.
"""
- # pylint: disable=protected-access
- switch_name = completion_waffle.ENABLE_COMPLETION_TRACKING_SWITCH.name
try:
- return Switch.objects.get(name=switch_name).created
+ return Switch.objects.get(name=ENABLE_COMPLETION_TRACKING_SWITCH.name).created
except Switch.DoesNotExist:
return DEFAULT_COMPLETION_TRACKING_START
--
GitLab