Support specifying metadata in DatabaseFlagsSource

This change supports specifying metadata in the DatabaseFlagsSource by using a FlagMetadata model.  It also modifies the DatabaseFlagSource to return a three-tuple instead of the dict that was deprecated in b1a752f.

docs/releasenotes.md

@@ -6,8 +6,16 @@
 
 - Added Django 3.0 support
 - Added validator support to ensure that the values that flag conditions test against are valid.
+- Added metadata support for flags.
+
+### Deprecations
+
 - Deprecated the optional `flags.middleware.FlagConditionsMiddleware` in favor of always lazily caching flags on the request object.
 
+### Removals
+
+- Django Flags 4.1 deprecated support for using a single dictionary to hold key/values of conditions for a settings-based feature flag, and this has been removed. Use [a list of dictionaries or tuples instead](/settings/#flags).
+
 ## 4.2.4
 
 ### What's new?

docs/settings.md

@@ -12,7 +12,7 @@ A list or tuple containing the full Python path strings to classes that provides
 
 Default: `{}`
 
-A dictionary of feature flags and optional conditions used when `'flags.sources.SettingsFlagsSource'` is in [`FLAG_SOURCES`](#flag_sources).
+A dictionary of feature flags with a list of optional conditions and metadata used when `'flags.sources.SettingsFlagsSource'` is in [`FLAG_SOURCES`](#flag_sources).
 
 Each key is the name of a feature flag, and the value should be a list contining conditions and metadata for that flag.
 
@@ -31,6 +31,8 @@ Metadata is defined in a dictionary along with conditions with in the format:
 {'help_text': 'enable a cool new future', 'category': 'temporary'}
 ```
 
+There should be only one metadata dictionary per flag. Any dictionary within the feature flag's list that does not contain `condition` and `value` keys will be treated as the metadata dictionary.
+
 For example:
 
 ```python

flags/admin.py

@@ -1,11 +1,16 @@
 from django.contrib import admin
 
-from flags.forms import FlagStateForm
-from flags.models import FlagState
+from flags.forms import FlagMetadataForm, FlagStateForm
+from flags.models import FlagMetadata, FlagState
+
+
+class FlagMetadataAdmin(admin.ModelAdmin):
+    form = FlagMetadataForm
 
 
 class FlagStateAdmin(admin.ModelAdmin):
     form = FlagStateForm
 
 
+admin.site.register(FlagMetadata, FlagMetadataAdmin)
 admin.site.register(FlagState, FlagStateAdmin)

flags/forms.py

@@ -1,10 +1,27 @@
 from django import forms
 
 from flags.conditions import get_condition, get_conditions
-from flags.models import FlagState
+from flags.models import FlagMetadata, FlagState
 from flags.sources import get_flags
 
 
+class FlagMetadataForm(forms.ModelForm):
+    name = forms.ChoiceField(label="Flag", required=True)
+    key = forms.CharField(label="Key", required=True)
+    value = forms.CharField(label="Value", required=True)
+
+    def __init__(self, *args, **kwargs):
+        super(FlagMetadataForm, self).__init__(*args, **kwargs)
+
+        self.fields["name"].choices = [
+            (f, f) for f in sorted(get_flags().keys())
+        ]
+
+    class Meta:
+        model = FlagMetadata
+        fields = ("name", "key", "value")
+
+
 class FlagStateForm(forms.ModelForm):
     name = forms.ChoiceField(label="Flag", required=True)
     condition = forms.ChoiceField(label="Condition name", required=True)

flags/migrations/0014_flagmetadata.py

@@ -0,0 +1,25 @@
+# Generated by Django 2.2.12 on 2020-05-28 20:37
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('flags', '0013_add_required_field'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='FlagMetadata',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('name', models.CharField(max_length=64)),
+                ('key', models.CharField(max_length=64)),
+                ('value', models.TextField()),
+            ],
+            options={
+                'unique_together': {('name', 'key')},
+            },
+        ),
+    ]

flags/models.py

@@ -12,12 +12,21 @@ class Meta:
         unique_together = ("name", "condition", "value")
 
     def __str__(self):
+        required_str = " (required)" if self.required else ""
         return (
-            "{name} is enabled when {condition} is "
-            "{value}{required}".format(
-                name=self.name,
-                condition=self.condition,
-                value=self.value,
-                required=" (required)" if self.required else "",
-            )
+            f"{self.name} is enabled when {self.condition} is "
+            f"{self.value}{required_str}"
         )
+
+
+class FlagMetadata(models.Model):
+    name = models.CharField(max_length=64)
+    key = models.CharField(max_length=64)
+    value = models.TextField()
+
+    class Meta:
+        app_label = "flags"
+        unique_together = ("name", "key")
+
+    def __str__(self):
+        return f"{self.name} ({self.key}: {self.value})"

flags/sources.py

@@ -148,17 +148,30 @@ def get_queryset(self):
         FlagState = apps.get_model("flags", "FlagState")
         return FlagState.objects.all()
 
+    def get_metadata_queryset(self):
+        FlagMetadata = apps.get_model("flags", "FlagMetadata")
+        return FlagMetadata.objects.all()
+
     def get_flags(self):
         flags = {}
-        for o in self.get_queryset():
+        for o in self.get_queryset().all():
             if o.name not in flags:
-                flags[o.name] = []
-            flags[o.name].append(
+                flags[o.name] = {"conditions": [], "metadata": {}}
+            flags[o.name]["conditions"].append(
                 DatabaseCondition(
                     o.condition, o.value, required=o.required, obj=o
                 )
             )
-        return flags
+
+        for o in self.get_metadata_queryset().all():
+            if o.name not in flags:
+                flags[o.name] = {"conditions": [], "metadata": {}}
+            flags[o.name]["metadata"][o.key] = o.value
+
+        return [
+            (flag, flags[flag]["conditions"], flags[flag]["metadata"])
+            for flag in flags
+        ]
 
 
 def get_flags(sources=None, ignore_errors=False, request=None):

flags/tests/test_forms.py

@@ -1,10 +1,38 @@
 from django.test import TestCase
 
 from flags.conditions.registry import _conditions, register
-from flags.forms import FlagStateForm
+from flags.forms import FlagMetadataForm, FlagStateForm
 
 
-class FormTestCase(TestCase):
+class FlagMetadataFormTestCase(TestCase):
+    def test_valid_data(self):
+        form = FlagMetadataForm(
+            {
+                "name": "FLAG_ENABLED",
+                "key": "help_text",
+                "value": "enable a cool thing",
+            }
+        )
+        self.assertTrue(form.is_valid())
+        metadata = form.save()
+        self.assertEqual(metadata.name, "FLAG_ENABLED")
+        self.assertEqual(metadata.key, "help_text")
+        self.assertEqual(metadata.value, "enable a cool thing")
+
+    def test_blank_data(self):
+        form = FlagMetadataForm({})
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors,
+            {
+                "name": ["This field is required."],
+                "key": ["This field is required."],
+                "value": ["This field is required."],
+            },
+        )
+
+
+class FlagStateFormTestCase(TestCase):
     def test_valid_data(self):
         form = FlagStateForm(
             {"name": "FLAG_ENABLED", "condition": "boolean", "value": "True"}

flags/tests/test_models.py

@@ -1,6 +1,6 @@
 from django.test import TestCase
 
-from flags.models import FlagState
+from flags.models import FlagMetadata, FlagState
 
 
 class FlagStateTestCase(TestCase):
@@ -17,3 +17,13 @@ def test_flag_str_required(self):
         self.assertEqual(
             str(state), "MY_FLAG is enabled when boolean is True (required)"
         )
+
+
+class FlagMetadataTestCase(TestCase):
+    def test_flag_str(self):
+        metadata = FlagMetadata.objects.create(
+            name="MY_FLAG", key="help_text", value="enable a cool thing"
+        )
+        self.assertEqual(
+            str(metadata), "MY_FLAG (help_text: enable a cool thing)"
+        )

flags/tests/test_sources.py

@@ -1,7 +1,7 @@
 from django.http import HttpRequest
 from django.test import TestCase, override_settings
 
-from flags.models import FlagState
+from flags.models import FlagMetadata, FlagState
 from flags.sources import (
     Condition,
     DatabaseFlagsSource,
@@ -148,7 +148,21 @@ def test_get_flags(self):
         )
         source = DatabaseFlagsSource()
         flags = source.get_flags()
-        self.assertEqual(flags, {"MY_FLAG": [Condition("boolean", "False")]})
+        self.assertEqual(
+            flags, [("MY_FLAG", [Condition("boolean", "False")], {})]
+        )
+
+    def test_get_flags_metadata(self):
+        FlagMetadata.objects.create(
+            name="MY_FLAG", key="help_text", value="enable a cool thing"
+        )
+        source = DatabaseFlagsSource()
+
+        flags = source.get_flags()
+
+        self.assertEqual(
+            flags, [("MY_FLAG", [], {"help_text": "enable a cool thing"})]
+        )
 
 
 class ConditionTestCase(TestCase):
@@ -296,7 +310,7 @@ def test_uses_cached_flags_from_request(self):
         request = HttpRequest()
 
         # The initial call looks up flag conditions from the database source.
-        with self.assertNumQueries(1):
+        with self.assertNumQueries(2):
             get_flags(request=request)
 
         # Subsequent calls with a request object don't need to redo the lookup
@@ -305,5 +319,5 @@ def test_uses_cached_flags_from_request(self):
             get_flags(request=request)
 
         # But subsequent calls without a request object still redo the lookup.
-        with self.assertNumQueries(1):
+        with self.assertNumQueries(2):
             get_flags()