diff --git a/docs/developer/adding-feature-flags.md b/docs/developer/adding-feature-flags.md index 711b6b7b6..99cede39e 100644 --- a/docs/developer/adding-feature-flags.md +++ b/docs/developer/adding-feature-flags.md @@ -20,9 +20,6 @@ Follow these steps to achieve this: 5. Rename the copied migration to match the increment. For instance, if `0091_create_waffle_flags_v01` exists, you will rename your migration to `0091_create_waffle_flags_v02`. 6. Modify the migration dependency to match the last migration in the stack. -## Modifying an existing feature flag through the CLI -Waffle comes with built in management commands that you can use to update records remotely. [Read here](https://waffle.readthedocs.io/en/stable/usage/cli.html) for information on how to use them. - ## Using feature flags as boolean values Waffle [provides a boolean](https://waffle.readthedocs.io/en/stable/usage/views.html) called `flag_is_active` that you can use as you otherwise would a boolean. This boolean requires a request object and the flag name. diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ee25a3a73..95ec90cb1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -15,7 +15,8 @@ from django.contrib.contenttypes.models import ContentType from django.urls import reverse from dateutil.relativedelta import relativedelta # type: ignore from epplibwrapper.errors import ErrorCode, RegistryError -from waffle.admin import FlagAdmin, SampleAdmin, SwitchAdmin +from waffle.admin import FlagAdmin +from waffle.models import Sample, Switch from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.views.utility.mixins import OrderableFieldsMixin @@ -2166,23 +2167,6 @@ class WaffleFlagAdmin(FlagAdmin): model = models.WaffleFlag fields = "__all__" - -class WaffleSwitchAdmin(SwitchAdmin): - class Meta: - """Contains meta information about this class""" - - model = models.WaffleSwitch - fields = "__all__" - - -class WaffleSampleAdmin(SampleAdmin): - class Meta: - """Contains meta information about this class""" - - model = models.WaffleSample - fields = "__all__" - - admin.site.unregister(LogEntry) # Unregister the default registration admin.site.register(LogEntry, CustomLogEntryAdmin) @@ -2209,5 +2193,7 @@ admin.site.register(models.VerifiedByStaff, VerifiedByStaffAdmin) # Register our custom waffle implementations admin.site.register(models.WaffleFlag, WaffleFlagAdmin) -admin.site.register(models.WaffleSample, WaffleSampleAdmin) -admin.site.register(models.WaffleSwitch, WaffleSwitchAdmin) + +# Unregister Sample and Switch from the waffle library +admin.site.unregister(Sample) +admin.site.unregister(Switch) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index b14ea231b..2df448d8c 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -331,14 +331,6 @@ WAFFLE_CREATE_MISSING_FLAGS = False # Used to replace the default flag class (for customization purposes). WAFFLE_FLAG_MODEL = "registrar.WaffleFlag" -# The model that will be used to keep track of switches. Extends AbstractBaseSwitch. -# Used to replace the default switch class (for customization purposes). -WAFFLE_SWITCH_MODEL = "registrar.WaffleSwitch" - -# The model that will be used to keep track of samples. Extends AbstractBaseSample. -# Used to replace the default sample class (for customization purposes). -WAFFLE_SAMPLE_MODEL = "registrar.WaffleSample" - # endregion # region: Headers-----------------------------------------------------------### diff --git a/src/registrar/migrations/0090_wafflesample_waffleswitch_waffleflag.py b/src/registrar/migrations/0090_waffleflag.py similarity index 57% rename from src/registrar/migrations/0090_wafflesample_waffleswitch_waffleflag.py rename to src/registrar/migrations/0090_waffleflag.py index 3830ca0e4..3edef9a7e 100644 --- a/src/registrar/migrations/0090_wafflesample_waffleswitch_waffleflag.py +++ b/src/registrar/migrations/0090_waffleflag.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-05-01 15:10 +# Generated by Django 4.2.10 on 2024-05-02 17:47 from django.conf import settings from django.db import migrations, models @@ -13,93 +13,6 @@ class Migration(migrations.Migration): ] operations = [ - migrations.CreateModel( - name="WaffleSample", - fields=[ - ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), - ( - "name", - models.CharField( - help_text="The human/computer readable name.", max_length=100, unique=True, verbose_name="Name" - ), - ), - ( - "percent", - models.DecimalField( - decimal_places=1, - help_text="A number between 0.0 and 100.0 to indicate a percentage of the time this sample will be active.", - max_digits=4, - verbose_name="Percent", - ), - ), - ( - "note", - models.TextField(blank=True, help_text="Note where this Sample is used.", verbose_name="Note"), - ), - ( - "created", - models.DateTimeField( - db_index=True, - default=django.utils.timezone.now, - help_text="Date when this Sample was created.", - verbose_name="Created", - ), - ), - ( - "modified", - models.DateTimeField( - default=django.utils.timezone.now, - help_text="Date when this Sample was last modified.", - verbose_name="Modified", - ), - ), - ], - options={ - "verbose_name": "waffle sample", - "verbose_name_plural": "Waffle samples", - }, - ), - migrations.CreateModel( - name="WaffleSwitch", - fields=[ - ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), - ( - "name", - models.CharField( - help_text="The human/computer readable name.", max_length=100, unique=True, verbose_name="Name" - ), - ), - ( - "active", - models.BooleanField(default=False, help_text="Is this switch active?", verbose_name="Active"), - ), - ( - "note", - models.TextField(blank=True, help_text="Note where this Switch is used.", verbose_name="Note"), - ), - ( - "created", - models.DateTimeField( - db_index=True, - default=django.utils.timezone.now, - help_text="Date when this Switch was created.", - verbose_name="Created", - ), - ), - ( - "modified", - models.DateTimeField( - default=django.utils.timezone.now, - help_text="Date when this Switch was last modified.", - verbose_name="Modified", - ), - ), - ], - options={ - "verbose_name": "waffle switch", - "verbose_name_plural": "Waffle switches", - }, - ), migrations.CreateModel( name="WaffleFlag", fields=[ diff --git a/src/registrar/migrations/0091_create_waffle_flags_v01.py b/src/registrar/migrations/0091_create_waffle_flags_v01.py index 6b1bd678d..ca7fcb751 100644 --- a/src/registrar/migrations/0091_create_waffle_flags_v01.py +++ b/src/registrar/migrations/0091_create_waffle_flags_v01.py @@ -33,7 +33,7 @@ def delete_flags(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ("registrar", "0090_wafflesample_waffleswitch_waffleflag"), + ("registrar", "0090_waffleflag"), ] operations = [ diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index 75abb03e9..f084a5d8b 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -16,8 +16,7 @@ from .website import Website from .transition_domain import TransitionDomain from .verified_by_staff import VerifiedByStaff from .waffle_flag import WaffleFlag -from .waffle_sample import WaffleSample -from .waffle_switch import WaffleSwitch + __all__ = [ "Contact", @@ -37,8 +36,6 @@ __all__ = [ "TransitionDomain", "VerifiedByStaff", "WaffleFlag", - "WaffleSwitch", - "WaffleSample", ] auditlog.register(Contact) @@ -58,5 +55,3 @@ auditlog.register(Website) auditlog.register(TransitionDomain) auditlog.register(VerifiedByStaff) auditlog.register(WaffleFlag) -auditlog.register(WaffleSample) -auditlog.register(WaffleSwitch) diff --git a/src/registrar/models/waffle_flag.py b/src/registrar/models/waffle_flag.py index 044aa3d37..6d2b3a9a8 100644 --- a/src/registrar/models/waffle_flag.py +++ b/src/registrar/models/waffle_flag.py @@ -41,9 +41,7 @@ class WaffleFlag(AbstractUserFlag): """ logger.info("Creating default waffle flags...") WaffleFlag = apps.get_model("registrar", "WaffleFlag") - # Flags can be changed through the command line or through django admin. - # To keep the scope of this function minimal and simple, if we require additional - # config on these flag, it should be done in a seperate function or as a command. + # Flags can be changed through django admin if necessary. for flag_name, flag_note in default_waffle_flags: try: WaffleFlag.objects.update_or_create( diff --git a/src/registrar/models/waffle_sample.py b/src/registrar/models/waffle_sample.py deleted file mode 100644 index 6b2ae59c6..000000000 --- a/src/registrar/models/waffle_sample.py +++ /dev/null @@ -1,19 +0,0 @@ -from waffle.models import AbstractBaseSample -import logging - -logger = logging.getLogger(__name__) - - -class WaffleSample(AbstractBaseSample): - """ - Custom implementation of django-waffles 'sample' object. - Read more here: https://waffle.readthedocs.io/en/stable/types/sample.html - - Use this class when dealing with samples. - """ - - class Meta: - """Contains meta information about this class""" - - verbose_name = "waffle sample" - verbose_name_plural = "Waffle samples" diff --git a/src/registrar/models/waffle_switch.py b/src/registrar/models/waffle_switch.py deleted file mode 100644 index 640b2b767..000000000 --- a/src/registrar/models/waffle_switch.py +++ /dev/null @@ -1,19 +0,0 @@ -from waffle.models import AbstractBaseSwitch -import logging - -logger = logging.getLogger(__name__) - - -class WaffleSwitch(AbstractBaseSwitch): - """ - Custom implementation of django-waffles 'switch' object. - Read more here: https://waffle.readthedocs.io/en/stable/types/switch.html - - Use this class when dealing with switches. - """ - - class Meta: - """Contains meta information about this class""" - - verbose_name = "waffle switch" - verbose_name_plural = "Waffle switches" diff --git a/src/registrar/tests/test_feature_flags.py b/src/registrar/tests/test_feature_flags.py deleted file mode 100644 index 62e15e3bf..000000000 --- a/src/registrar/tests/test_feature_flags.py +++ /dev/null @@ -1,167 +0,0 @@ -from waffle.decorators import flag_is_active -from django.test import TestCase, Client, RequestFactory -from registrar.models import ( - WaffleFlag, - User, - Contact, - UserGroup, -) -from registrar.tests.common import create_superuser, create_staffuser, create_user - - -class TestFeatureFlags(TestCase): - def setUp(self): - super().setUp() - self.client = Client(HTTP_HOST="localhost:8080") - self.factory = RequestFactory() - self.superuser = create_superuser() - - # For testing purposes, lets set this to false. - self.superuser.is_staff = False - self.superuser.save() - - self.staffuser = create_staffuser() - self.user = create_user() - - def tearDown(self): - super().tearDown() - WaffleFlag.objects.all().delete() - User.objects.all().delete() - Contact.objects.all().delete() - - def assert_flag_active(self, request_user, flag_name, location="/"): - """ - Checks if the given `request_user` has `flag_name` active - using waffles `flag_is_active` function. - """ - request = self.factory.get(location) - request.user = request_user - self.assertTrue(flag_is_active(request, flag_name)) - - def assert_flag_not_active(self, request_user, flag_name, location="/"): - """ - Checks if the given `request_user` has `flag_name` not active - using waffles `flag_is_active` function. - """ - request = self.factory.get(location) - request.user = request_user - self.assertFalse(flag_is_active(request, flag_name)) - - def test_flag_active_for_superuser(self): - """ - Tests flag_is_active for a flag with `superuser = True` - """ - flag, _ = WaffleFlag.objects.get_or_create( - name="test_superuser_flag", - superusers=True, - staff=False, - ) - # Test if superusers can access this flag - self.assert_flag_active(request_user=self.superuser, flag_name=flag.name) - - # Ensure that regular staff cannot access this flag - self.assert_flag_not_active(request_user=self.staffuser, flag_name=flag.name) - - # Ensure that a normal user also can't access this flag - self.assert_flag_not_active(request_user=self.user, flag_name=flag.name) - - def test_flag_active_for_is_staff(self): - """ - Tests flag_is_active for a flag with `is_staff = True` - """ - # We should actually expect superusers - # to not see this feature - otherwise the two distinct booleans aren't useful. - # In practice, we would usually use groups for toggling features. - flag, _ = WaffleFlag.objects.get_or_create( - name="test_superuser_flag", - superusers=False, - staff=True, - ) - - # Ensure that regular staff can access this flag - self.assert_flag_active(request_user=self.staffuser, flag_name=flag.name) - - # Ensure that superusers cannot access this flag - self.assert_flag_not_active(request_user=self.superuser, flag_name=flag.name) - - # Ensure that a normal user also can't access this flag - self.assert_flag_not_active(request_user=self.user, flag_name=flag.name) - - def test_flag_active_for_everyone(self): - """ - Tests flag_is_active for a flag with `everyone = True` - """ - flag, _ = WaffleFlag.objects.get_or_create( - name="test_superuser_flag", - everyone=True, - ) - - # Ensure that regular staff can access this flag - self.assert_flag_active(request_user=self.staffuser, flag_name=flag.name) - - # Ensure that superusers can access this flag - self.assert_flag_active(request_user=self.superuser, flag_name=flag.name) - - # Ensure that normal users can access this flag - self.assert_flag_active(request_user=self.user, flag_name=flag.name) - - def test_flag_active_for_everyone_is_false(self): - """ - Tests flag_is_active for a flag with `everyone = False` - """ - flag, _ = WaffleFlag.objects.get_or_create( - name="test_superuser_flag", - everyone=False, - ) - - # Ensure that regular staff cannot access this flag - self.assert_flag_not_active(request_user=self.staffuser, flag_name=flag.name) - - # Ensure that superusers cannot access this flag - self.assert_flag_not_active(request_user=self.superuser, flag_name=flag.name) - - # Ensure that normal users cannot access this flag - self.assert_flag_not_active(request_user=self.user, flag_name=flag.name) - - def test_admin_group(self): - """ - Tests flag_is_active for the admin user group - """ - flag, _ = WaffleFlag.objects.get_or_create( - name="test_superuser_flag", - ) - - # Add the full access group to this flag - group, _ = UserGroup.objects.get_or_create(name="full_access_group") - flag.groups.set([group]) - - # Ensure that regular staff cannot access this flag - self.assert_flag_not_active(request_user=self.staffuser, flag_name=flag.name) - - # Ensure that superusers can access this flag - self.assert_flag_active(request_user=self.superuser, flag_name=flag.name) - - # Ensure that normal users cannot access this flag - self.assert_flag_not_active(request_user=self.user, flag_name=flag.name) - - def test_staff_group(self): - """ - Tests flag_is_active for the staff user group - """ - flag, _ = WaffleFlag.objects.get_or_create( - name="test_superuser_flag", - ) - - # Add the analyst group to this flag - analyst_group, _ = UserGroup.objects.get_or_create(name="cisa_analysts_group") - flag.groups.set([analyst_group]) - - # Ensure that regular staff can access this flag - self.assert_flag_active(request_user=self.staffuser, flag_name=flag.name) - - # Ensure that superusers can access this flag. - # This permission encompasses cisa_analysts_group. - self.assert_flag_active(request_user=self.superuser, flag_name=flag.name) - - # Ensure that normal users cannot access this flag - self.assert_flag_not_active(request_user=self.user, flag_name=flag.name)