Skip to content

Commit

Permalink
fix: incorrect default_percentage_allocation on import, binary flags …
Browse files Browse the repository at this point in the history
…imported as multivariate (#2841)
  • Loading branch information
khvn26 authored Oct 12, 2023
1 parent 14fec1f commit 619c3f5
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 37 deletions.
8 changes: 6 additions & 2 deletions api/features/feature_types.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
MULTIVARIATE = "MULTIVARIATE"
STANDARD = "STANDARD"
from typing import Literal

FeatureType = Literal["STANDARD", "MULTIVARIATE"]

MULTIVARIATE: FeatureType = "MULTIVARIATE"
STANDARD: FeatureType = "STANDARD"

# the following two types have been merged in terms of functionality
# but kept for now until the FE is updated
Expand Down
96 changes: 73 additions & 23 deletions api/integrations/launch_darkly/services.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
from contextlib import contextmanager
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING, Callable, Tuple

from django.core import signing
from django.utils import timezone
from requests.exceptions import HTTPError, RequestException

from environments.models import Environment
from features.feature_types import MULTIVARIATE, STANDARD
from features.feature_types import MULTIVARIATE, STANDARD, FeatureType
from features.models import Feature, FeatureState, FeatureStateValue
from features.multivariate.models import (
MultivariateFeatureOption,
Expand Down Expand Up @@ -102,7 +102,7 @@ def _create_tags_from_ld(
return tags_by_ld_tag


def _create_standard_feature_states(
def _create_boolean_feature_states(
ld_flag: ld_types.FeatureFlag,
feature: Feature,
environments_by_ld_environment_key: dict[str, Environment],
Expand All @@ -114,9 +114,47 @@ def _create_standard_feature_states(
environment=environment,
defaults={"enabled": ld_flag_config["on"]},
)

FeatureStateValue.objects.update_or_create(
feature_state=feature_state,
defaults={"feature_state": feature_state},
)


def _create_string_feature_states(
ld_flag: ld_types.FeatureFlag,
feature: Feature,
environments_by_ld_environment_key: dict[str, Environment],
) -> None:
variations_by_idx = {
str(idx): variation for idx, variation in enumerate(ld_flag["variations"])
}

for ld_environment_key, environment in environments_by_ld_environment_key.items():
ld_flag_config = ld_flag["environments"][ld_environment_key]

is_flag_on = ld_flag_config["on"]
if is_flag_on:
variation_config_key = "isFallthrough"
else:
variation_config_key = "isOff"

string_value = ""

if ld_flag_config_summary := ld_flag_config.get("_summary"):
enabled_variations = ld_flag_config_summary.get("variations") or {}
for idx, variation_config in enabled_variations.items():
if variation_config.get(variation_config_key):
string_value = variations_by_idx[idx]["value"]
break

feature_state, _ = FeatureState.objects.update_or_create(
feature=feature,
environment=environment,
defaults={"enabled": is_flag_on},
)
FeatureStateValue.objects.update_or_create(
feature_state=feature_state,
defaults={"type": STRING, "string_value": string_value},
)


Expand All @@ -129,16 +167,15 @@ def _create_mv_feature_states(
mv_feature_options_by_variation: dict[str, MultivariateFeatureOption] = {}

for idx, variation in enumerate(variations):
mv_feature_options_by_variation[str(idx)] = MultivariateFeatureOption(
(
mv_feature_options_by_variation[str(idx)],
_,
) = MultivariateFeatureOption.objects.update_or_create(
feature=feature,
type=STRING,
string_value=variation["value"],
defaults={"default_percentage_allocation": 0, "type": STRING},
)

MultivariateFeatureOption.objects.bulk_create(
mv_feature_options_by_variation.values(),
)

for ld_environment_key, environment in environments_by_ld_environment_key.items():
ld_flag_config = ld_flag["environments"][ld_environment_key]
feature_state, _ = FeatureState.objects.update_or_create(
Expand Down Expand Up @@ -178,16 +215,38 @@ def _create_mv_feature_states(
)


def _get_feature_type_and_feature_state_factory(
ld_flag: ld_types.FeatureFlag,
) -> Tuple[
FeatureType,
Callable[[ld_types.FeatureFlag, Feature, dict[str, Environment]], None],
]:
match ld_flag["kind"]:
case "multivariate" if len(ld_flag["variations"]) > 2:
feature_type = MULTIVARIATE
feature_state_factory = _create_mv_feature_states
case "multivariate":
feature_type = STANDARD
feature_state_factory = _create_string_feature_states
case _: # assume boolean
feature_type = STANDARD
feature_state_factory = _create_boolean_feature_states

return feature_type, feature_state_factory


def _create_feature_from_ld(
ld_flag: ld_types.FeatureFlag,
environments_by_ld_environment_key: dict[str, Environment],
tags_by_ld_tag: dict[str, Tag],
project_id: int,
) -> Feature:
feature_type, feature_state_factory = {
"boolean": (STANDARD, _create_standard_feature_states),
"multivariate": (MULTIVARIATE, _create_mv_feature_states),
}[ld_flag["kind"]]
(
feature_type,
feature_state_factory,
) = _get_feature_type_and_feature_state_factory(
ld_flag,
)

tags = [
tags_by_ld_tag[LAUNCH_DARKLY_IMPORTED_DEFAULT_TAG_LABEL],
Expand Down Expand Up @@ -232,15 +291,6 @@ def _create_features_from_ld(
]


def get_import_request(
project: "Project", ld_project_key: str
) -> Optional[LaunchDarklyImportRequest]:
return LaunchDarklyImportRequest.objects.get(
project=project,
ld_project_key=ld_project_key,
)


def create_import_request(
project: "Project",
user: "FFAdminUser",
Expand Down
50 changes: 38 additions & 12 deletions api/tests/unit/integrations/launch_darkly/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,45 @@ def test_process_import_request__success__expected_status(
boolean_standard_feature_states_by_env_name["Test"].enabled is True
boolean_standard_feature_states_by_env_name["Production"].enabled is False

value_standard_feature = Feature.objects.get(project=project, name="flag2_value")
value_standard_feature_states_by_env_name = {
string_standard_feature = Feature.objects.get(project=project, name="flag2_value")
string_standard_feature_states_by_env_name = {
fs.environment.name: fs
for fs in FeatureState.objects.filter(feature=value_standard_feature)
for fs in FeatureState.objects.filter(feature=string_standard_feature)
}
value_standard_feature_states_by_env_name["Test"].enabled is True
value_standard_feature_states_by_env_name[
"Test"
].get_feature_state_value() == "123123"
value_standard_feature_states_by_env_name["Production"].enabled is False
value_standard_feature_states_by_env_name[
"Production"
].get_feature_state_value() == ""
assert string_standard_feature_states_by_env_name["Test"].enabled is True
assert (
string_standard_feature_states_by_env_name["Test"].get_feature_state_value()
== "123123"
)
assert (
string_standard_feature_states_by_env_name["Test"].feature_state_value.type
== "unicode"
)
assert (
string_standard_feature_states_by_env_name[
"Test"
].feature_state_value.string_value
== "123123"
)
assert string_standard_feature_states_by_env_name["Production"].enabled is False
assert (
string_standard_feature_states_by_env_name[
"Production"
].get_feature_state_value()
== ""
)
assert (
string_standard_feature_states_by_env_name[
"Production"
].feature_state_value.type
== "unicode"
)
assert (
string_standard_feature_states_by_env_name[
"Production"
].feature_state_value.string_value
== ""
)

# Multivariate feature states with percentage rollout have expected values.
percentage_mv_feature = Feature.objects.get(
Expand All @@ -168,7 +194,7 @@ def test_process_import_request__success__expected_status(
"multivariate_feature_option__string_value",
"percentage_allocation",
)
) == [("variation1", 100), ("variation2", 0)]
) == [("variation1", 100), ("variation2", 0), ("variation3", 0)]

assert percentage_mv_feature_states_by_env_name["Production"].enabled is True
assert list(
Expand Down

3 comments on commit 619c3f5

@vercel
Copy link

@vercel vercel bot commented on 619c3f5 Oct 12, 2023

Choose a reason for hiding this comment

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

@vercel
Copy link

@vercel vercel bot commented on 619c3f5 Oct 12, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

docs – ./docs

docs-flagsmith.vercel.app
docs-git-main-flagsmith.vercel.app
docs.bullet-train.io
docs.flagsmith.com

@vercel
Copy link

@vercel vercel bot commented on 619c3f5 Oct 12, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.