diff --git a/api/edge_api/identities/models.py b/api/edge_api/identities/models.py index aad70198f88d..e332eb7c38c2 100644 --- a/api/edge_api/identities/models.py +++ b/api/edge_api/identities/models.py @@ -203,7 +203,11 @@ def _update_feature_overrides( kwargs = { "environment_api_key": self.environment_api_key, "identifier": self.identifier, - "user_id": getattr(user, "id", None), + "user_id": ( + user.id + if not getattr(user, "is_master_api_key_user", False) + else None + ), "changes": changeset, "identity_uuid": str(self.identity_uuid), "master_api_key_id": ( diff --git a/api/edge_api/identities/tasks.py b/api/edge_api/identities/tasks.py index 9bd8cd510adf..832c2b232b57 100644 --- a/api/edge_api/identities/tasks.py +++ b/api/edge_api/identities/tasks.py @@ -11,6 +11,7 @@ from environments.dynamodb import DynamoEnvironmentV2Wrapper from environments.models import Environment, Webhook from features.models import Feature, FeatureState +from users.models import FFAdminUser from util.mappers import map_identity_changeset_to_identity_override_changeset from webhooks.webhooks import WebhookEventType, call_environment_webhooks @@ -23,8 +24,9 @@ def call_environment_webhook_for_feature_state_change( environment_api_key: str, identity_id: typing.Union[id, str], identity_identifier: str, - changed_by: str, timestamp: str, + changed_by_user_id: int = None, # deprecated(use changed_by) + changed_by: str = None, new_enabled_state: bool = None, new_value: typing.Union[bool, int, str] = None, previous_enabled_state: bool = None, @@ -39,6 +41,8 @@ def call_environment_webhook_for_feature_state_change( return feature = Feature.objects.get(id=feature_id) + if changed_by_user_id: + changed_by = FFAdminUser.objects.get(id=changed_by_user_id).email data = { "changed_by": changed_by, diff --git a/api/tests/integration/edge_api/identities/conftest.py b/api/tests/integration/edge_api/identities/conftest.py index 298bd6165ee6..3781cec6d76d 100644 --- a/api/tests/integration/edge_api/identities/conftest.py +++ b/api/tests/integration/edge_api/identities/conftest.py @@ -8,6 +8,7 @@ from environments.dynamodb.wrappers.environment_wrapper import ( DynamoEnvironmentV2Wrapper, ) +from users.models import FFAdminUser @pytest.fixture() @@ -23,13 +24,14 @@ def identity_overrides_v2( identity_document_without_fs: dict[str, Any], identity_document: dict[str, Any], dynamodb_wrapper_v2: DynamoEnvironmentV2Wrapper, + admin_user: FFAdminUser, ) -> list[str]: edge_identity = EdgeIdentity.from_identity_document(identity_document_without_fs) for feature_override in IdentityModel.model_validate( identity_document ).identity_features: edge_identity.add_feature_override(feature_override) - edge_identity.save() + edge_identity.save(admin_user) return [ item["document_key"] for item in dynamodb_wrapper_v2.query_get_all_items( 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 4b86aa448494..87d5052cfa6e 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 @@ -374,7 +374,7 @@ def test_search_for_identities_by_dashboard_alias( def test_update_edge_identity( - admin_client: APIClient, + admin_client_new: APIClient, dynamo_enabled_environment: Environment, environment_api_key: str, identity_document: dict[str, Any], @@ -394,7 +394,7 @@ def test_update_edge_identity( ) # When - response = admin_client.patch(url, data={"dashboard_alias": dashboard_alias}) + response = admin_client_new.patch(url, data={"dashboard_alias": dashboard_alias}) # Then assert response.status_code == status.HTTP_200_OK diff --git a/api/tests/unit/edge_api/identities/test_unit_edge_api_identities_tasks.py b/api/tests/unit/edge_api/identities/test_unit_edge_api_identities_tasks.py index 26d61d6da33b..253fa3b15997 100644 --- a/api/tests/unit/edge_api/identities/test_unit_edge_api_identities_tasks.py +++ b/api/tests/unit/edge_api/identities/test_unit_edge_api_identities_tasks.py @@ -16,7 +16,10 @@ IdentityOverridesV2Changeset, IdentityOverrideV2, ) +from environments.identities.models import Identity from environments.models import Environment, Webhook +from features.models import Feature +from users.models import FFAdminUser from webhooks.webhooks import WebhookEventType @@ -128,6 +131,54 @@ def test_call_environment_webhook_for_feature_state_change_with_previous_state_o assert data["timestamp"] == now_isoformat +def test_call_environment_webhook_for_feature_state_change_with_changed_by_user_id( + mocker: MockerFixture, + environment: Environment, + feature: Feature, + identity: Identity, + admin_user: FFAdminUser, +) -> None: + # Given + mock_call_environment_webhooks = mocker.patch( + "edge_api.identities.tasks.call_environment_webhooks" + ) + Webhook.objects.create(environment=environment, url="https://foo.com/webhook") + + mock_feature_state_data = mocker.MagicMock() + mock_generate_webhook_feature_state_data = mocker.patch.object( + Webhook, + "generate_webhook_feature_state_data", + return_value=mock_feature_state_data, + ) + + now_isoformat = timezone.now().isoformat() + previous_enabled_state = True + previous_value = "foo" + + # When + call_environment_webhook_for_feature_state_change( + feature_id=feature.id, + environment_api_key=environment.api_key, + identity_id=identity.id, + identity_identifier=identity.identifier, + changed_by_user_id=admin_user.id, + timestamp=now_isoformat, + previous_enabled_state=previous_enabled_state, + previous_value=previous_value, + ) + + # Then + mock_call_environment_webhooks.assert_called_once() + mock_generate_webhook_feature_state_data.assert_called_once_with( + feature=feature, + environment=environment, + identity_id=identity.id, + identity_identifier=identity.identifier, + enabled=previous_enabled_state, + value=previous_value, + ) + + @pytest.mark.parametrize( "previous_enabled_state, previous_value, new_enabled_state, new_value", ( diff --git a/api/tests/unit/features/test_unit_features_features_service.py b/api/tests/unit/features/test_unit_features_features_service.py index b37b88a820a6..f59f94dd8590 100644 --- a/api/tests/unit/features/test_unit_features_features_service.py +++ b/api/tests/unit/features/test_unit_features_features_service.py @@ -12,6 +12,7 @@ ) from features.models import Feature, FeatureSegment, FeatureState from projects.models import EdgeV2MigrationStatus +from users.models import FFAdminUser from util.mappers.engine import ( map_feature_state_to_engine, map_identity_to_engine, @@ -221,6 +222,7 @@ def test_feature_get_edge_overrides_data( distinct_identity_featurestate: FeatureState, dynamodb_identity_wrapper: "DynamoIdentityWrapper", dynamodb_wrapper_v2: "DynamoEnvironmentV2Wrapper", + admin_user: FFAdminUser, ) -> None: # Given # replicate identity to Edge @@ -231,7 +233,7 @@ def test_feature_get_edge_overrides_data( edge_identity.add_feature_override( map_feature_state_to_engine(distinct_identity_featurestate), ) - edge_identity.save() + edge_identity.save(admin_user) # When overrides_data = get_edge_overrides_data(environment) @@ -268,6 +270,7 @@ def test_get_edge_overrides_data_skips_deleted_features( distinct_identity_featurestate: FeatureState, dynamodb_identity_wrapper: "DynamoIdentityWrapper", dynamodb_wrapper_v2: "DynamoEnvironmentV2Wrapper", + admin_user: FFAdminUser, ): # Given # replicate identity to Edge @@ -279,7 +282,7 @@ def test_get_edge_overrides_data_skips_deleted_features( edge_identity.add_feature_override( map_feature_state_to_engine(distinct_identity_featurestate), ) - edge_identity.save() + edge_identity.save(admin_user) # Now, delete one of the feature feature.delete()