fix: SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED does not affect rules (#5866)

Author: khvn26

api/integrations/launch_darkly/services.py

@@ -352,7 +352,7 @@ def _create_segment_rule_for_segment(
                 f" skipping for segment: {segment.name}",
             )
 
-    return parent_rule  # type: ignore[no-any-return]
+    return parent_rule
 
 
 def _create_feature_segment_from_clauses(

api/segments/models.py

@@ -32,9 +32,26 @@
 
 from .managers import LiveSegmentManager, SegmentManager
 
+ModelT = typing.TypeVar("ModelT", bound=models.Model)
+
 logger = logging.getLogger(__name__)
 
 
+class ConfiguredOrderManager(SoftDeleteExportableManager, models.Manager[ModelT]):
+    def get_queryset(
+        self,
+    ) -> models.QuerySet[ModelT]:
+        # Effectively `<ModelT>.Meta.ordering = ("id",) if ... else ()`,
+        # but avoid the weirdness of a setting-dependant migration
+        # and having to reload everything in tests
+        qs: models.QuerySet[ModelT]
+        if settings.SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED:
+            qs = super().get_queryset().order_by("id")
+        else:
+            qs = super().get_queryset()
+        return qs
+
+
 class Segment(
     LifecycleModelMixin,  # type: ignore[misc]
     SoftDeleteExportableModel,
@@ -168,7 +185,8 @@ def copy_rules_and_conditions_from(self, source_segment: "Segment") -> None:
             cloned_rule.pk = None
             cloned_rule.uuid = uuid.uuid4()
             cloned_rule.segment = self if rule.segment else None
-            cloned_rule.rule = rule_to_cloned_rule_map.get(rule.rule)
+            if rule.rule in rule_to_cloned_rule_map:
+                cloned_rule.rule = rule_to_cloned_rule_map[rule.rule]
             cloned_rule.save()
             rule_to_cloned_rule_map[rule] = cloned_rule
 
@@ -217,6 +235,10 @@ class SegmentRule(
 
     history_record_class_path = "segments.models.HistoricalSegmentRule"
 
+    objects: typing.ClassVar[ConfiguredOrderManager["SegmentRule"]] = (
+        ConfiguredOrderManager()
+    )
+
     def __str__(self):  # type: ignore[no-untyped-def]
         return "%s rule for %s" % (
             self.type,
@@ -241,21 +263,6 @@ def get_skip_create_audit_log(self) -> bool:
         return True
 
 
-class ConditionManager(SoftDeleteExportableManager):
-    def get_queryset(
-        self,
-    ) -> models.QuerySet["Condition"]:
-        # Effectively `Condition.Meta.ordering = ("id",) if ... else ()`,
-        # but avoid the weirdness of a setting-dependant migration
-        # and having to reload everything in tests
-        qs: models.QuerySet["Condition"]
-        if settings.SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED:
-            qs = super().get_queryset().order_by("id")
-        else:
-            qs = super().get_queryset()
-        return qs
-
-
 class Condition(
     SoftDeleteExportableModel,
     abstract_base_auditable_model_factory(["uuid"]),  # type: ignore[misc]
@@ -299,7 +306,9 @@ class Condition(
     created_at = models.DateTimeField(null=True, auto_now_add=True)
     updated_at = models.DateTimeField(null=True, auto_now=True)
 
-    objects: typing.ClassVar[ConditionManager] = ConditionManager()
+    objects: typing.ClassVar[ConfiguredOrderManager["Condition"]] = (
+        ConfiguredOrderManager()
+    )
 
     def __str__(self) -> str:
         return "Condition for %s: %s %s %s" % (

api/tests/unit/environments/identities/test_unit_identities_views.py

@@ -603,7 +603,9 @@ def test_identities_endpoint_returns_value_for_segment_if_rule_type_percentage_s
     )
     Condition.objects.create(
         operator=PERCENTAGE_SPLIT,
-        value=(identity_percentage_value + (1 - identity_percentage_value) / 2) * 100.0,
+        value=int(
+            (identity_percentage_value + (1 - identity_percentage_value) / 2) * 100.0
+        ),
         rule=segment_rule,
     )
     feature_segment = FeatureSegment.objects.create(
@@ -654,7 +656,7 @@ def test_identities_endpoint_returns_default_value_if_rule_type_percentage_split
     )
     Condition.objects.create(
         operator=PERCENTAGE_SPLIT,
-        value=identity_percentage_value / 2,
+        value=int(identity_percentage_value / 2),
         rule=segment_rule,
     )
     feature_segment = FeatureSegment.objects.create(

api/tests/unit/segments/test_unit_segments_views.py

@@ -707,8 +707,9 @@ def test_update_segment_add_new_condition(
     assert response.status_code == status.HTTP_200_OK
 
     assert nested_rule.conditions.count() == 2
-    assert nested_rule.conditions.last().property == new_condition_property
-    assert nested_rule.conditions.last().value == new_condition_value
+    assert (expected_new_condition := nested_rule.conditions.last())
+    assert expected_new_condition.property == new_condition_property
+    assert expected_new_condition.value == new_condition_value
 
 
 def test_update_segment_versioned_segment(

infrastructure/aws/staging/ecs-task-definition-web.json

@@ -217,6 +217,10 @@
                     "name": "PROMETHEUS_ENABLED",
                     "value": "True"
                 },
+                {
+                    "name": "SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED",
+                    "value": "True"
+                },
                 {
                     "name": "FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE",
                     "value": "False"