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

fix: incorrect default_percentage_allocation on import, binary flags imported as multivariate #2841

Merged
merged 1 commit into from
Oct 12, 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
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