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

fix(webhook/changed_by): Return name of the master api key #4602

Merged
merged 2 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@ def environment_api_key(environment):
)


@pytest.fixture()
def master_api_key_name() -> str:
return "test-key"


@pytest.fixture()
def admin_master_api_key(organisation: Organisation) -> typing.Tuple[MasterAPIKey, str]:
master_api_key, key = MasterAPIKey.objects.create_key(
Expand All @@ -646,13 +651,20 @@ def admin_master_api_key(organisation: Organisation) -> typing.Tuple[MasterAPIKe


@pytest.fixture()
def master_api_key(organisation: Organisation) -> typing.Tuple[MasterAPIKey, str]:
def master_api_key(
master_api_key_name: str, organisation: Organisation
) -> typing.Tuple[MasterAPIKey, str]:
master_api_key, key = MasterAPIKey.objects.create_key(
name="test_key", organisation=organisation, is_admin=False
name=master_api_key_name, organisation=organisation, is_admin=False
)
return master_api_key, key


@pytest.fixture()
def admin_user_email(admin_user: FFAdminUser) -> str:
return admin_user.email


@pytest.fixture
def master_api_key_object(
master_api_key: typing.Tuple[MasterAPIKey, str]
Expand Down
9 changes: 6 additions & 3 deletions api/features/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@ def trigger_feature_state_change_webhooks(
else ""
)
changed_by = (
str(history_instance.history_user)
history_instance.history_user.email
if history_instance and history_instance.history_user
else ""
else (
history_instance.master_api_key.name
if history_instance and history_instance.master_api_key
else ""
)
)

new_state = (
None
if event_type == WebhookEventType.FLAG_DELETED
Expand Down
27 changes: 26 additions & 1 deletion api/tests/unit/features/test_unit_features_tasks.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,35 @@
import pytest
from pytest_lazyfixture import lazy_fixture
from pytest_mock import MockerFixture

from api_keys.models import MasterAPIKey
from environments.models import Environment
from features.models import Feature, FeatureState
from features.tasks import trigger_feature_state_change_webhooks
from organisations.models import Organisation
from projects.models import Project
from users.models import FFAdminUser
from webhooks.webhooks import WebhookEventType


@pytest.mark.parametrize(
"user, api_key, changed_by",
[
(lazy_fixture("admin_user"), None, lazy_fixture("admin_user_email")),
(
None,
lazy_fixture("master_api_key_object"),
lazy_fixture("master_api_key_name"),
),
],
)
@pytest.mark.django_db
def test_trigger_feature_state_change_webhooks(mocker: MockerFixture):
def test_trigger_feature_state_change_webhooks(
mocker: MockerFixture,
user: FFAdminUser | None,
api_key: MasterAPIKey | None,
changed_by: str,
) -> None:
# Given
initial_value = "initial"
new_value = "new"
Expand All @@ -23,6 +42,11 @@ def test_trigger_feature_state_change_webhooks(mocker: MockerFixture):
)
feature_state = FeatureState.objects.get(feature=feature, environment=environment)

# Set user/master_api_key
mocked_historical_record = mocker.patch("core.signals.HistoricalRecords")
mocked_historical_record.thread.request.user.key = api_key
feature_state._history_user = user

# update the feature state value and save both objects to ensure that the history is updated
feature_state.feature_state_value.string_value = new_value
feature_state.feature_state_value.save()
Expand Down Expand Up @@ -56,6 +80,7 @@ def test_trigger_feature_state_change_webhooks(mocker: MockerFixture):
event_type = environment_webhook_call_args[2]
assert data["new_state"]["feature_state_value"] == new_value
assert data["previous_state"]["feature_state_value"] == initial_value
assert data["changed_by"] == changed_by
assert event_type == WebhookEventType.FLAG_UPDATED.value


Expand Down
2 changes: 1 addition & 1 deletion api/webhooks/sample_webhook_data.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
environment_webhook_data = {
"data": {
"changed_by": "John Doe",
"changed_by": "[email protected]",
"new_state": {
"enabled": True,
"environment": {"id": 1, "name": "Development"},
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/system-administration/webhooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Environment:
```json
{
"data": {
"changed_by": "Ben Rometsch",
"changed_by": "[email protected]"(or the name of the Organisation API Key),
"new_state": {
"enabled": true,
"environment": {
Expand Down
Loading