Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Rely on Flagsmith Engine for segment evaluation #2865

Merged
merged 3 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest
from django.contrib.contenttypes.models import ContentType
from django.core.cache import cache
from flag_engine.segments.constants import EQUAL
from rest_framework.authtoken.models import Token
from rest_framework.test import APIClient

Expand Down Expand Up @@ -45,7 +46,7 @@
)
from projects.permissions import VIEW_PROJECT
from projects.tags.models import Tag
from segments.models import EQUAL, Condition, Segment, SegmentRule
from segments.models import Condition, Segment, SegmentRule
from task_processor.task_run_method import TaskRunMethod
from users.models import FFAdminUser, UserPermissionGroup

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
from core.constants import INTEGER
from django.core.exceptions import ObjectDoesNotExist
from flag_engine.identities.builders import build_identity_model
from flag_engine.segments.constants import IN
from rest_framework.exceptions import NotFound

from environments.dynamodb import DynamoIdentityWrapper
from environments.identities.models import Identity
from environments.identities.traits.models import Trait
from segments.models import IN, Condition, Segment, SegmentRule
from segments.models import Condition, Segment, SegmentRule
from util.mappers import (
map_environment_to_environment_document,
map_identity_to_identity_document,
Expand Down
18 changes: 8 additions & 10 deletions api/environments/identities/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import pytest
from core.constants import FLOAT
from django.utils import timezone
from flag_engine.segments.constants import (
EQUAL,
GREATER_THAN,
GREATER_THAN_INCLUSIVE,
LESS_THAN_INCLUSIVE,
NOT_EQUAL,
)
from rest_framework.test import APITestCase

from environments.identities.models import Identity
Expand All @@ -15,16 +22,7 @@
from features.value_types import BOOLEAN, INTEGER, STRING
from organisations.models import Organisation
from projects.models import Project
from segments.models import (
EQUAL,
GREATER_THAN,
GREATER_THAN_INCLUSIVE,
LESS_THAN_INCLUSIVE,
NOT_EQUAL,
Condition,
Segment,
SegmentRule,
)
from segments.models import Condition, Segment, SegmentRule

from .helpers import (
create_trait_for_identity,
Expand Down
18 changes: 9 additions & 9 deletions api/environments/identities/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
from unittest.case import TestCase

import pytest
from core.constants import FLAGSMITH_UPDATED_AT_HEADER
from core.constants import FLAGSMITH_UPDATED_AT_HEADER, STRING
from django.test import override_settings
from django.urls import reverse
from flag_engine.segments.constants import PERCENTAGE_SPLIT
from rest_framework import status
from rest_framework.test import APIClient, APITestCase

Expand All @@ -20,7 +21,6 @@
from integrations.amplitude.models import AmplitudeConfiguration
from organisations.models import Organisation, OrganisationRole
from projects.models import Project
from segments import models
from segments.models import Condition, Segment, SegmentRule
from util.tests import Helper

Expand Down Expand Up @@ -370,7 +370,7 @@ def test_identities_endpoint_returns_traits(self, mock_amplitude_wrapper):
trait = Trait.objects.create(
identity=self.identity,
trait_key="trait_key",
value_type="STRING",
value_type=STRING,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to think this is because those tests were wrong (and we were lucky that the default type is string) but it's worth noting that this is a slight change "STRING" -> "unicode".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. The engine mapper relies on Trait's trait_value property which doesn't employ the default string fallback.

string_value="trait_value",
)

Expand Down Expand Up @@ -422,7 +422,7 @@ def test_identities_endpoint_returns_value_for_segment_if_identity_in_segment(
Trait.objects.create(
identity=self.identity,
trait_key=trait_key,
value_type="STRING",
value_type=STRING,
string_value=trait_value,
)
segment = Segment.objects.create(name="Test Segment", project=self.project)
Expand Down Expand Up @@ -476,7 +476,7 @@ def test_identities_endpoint_returns_value_for_segment_if_identity_in_segment_an
Trait.objects.create(
identity=self.identity,
trait_key=trait_key,
value_type="STRING",
value_type=STRING,
string_value=trait_value,
)
segment = Segment.objects.create(name="Test Segment", project=self.project)
Expand Down Expand Up @@ -528,7 +528,7 @@ def test_identities_endpoint_returns_value_for_segment_if_rule_type_percentage_s
[segment.id, self.identity.id]
)
Condition.objects.create(
operator=models.PERCENTAGE_SPLIT,
operator=PERCENTAGE_SPLIT,
value=(identity_percentage_value + (1 - identity_percentage_value) / 2)
* 100.0,
rule=segment_rule,
Expand Down Expand Up @@ -575,7 +575,7 @@ def test_identities_endpoint_returns_default_value_if_rule_type_percentage_split
[segment.id, self.identity.id]
)
Condition.objects.create(
operator=models.PERCENTAGE_SPLIT,
operator=PERCENTAGE_SPLIT,
value=identity_percentage_value / 2,
rule=segment_rule,
)
Expand Down Expand Up @@ -628,13 +628,13 @@ def test_post_identify_deletes_a_trait_if_trait_value_is_none(self):
trait_1 = Trait.objects.create(
identity=self.identity,
trait_key="trait_key_1",
value_type="STRING",
value_type=STRING,
string_value="trait_value",
)
trait_2 = Trait.objects.create(
identity=self.identity,
trait_key="trait_key_2",
value_type="STRING",
value_type=STRING,
string_value="trait_value",
)

Expand Down
3 changes: 2 additions & 1 deletion api/features/feature_segments/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import pytest
from core.constants import STRING
from django.test import TestCase
from flag_engine.segments.constants import EQUAL

from environments.identities.models import Identity
from environments.identities.traits.models import Trait
from environments.models import Environment
from features.models import Feature, FeatureSegment
from organisations.models import Organisation
from projects.models import Project
from segments.models import EQUAL, Condition, Segment, SegmentRule
from segments.models import Condition, Segment, SegmentRule


@pytest.mark.django_db
Expand Down
Loading