diff --git a/api/app/settings/common.py b/api/app/settings/common.py index 7436f7a87b0f..be5bea5f1f68 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -1357,6 +1357,10 @@ SEGMENT_RULES_CONDITIONS_LIMIT = env.int("SEGMENT_RULES_CONDITIONS_LIMIT", 100) +SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED = env.bool( + "SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED", default=False +) + WEBHOOK_BACKOFF_BASE = env.int("WEBHOOK_BACKOFF_BASE", default=2) WEBHOOK_BACKOFF_RETRIES = env.int("WEBHOOK_BACKOFF_RETRIES", default=3) diff --git a/api/core/models.py b/api/core/models.py index 01db00e92a0c..100c3a6d740d 100644 --- a/api/core/models.py +++ b/api/core/models.py @@ -66,7 +66,9 @@ class SoftDeleteExportableManager(UUIDNaturalKeyManagerMixin, SoftDeleteManager) class SoftDeleteExportableModel(SoftDeleteObject, AbstractBaseExportableModel): # type: ignore[misc] - objects = SoftDeleteExportableManager() # type: ignore[misc] + objects: typing.ClassVar[SoftDeleteExportableManager] = ( + SoftDeleteExportableManager() + ) class Meta: abstract = True diff --git a/api/segments/models.py b/api/segments/models.py index bfebac8a3d02..e9ade72dd088 100644 --- a/api/segments/models.py +++ b/api/segments/models.py @@ -22,6 +22,7 @@ ) from audit.related_object_type import RelatedObjectType from core.models import ( + SoftDeleteExportableManager, SoftDeleteExportableModel, abstract_base_auditable_model_factory, ) @@ -300,6 +301,21 @@ def deep_clone(self, cloned_segment: Segment) -> "SegmentRule": return cloned_rule +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] @@ -343,6 +359,8 @@ 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() + def __str__(self): # type: ignore[no-untyped-def] return "Condition for %s: %s %s %s" % ( str(self.rule), diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index 856f5bd7fe26..f032f1b3ce5b 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -648,8 +648,11 @@ def test_update_segment_add_new_condition( admin_client_new: APIClient, segment: Segment, segment_rule: SegmentRule, + settings: SettingsWrapper, ) -> None: # Given + settings.SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED = True + url = reverse( "api-v1:projects:project-segments-detail", args=[project.id, segment.id] ) @@ -705,11 +708,8 @@ 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.order_by("-id").first().property - == new_condition_property - ) - assert nested_rule.conditions.order_by("-id").first().value == new_condition_value + assert nested_rule.conditions.last().property == new_condition_property + assert nested_rule.conditions.last().value == new_condition_value def test_update_mismatched_rule_and_segment( diff --git a/docs/docs/deployment/hosting/locally-api.md b/docs/docs/deployment/hosting/locally-api.md index 73e8629c76e9..4d3f6d3e092c 100644 --- a/docs/docs/deployment/hosting/locally-api.md +++ b/docs/docs/deployment/hosting/locally-api.md @@ -222,6 +222,7 @@ the below variables will be ignored. and hence should not be modified for already running instances of flagsmith. It should only be used for new installations, and should not be modified. WARNING: setting this to a higher limit may prevent imports to our SaaS platform if required in the future. +- `SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED`: Forces segment rule condition ordering by primary key, ascending. Default is `False` (no guaranteed order). **Warning**: changing this setting might affect evaluation for existing segments. - `ENABLE_API_USAGE_TRACKING`: Enable tracking of all API requests in Postgres / Influx. Default is True. Setting to False will mean that the Usage tab in the Organisation Settings will not show any data. Useful when using Postgres for analytics in high traffic environments to limit the size of database. @@ -451,7 +452,7 @@ increasing the performance of your task processor. | Environment Variable | Description | Example value | Default | -|---------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------|-----------------------------------------------| +| ------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | --------------------------------------------- | | `CACHE_ENVIRONMENT_DOCUMENT_MODE` | The caching mode. One of `PERSISTENT` or `EXPIRING`. Note that although the default is `EXPIRING` there is no caching by default due to the default value of `CACHE_ENVIRONMENT_DOCUMENT_SECONDS` | `PERSISTENT` | `EXPIRING` | | `CACHE_ENVIRONMENT_DOCUMENT_SECONDS` | Number of seconds to cache the environment for (only relevant when `CACHE_ENVIRONMENT_DOCUMENT_MODE=EXPIRING`) | `60` | `0` ( = don't cache) | | `CACHE_ENVIRONMENT_DOCUMENT_BACKEND` | Python path to the django cache backend chosen. See documentation [here](https://docs.djangoproject.com/en/4.2/topics/cache/). | `django.core.cache.backends.memcached.PyMemcacheCache` | `django.core.cache.backends.db.DatabaseCache` |