From 5c02c1effe450e8f2301bbeb8e614e67e119b98f Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Wed, 4 Sep 2024 15:44:59 +0100 Subject: [PATCH] feat: search identities by dashboard alias (#4569) --- api/conftest.py | 23 +++- api/edge_api/identities/search.py | 28 ++++ api/edge_api/identities/serializers.py | 67 +++++++++- api/edge_api/identities/views.py | 56 ++++---- .../dynamodb/wrappers/identity_wrapper.py | 22 +-- api/poetry.lock | 12 +- api/tests/integration/conftest.py | 2 +- ...est_edge_identity_featurestates_viewset.py | 126 +++++++++--------- .../identities/test_edge_identity_viewset.py | 117 +++++++++++++--- .../unit/edge_api/identities/conftest.py | 2 +- .../test_unit_dynamodb_identity_wrapper.py | 16 ++- .../mappers/test_unit_mappers_dynamodb.py | 1 - .../util/mappers/test_unit_mappers_sdk.py | 1 + api/util/mappers/dynamodb.py | 3 + frontend/common/types/responses.ts | 1 + frontend/web/components/pages/UsersPage.tsx | 7 +- 16 files changed, 346 insertions(+), 138 deletions(-) create mode 100644 api/edge_api/identities/search.py diff --git a/api/conftest.py b/api/conftest.py index 9aaa0e840a97..3256a79e49f4 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -12,6 +12,7 @@ from flag_engine.segments.constants import EQUAL from moto import mock_dynamodb from mypy_boto3_dynamodb.service_resource import DynamoDBServiceResource, Table +from pytest_django.fixtures import SettingsWrapper from pytest_django.plugin import blocking_manager_key from pytest_mock import MockerFixture from rest_framework.authtoken.models import Token @@ -977,8 +978,10 @@ def dynamodb(aws_credentials): @pytest.fixture() -def flagsmith_identities_table(dynamodb: DynamoDBServiceResource) -> Table: - return dynamodb.create_table( +def flagsmith_identities_table( + dynamodb: DynamoDBServiceResource, settings: SettingsWrapper +) -> Table: + table = dynamodb.create_table( TableName="flagsmith_identities", KeySchema=[ { @@ -991,6 +994,7 @@ def flagsmith_identities_table(dynamodb: DynamoDBServiceResource) -> Table: {"AttributeName": "environment_api_key", "AttributeType": "S"}, {"AttributeName": "identifier", "AttributeType": "S"}, {"AttributeName": "identity_uuid", "AttributeType": "S"}, + {"AttributeName": "dashboard_alias", "AttributeType": "S"}, ], GlobalSecondaryIndexes=[ { @@ -1006,9 +1010,24 @@ def flagsmith_identities_table(dynamodb: DynamoDBServiceResource) -> Table: "KeySchema": [{"AttributeName": "identity_uuid", "KeyType": "HASH"}], "Projection": {"ProjectionType": "ALL"}, }, + { + "IndexName": "environment_api_key-dashboard_alias-index", + "KeySchema": [ + {"AttributeName": "environment_api_key", "KeyType": "HASH"}, + {"AttributeName": "dashboard_alias", "KeyType": "RANGE"}, + ], + "Projection": { + "ProjectionType": "INCLUDE", + "NonKeyAttributes": [ + "identifier", + ], + }, + }, ], BillingMode="PAY_PER_REQUEST", ) + settings.IDENTITIES_TABLE_NAME_DYNAMO = table.name + return table @pytest.fixture() diff --git a/api/edge_api/identities/search.py b/api/edge_api/identities/search.py new file mode 100644 index 000000000000..dcc4fd38d1ef --- /dev/null +++ b/api/edge_api/identities/search.py @@ -0,0 +1,28 @@ +import enum +from dataclasses import dataclass + +IDENTIFIER_ATTRIBUTE = "identifier" +DASHBOARD_ALIAS_ATTRIBUTE = "dashboard_alias" +DASHBOARD_ALIAS_SEARCH_PREFIX = f"{DASHBOARD_ALIAS_ATTRIBUTE}:" + + +class EdgeIdentitySearchType(enum.Enum): + EQUAL = "EQUAL" + BEGINS_WITH = "BEGINS_WITH" + + +@dataclass +class EdgeIdentitySearchData: + search_term: str + search_type: EdgeIdentitySearchType + search_attribute: str + + @property + def dynamo_search_method(self): + return ( + "eq" if self.search_type == EdgeIdentitySearchType.EQUAL else "begins_with" + ) + + @property + def dynamo_index_name(self): + return f"environment_api_key-{self.search_attribute}-index" diff --git a/api/edge_api/identities/serializers.py b/api/edge_api/identities/serializers.py index 00e937520b48..ce22aa8d9bbe 100644 --- a/api/edge_api/identities/serializers.py +++ b/api/edge_api/identities/serializers.py @@ -32,18 +32,29 @@ from webhooks.constants import WEBHOOK_DATETIME_FORMAT from .models import EdgeIdentity +from .search import ( + DASHBOARD_ALIAS_ATTRIBUTE, + DASHBOARD_ALIAS_SEARCH_PREFIX, + IDENTIFIER_ATTRIBUTE, + EdgeIdentitySearchData, + EdgeIdentitySearchType, +) from .tasks import call_environment_webhook_for_feature_state_change class EdgeIdentitySerializer(serializers.Serializer): identity_uuid = serializers.CharField(read_only=True) identifier = serializers.CharField(required=True, max_length=2000) + dashboard_alias = serializers.CharField(required=False, max_length=100) - def save(self, **kwargs): + def create(self, *args, **kwargs): identifier = self.validated_data.get("identifier") + dashboard_alias = self.validated_data.get("dashboard_alias") environment_api_key = self.context["view"].kwargs["environment_api_key"] self.instance = EngineIdentity( - identifier=identifier, environment_api_key=environment_api_key + identifier=identifier, + environment_api_key=environment_api_key, + dashboard_alias=dashboard_alias, ) if EdgeIdentity.dynamo_wrapper.get_item(self.instance.composite_key): raise ValidationError( @@ -55,6 +66,28 @@ def save(self, **kwargs): return self.instance +class EdgeIdentityUpdateSerializer(EdgeIdentitySerializer): + def get_fields(self): + fields = super().get_fields() + fields["identifier"].read_only = True + return fields + + def update( + self, instance: dict[str, typing.Any], validated_data: dict[str, typing.Any] + ) -> EngineIdentity: + engine_identity = EngineIdentity.model_validate(instance) + + engine_identity.dashboard_alias = ( + self.validated_data.get("dashboard_alias") + or engine_identity.dashboard_alias + ) + + edge_identity = EdgeIdentity(engine_identity) + edge_identity.save() + + return engine_identity + + class EdgeMultivariateFeatureOptionField(serializers.IntegerField): def to_internal_value( self, @@ -238,6 +271,36 @@ class GetEdgeIdentityOverridesQuerySerializer(serializers.Serializer): feature = serializers.IntegerField(required=False) +class EdgeIdentitySearchField(serializers.CharField): + def to_internal_value(self, data: str) -> EdgeIdentitySearchData: + kwargs = {} + search_term = data + + if search_term.startswith(DASHBOARD_ALIAS_SEARCH_PREFIX): + kwargs["search_attribute"] = DASHBOARD_ALIAS_ATTRIBUTE + search_term = search_term.lstrip(DASHBOARD_ALIAS_SEARCH_PREFIX) + else: + kwargs["search_attribute"] = IDENTIFIER_ATTRIBUTE + + if search_term.startswith('"') and search_term.endswith('"'): + kwargs["search_type"] = EdgeIdentitySearchType.EQUAL + search_term = search_term[1:-1] + else: + kwargs["search_type"] = EdgeIdentitySearchType.BEGINS_WITH + + return EdgeIdentitySearchData(**kwargs, search_term=search_term) + + +class ListEdgeIdentitiesQuerySerializer(serializers.Serializer): + page_size = serializers.IntegerField(required=False) + q = EdgeIdentitySearchField( + required=False, + help_text="Search string to look for. Prefix with 'dashboard_alias:' " + "to search over aliases instead of identifiers.", + ) + last_evaluated_key = serializers.CharField(required=False, allow_null=True) + + class GetEdgeIdentityOverridesResultSerializer(serializers.Serializer): identifier = serializers.CharField() identity_uuid = serializers.CharField() diff --git a/api/edge_api/identities/views.py b/api/edge_api/identities/views.py index 76d2eef94b6f..ac2994277e89 100644 --- a/api/edge_api/identities/views.py +++ b/api/edge_api/identities/views.py @@ -3,7 +3,6 @@ import typing import pydantic -from boto3.dynamodb.conditions import Key from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator from drf_yasg.utils import swagger_auto_schema @@ -22,6 +21,7 @@ DestroyModelMixin, ListModelMixin, RetrieveModelMixin, + UpdateModelMixin, ) from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request @@ -40,10 +40,12 @@ EdgeIdentitySerializer, EdgeIdentitySourceIdentityRequestSerializer, EdgeIdentityTraitsSerializer, + EdgeIdentityUpdateSerializer, EdgeIdentityWithIdentifierFeatureStateDeleteRequestBody, EdgeIdentityWithIdentifierFeatureStateRequestBody, GetEdgeIdentityOverridesQuerySerializer, GetEdgeIdentityOverridesSerializer, + ListEdgeIdentitiesQuerySerializer, ) from environments.identities.serializers import ( IdentityAllFeatureStatesSerializer, @@ -66,6 +68,7 @@ EdgeIdentityWithIdentifierViewPermissions, GetEdgeIdentityOverridesPermission, ) +from .search import EdgeIdentitySearchData @method_decorator( @@ -81,14 +84,10 @@ class EdgeIdentityViewSet( RetrieveModelMixin, DestroyModelMixin, ListModelMixin, + UpdateModelMixin, ): - serializer_class = EdgeIdentitySerializer pagination_class = EdgeIdentityPagination lookup_field = "identity_uuid" - dynamo_identifier_search_functions = { - "EQUAL": lambda identifier: Key("identifier").eq(identifier), - "BEGINS_WITH": lambda identifier: Key("identifier").begins_with(identifier), - } def initial(self, request, *args, **kwargs): environment = self.get_environment_from_request() @@ -97,44 +96,43 @@ def initial(self, request, *args, **kwargs): super().initial(request, *args, **kwargs) - def _get_search_function_and_value( - self, - search_query: str, - ) -> typing.Tuple[typing.Callable, str]: - if search_query.startswith('"') and search_query.endswith('"'): - return self.dynamo_identifier_search_functions[ - "EQUAL" - ], search_query.replace('"', "") - return self.dynamo_identifier_search_functions["BEGINS_WITH"], search_query + def get_serializer_class(self): + if self.action in ("update", "partial_update"): + return EdgeIdentityUpdateSerializer + return EdgeIdentitySerializer def get_object(self): + # TODO: should this return an EdgeIdentity object instead of a dict? return EdgeIdentity.dynamo_wrapper.get_item_from_uuid_or_404( self.kwargs["identity_uuid"] ) def get_queryset(self): page_size = self.pagination_class().get_page_size(self.request) - previous_last_evaluated_key = self.request.GET.get("last_evaluated_key") - search_query = self.request.query_params.get("q") + + query_serializer = ListEdgeIdentitiesQuerySerializer( + data=self.request.query_params + ) + query_serializer.is_valid(raise_exception=True) + start_key = None - if previous_last_evaluated_key: + if previous_last_evaluated_key := query_serializer.validated_data.get( + "last_evaluated_key" + ): start_key = json.loads(base64.b64decode(previous_last_evaluated_key)) - if not search_query: + search_query: typing.Optional[EdgeIdentitySearchData] + if not (search_query := query_serializer.validated_data.get("q")): return EdgeIdentity.dynamo_wrapper.get_all_items( self.kwargs["environment_api_key"], page_size, start_key ) - search_func, search_identifier = self._get_search_function_and_value( - search_query - ) - identity_documents = EdgeIdentity.dynamo_wrapper.search_items_with_identifier( - self.kwargs["environment_api_key"], - search_identifier, - search_func, - page_size, - start_key, + + return EdgeIdentity.dynamo_wrapper.search_items( + environment_api_key=self.kwargs["environment_api_key"], + search_data=search_query, + limit=page_size, + start_key=start_key, ) - return identity_documents def get_permissions(self): return [ diff --git a/api/environments/dynamodb/wrappers/identity_wrapper.py b/api/environments/dynamodb/wrappers/identity_wrapper.py index 1a566d5f06ba..a9b605432bb2 100644 --- a/api/environments/dynamodb/wrappers/identity_wrapper.py +++ b/api/environments/dynamodb/wrappers/identity_wrapper.py @@ -12,6 +12,7 @@ from flag_engine.segments.evaluator import get_identity_segments from rest_framework.exceptions import NotFound +from edge_api.identities.search import EdgeIdentitySearchData from environments.dynamodb.constants import IDENTITIES_PAGINATION_LIMIT from environments.dynamodb.wrappers.exceptions import CapacityBudgetExceeded from util.mappers import map_identity_to_identity_document @@ -147,24 +148,29 @@ def iter_all_items_paginated( if last_evaluated_key := query_response.get("LastEvaluatedKey"): get_all_items_kwargs["start_key"] = last_evaluated_key - def search_items_with_identifier( + def search_items( self, environment_api_key: str, - identifier: str, - search_function: typing.Callable, + search_data: EdgeIdentitySearchData, limit: int, start_key: dict = None, - ): - filter_expression = Key("environment_api_key").eq( + ) -> "QueryOutputTableTypeDef": + partition_key_search_expression = Key("environment_api_key").eq( environment_api_key - ) & search_function(identifier) + ) + sort_key_search_expression = getattr( + Key(search_data.search_attribute), search_data.dynamo_search_method + )(search_data.search_term) + query_kwargs = { - "IndexName": "environment_api_key-identifier-index", + "IndexName": search_data.dynamo_index_name, "Limit": limit, - "KeyConditionExpression": filter_expression, + "KeyConditionExpression": partition_key_search_expression + & sort_key_search_expression, } if start_key: query_kwargs.update(ExclusiveStartKey=start_key) + return self.query_items(**query_kwargs) def get_segment_ids( diff --git a/api/poetry.lock b/api/poetry.lock index d5d5e9e7be57..234bb6ce3baf 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "annotated-types" @@ -3320,7 +3320,6 @@ files = [ {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:69b023b2b4daa7548bcfbd4aa3da05b3a74b772db9e23b982788168117739938"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:81e0b275a9ecc9c0c0c07b4b90ba548307583c125f54d5b6946cfee6360c733d"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:ba336e390cd8e4d1739f42dfe9bb83a3cc2e80f567d8805e11b46f4a943f5515"}, - {file = "PyYAML-6.0.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:326c013efe8048858a6d312ddd31d56e468118ad4cdeda36c719bf5bb6192290"}, {file = "PyYAML-6.0.1-cp310-cp310-win32.whl", hash = "sha256:bd4af7373a854424dabd882decdc5579653d7868b8fb26dc7d0e99f823aa5924"}, {file = "PyYAML-6.0.1-cp310-cp310-win_amd64.whl", hash = "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d"}, {file = "PyYAML-6.0.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:6965a7bc3cf88e5a1c3bd2e0b5c22f8d677dc88a455344035f03399034eb3007"}, @@ -3328,15 +3327,8 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:42f8152b8dbc4fe7d96729ec2b99c7097d656dc1213a3229ca5383f973a5ed6d"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d2b04aac4d386b172d5b9692e2d2da8de7bfb6c387fa4f801fbf6fb2e6ba4673"}, - {file = "PyYAML-6.0.1-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:e7d73685e87afe9f3b36c799222440d6cf362062f78be1013661b00c5c6f678b"}, {file = "PyYAML-6.0.1-cp311-cp311-win32.whl", hash = "sha256:1635fd110e8d85d55237ab316b5b011de701ea0f29d07611174a1b42f1444741"}, {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, - {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, - {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, - {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, - {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, - {file = "PyYAML-6.0.1-cp312-cp312-win_amd64.whl", hash = "sha256:0d3304d8c0adc42be59c5f8a4d9e3d7379e6955ad754aa9d6ab7a398b59dd1df"}, {file = "PyYAML-6.0.1-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:50550eb667afee136e9a77d6dc71ae76a44df8b3e51e41b77f6de2932bfe0f47"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:1fe35611261b29bd1de0070f0b2f47cb6ff71fa6595c077e42bd0c419fa27b98"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:704219a11b772aea0d8ecd7058d0082713c3562b4e271b849ad7dc4a5c90c13c"}, @@ -3353,7 +3345,6 @@ files = [ {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:28c119d996beec18c05208a8bd78cbe4007878c6dd15091efb73a30e90539696"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:7e07cbde391ba96ab58e532ff4803f79c4129397514e1413a7dc761ccd755735"}, - {file = "PyYAML-6.0.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:49a183be227561de579b4a36efbb21b3eab9651dd81b1858589f796549873dd6"}, {file = "PyYAML-6.0.1-cp38-cp38-win32.whl", hash = "sha256:184c5108a2aca3c5b3d3bf9395d50893a7ab82a38004c8f61c258d4428e80206"}, {file = "PyYAML-6.0.1-cp38-cp38-win_amd64.whl", hash = "sha256:1e2722cc9fbb45d9b87631ac70924c11d3a401b2d7f410cc0e3bbf249f2dca62"}, {file = "PyYAML-6.0.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8"}, @@ -3361,7 +3352,6 @@ files = [ {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:5773183b6446b2c99bb77e77595dd486303b4faab2b086e7b17bc6bef28865f6"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:b786eecbdf8499b9ca1d697215862083bd6d2a99965554781d0d8d1ad31e13a0"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:bc1bf2925a1ecd43da378f4db9e4f799775d6367bdb94671027b73b393a7c42c"}, - {file = "PyYAML-6.0.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5"}, {file = "PyYAML-6.0.1-cp39-cp39-win32.whl", hash = "sha256:faca3bdcf85b2fc05d06ff3fbc1f83e1391b3e724afa3feba7d13eeab355484c"}, {file = "PyYAML-6.0.1-cp39-cp39-win_amd64.whl", hash = "sha256:510c9deebc5c0225e8c96813043e62b680ba2f9c50a08d3724c7f28a747d1486"}, {file = "PyYAML-6.0.1.tar.gz", hash = "sha256:bfdf460b1736c775f2ba9f6a92bca30bc2095067b8a9d77876d1fad6cc3b4a43"}, diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index 806703c2d0bd..b5d79c20155c 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -432,7 +432,7 @@ def identity_document( } return { "composite_key": f"{environment_api_key}_user_1_test", - "dashboard_alias": None, + "dashboard_alias": "dashboard-alias", "identity_traits": identity_traits, "identity_features": [ _environment_feature_state_1_document, diff --git a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py index 101d7e22ded8..55a18d30c68b 100644 --- a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py +++ b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py @@ -1,7 +1,8 @@ +import copy import json import typing import uuid -from unittest.mock import MagicMock +from unittest import mock import pytest from core.constants import BOOLEAN, INTEGER, STRING @@ -30,6 +31,7 @@ DynamoEnvironmentWrapper, DynamoIdentityWrapper, ) +from environments.models import Environment from features.models import Feature from features.multivariate.models import MultivariateFeatureOption from projects.models import Project @@ -357,10 +359,10 @@ def test_edge_identities_create_featurestate( environment: int, environment_api_key: str, identity_document_without_fs: dict, - edge_identity_dynamo_wrapper_mock: MagicMock, + edge_identity_dynamo_wrapper_mock: mock.MagicMock, feature: int, feature_name: str, - webhook_mock: MagicMock, + webhook_mock: mock.MagicMock, ): # Given edge_identity_dynamo_wrapper_mock.get_item_from_uuid_or_404.return_value = ( @@ -496,19 +498,16 @@ def test_edge_identities_create_mv_featurestate( def test_edge_identities_update_featurestate( - dynamodb_wrapper_v2, - admin_client, - environment, - environment_api_key, - identity_document, - edge_identity_dynamo_wrapper_mock, - feature, - webhook_mock, + dynamodb_wrapper_v2: DynamoEnvironmentV2Wrapper, + admin_client: APIClient, + environment: Environment, + environment_api_key: str, + identity_document: dict[str, typing.Any], + feature: Feature, + webhook_mock: mock.MagicMock, + flagsmith_identities_table: Table, ): # Given - edge_identity_dynamo_wrapper_mock.get_item_from_uuid_or_404.return_value = ( - identity_document - ) identity_uuid = identity_document["identity_uuid"] featurestate_uuid = identity_document["identity_features"][0]["featurestate_uuid"] url = reverse( @@ -526,27 +525,25 @@ def test_edge_identities_update_featurestate( "identity_uuid": identity_uuid, } + flagsmith_identities_table.put_item(Item=identity_document) + # When response = admin_client.put( url, data=json.dumps(data), content_type="application/json" ) + # Then assert response.status_code == status.HTTP_200_OK assert response.json()["feature"] == feature assert response.json()["feature_state_value"] == expected_feature_state_value assert response.json()["enabled"] == data["enabled"] - edge_identity_dynamo_wrapper_mock.get_item_from_uuid_or_404.assert_called_with( - identity_uuid - ) - name, args, _ = edge_identity_dynamo_wrapper_mock.mock_calls[1] - assert name == "put_item" - # Next, let's verify that the document that we put # have correct updates # First, let's create the copy of the original document - expected_identity_document = identity_document + expected_identity_document = copy.deepcopy(identity_document) + # Next, let's modify the fs value that we updated expected_identity_document["identity_features"][0][ "feature_state_value" @@ -556,7 +553,12 @@ def test_edge_identities_update_featurestate( expected_identity_document["identity_features"][0]["enabled"] = expected_fs_enabled # Finally, let's compare them - assert args[0] == expected_identity_document + assert ( + flagsmith_identities_table.get_item( + Key={"composite_key": identity_document["composite_key"]} + )["Item"] + == expected_identity_document + ) def test_edge_identities_patch_returns_405( @@ -585,21 +587,18 @@ def test_edge_identities_patch_returns_405( def test_edge_identities_update_mv_featurestate( - dynamodb_wrapper_v2, - admin_client, - environment, - environment_api_key, - identity_document, - edge_identity_dynamo_wrapper_mock, - feature, - mv_option_50_percent, - mv_option_value, - webhook_mock, + dynamodb_wrapper_v2: DynamoEnvironmentV2Wrapper, + admin_client: APIClient, + environment: Environment, + environment_api_key: str, + identity_document: dict[str, typing.Any], + feature: Feature, + mv_option_50_percent: MultivariateFeatureOption, + mv_option_value: str, + webhook_mock: mock.MagicMock, + flagsmith_identities_table: Table, ): # Given - edge_identity_dynamo_wrapper_mock.get_item_from_uuid_or_404.return_value = ( - identity_document - ) identity_uuid = identity_document["identity_uuid"] featurestate_uuid = identity_document["identity_features"][2]["featurestate_uuid"] url = reverse( @@ -624,6 +623,8 @@ def test_edge_identities_update_mv_featurestate( "feature_state_value": expected_feature_state_value, } + flagsmith_identities_table.put_item(Item=identity_document) + # When response = admin_client.put( url, data=json.dumps(data), content_type="application/json" @@ -636,17 +637,12 @@ def test_edge_identities_update_mv_featurestate( == new_mv_allocation ) - edge_identity_dynamo_wrapper_mock.get_item_from_uuid_or_404.assert_called_with( - identity_uuid - ) - name, args, _ = edge_identity_dynamo_wrapper_mock.mock_calls[1] - assert name == "put_item" - # Next, let's verify that the document that we put # have correct updates # First, let's create the copy of the original document - expected_identity_document = identity_document + expected_identity_document = copy.deepcopy(identity_document) + # Next, let's modify the fs value that we updated expected_identity_document["identity_features"][2][ "feature_state_value" @@ -666,15 +662,17 @@ def test_edge_identities_update_mv_featurestate( "id": mv_option_50_percent, "value": mv_option_value, }, + "mv_fs_value_uuid": mock.ANY, } ] - # Remove the uuid before comparing because it was generated by the engine - # and we can't patch it - args[0]["identity_features"][2]["multivariate_feature_state_values"][0].pop( - "mv_fs_value_uuid" - ) + # Finally, let's compare them - assert args[0] == expected_identity_document + assert ( + flagsmith_identities_table.get_item( + Key={"composite_key": identity_document["composite_key"]} + )["Item"] + == expected_identity_document + ) def test_edge_identities_post_returns_400_for_invalid_mvfs_allocation( @@ -840,17 +838,16 @@ def test_edge_identities_with_identifier_delete_featurestate( def test_edge_identities_with_identifier_update_featurestate( - dynamodb_wrapper_v2, - admin_client, - environment, - environment_api_key, - identity_document, - edge_identity_dynamo_wrapper_mock, - feature, - webhook_mock, + dynamodb_wrapper_v2: DynamoEnvironmentV2Wrapper, + admin_client: APIClient, + environment: Environment, + environment_api_key: str, + identity_document: dict[str, typing.Any], + feature: Feature, + webhook_mock: mock.MagicMock, + flagsmith_identities_table: Table, ): # Given - edge_identity_dynamo_wrapper_mock.get_item.return_value = identity_document identifier = identity_document["identifier"] url = reverse( "api-v1:environments:edge-identities-with-identifier-featurestates", @@ -866,20 +863,18 @@ def test_edge_identities_with_identifier_update_featurestate( "identifier": identifier, } + flagsmith_identities_table.put_item(Item=identity_document) + # When response = admin_client.put( url, data=json.dumps(data), content_type="application/json" ) + # Then assert response.status_code == status.HTTP_200_OK assert response.json()["feature"] == feature assert response.json()["feature_state_value"] == expected_feature_state_value assert response.json()["enabled"] == data["enabled"] - edge_identity_dynamo_wrapper_mock.get_item.assert_called_with( - f"{environment_api_key}_{identifier}" - ) - name, args, _ = edge_identity_dynamo_wrapper_mock.mock_calls[1] - assert name == "put_item" # Next, let's verify that the document that we put # have correct updates @@ -893,7 +888,12 @@ def test_edge_identities_with_identifier_update_featurestate( identity_document["identity_features"][0]["enabled"] = expected_fs_enabled # Finally, let's compare them - assert args[0] == identity_document + assert ( + flagsmith_identities_table.get_item( + Key={"composite_key": identity_document["composite_key"]} + )["Item"] + == identity_document + ) @pytest.mark.parametrize( diff --git a/api/tests/integration/edge_api/identities/test_edge_identity_viewset.py b/api/tests/integration/edge_api/identities/test_edge_identity_viewset.py index ffdadfe5d6a1..4b86aa448494 100644 --- a/api/tests/integration/edge_api/identities/test_edge_identity_viewset.py +++ b/api/tests/integration/edge_api/identities/test_edge_identity_viewset.py @@ -4,17 +4,24 @@ from boto3.dynamodb.conditions import Key from django.urls import reverse +from mypy_boto3_dynamodb.service_resource import Table from rest_framework import status from rest_framework.exceptions import NotFound from rest_framework.test import APIClient -from edge_api.identities.views import EdgeIdentityViewSet +from edge_api.identities.search import ( + DASHBOARD_ALIAS_SEARCH_PREFIX, + IDENTIFIER_ATTRIBUTE, + EdgeIdentitySearchData, + EdgeIdentitySearchType, +) from environments.dynamodb.wrappers.environment_wrapper import ( DynamoEnvironmentV2Wrapper, ) from environments.dynamodb.wrappers.identity_wrapper import ( DynamoIdentityWrapper, ) +from environments.models import Environment def test_get_identities_returns_bad_request_if_dynamo_is_not_enabled( @@ -267,7 +274,7 @@ def test_search_identities_without_exact_match( ) url = "%s?q=%s" % (base_url, identifier) - edge_identity_dynamo_wrapper_mock.search_items_with_identifier.return_value = { + edge_identity_dynamo_wrapper_mock.search_items.return_value = { "Items": [identity_document], "Count": 1, } @@ -279,12 +286,15 @@ def test_search_identities_without_exact_match( assert response.json()["results"][0]["identifier"] == identifier assert len(response.json()["results"]) == 1 - edge_identity_dynamo_wrapper_mock.search_items_with_identifier.assert_called_with( - environment_api_key, - identifier, - EdgeIdentityViewSet.dynamo_identifier_search_functions["BEGINS_WITH"], - 100, - None, + edge_identity_dynamo_wrapper_mock.search_items.assert_called_with( + environment_api_key=environment_api_key, + search_data=EdgeIdentitySearchData( + search_term=identifier, + search_type=EdgeIdentitySearchType.BEGINS_WITH, + search_attribute=IDENTIFIER_ATTRIBUTE, + ), + limit=100, + start_key=None, ) @@ -306,7 +316,7 @@ def test_search_for_identities_with_exact_match( base_url, urllib.parse.urlencode({"q": f'"{identifier}"'}), ) - edge_identity_dynamo_wrapper_mock.search_items_with_identifier.return_value = { + edge_identity_dynamo_wrapper_mock.search_items.return_value = { "Items": [identity_document], "Count": 1, } @@ -318,14 +328,91 @@ def test_search_for_identities_with_exact_match( assert response.json()["results"][0]["identifier"] == identifier assert len(response.json()["results"]) == 1 - edge_identity_dynamo_wrapper_mock.search_items_with_identifier.assert_called_with( - environment_api_key, - identifier, - EdgeIdentityViewSet.dynamo_identifier_search_functions["EQUAL"], - 100, - None, + edge_identity_dynamo_wrapper_mock.search_items.assert_called_with( + environment_api_key=environment_api_key, + search_data=EdgeIdentitySearchData( + search_term=identifier, + search_type=EdgeIdentitySearchType.EQUAL, + search_attribute=IDENTIFIER_ATTRIBUTE, + ), + limit=100, + start_key=None, + ) + + +def test_search_for_identities_by_dashboard_alias( + admin_client: APIClient, + dynamo_enabled_environment: Environment, + environment_api_key: str, + identity_document: dict[str, Any], + flagsmith_identities_table: Table, +) -> None: + # Given + identifier = identity_document["identifier"] + dashboard_alias = identity_document["dashboard_alias"] + + flagsmith_identities_table.put_item(Item=identity_document) + + base_url = reverse( + "api-v1:environments:environment-edge-identities-list", + args=[environment_api_key], + ) + url = "%s?%s" % ( + base_url, + urllib.parse.urlencode( + {"q": f'{DASHBOARD_ALIAS_SEARCH_PREFIX}"{dashboard_alias}"'} + ), + ) + + # When + response = admin_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["results"][0]["identifier"] == identifier + assert len(response.json()["results"]) == 1 + + +def test_update_edge_identity( + admin_client: APIClient, + dynamo_enabled_environment: Environment, + environment_api_key: str, + identity_document: dict[str, Any], + flagsmith_identities_table: Table, +) -> None: + # Given + identity_uuid = identity_document["identity_uuid"] + composite_key = identity_document["composite_key"] + + dashboard_alias = "new-dashboard-alias" + + flagsmith_identities_table.put_item(Item=identity_document) + + url = reverse( + "api-v1:environments:environment-edge-identities-detail", + args=[environment_api_key, identity_uuid], ) + # When + response = admin_client.patch(url, data={"dashboard_alias": dashboard_alias}) + + # Then + assert response.status_code == status.HTTP_200_OK + + assert response.json() == { + "dashboard_alias": dashboard_alias, + "identifier": identity_document["identifier"], + "identity_uuid": identity_uuid, + } + + # Let's also validate that the fields are the same between the original document, + # and from the updated database, apart from the dashboard_alias which should have + # been updated as expected. + identity_from_db = flagsmith_identities_table.get_item( + Key={"composite_key": composite_key} + )["Item"] + assert {**identity_document, "dashboard_alias": dashboard_alias} == identity_from_db + def test_edge_identities_traits_list( admin_client, diff --git a/api/tests/unit/edge_api/identities/conftest.py b/api/tests/unit/edge_api/identities/conftest.py index ab0aed1bdc83..3b9b8bb5a35b 100644 --- a/api/tests/unit/edge_api/identities/conftest.py +++ b/api/tests/unit/edge_api/identities/conftest.py @@ -41,7 +41,7 @@ def identity_document_without_fs(environment): "environment_api_key": environment.api_key, "identity_uuid": "59efa2a7-6a45-46d6-b953-a7073a90eacf", "django_id": None, - "dashboard_alias": None, + "dashboard_alias": "dashboard-alias", } diff --git a/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py b/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py index 3beee0c1c671..a555aec1b0e9 100644 --- a/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py +++ b/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py @@ -11,6 +11,11 @@ from pytest_mock import MockerFixture from rest_framework.exceptions import NotFound +from edge_api.identities.search import ( + IDENTIFIER_ATTRIBUTE, + EdgeIdentitySearchData, + EdgeIdentitySearchType, +) from environments.dynamodb import DynamoIdentityWrapper from environments.dynamodb.wrappers.exceptions import CapacityBudgetExceeded from environments.identities.models import Identity @@ -189,8 +194,15 @@ def test_search_items_with_identifier_calls_query_with_correct_arguments(mocker) search_function = lambda x: Key("identifier").eq(x) # noqa: E731 # When - dynamo_identity_wrapper.search_items_with_identifier( - environment_key, identifier, search_function, 999, start_key + dynamo_identity_wrapper.search_items( + environment_key, + EdgeIdentitySearchData( + search_term=identifier, + search_type=EdgeIdentitySearchType.EQUAL, + search_attribute=IDENTIFIER_ATTRIBUTE, + ), + 999, + start_key, ) # Then diff --git a/api/tests/unit/util/mappers/test_unit_mappers_dynamodb.py b/api/tests/unit/util/mappers/test_unit_mappers_dynamodb.py index 27c75ed91cdc..4e440174ad06 100644 --- a/api/tests/unit/util/mappers/test_unit_mappers_dynamodb.py +++ b/api/tests/unit/util/mappers/test_unit_mappers_dynamodb.py @@ -131,7 +131,6 @@ def test_map_identity_to_identity_document__call_expected( "identity_features": [], "identity_traits": [{"trait_key": "key1", "trait_value": "value1"}], "identity_uuid": mocker.ANY, - "dashboard_alias": None, } assert uuid.UUID(result["identity_uuid"]) diff --git a/api/tests/unit/util/mappers/test_unit_mappers_sdk.py b/api/tests/unit/util/mappers/test_unit_mappers_sdk.py index ce849a4485cc..e1a4b4f82bdb 100644 --- a/api/tests/unit/util/mappers/test_unit_mappers_sdk.py +++ b/api/tests/unit/util/mappers/test_unit_mappers_sdk.py @@ -83,6 +83,7 @@ def test_map_environment_to_sdk_document__return_expected( ], "identity_traits": [], "identity_uuid": mocker.ANY, + "dashboard_alias": None, } ], "hide_disabled_flags": None, diff --git a/api/util/mappers/dynamodb.py b/api/util/mappers/dynamodb.py index b4da6bcdb903..79ad3b2aea29 100644 --- a/api/util/mappers/dynamodb.py +++ b/api/util/mappers/dynamodb.py @@ -40,6 +40,8 @@ DocumentValue: TypeAlias = Union[Dict[str, "DocumentValue"], str, bool, None, Decimal] Document: TypeAlias = Dict[str, DocumentValue] +_NULLABLE_IDENTITY_KEY_ATTRIBUTES = {"dashboard_alias"} + def map_environment_to_environment_document( environment: "Environment", @@ -81,6 +83,7 @@ def map_engine_identity_to_identity_document( response = { field_name: _map_value_to_document_value(value) for field_name, value in engine_identity + if (value is not None or field_name not in _NULLABLE_IDENTITY_KEY_ATTRIBUTES) } response["composite_key"] = engine_identity.composite_key return response diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 487c55ada915..b26f76c33d97 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -292,6 +292,7 @@ export type Identity = { id?: string identifier: string identity_uuid?: string + dashboard_alias?: string } export type AvailablePermission = { diff --git a/frontend/web/components/pages/UsersPage.tsx b/frontend/web/components/pages/UsersPage.tsx index 6096f6f37908..8bf1cda3c76b 100644 --- a/frontend/web/components/pages/UsersPage.tsx +++ b/frontend/web/components/pages/UsersPage.tsx @@ -60,7 +60,7 @@ const UsersPage: FC = (props) => { pageType: page.pageType, page_size: 10, pages: page.pages, - search, + q: search, }) const { environmentId } = props.match.params @@ -152,7 +152,7 @@ const UsersPage: FC = (props) => { renderFooter={() => ( )} @@ -193,7 +193,7 @@ const UsersPage: FC = (props) => { }) }} renderRow={( - { id, identifier, identity_uuid }: Identity, + { dashboard_alias, id, identifier, identity_uuid }: Identity, index: number, ) => permission ? ( @@ -211,6 +211,7 @@ const UsersPage: FC = (props) => { >
+ {dashboard_alias ? ` (alias: ${dashboard_alias})` : ''}