diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 3759ec5688..f0df220933 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -42,8 +42,8 @@ class CourseWaffleFlag(LegacyWaffleFlag): overriding any behavior configured on the waffle flag itself. "Force off" will disable the waffle flag for all users in a course, overriding any behavior configured on the waffle flag itself. Requires "Enabled" (see below) to apply. - Enabled: This must be marked as enabled in order for the override to be applied. These settings can't be - deleted, so instead, you need to disable if it should no longer apply. + Enabled: Must be marked as "enabled" in order for the override to be applied. These settings can't be + deleted, so instead, you need to add another disabled override entry to disable the override. To configure an org-level override, go to Django Admin "waffle_utils" -> "Waffle flag org overrides". @@ -53,8 +53,8 @@ class CourseWaffleFlag(LegacyWaffleFlag): overriding any behavior configured on the waffle flag itself. "Force off" will disable the waffle flag for all users in a org's courses, overriding any behavior configured on the waffle flag itself. Requires "Enabled" (see below) to apply. - Enabled: This must be marked as enabled in order for the override to be applied. These settings can't be - deleted, so instead, you need to disable if it should no longer apply. + Enabled: Must be marked as "enabled" in order for the override to be applied. These settings can't be + deleted, so instead, you need to add another disabled override entry to disable the override. """ diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_views.py b/openedx/core/djangoapps/waffle_utils/tests/test_views.py index c1bb6f337b..496b7c6d0b 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_views.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_views.py @@ -44,20 +44,38 @@ class ToggleStateViewTests(TestCase): # lint-amnesty, pylint: disable=missing-c assert "on" == response.data["waffle_flags"][0]["course_overrides"][0]["force"] assert "both" == response.data["waffle_flags"][0]["computed_status"] - def test_course_overrides(self): - models.WaffleFlagCourseOverrideModel.objects.create(waffle_flag="my.flag", enabled=True) - course_overrides = {} + def test_response_with_org_override(self): + models.WaffleFlagOrgOverrideModel.objects.create(waffle_flag="my.flag", enabled=True) + response = get_toggle_state_response() + assert response.data["waffle_flags"] + assert "my.flag" == response.data["waffle_flags"][0]["name"] + assert response.data["waffle_flags"][0]["org_overrides"] + assert "" == response.data["waffle_flags"][0]["org_overrides"][0]["org"] + assert "on" == response.data["waffle_flags"][0]["org_overrides"][0]["force"] + assert "both" == response.data["waffle_flags"][0]["computed_status"] - report = toggle_state_views.CourseOverrideToggleStateReport() - report.add_waffle_flag_instances(course_overrides) - report.add_waffle_flag_computed_status(course_overrides) + def test_course_and_org_overrides(self): + models.WaffleFlagCourseOverrideModel.objects.create(waffle_flag='my.flag', enabled=True) + models.WaffleFlagOrgOverrideModel.objects.create(waffle_flag='my.flag', enabled=True) + overrides = {} - assert 'my.flag' in course_overrides - assert 'course_overrides' in course_overrides['my.flag'] - assert 1 == len(course_overrides['my.flag']['course_overrides']) - assert 'None' == course_overrides['my.flag']['course_overrides'][0]['course_id'] - assert 'on' == course_overrides['my.flag']['course_overrides'][0]['force'] - assert 'both' == course_overrides['my.flag']['computed_status'] + report = toggle_state_views.OverrideToggleStateReport() + report.add_waffle_flag_instances(overrides) + report.add_waffle_flag_computed_status(overrides) + + assert 'my.flag' in overrides + + assert 'course_overrides' in overrides['my.flag'] + assert 1 == len(overrides['my.flag']['course_overrides']) + assert 'None' == overrides['my.flag']['course_overrides'][0]['course_id'] + assert 'on' == overrides['my.flag']['course_overrides'][0]['force'] + assert 'both' == overrides['my.flag']['computed_status'] + + assert 'org_overrides' in overrides['my.flag'] + assert 1 == len(overrides['my.flag']['org_overrides']) + assert '' == overrides['my.flag']['org_overrides'][0]['org'] + assert 'on' == overrides['my.flag']['org_overrides'][0]['force'] + assert 'both' == overrides['my.flag']['computed_status'] def test_computed_status(self): models.WaffleFlagCourseOverrideModel.objects.create( @@ -69,21 +87,39 @@ class ToggleStateViewTests(TestCase): # lint-amnesty, pylint: disable=missing-c models.WaffleFlagCourseOverrideModel.objects.create( waffle_flag="my.disabledflag1", enabled=False, course_id="org/course/id" ) + models.WaffleFlagOrgOverrideModel.objects.create( + waffle_flag="my.overriddenflag3", enabled=True, org="org" + ) + models.WaffleFlagOrgOverrideModel.objects.create( + waffle_flag="my.overriddenflag4", enabled=True + ) + models.WaffleFlagOrgOverrideModel.objects.create( + waffle_flag="my.disabledflag2", enabled=False, org="org" + ) - course_overrides = {} - report = toggle_state_views.CourseOverrideToggleStateReport() - report.add_waffle_flag_instances(course_overrides) - report.add_waffle_flag_computed_status(course_overrides) + overrides = {} + report = toggle_state_views.OverrideToggleStateReport() + report.add_waffle_flag_instances(overrides) + report.add_waffle_flag_computed_status(overrides) - assert "both" == course_overrides["my.overriddenflag1"]["computed_status"] - assert "org/course/id" == course_overrides["my.overriddenflag1"]["course_overrides"][0]["course_id"] - assert "on" == course_overrides["my.overriddenflag1"]["course_overrides"][0]["force"] + assert "both" == overrides["my.overriddenflag1"]["computed_status"] + assert "org/course/id" == overrides["my.overriddenflag1"]["course_overrides"][0]["course_id"] + assert "on" == overrides["my.overriddenflag1"]["course_overrides"][0]["force"] - assert "both" == course_overrides["my.overriddenflag2"]["computed_status"] - assert "None" == course_overrides["my.overriddenflag2"]["course_overrides"][0]["course_id"] - assert "on" == course_overrides["my.overriddenflag2"]["course_overrides"][0]["force"] + assert "both" == overrides["my.overriddenflag2"]["computed_status"] + assert "None" == overrides["my.overriddenflag2"]["course_overrides"][0]["course_id"] + assert "on" == overrides["my.overriddenflag2"]["course_overrides"][0]["force"] - assert "my.disabledflag1" not in course_overrides + assert "both" == overrides["my.overriddenflag3"]["computed_status"] + assert "org" == overrides["my.overriddenflag3"]["org_overrides"][0]["org"] + assert "on" == overrides["my.overriddenflag3"]["org_overrides"][0]["force"] + + assert "both" == overrides["my.overriddenflag4"]["computed_status"] + assert "" == overrides["my.overriddenflag4"]["org_overrides"][0]["org"] + assert "on" == overrides["my.overriddenflag4"]["org_overrides"][0]["force"] + + assert "my.disabledflag1" not in overrides + assert "my.disabledflag2" not in overrides def get_toggle_state_response(is_staff=True): diff --git a/openedx/core/djangoapps/waffle_utils/views.py b/openedx/core/djangoapps/waffle_utils/views.py index ce0c1767dc..2a26430d18 100644 --- a/openedx/core/djangoapps/waffle_utils/views.py +++ b/openedx/core/djangoapps/waffle_utils/views.py @@ -2,6 +2,7 @@ Views that we will use to view toggle state in edx-platform. """ from collections import OrderedDict +from enum import Enum from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.permissions import IsStaff @@ -10,34 +11,40 @@ from rest_framework import views from rest_framework.authentication import SessionAuthentication from rest_framework.response import Response -from .models import WaffleFlagCourseOverrideModel +from .models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel -class CourseOverrideToggleStateReport(ToggleStateReport): +class OverrideToggleStateReport(ToggleStateReport): """ - Override some of the methods from ToggleStateReport to expose toggles from WaffleFlagCourseOverrideModel objects. + Override some of the methods from ToggleStateReport to expose toggles from both the + WaffleFlagCourseOverrideModel and WaffleFlagOrgOverrideModel objects. """ def add_waffle_flag_instances(self, flags_dict): """ - Append objects from WaffleFlagCourseOverrideModel. + Append objects from WaffleFlagCourseOverrideModel and WaffleFlagOrgOverrideModel. """ super().add_waffle_flag_state(flags_dict) - _add_waffle_flag_course_override_state(flags_dict) + _add_waffle_flag_override_states(flags_dict) def get_waffle_flag_computed_status(self, flag): """ - Produce correct "computed_status" values for WaffleFlagCourseOverrideModel instances. + Produce correct "computed_status" values for WaffleFlagCourseOverrideModel or + WaffleFlagOrgOverrideModel instances. """ computed_status = super().get_waffle_flag_computed_status(flag) - # check course overrides only if computed_status is not already 'both' - if computed_status != "both" and "course_overrides" in flag: - has_force_on = any( - override["force"] == "on" for override in flag["course_overrides"] - ) - has_force_off = any( - override["force"] == "off" for override in flag["course_overrides"] - ) + # check course/org overrides only if computed_status is not already 'both' + if computed_status != "both": + has_force_on = False + has_force_off = False + for override_key in ("course_overrides", "org_overrides"): + if override_key in flag: + has_force_on = has_force_on or any( + override["force"] == "on" for override in flag[override_key] + ) + has_force_off = has_force_off or any( + override["force"] == "off" for override in flag[override_key] + ) if has_force_on and has_force_off: computed_status = "both" elif has_force_on: @@ -62,61 +69,87 @@ class ToggleStateView(views.APIView): """ Expose toggle state report dict as a view. """ - report = CourseOverrideToggleStateReport().as_dict() + report = OverrideToggleStateReport().as_dict() return Response(report) -def _add_waffle_flag_course_override_state(flags_dict): +class FlagOverride(Enum): """ - Add waffle flag course override state from the WaffleFlagCourseOverrideModel model. + Enumerates the supported CourseWaffleFlag overrides. """ - - flag_course_overrides = _get_flag_course_overrides() - for flag_name, course_overrides_dict in flag_course_overrides.items(): - course_overrides = [ - course_override - for course_override in course_overrides_dict.values() - if not course_override.get("disabled") - ] - if course_overrides: - flag = get_or_create_toggle_response(flags_dict, flag_name) - flag["course_overrides"] = course_overrides + COURSE = "COURSE" + ORG = "ORG" -def _get_flag_course_overrides(): +def _add_waffle_flag_override_states(flags_dict): """ - Return flag objects from WaffleFlagCourseOverrideModel instances. + Add waffle flag courserun & org override state from both the + WaffleFlagCourseOverrideModel and WaffleFlagOrgOverrideModel models. """ - # This dict is keyed by flag name, and contains dicts keyed by course_id, the contains + for override_type, override_key in ( + (FlagOverride.COURSE, "course_overrides"), + (FlagOverride.ORG, "org_overrides") + ): + flag_overrides = _get_flag_overrides(override_type) + for flag_name, overrides_dict in flag_overrides.items(): + enabled_overrides = [ + override + for override in overrides_dict.values() + if not override.get("disabled") + ] + if enabled_overrides: + flag = get_or_create_toggle_response(flags_dict, flag_name) + flag[override_key] = enabled_overrides + + +def _get_flag_overrides(course_or_org: FlagOverride): + """ + Return flag objects from the requested override model instances. + """ + # This dict is keyed by flag name, and contains dicts keyed by organization name, the contains # the final dict of metadata for a single course override that will be returned. - flag_course_overrides = OrderedDict() + flag_overrides = OrderedDict() # Note: We can't just get enabled records, because if a historical record is enabled but # the current record is disabled, we would not know this. We get all records, and mark # some overrides as disabled, and then later filter the disabled records. - course_overrides_data = WaffleFlagCourseOverrideModel.objects.all().order_by( - "waffle_flag", "course_id", "-change_date" - ) - for course_override_data in course_overrides_data: - if course_override_data.enabled: - course_override_fields = { - "force": course_override_data.override_choice, - "modified": str(course_override_data.change_date), + if course_or_org == FlagOverride.COURSE: + override_key = "course_id" + overrides_data = WaffleFlagCourseOverrideModel.objects.all().order_by( + "waffle_flag", "course_id", "-change_date" + ) + elif course_or_org == FlagOverride.ORG: + override_key = "org" + overrides_data = WaffleFlagOrgOverrideModel.objects.all().order_by( + "waffle_flag", "org", "-change_date" + ) + else: + return {} + + for override in overrides_data: + if course_or_org == FlagOverride.COURSE: + override_match_val = str(override.course_id) + elif course_or_org == FlagOverride.ORG: + override_match_val = str(override.org) + + if override.enabled: + override_fields = { + "force": override.override_choice, + "modified": str(override.change_date), } else: # The current record may be disabled, but later history might be enabled. - # We'll filter these disabled records below. - course_override_fields = {"disabled": True} - course_override_created_at = str(course_override_data.change_date) + # Disabled records are filtered elsewhere. + override_fields = {"disabled": True} - flag_name = course_override_data.waffle_flag - course_id = str(course_override_data.course_id) - course_override = flag_course_overrides.setdefault( - flag_name, OrderedDict() - ).setdefault(course_id, OrderedDict()) + one_override = flag_overrides.setdefault( + override.waffle_flag, OrderedDict() + ).setdefault( + override_match_val, OrderedDict() + ) # data is reverse ordered by date, so the first record is the current record - if "course_id" not in course_override: - course_override["course_id"] = course_id - course_override.update(course_override_fields) + if override_key not in one_override: + one_override[override_key] = override_match_val + one_override.update(override_fields) # data is reverse ordered by date, so the last record is the oldest record - course_override["created"] = course_override_created_at - return flag_course_overrides + one_override["created"] = str(override.change_date) + return flag_overrides