-
Notifications
You must be signed in to change notification settings - Fork 429
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: add endpoint to list (edge) identity overrides for a feature #3116
Changes from 20 commits
c5bc148
e8063f8
da0bf47
5ecabe6
5ef1cfa
ad00da6
f950683
17b5a1a
360758c
b646708
87c33c3
99699b1
fe23b36
aa9fd3c
96571d5
d7c99c3
b74809f
c01f507
967da77
7ce4b10
7a044be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import typing | ||
|
||
from environments.dynamodb.dynamodb_wrapper import DynamoEnvironmentV2Wrapper | ||
from environments.dynamodb.types import IdentityOverrideV2 | ||
|
||
ddb_environment_v2_wrapper = DynamoEnvironmentV2Wrapper() | ||
|
||
|
||
def get_edge_identity_overrides( | ||
environment_id: int, feature_id: int | ||
) -> typing.List[IdentityOverrideV2]: | ||
override_items = ddb_environment_v2_wrapper.get_identity_overrides_by_feature_id( | ||
environment_id=environment_id, feature_id=feature_id | ||
) | ||
return [IdentityOverrideV2.parse_obj(item) for item in override_items] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
from rest_framework import serializers | ||
from rest_framework.exceptions import ValidationError | ||
|
||
from environments.dynamodb.types import IdentityOverrideV2 | ||
from environments.models import Environment | ||
from features.models import Feature, FeatureState, FeatureStateValue | ||
from features.multivariate.models import MultivariateFeatureOption | ||
|
@@ -77,7 +78,12 @@ def to_internal_value(self, data): | |
|
||
class FeatureStateValueEdgeIdentityField(serializers.Field): | ||
def to_representation(self, obj): | ||
identity_id = self.parent.get_identity_uuid() | ||
identity: EdgeIdentity = self.parent.context["identity"] | ||
environment: Environment = self.parent.context["environment"] | ||
identity_id = identity.get_hash_key( | ||
environment.use_identity_composite_key_for_hashing | ||
) | ||
|
||
Comment on lines
+81
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is technically out of the scope of this PR, but this was an issue in the current implementation of the views which retrieve the feature states for a given identity. The previous logic, although wrong, wouldn't have actually given any incorrect data (at least for users that are creating identity overrides via the dashboard, not via the API) since it only would have affected multivariate features with more than one weighted option, which we don't allow for identity overrides (you have to select a single option, which will be given 100% weighting). Since I had to also implement this functionality for the new endpoint, however, it made sense to fix it in this PR. |
||
return obj.get_value(identity_id=identity_id) | ||
|
||
def get_attribute(self, instance): | ||
|
@@ -124,7 +130,7 @@ class Meta: | |
swagger_schema_fields = {"type": "integer/string"} | ||
|
||
|
||
class EdgeIdentityFeatureStateSerializer(serializers.Serializer): | ||
class BaseEdgeIdentityFeatureStateSerializer(serializers.Serializer): | ||
feature_state_value = FeatureStateValueEdgeIdentityField( | ||
allow_null=True, required=False, default=None | ||
) | ||
|
@@ -133,13 +139,8 @@ class EdgeIdentityFeatureStateSerializer(serializers.Serializer): | |
many=True, required=False | ||
) | ||
enabled = serializers.BooleanField(required=False, default=False) | ||
identity_uuid = serializers.SerializerMethodField() | ||
|
||
featurestate_uuid = serializers.CharField(required=False, read_only=True) | ||
|
||
def get_identity_uuid(self, obj=None): | ||
return self.context["view"].identity.identity_uuid | ||
|
||
def save(self, **kwargs): | ||
view = self.context["view"] | ||
request = self.context["request"] | ||
|
@@ -200,6 +201,13 @@ def save(self, **kwargs): | |
return self.instance | ||
|
||
|
||
class EdgeIdentityFeatureStateSerializer(BaseEdgeIdentityFeatureStateSerializer): | ||
identity_uuid = serializers.SerializerMethodField() | ||
|
||
def get_identity_uuid(self, obj=None): | ||
return self.context["view"].identity.identity_uuid | ||
|
||
|
||
class EdgeIdentityIdentifierSerializer(serializers.Serializer): | ||
identifier = serializers.CharField(required=True, max_length=2000) | ||
|
||
|
@@ -227,3 +235,32 @@ class EdgeIdentityFsQueryparamSerializer(serializers.Serializer): | |
feature = serializers.IntegerField( | ||
required=False, help_text="ID of the feature to filter by" | ||
) | ||
|
||
|
||
class GetEdgeIdentityOverridesQuerySerializer(serializers.Serializer): | ||
feature = serializers.IntegerField(required=False) | ||
|
||
|
||
class GetEdgeIdentityOverridesResultSerializer(serializers.Serializer): | ||
identifier = serializers.CharField() | ||
identity_uuid = serializers.CharField() | ||
feature_state = BaseEdgeIdentityFeatureStateSerializer() | ||
|
||
def to_representation(self, instance: IdentityOverrideV2): | ||
# Since the FeatureStateValueEdgeIdentityField relies on having this data | ||
# available to generate the value of the feature state, we need to set this | ||
# and make it available to the field class. to_representation seems like the | ||
# best place for this since we only care about serialization here (not | ||
# deserialization). | ||
self.context["identity"] = EdgeIdentity.from_identity_document( | ||
{ | ||
"identifier": instance.identifier, | ||
"identity_uuid": instance.identity_uuid, | ||
"environment_api_key": self.context["environment"].api_key, | ||
} | ||
) | ||
return super().to_representation(instance) | ||
|
||
|
||
class GetEdgeIdentityOverridesSerializer(serializers.Serializer): | ||
results = GetEdgeIdentityOverridesResultSerializer(many=True) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
EdgeIdentityFeatureStateViewSet, | ||
EdgeIdentityViewSet, | ||
EdgeIdentityWithIdentifierFeatureStateView, | ||
get_edge_identity_overrides, | ||
) | ||
from features.views import ( | ||
EnvironmentFeatureStateViewSet, | ||
|
@@ -142,4 +143,9 @@ | |
create_segment_override, | ||
name="create-segment-override", | ||
), | ||
path( | ||
"<int:environment_pk>/edge-identity-overrides", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure whether to use the |
||
get_edge_identity_overrides, | ||
name="edge-identity-overrides", | ||
), | ||
] |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@khvn26, I had to add the identifier here as I need it in the response to the FE. I believe we'll also need it to power identity override in local evaluation too so I think it's data worth having.