fix: Feature.type field is unbounded (#6888)

Author: khvn26

api/features/feature_types.py

@@ -1,11 +1,24 @@
-from typing import Literal
+from typing import get_args
 
-FeatureType = Literal["STANDARD", "MULTIVARIATE"]
+from flagsmith_schemas.types import FeatureType
+
+__all__ = (
+    "FeatureType",
+    "CONFIG",
+    "FEATURE_TYPE_CHOICES",
+    "FLAG",
+    "MULTIVARIATE",
+    "STANDARD",
+)
 
 MULTIVARIATE: FeatureType = "MULTIVARIATE"
 STANDARD: FeatureType = "STANDARD"
 
+FEATURE_TYPE_CHOICES = [(v, v) for v in get_args(FeatureType)]
+
 # the following two types have been merged in terms of functionality
 # but kept for now until the FE is updated
 CONFIG = "CONFIG"
+"""Deprecated."""
 FLAG = "FLAG"
+"""Deprecated."""

api/features/migrations/0066_constrain_feature_type.py

@@ -0,0 +1,46 @@
+from django.apps.registry import Apps
+from django.db import migrations, models
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+
+from features.feature_types import FEATURE_TYPE_CHOICES, MULTIVARIATE, STANDARD
+
+
+def fix_feature_type(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    Feature = apps.get_model("features", "Feature")
+    # Features with multivariate options should be MULTIVARIATE.
+    Feature.objects.filter(multivariate_options__isnull=False).exclude(
+        type=MULTIVARIATE
+    ).update(type=MULTIVARIATE)
+    # All remaining features with an invalid type should be STANDARD.
+    Feature.objects.exclude(type__in=[STANDARD, MULTIVARIATE]).update(type=STANDARD)
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("features", "0065_make_feature_value_size_configurable"),
+        ("multivariate", "0007_alter_boolean_values"),
+    ]
+
+    operations = [
+        migrations.RunPython(fix_feature_type, reverse_code=migrations.RunPython.noop),
+        migrations.AlterField(
+            model_name="feature",
+            name="type",
+            field=models.CharField(
+                blank=True,
+                choices=FEATURE_TYPE_CHOICES,
+                default=STANDARD,
+                max_length=50,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="historicalfeature",
+            name="type",
+            field=models.CharField(
+                blank=True,
+                choices=FEATURE_TYPE_CHOICES,
+                default=STANDARD,
+                max_length=50,
+            ),
+        ),
+    ]

api/features/models.py

@@ -55,7 +55,7 @@
 from features.constants import ENVIRONMENT, FEATURE_SEGMENT, IDENTITY
 from features.custom_lifecycle import CustomLifecycleModelMixin
 from features.feature_states.models import AbstractBaseFeatureValueModel
-from features.feature_types import MULTIVARIATE, STANDARD
+from features.feature_types import FEATURE_TYPE_CHOICES, MULTIVARIATE, STANDARD
 from features.helpers import get_correctly_typed_value
 from features.managers import (
     FeatureManager,
@@ -115,7 +115,9 @@ class Feature(  # type: ignore[django-manager-missing]
     )
     description = models.TextField(null=True, blank=True)
     default_enabled = models.BooleanField(default=False)
-    type = models.CharField(max_length=50, blank=True, default=STANDARD)
+    type = models.CharField(
+        max_length=50, blank=True, default=STANDARD, choices=FEATURE_TYPE_CHOICES
+    )
     tags = models.ManyToManyField(Tag, blank=True)
     is_archived = models.BooleanField(default=False)
     owners = models.ManyToManyField(

api/features/serializers.py

@@ -224,7 +224,13 @@ class Meta:
             "last_modified_in_any_environment",
             "last_modified_in_current_environment",
         )
-        read_only_fields = ("feature_segments", "created_date", "uuid", "project")
+        read_only_fields = (
+            "feature_segments",
+            "created_date",
+            "uuid",
+            "project",
+            "type",
+        )
 
     def to_internal_value(self, data):  # type: ignore[no-untyped-def]
         if data.get("initial_value") and not isinstance(data["initial_value"], str):

api/tests/unit/features/test_migrations.py

@@ -226,6 +226,54 @@ def test_fix_feature_type_migration(migrator: Migrator) -> None:
     assert NewFeature.objects.get(id=mv_feature.id).type == MULTIVARIATE
 
 
+def test_constrain_feature_type__invalid_types__fixes_to_standard_or_multivariate(
+    migrator: Migrator,
+) -> None:
+    # Given
+    old_state = migrator.apply_initial_migration(
+        ("features", "0065_make_feature_value_size_configurable")
+    )
+
+    Organisation = old_state.apps.get_model("organisations", "Organisation")
+    Project = old_state.apps.get_model("projects", "Project")
+    Feature = old_state.apps.get_model("features", "Feature")
+    MultivariateFeatureOption = old_state.apps.get_model(
+        "multivariate", "MultivariateFeatureOption"
+    )
+
+    organisation = Organisation.objects.create(name="test org")
+    project = Project.objects.create(
+        name="test project", organisation_id=organisation.id
+    )
+    standard_feature = Feature.objects.create(
+        name="standard_feature", project_id=project.id
+    )
+    mv_feature = Feature.objects.create(
+        name="mv_feature", project_id=project.id, type=MULTIVARIATE
+    )
+    # Feature with an invalid type and no MV options → should become STANDARD.
+    bad_type_feature = Feature.objects.create(
+        name="bad_type_feature", project_id=project.id, type="boolean"
+    )
+    # Feature with an invalid type but has MV options → should become MULTIVARIATE.
+    bad_type_mv_feature = Feature.objects.create(
+        name="bad_type_mv_feature", project_id=project.id, type="flag"
+    )
+    MultivariateFeatureOption.objects.create(feature_id=bad_type_mv_feature.id)
+
+    # When
+    new_state = migrator.apply_tested_migration(
+        ("features", "0066_constrain_feature_type")
+    )
+
+    # Then
+    NewFeature = new_state.apps.get_model("features", "Feature")
+    assert NewFeature.objects.get(id=standard_feature.id).type == STANDARD
+    assert NewFeature.objects.get(id=mv_feature.id).type == MULTIVARIATE
+    assert NewFeature.objects.get(id=bad_type_feature.id).type == STANDARD
+    assert NewFeature.objects.get(id=bad_type_mv_feature.id).type == MULTIVARIATE
+
+
 def test_migrate_sample_to_webhook_forward(migrator: Migrator) -> None:
     # Given
     old_state = migrator.apply_initial_migration(

api/tests/unit/features/test_unit_features_views.py

@@ -45,7 +45,7 @@
 from environments.permissions.models import UserEnvironmentPermission
 from features import views
 from features.dataclasses import EnvironmentFeatureOverridesData
-from features.feature_types import MULTIVARIATE
+from features.feature_types import MULTIVARIATE, STANDARD
 from features.models import Feature, FeatureSegment, FeatureState
 from features.multivariate.models import MultivariateFeatureOption
 from features.value_types import STRING
@@ -1816,8 +1816,8 @@ def test_audit_log_created_when_feature_updated(
         args=[project.id, feature.id],
     )
     data = {
-        "name": "Test Feature updated",
-        "type": "FLAG",
+        "name": feature.name,
+        "description": "Updated description",
         "project": project.id,
     }
 
@@ -4488,3 +4488,42 @@ def test_list_features__edge_v2_project__makes_one_dynamo_query(
     # Validate that only a single query is made to dynamodb, not one
     # per feature.
     assert mock_table.query.call_count == 1
+
+
+def test_create_feature__type_provided__ignores_type_and_defaults_to_standard(
+    admin_client_new: APIClient,
+    project: Project,
+) -> None:
+    # Given
+    url = reverse("api-v1:projects:project-features-list", args=[project.id])
+    data = {"name": "test_feature_type_readonly", "type": "boolean"}
+
+    # When
+    response = admin_client_new.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_201_CREATED
+    assert response.json()["type"] == STANDARD
+
+
+def test_create_feature__multivariate_options_provided__sets_type_to_multivariate(
+    admin_client_new: APIClient,
+    project: Project,
+) -> None:
+    # Given
+    url = reverse("api-v1:projects:project-features-list", args=[project.id])
+    data = {
+        "name": "test_feature_mv_type",
+        "multivariate_options": [{"type": "unicode", "string_value": "option-a"}],
+    }
+
+    # When
+    response = admin_client_new.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_201_CREATED
+    assert response.json()["type"] == MULTIVARIATE

sdk/openapi.yaml

@@ -150,6 +150,9 @@ paths:
 components:
   schemas:
     TypeD77Enum:
+      description: |-
+        * `STANDARD` - STANDARD
+        * `MULTIVARIATE` - MULTIVARIATE
       type: string
       enum:
         - STANDARD