Skip to content

Commit

Permalink
fix(edge-api/tasks): Add change_by_user_id (#4591)
Browse files Browse the repository at this point in the history
  • Loading branch information
gagantrivedi authored Sep 5, 2024
1 parent a891114 commit 940a56d
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 7 deletions.
6 changes: 5 additions & 1 deletion api/edge_api/identities/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": (
Expand Down
6 changes: 5 additions & 1 deletion api/edge_api/identities/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion api/tests/integration/edge_api/identities/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from environments.dynamodb.wrappers.environment_wrapper import (
DynamoEnvironmentV2Wrapper,
)
from users.models import FFAdminUser


@pytest.fixture()
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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",
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down

0 comments on commit 940a56d

Please sign in to comment.