fix: metadata-feature-changes-erases-existing-value (#5362)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: Zaimwa9

api/environments/serializers.py

@@ -1,9 +1,11 @@
 import typing
+from typing import cast
 
 from common.metadata.serializers import (
     MetadataSerializer,
     SerializerWithMetadata,
 )
+from django.db import models
 from rest_framework import serializers
 
 from environments.models import Environment, EnvironmentAPIKey, Webhook
@@ -97,6 +99,15 @@ def get_project(
             "Unable to retrieve project for metadata validation."
         )
 
+    def update(
+        self, instance: models.Model, validated_data: dict[str, typing.Any]
+    ) -> Environment:
+        metadata_items = validated_data.pop("metadata", [])
+        environment = cast(Environment, super().update(instance, validated_data))
+        self.update_metadata(environment, metadata_items)
+        environment.refresh_from_db()
+        return environment
+
 
 class EnvironmentRetrieveSerializerWithMetadata(EnvironmentSerializerWithMetadata):
     total_segment_overrides = serializers.IntegerField()

api/features/serializers.py

@@ -13,6 +13,7 @@
     MetadataSerializer,
     SerializerWithMetadata,
 )
+from django.db import models
 from drf_writable_nested import (  # type: ignore[attr-defined]
     WritableNestedModelSerializer,
 )
@@ -344,6 +345,15 @@ def get_project(
                 "Unable to retrieve project for metadata validation."
             )
 
+    def update(
+        self, instance: models.Model, validated_data: dict[str, typing.Any]
+    ) -> Feature:
+        metadata_items = validated_data.pop("metadata", [])
+        feature = typing.cast(Feature, super().update(instance, validated_data))
+        self.update_metadata(feature, metadata_items)
+        feature.refresh_from_db()
+        return feature
+
 
 class UpdateFeatureSerializerWithMetadata(FeatureSerializerWithMetadata):
     """prevent users from changing certain values after creation"""

api/poetry.lock

@@ -160,7 +160,7 @@ pygithub = "2.1.1"
 hubspot-api-client = "^8.2.1"
 djangorestframework-dataclasses = "^1.3.1"
 pyotp = "^2.9.0"
-flagsmith-common = "^1.12.0"
+flagsmith-common = "^1.12.1"
 django-stubs = "^5.1.3"
 tzdata = "^2024.1"
 djangorestframework-simplejwt = "^5.3.1"

api/pyproject.toml

@@ -1106,6 +1106,69 @@ def test_update_segment_delete_existing_rule(project, client, segment, segment_r
     assert segment_rule.conditions.count() == 0
 
 
+@pytest.mark.parametrize(
+    "client",
+    [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],
+)
+def test_update_segment_metadata_create_correct_number_of_metadata(
+    project: Project,
+    client: APIClient,
+    required_a_segment_metadata_field: MetadataModelField,
+) -> None:
+    # Given
+    url = reverse("api-v1:projects:project-segments-list", args=[project.id])
+    description = "This is the description"
+    field_value = 10
+    first_segment_data = {
+        "name": "Test Segment",
+        "description": description,
+        "project": project.id,
+        "rules": [{"type": "ALL", "rules": [], "conditions": []}],
+        "metadata": [
+            {
+                "model_field": required_a_segment_metadata_field.id,
+                "field_value": field_value,
+            },
+        ],
+    }
+    second_segment_data = {
+        "name": "Test Segment",
+        "description": description,
+        "project": project.id,
+        "rules": [{"type": "ALL", "rules": [], "conditions": []}],
+        "metadata": [
+            {
+                "model_field": required_a_segment_metadata_field.id,
+                "field_value": field_value,
+            },
+        ],
+    }
+
+    # When
+    response_first = client.post(
+        url, data=json.dumps(first_segment_data), content_type="application/json"
+    )
+    response_second = client.post(
+        url, data=json.dumps(second_segment_data), content_type="application/json"
+    )
+    # Then
+    assert response_first.status_code == status.HTTP_201_CREATED
+    assert response_second.status_code == status.HTTP_201_CREATED
+    assert (
+        response_first.json()["metadata"][0]["model_field"]
+        == required_a_segment_metadata_field.id
+    )
+    assert (
+        response_second.json()["metadata"][0]["model_field"]
+        == required_a_segment_metadata_field.id
+    )
+
+    metadata = Metadata.objects.filter(
+        model_field=required_a_segment_metadata_field.id
+    ).all()
+    assert metadata.count() == 2
+
+
 @pytest.mark.parametrize(
     "client",
     [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")],

api/tests/unit/segments/test_unit_segments_views.py

@@ -1,3 +1,6 @@
+from typing import Any
+
+from django.db import models
 from drf_writable_nested import (  # type: ignore[attr-defined]
     NestedCreateMixin,
     NestedUpdateMixin,
@@ -10,7 +13,9 @@ class NestedUpdateMixinDeleteBeforeUpdate(NestedUpdateMixin):
     ref: https://github.com/beda-software/drf-writable-nested/issues/158
     """
 
-    def update(self, instance, validated_data):  # type: ignore[no-untyped-def]
+    def update(
+        self, instance: models.Model, validated_data: dict[str, Any]
+    ) -> models.Model:
         relations, reverse_relations = self._extract_relations(validated_data)  # type: ignore[no-untyped-call]
 
         # Create or update direct relations (foreign key, one-to-one)