diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 6ae491ea30..2898a60afc 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -220,14 +220,14 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=14 if with_storage_backing else 13, + expected_sql_queries=15 if with_storage_backing else 14, ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 5, True, 24), - (ModuleStoreEnum.Type.mongo, 5, False, 14), - (ModuleStoreEnum.Type.split, 2, True, 24), - (ModuleStoreEnum.Type.split, 2, False, 14), + (ModuleStoreEnum.Type.mongo, 5, True, 25), + (ModuleStoreEnum.Type.mongo, 5, False, 15), + (ModuleStoreEnum.Type.split, 2, True, 25), + (ModuleStoreEnum.Type.split, 2, False, 15), ) @ddt.unpack def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries): diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index 65042dc035..da1ee79f22 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -431,12 +431,13 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase): discussion_target='Target Discussion', )) - # 4 queries are required to do first discussion xblock render: + # 5 queries are required to do first discussion xblock render: # * waffle_utils_wafflecourseoverridemodel + # * waffle_utils_waffleorgoverridemodel # * waffle_flag # * django_comment_client_role # * lms_xblock_xblockasidesconfig - num_queries = 4 + num_queries = 5 for discussion in discussions: discussion_xblock = get_module_for_descriptor_internal( user=user, diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index f3fe8cb5ab..9c40423621 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -162,10 +162,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest assert mock_block_structure_create.call_count == 1 @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 40, True), - (ModuleStoreEnum.Type.mongo, 1, 40, False), - (ModuleStoreEnum.Type.split, 2, 40, True), - (ModuleStoreEnum.Type.split, 2, 40, False), + (ModuleStoreEnum.Type.mongo, 1, 42, True), + (ModuleStoreEnum.Type.mongo, 1, 42, False), + (ModuleStoreEnum.Type.split, 2, 42, True), + (ModuleStoreEnum.Type.split, 2, 42, False), ) @ddt.unpack @@ -178,8 +178,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 40), - (ModuleStoreEnum.Type.split, 2, 40), + (ModuleStoreEnum.Type.mongo, 1, 42), + (ModuleStoreEnum.Type.split, 2, 42), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -224,8 +224,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 23), - (ModuleStoreEnum.Type.split, 2, 23), + (ModuleStoreEnum.Type.mongo, 1, 25), + (ModuleStoreEnum.Type.split, 2, 25), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -239,8 +239,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest assert len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)) == 0 @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 41), - (ModuleStoreEnum.Type.split, 2, 41), + (ModuleStoreEnum.Type.mongo, 1, 43), + (ModuleStoreEnum.Type.split, 2, 43), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index dfe3ed2927..8cf5b125c3 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -371,8 +371,8 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): self._verify_cell_data_for_user(verified_user.username, course.id, 'Certificate Eligible', 'Y', num_rows=2) @ddt.data( - (ModuleStoreEnum.Type.mongo, 4, 46), - (ModuleStoreEnum.Type.split, 3, 46), + (ModuleStoreEnum.Type.mongo, 4, 47), + (ModuleStoreEnum.Type.split, 3, 47), ) @ddt.unpack def test_query_counts(self, store_type, mongo_count, expected_query_count): @@ -2081,7 +2081,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(82): + with self.assertNumQueries(83): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data( diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 2e5f9f96e1..392f1fb0f4 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -247,7 +247,7 @@ class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStor def test_schedule_context(self): resolver = self.create_resolver() # using this to make sure the select_related stays intact - with self.assertNumQueries(36): + with self.assertNumQueries(38): sc = resolver.get_schedules() schedules = list(sc) diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index c8f5c86560..3759ec5688 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -21,7 +21,8 @@ log = logging.getLogger(__name__) 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. + WaffleFlag when in the context of a course. This class will also respect any org-level overrides, though + course-level overrides will take precedence. Uses a cached waffle namespace. @@ -33,7 +34,7 @@ class CourseWaffleFlag(LegacyWaffleFlag): SOME_COURSE_FLAG.is_enabled(course_key) - To configure, go to "Waffle flag course overrides" under the Django Admin "waffle_utils" section. + To configure a course-level override, go to Django Admin "waffle_utils" -> "Waffle flag course overrides". Waffle flag: Set this to the flag name (e.g. my_namespace.some_course_feature). Course id: Set this to the course id (e.g. course-v1:edx+100+Demo) @@ -44,6 +45,17 @@ class CourseWaffleFlag(LegacyWaffleFlag): 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. + To configure an org-level override, go to Django Admin "waffle_utils" -> "Waffle flag org overrides". + + Waffle flag: Set this to the flag name (e.g. my_namespace.some_course_feature). + Org name: Set this to the organization name (e.g. edx) + Override choice: (Force on/Force off). "Force on" will enable the waffle flag for all users in an org's courses, + 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. + """ def _get_course_override_value(self, course_key): @@ -57,21 +69,39 @@ class CourseWaffleFlag(LegacyWaffleFlag): course_key (CourseKey): The course to check for override before checking waffle. """ # Import is placed here to avoid model import at project startup. - from .models import WaffleFlagCourseOverrideModel + from .models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel - cache_key = f"{self.name}.{str(course_key)}" - course_override = self.cached_flags().get(cache_key) + course_cache_key = f"{self.name}.cwaffle.{str(course_key)}" + course_override = self.cached_flags().get(course_cache_key) if course_override is None: course_override = WaffleFlagCourseOverrideModel.override_value( self.name, course_key ) - self.cached_flags()[cache_key] = course_override + self.cached_flags()[course_cache_key] = course_override if course_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.on: return True if course_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.off: return False + + # Since no course-specific override was found, fall back to checking at the org-level. + if course_key: + org = course_key.org + org_cache_key = f"{self.name}.owaffle.{org}" + org_override = self.cached_flags().get(org_cache_key) + + if org_override is None: + org_override = WaffleFlagOrgOverrideModel.override_value( + self.name, org + ) + self.cached_flags()[org_cache_key] = org_override + + if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.on: + return True + if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.off: + return False + return None def is_enabled(self, course_key=None): # pylint: disable=arguments-differ diff --git a/openedx/core/djangoapps/waffle_utils/admin.py b/openedx/core/djangoapps/waffle_utils/admin.py index f89fd47c7a..bd905a7c9e 100644 --- a/openedx/core/djangoapps/waffle_utils/admin.py +++ b/openedx/core/djangoapps/waffle_utils/admin.py @@ -7,8 +7,8 @@ from django.contrib import admin from config_models.admin import KeyedConfigurationModelAdmin -from .forms import WaffleFlagCourseOverrideAdminForm -from .models import WaffleFlagCourseOverrideModel +from .forms import WaffleFlagCourseOverrideAdminForm, WaffleFlagOrgOverrideAdminForm +from .models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel class WaffleFlagCourseOverrideAdmin(KeyedConfigurationModelAdmin): @@ -23,8 +23,28 @@ class WaffleFlagCourseOverrideAdmin(KeyedConfigurationModelAdmin): fieldsets = ( (None, { 'fields': ('waffle_flag', 'course_id', 'note', 'override_choice', 'enabled'), - 'description': 'Enter a valid course id and an existing waffle flag. The waffle flag name is not validated.' + 'description': + 'Enter a valid course id and an existing waffle flag. The waffle flag name is not validated.' + }), + ) + + +class WaffleFlagOrgOverrideAdmin(KeyedConfigurationModelAdmin): + """ + Admin for org override of waffle flags. + + Includes search by org and waffle_flag. + + """ + form = WaffleFlagOrgOverrideAdminForm + search_fields = ['waffle_flag', 'org'] + fieldsets = ( + (None, { + 'fields': ('waffle_flag', 'org', 'note', 'override_choice', 'enabled'), + 'description': + 'Enter a valid organization and an existing waffle flag. The waffle flag name is not validated.' }), ) admin.site.register(WaffleFlagCourseOverrideModel, WaffleFlagCourseOverrideAdmin) +admin.site.register(WaffleFlagOrgOverrideModel, WaffleFlagOrgOverrideAdmin) diff --git a/openedx/core/djangoapps/waffle_utils/forms.py b/openedx/core/djangoapps/waffle_utils/forms.py index 374487e5d7..18592eddac 100644 --- a/openedx/core/djangoapps/waffle_utils/forms.py +++ b/openedx/core/djangoapps/waffle_utils/forms.py @@ -1,17 +1,17 @@ """ -Defines a form for providing validation of subsection grade templates. +Defines a form for providing validation of waffle overrides. """ from django import forms from openedx.core.lib.courses import clean_course_id -from .models import WaffleFlagCourseOverrideModel +from .models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel class WaffleFlagCourseOverrideAdminForm(forms.ModelForm): """ - Input form for course override of waffle flags, allowing us to verify data. + Input form for course-level override of waffle flags, allowing us to verify data. """ class Meta: model = WaffleFlagCourseOverrideModel @@ -25,12 +25,45 @@ class WaffleFlagCourseOverrideAdminForm(forms.ModelForm): def clean_waffle_flag(self): """ - Validate the waffle flag is an existing flag. + Validate the waffle flag is specified. """ - cleaned_flag = self.cleaned_data['waffle_flag'] + cleaned_flag = self.cleaned_data['waffle_flag'].strip() if not cleaned_flag: msg = 'Waffle flag must be supplied.' raise forms.ValidationError(msg) - return cleaned_flag.strip() + return cleaned_flag + + +class WaffleFlagOrgOverrideAdminForm(forms.ModelForm): + """ + Input form for org-level override of waffle flags, allowing us to verify data. + """ + class Meta: + model = WaffleFlagOrgOverrideModel + fields = '__all__' + + def clean_org(self): + """ + Validate the org. + """ + cleaned_flag = self.cleaned_data['org'].strip() + + if not cleaned_flag: + msg = 'Organization must be supplied.' + raise forms.ValidationError(msg) + + return cleaned_flag + + def clean_waffle_flag(self): + """ + Validate the waffle flag is specified. + """ + cleaned_flag = self.cleaned_data['waffle_flag'].strip() + + if not cleaned_flag: + msg = 'Waffle flag must be supplied.' + raise forms.ValidationError(msg) + + return cleaned_flag diff --git a/openedx/core/djangoapps/waffle_utils/migrations/0003_add_org_level_waffle_override.py b/openedx/core/djangoapps/waffle_utils/migrations/0003_add_org_level_waffle_override.py new file mode 100644 index 0000000000..2f9c7a67ad --- /dev/null +++ b/openedx/core/djangoapps/waffle_utils/migrations/0003_add_org_level_waffle_override.py @@ -0,0 +1,37 @@ +# Generated by Django 3.2.11 on 2022-01-13 18:42 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('waffle_utils', '0002_waffleflagcourseoverridemodel_note'), + ] + + operations = [ + migrations.CreateModel( + name='WaffleFlagOrgOverrideModel', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('waffle_flag', models.CharField(db_index=True, max_length=255)), + ('org', models.CharField(db_index=True, max_length=255)), + ('override_choice', models.CharField(choices=[('on', 'Force On'), ('off', 'Force Off')], default='on', max_length=3)), + ('note', models.TextField(blank=True, help_text='e.g. why this exists and when/if it can be dropped')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ], + options={ + 'verbose_name': 'Waffle flag org override', + 'verbose_name_plural': 'Waffle flag org overrides', + }, + ), + migrations.AddIndex( + model_name='waffleflagorgoverridemodel', + index=models.Index(fields=['org', 'waffle_flag'], name='waffle_org_and_waffle_flag'), + ), + ] diff --git a/openedx/core/djangoapps/waffle_utils/models.py b/openedx/core/djangoapps/waffle_utils/models.py index 6e78e8106d..a89201ef3a 100644 --- a/openedx/core/djangoapps/waffle_utils/models.py +++ b/openedx/core/djangoapps/waffle_utils/models.py @@ -2,7 +2,7 @@ Models for configuring waffle utils. """ -from django.db.models import CharField, TextField +from django.db.models import CharField, TextField, Index from django.utils.translation import gettext_lazy as _ from model_utils import Choices from opaque_keys.edx.django.models import CourseKeyField @@ -15,6 +15,14 @@ class WaffleFlagCourseOverrideModel(ConfigurationModel): """ Used to force a waffle flag on or off for a course. + Prioritization: + A course-level waffle flag overrides a relevant org-level waffle flag. + An org-level waffle flag overrides any defaults. + + So: Course level overrides (THIS MODEL) (highest priority) -> + Org level overrides -> + Defaults (lowest priority) + .. no_pii: """ OVERRIDE_CHOICES = Choices(('on', _('Force On')), ('off', _('Force Off'))) @@ -62,3 +70,70 @@ class WaffleFlagCourseOverrideModel(ConfigurationModel): def __str__(self): enabled_label = 'Enabled' if self.enabled else 'Not Enabled' return f'Course {str(self.course_id)}: Waffle Override {enabled_label}' + + +class WaffleFlagOrgOverrideModel(ConfigurationModel): + """ + Used to force a waffle flag on or off for an organization. + + This class mostly mirrors WaffleFlagCourseOverrideModel. + + Prioritization: + A course-level waffle flag overrides a relevant org-level waffle flag. + An org-level waffle flag overrides any defaults. + + So: Course level overrides (highest priority) -> + Org level overrides (THIS MODEL) -> + Defaults (lowest priority) + + .. no_pii: + """ + OVERRIDE_CHOICES = Choices(('on', _('Force On')), ('off', _('Force Off'))) + ALL_CHOICES = OVERRIDE_CHOICES + Choices('unset') + + KEY_FIELDS = ('waffle_flag', 'org') + + # The course that these features are attached to. + waffle_flag = CharField(max_length=255, db_index=True) + org = CharField(max_length=255, db_index=True) + override_choice = CharField(choices=OVERRIDE_CHOICES, default=OVERRIDE_CHOICES.on, max_length=3) + note = TextField(blank=True, help_text='e.g. why this exists and when/if it can be dropped') + + @classmethod + @request_cached() + def override_value(cls, waffle_flag, org): + """ + Returns whether the waffle flag was overridden (on or off) for the + org, or is unset. + + Arguments: + waffle_flag (String): The name of the flag. + org (String): The org for which the flag may have been overridden. + + If the current config is not set or disabled for this waffle flag and + org, returns ALL_CHOICES.unset. + Otherwise, returns ALL_CHOICES.on or ALL_CHOICES.off as configured for + the override_choice. + """ + if not org or not waffle_flag: + return cls.ALL_CHOICES.unset + + effective = cls.objects.filter(waffle_flag=waffle_flag, org=org).order_by('-change_date').first() + if effective and effective.enabled: + return effective.override_choice + return cls.ALL_CHOICES.unset + + class Meta: + app_label = 'waffle_utils' + verbose_name = 'Waffle flag org override' + verbose_name_plural = 'Waffle flag org overrides' + indexes = [ + Index( + name="waffle_org_and_waffle_flag", + fields=["org", "waffle_flag"], + ) + ] + + def __str__(self): + enabled_label = 'Enabled' if self.enabled else 'Not Enabled' + return f'Org {str(self.org)}: Waffle Override {enabled_label}' diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index f275a0d254..546ec2b7b5 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -6,18 +6,19 @@ Tests for waffle utils features. from unittest.mock import patch import crum import ddt -from django.test import TestCase from django.test.client import RequestFactory from edx_django_utils.cache import RequestCache from opaque_keys.edx.keys import CourseKey from waffle.testutils import override_flag +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase + from .. import CourseWaffleFlag -from ..models import WaffleFlagCourseOverrideModel +from ..models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel @ddt.ddt -class TestCourseWaffleFlag(TestCase): +class TestCourseWaffleFlag(CacheIsolationTestCase): """ Tests the CourseWaffleFlag. """ @@ -28,8 +29,10 @@ class TestCourseWaffleFlag(TestCase): FLAG_2_NAME = "test_flag_2" NAMESPACED_FLAG_2_NAME = NAMESPACE_NAME + "." + FLAG_2_NAME - TEST_COURSE_KEY = CourseKey.from_string("edX/DemoX/Demo_Course") - TEST_COURSE_2_KEY = CourseKey.from_string("edX/DemoX/Demo_Course_2") + TEST_ORG = "edX" + TEST_COURSE_KEY = CourseKey.from_string(f"{TEST_ORG}/DemoX/Demo_Course") + TEST_COURSE_2_KEY = CourseKey.from_string(f"{TEST_ORG}/DemoX/Demo_Course_2") + TEST_COURSE_3_KEY = CourseKey.from_string("CollegeX/DemoX/Demo_Course") TEST_COURSE_FLAG = CourseWaffleFlag(NAMESPACE_NAME, FLAG_NAME, __name__) def setUp(self): @@ -40,22 +43,22 @@ class TestCourseWaffleFlag(TestCase): RequestCache.clear_all_namespaces() @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}, + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, True), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, True), + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, False), ) - def test_course_waffle_flag(self, data): + @ddt.unpack + def test_course_waffle_flag(self, waffle_enabled, course_override, result): """ - Tests various combinations of a flag being set in waffle and overridden - for a course. + Tests various combinations of a flag being set in waffle and overridden for a course. """ - with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']): - with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']): + with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=course_override): + with override_flag(self.NAMESPACED_FLAG_NAME, active=waffle_enabled): # check twice to test that the result is properly cached - assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) == data['result'] - assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) == data['result'] - # result is cached, so override check should happen once + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) == result + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) == result + # result is cached, so override check should happen only once # pylint: disable=no-member WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( self.NAMESPACED_FLAG_NAME, @@ -63,16 +66,131 @@ class TestCourseWaffleFlag(TestCase): ) # check flag for a second course - if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset: + if course_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset: # When course override wasn't set for the first course, the second course will get the same # cached value from waffle. - second_value = data['waffle_enabled'] - assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY) == second_value + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY) == waffle_enabled else: # When course override was set for the first course, it should not apply to the second # course which should get the default value of False. - second_value = False - assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY) == second_value + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY) is False + + @ddt.data( + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + ) + @ddt.unpack + def test_matching_org_override_waffle_flag(self, waffle_enabled, org_override_choice, is_enabled): + """ + Tests various combinations of a flag being set in waffle and overridden for an org + which is the org which authored/owns the course. + Since the org-level override has the same org as the course being checked, the org-level + override's on/off/unset state determines whether is CourseWaffleFlag is active or not. + + on = active (enabled) + off = inactive (disabled) + unset = mirror the base waffle flag's activity + """ + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=self.NAMESPACED_FLAG_NAME, + org=self.TEST_ORG, + override_choice=org_override_choice, + note='', + enabled=True + ) + # Both course keys should match the org-level override. + with override_flag(self.NAMESPACED_FLAG_NAME, active=waffle_enabled): + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) == is_enabled + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY) == is_enabled + + @ddt.data( + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, True), + ) + @ddt.unpack + def test_not_matching_org_override_waffle_flag(self, waffle_enabled, org_override_choice, is_enabled): + """ + Tests various combinations of a flag being set in waffle and overridden for an org + which is *not* the target course's org. + Since the org-level override isn't relevant to the course being checked, whether the + waffle flag is active/inactive determines whether the CourseWaffleFlag is active or not. + + So whether the non-matching org override is on/off/unset, simply mirror the base waffle flag's activity. + """ + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=self.NAMESPACED_FLAG_NAME, + org=self.TEST_ORG, + override_choice=org_override_choice, + note='', + enabled=True + ) + # Org doesn't match the course key, so should never be enabled. + with override_flag(self.NAMESPACED_FLAG_NAME, active=waffle_enabled): + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_3_KEY) == is_enabled + + @ddt.data( + # "unset" isn't a typical override value - it nullifies the presence of the override. + # Since both overrides are "unset", use the legacy waffle flag behavior and reflect the flag's active value. + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + # Since the course override matches the course ID and is on, the waffle flag is enabled. + # The org override isn't relevant in this situation. + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, True), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.on, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, True), + # Since the course override is nullified and the org override matches + # the course ID and is on, the waffle flag is enabled. + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + # Since the course override matches the course ID but is off, the waffle flag is *not* enabled. + # The org override isn't relevant in this situation - it's overridden by the course override. + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, False), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, False), + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + # Since the either the course override or the org override matches the course ID but is off + # AND the other course/org override is unset/nullified, the waffle flag is *not* enabled. + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.off, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (False, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + ) + @ddt.unpack + def test_matching_course_and_org_override_waffle_flag( + self, waffle_enabled, course_override_choice, org_override_choice, is_enabled + ): + """ + Tests various combinations of a flag being set in waffle and overridden for both a matching + course ID and a matching org - the org which authored/owns the course. + Demonstrates the priorities of the two overrides - course and org. + """ + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag=self.NAMESPACED_FLAG_NAME, + course_id=self.TEST_COURSE_KEY, + override_choice=course_override_choice, + note='', + enabled=True + ) + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=self.NAMESPACED_FLAG_NAME, + org=self.TEST_ORG, + override_choice=org_override_choice, + note='', + enabled=True + ) + with override_flag(self.NAMESPACED_FLAG_NAME, active=waffle_enabled): + assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) == is_enabled def test_undefined_waffle_flag(self): """ diff --git a/openedx/core/djangoapps/waffle_utils/testutils.py b/openedx/core/djangoapps/waffle_utils/testutils.py index fcaf8f1ab2..3f5c510e4c 100644 --- a/openedx/core/djangoapps/waffle_utils/testutils.py +++ b/openedx/core/djangoapps/waffle_utils/testutils.py @@ -8,6 +8,7 @@ Test utilities for waffle utilities. # with self.assertNumQueries(6, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): WAFFLE_TABLES = [ "waffle_utils_waffleflagcourseoverridemodel", + "waffle_utils_waffleflagorgoverridemodel", "waffle_flag", "waffle_switch", "waffle_sample",