fix: ensure feature segments are cloned correctly (#2706)

api/environments/models.py

@@ -162,8 +162,6 @@ def clone(self, name: str, api_key: str = None) -> "Environment":
         clone.name = name
         clone.api_key = api_key if api_key else create_hash()
         clone.save()
-        for feature_segment in self.feature_segments.all():
-            feature_segment.clone(clone)
 
         # Since identities are closely tied to the enviroment
         # it does not make much sense to clone them, hence

api/features/models.py

@@ -559,15 +559,17 @@ def clone(
         clone = deepcopy(self)
         clone.id = None
         clone.uuid = uuid.uuid4()
-        clone.feature_segment = (
-            FeatureSegment.objects.get(
-                environment=env,
-                feature=clone.feature,
-                segment=self.feature_segment.segment,
+
+        if self.feature_segment:
+            # For now, we can only create a new feature segment if we are cloning to another environment
+            # TODO: with feature versioning, ensure that we clone regardless.
+            #  see this PR: https://github.com/Flagsmith/flagsmith/pull/2382
+            clone.feature_segment = (
+                self.feature_segment.clone(environment=env)
+                if env != self.environment
+                else self.feature_segment
             )
-            if self.feature_segment
-            else None
-        )
+
         clone.environment = env
         clone.version = None if as_draft else version or self.version
         clone.live_from = live_from

api/tests/integration/conftest.py

@@ -223,6 +223,26 @@ def feature_segment(admin_client, segment, feature, environment):
     return response.json()["id"]
 
 
+@pytest.fixture()
+def segment_featurestate(
+    admin_client: APIClient,
+    segment: int,
+    feature: int,
+    environment: int,
+    feature_segment: int,
+) -> int:
+    data = {
+        "feature": feature,
+        "environment": environment,
+        "feature_segment": feature_segment,
+    }
+    url = reverse("api-v1:features:featurestates-list")
+    response = admin_client.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+    return response.json()["id"]
+
+
 @pytest.fixture()
 def identity_traits():
     return [

api/tests/integration/environments/test_clone_environment.py

@@ -4,6 +4,7 @@
 from django.urls import reverse
 from pytest_lazyfixture import lazy_fixture
 from rest_framework import status
+from rest_framework.test import APIClient
 from tests.integration.helpers import (
     get_env_feature_states_list_with_api,
     get_feature_segement_list_with_api,
@@ -98,7 +99,12 @@ def test_clone_environment_creates_admin_permission_with_the_current_user(
 
 
 def test_env_clone_creates_feature_segment(
-    admin_client, environment, environment_api_key, db, feature, feature_segment
+    admin_client: APIClient,
+    environment: int,
+    environment_api_key: str,
+    feature: int,
+    feature_segment: int,
+    segment_featurestate: int,
 ):
     # Firstly, let's clone the environment
     env_name = "Cloned env"
@@ -161,6 +167,9 @@ def test_env_clone_clones_segments_overrides(
             "feature_segment": feature_segment,
         },
     )
+    source_feature_segment_id = source_env_feature_states["results"][0][
+        "feature_segment"
+    ]
 
     # (fetch the feature segment id to filter feature states)
     clone_feature_segment_id = get_feature_segement_list_with_api(
@@ -199,3 +208,4 @@ def test_env_clone_clones_segments_overrides(
         clone_env_feature_states["results"][0]["feature_segment"]
         == clone_feature_segment_id
     )
+    assert clone_feature_segment_id != source_feature_segment_id

api/tests/unit/features/test_unit_features_models.py

@@ -404,3 +404,42 @@ def test_feature_state_gt_operator_for_segment_overrides_and_environment_default
 
     # Then
     assert segment_override > environment_default
+
+
+def test_feature_state_clone_for_segment_override_clones_feature_segment(
+    feature: Feature,
+    segment_featurestate: FeatureState,
+    environment: Environment,
+    environment_two: Environment,
+) -> None:
+    # When
+    cloned_fs = segment_featurestate.clone(env=environment_two, as_draft=True)
+
+    # Then
+    assert cloned_fs.feature_segment != segment_featurestate.feature_segment
+
+    assert (
+        cloned_fs.feature_segment.segment
+        == segment_featurestate.feature_segment.segment
+    )
+    assert (
+        cloned_fs.feature_segment.priority
+        == segment_featurestate.feature_segment.priority
+    )
+
+
+def test_feature_segment_clone(
+    feature_segment: FeatureSegment,
+    environment: Environment,
+    environment_two: Environment,
+) -> None:
+    # When
+    cloned_feature_segment = feature_segment.clone(environment=environment_two)
+
+    # Then
+    assert cloned_feature_segment.id != feature_segment.id
+
+    assert cloned_feature_segment.priority == feature_segment.priority
+    assert cloned_feature_segment.segment == feature_segment.segment
+    assert cloned_feature_segment.feature == feature_segment.feature
+    assert cloned_feature_segment.environment == environment_two