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: add endpoint to list (edge) identity overrides for a feature #3116

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
15 changes: 15 additions & 0 deletions api/edge_api/identities/edge_identity_service.py
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]
1 change: 1 addition & 0 deletions api/edge_api/identities/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def save(self, user: FFAdminUser = None, master_api_key: MasterAPIKey = None):
"environment_api_key": self.environment_api_key,
"changes": changes,
"identity_uuid": str(self.identity_uuid),
"identifier": self.identifier,
Copy link
Contributor Author

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.

}
)
self._reset_initial_state()
Expand Down
16 changes: 15 additions & 1 deletion api/edge_api/identities/permissions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
from contextlib import suppress

from django.http import HttpRequest
from django.views import View
from rest_framework.permissions import BasePermission

from environments.models import Environment
from environments.permissions.constants import UPDATE_FEATURE_STATE
from environments.permissions.constants import (
UPDATE_FEATURE_STATE,
VIEW_IDENTITIES,
)


class EdgeIdentityWithIdentifierViewPermissions(BasePermission):
Expand All @@ -15,3 +20,12 @@ def has_permission(self, request, view):
UPDATE_FEATURE_STATE, environment
)
return False


class GetEdgeIdentityOverridesPermission(BasePermission):
def has_permission(self, request: HttpRequest, view: View) -> bool:
environment_pk = view.kwargs.get("environment_pk")
with suppress(Environment.DoesNotExist):
environment = Environment.objects.get(pk=environment_pk)
return request.user.has_environment_permission(VIEW_IDENTITIES, environment)
return False
51 changes: 44 additions & 7 deletions api/edge_api/identities/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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):
Expand Down Expand Up @@ -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
)
Expand All @@ -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"]
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
2 changes: 2 additions & 0 deletions api/edge_api/identities/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def generate_audit_log_records(
def update_flagsmith_environments_v2_identity_overrides(
environment_api_key: str,
identity_uuid: str,
identifier: str,
changes: IdentityChangeset,
) -> None:
feature_override_changes = changes["feature_overrides"]
Expand All @@ -153,5 +154,6 @@ def update_flagsmith_environments_v2_identity_overrides(
identity_uuid=identity_uuid,
environment_api_key=environment_api_key,
environment_id=environment.id,
identifier=identifier,
)
dynamodb_wrapper_v2.update_identity_overrides(identity_override_changeset)
54 changes: 51 additions & 3 deletions api/edge_api/identities/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from flag_engine.identities.traits.models import TraitModel
from pyngo import drf_error_details
from rest_framework import status, viewsets
from rest_framework.decorators import action
from rest_framework.decorators import action, api_view, permission_classes
from rest_framework.exceptions import (
NotFound,
PermissionDenied,
Expand All @@ -24,6 +24,7 @@
RetrieveModelMixin,
)
from rest_framework.permissions import IsAuthenticated
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.views import APIView
from rest_framework.viewsets import GenericViewSet
Expand All @@ -40,6 +41,8 @@
EdgeIdentityTraitsSerializer,
EdgeIdentityWithIdentifierFeatureStateDeleteRequestBody,
EdgeIdentityWithIdentifierFeatureStateRequestBody,
GetEdgeIdentityOverridesQuerySerializer,
GetEdgeIdentityOverridesSerializer,
)
from environments.identities.serializers import (
IdentityAllFeatureStatesSerializer,
Expand All @@ -55,9 +58,13 @@
from projects.exceptions import DynamoNotEnabledError
from util.mappers import map_engine_identity_to_identity_document

from . import edge_identity_service
from .exceptions import TraitPersistenceError
from .models import EdgeIdentity
from .permissions import EdgeIdentityWithIdentifierViewPermissions
from .permissions import (
EdgeIdentityWithIdentifierViewPermissions,
GetEdgeIdentityOverridesPermission,
)


@method_decorator(
Expand Down Expand Up @@ -238,6 +245,15 @@ def get_object(self):

return feature_state

def get_serializer_context(self) -> dict:
return {
**super().get_serializer_context(),
"identity": self.identity,
"environment": Environment.objects.get(
api_key=self.kwargs["environment_api_key"]
),
}

@swagger_auto_schema(query_serializer=EdgeIdentityFsQueryparamSerializer())
def list(self, request, *args, **kwargs):
q_params_serializer = EdgeIdentityFsQueryparamSerializer(
Expand Down Expand Up @@ -314,7 +330,14 @@ def put(self, request, *args, **kwargs):
serializer = EdgeIdentityFeatureStateSerializer(
instance=feature_state,
data=request.data,
context={"view": self, "request": request},
context={
"view": self,
"request": request,
"identity": self.identity,
"environment": Environment.objects.get(
api_key=self.kwargs["environment_api_key"]
),
},
)
serializer.is_valid(raise_exception=True)
serializer.save()
Expand All @@ -334,3 +357,28 @@ def delete(self, request, *args, **kwargs):
master_api_key=getattr(request, "master_api_key", None),
)
return Response(status=status.HTTP_204_NO_CONTENT)


@swagger_auto_schema(
method="GET",
query_serializer=GetEdgeIdentityOverridesQuerySerializer(),
responses={200: GetEdgeIdentityOverridesSerializer()},
)
@api_view(http_method_names=["GET"])
@permission_classes([IsAuthenticated, GetEdgeIdentityOverridesPermission])
def get_edge_identity_overrides(
request: Request, environment_pk: int, **kwargs
) -> Response:
query_serializer = GetEdgeIdentityOverridesQuerySerializer(
data=request.query_params
)
query_serializer.is_valid(raise_exception=True)
feature_id = query_serializer.validated_data.get("feature")
environment = Environment.objects.get(pk=environment_pk)
items = edge_identity_service.get_edge_identity_overrides(
environment_pk, feature_id=feature_id
)
response_serializer = GetEdgeIdentityOverridesSerializer(
instance={"results": items}, context={"environment": environment}
)
return Response(response_serializer.data)
5 changes: 5 additions & 0 deletions api/environments/dynamodb/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ class IdentityOverrideV2(BaseModel):
environment_id: str
document_key: str
environment_api_key: str
identifier: str
feature_state: FeatureStateModel

@property
def identity_uuid(self) -> str:
return self.document_key.rsplit(":", maxsplit=1)[1]


@dataclass
class IdentityOverridesV2Changeset:
Expand Down
6 changes: 6 additions & 0 deletions api/environments/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
EdgeIdentityFeatureStateViewSet,
EdgeIdentityViewSet,
EdgeIdentityWithIdentifierFeatureStateView,
get_edge_identity_overrides,
)
from features.views import (
EnvironmentFeatureStateViewSet,
Expand Down Expand Up @@ -142,4 +143,9 @@
create_segment_override,
name="create-segment-override",
),
path(
"<int:environment_pk>/edge-identity-overrides",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure whether to use the pk here or the API key. I definitely prefer using the pk, and would like to move to using the pk everywhere in the future so I figured this would at least not add further tech debt. It does, however, cause some consistency issues.

get_edge_identity_overrides,
name="edge-identity-overrides",
),
]
22 changes: 1 addition & 21 deletions api/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading