fix(Multivariate): API accepts multivariate percentage splits over 100% (#8151)

Author: bardock-2393

api/features/multivariate/serializers.py

@@ -7,7 +7,10 @@
 from core.constants import BOOLEAN, INTEGER
 from features.constants import CONTROL_VARIANT_KEY, RESERVED_VARIANT_KEY_MESSAGE
 from features.models import Feature, FeatureState
-from features.multivariate.models import MultivariateFeatureOption
+from features.multivariate.models import (
+    MultivariateFeatureOption,
+    MultivariateFeatureStateValue,
+)
 
 
 class NestedMultivariateFeatureOptionSerializer(serializers.ModelSerializer):  # type: ignore[type-arg]
@@ -79,25 +82,59 @@ def validate_key(self, value: str | None) -> str | None:
 
     def validate(self, attrs):  # type: ignore[no-untyped-def]
         attrs = super().validate(attrs)
+        feature = attrs["feature"]
+        default_percentage_allocation = attrs["default_percentage_allocation"]
+
         total_sibling_percentage_allocation = (
-            self._get_siblings(attrs["feature"]).aggregate(
+            self._get_siblings(feature).aggregate(
                 total_percentage_allocation=Sum("default_percentage_allocation")
             )["total_percentage_allocation"]
             or 0
         )
         total_percentage_allocation = (
-            total_sibling_percentage_allocation + attrs["default_percentage_allocation"]
+            total_sibling_percentage_allocation + default_percentage_allocation
         )
 
         if total_percentage_allocation > 100:
             raise ValidationError(
                 {"default_percentage_allocation": "Invalid percentage allocation"}
             )
 
+        if self.instance is None:
+            # Creating an option adds `default_percentage_allocation` to every
+            # environment's default feature state (see `MultivariateFeatureOption
+            # .create_multivariate_feature_state_values`). A given environment's
+            # actual feature state values can have drifted from the option-level
+            # defaults checked above (e.g. via a per-environment edit), so check
+            # those too rather than allowing that step to silently push one
+            # environment over 100%.
+            self._validate_environment_allocations(
+                feature, default_percentage_allocation
+            )
+
         self._validate_key_is_unique(attrs)
 
         return attrs
 
+    def _validate_environment_allocations(
+        self, feature: Feature, default_percentage_allocation: float
+    ) -> None:
+        environment_overflows = (
+            MultivariateFeatureStateValue.objects.filter(
+                feature_state__in=feature.feature_states.filter(
+                    identity=None, feature_segment=None
+                ),
+            )
+            .values("feature_state_id")
+            .annotate(total_percentage_allocation=Sum("percentage_allocation"))
+            .filter(total_percentage_allocation__gt=100 - default_percentage_allocation)
+            .exists()
+        )
+        if environment_overflows:
+            raise ValidationError(
+                {"default_percentage_allocation": "Invalid percentage allocation"}
+            )
+
     def _validate_key_is_unique(self, attrs: dict[str, typing.Any]) -> None:
         key = attrs.get("key")
         if key is None:

api/tests/integration/features/multivariate/test_integration_multivariate.py

@@ -8,6 +8,7 @@
 
 from features.constants import RESERVED_VARIANT_KEY_MESSAGE
 from features.models import Feature
+from features.multivariate.models import MultivariateFeatureOption
 from organisations.models import Organisation
 from projects.models import Project
 from users.models import FFAdminUser
@@ -420,6 +421,91 @@ def test_create_mv_option__total_allocation_exceeds_100__returns_bad_request(  #
     ]
 
 
+@pytest.mark.parametrize(
+    "client",
+    [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],
+)
+def test_create_mv_option__environment_allocation_drifted_from_option_default__returns_bad_request(  # type: ignore[no-untyped-def]  # noqa: E501
+    project, environment, environment_api_key, mv_option_50_percent, client, feature
+):
+    """
+    Reproduces https://github.com/Flagsmith/flagsmith/issues/7369: an
+    environment's actual `MultivariateFeatureStateValue.percentage_allocation`
+    can drift from the option's `default_percentage_allocation` via a
+    per-environment edit. Adding a new option that only overflows 100% for
+    that one environment must still be rejected, not just options that
+    overflow the option-level defaults.
+    """
+    # Given - the existing option's allocation is bumped from 50% to 100% for
+    # this environment only, so this environment's actual feature state values
+    # (100%) have diverged from the option-level default (50%).
+    get_feature_states_url = reverse(
+        "api-v1:environments:environment-featurestates-list", args=[environment_api_key]
+    )
+    feature_state = next(
+        filter(
+            lambda fs: fs["feature"] == feature,
+            client.get(get_feature_states_url).json()["results"],
+        )
+    )
+    mv_fsv = feature_state["multivariate_feature_state_values"][0]
+    update_url = reverse(
+        "api-v1:environments:environment-featurestates-detail",
+        args=[environment_api_key, feature_state["id"]],
+    )
+    update_response = client.put(
+        update_url,
+        data=json.dumps(
+            {
+                "id": feature_state["id"],
+                "feature_state_value": "big",
+                "multivariate_feature_state_values": [
+                    {
+                        "multivariate_feature_option": mv_fsv[
+                            "multivariate_feature_option"
+                        ],
+                        "id": mv_fsv["id"],
+                        "percentage_allocation": 100,
+                    }
+                ],
+                "identity": None,
+                "enabled": False,
+                "feature": feature,
+                "environment": environment,
+                "feature_segment": None,
+            }
+        ),
+        content_type="application/json",
+    )
+    assert update_response.status_code == status.HTTP_200_OK
+
+    url = reverse(
+        "api-v1:projects:feature-mv-options-list",
+        args=[project, feature],
+    )
+    data = {
+        "type": "unicode",
+        "feature": feature,
+        "string_value": "bigger",
+        "default_percentage_allocation": 10,
+    }
+
+    # When - a new option is added. At the option level, 50% + 10% = 60% <= 100%,
+    # so this looks valid, but it would push this environment's actual
+    # allocation to 110%.
+    response = client.post(
+        url,
+        data=json.dumps(data),
+        content_type="application/json",
+    )
+
+    # Then
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    # The option must not be left half wired-up (created, but missing feature
+    # state values for the environment that rejected it).
+    assert MultivariateFeatureOption.objects.filter(feature=feature).count() == 1
+
+
 @pytest.mark.parametrize(
     "client",
     [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],