Skip to content
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
7 changes: 4 additions & 3 deletions api/audit/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from audit.serializers import AuditLogListSerializer
from audit.services import get_audited_instance_from_audit_log_record
from features.models import FeatureState, FeatureStateValue
from features.multivariate.models import MultivariateFeatureStateValue
from features.signals import feature_state_change_went_live
from integrations.common.models import IntegrationsModel
from integrations.datadog.datadog import DataDogWrapper
Expand Down Expand Up @@ -214,9 +215,9 @@ def send_audit_log_event_to_slack(sender, instance, **kwargs): # type: ignore[n
def send_feature_flag_went_live_signal(sender, instance, **kwargs): # type: ignore[no-untyped-def]
audited_instance = get_audited_instance_from_audit_log_record(instance)

# Handle both FeatureState and FeatureStateValue audit logs
# FeatureStateValue changes also have related_object_type=FEATURE_STATE
if isinstance(audited_instance, FeatureStateValue):
# Handle FeatureState, FeatureStateValue, and MultivariateFeatureStateValue audit logs
# All these types have related_object_type=FEATURE_STATE
if isinstance(audited_instance, (FeatureStateValue, MultivariateFeatureStateValue)):
feature_state = audited_instance.feature_state
elif isinstance(audited_instance, FeatureState):
feature_state = audited_instance
Expand Down
17 changes: 16 additions & 1 deletion api/environments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,8 @@ def generate_webhook_feature_state_data(
identity_id: int | str | None = None,
identity_identifier: str | None = None,
feature_segment: FeatureSegment | None = None,
multivariate_feature_state_values: list[MultivariateFeatureStateValue]
| None = None,
) -> dict: # type: ignore[type-arg]
if (identity_id or identity_identifier) and not (
identity_id and identity_identifier
Expand All @@ -647,8 +649,20 @@ def generate_webhook_feature_state_data(
if (identity_id and identity_identifier) and feature_segment:
raise ValueError("Cannot provide identity information and feature segment")

mv_values_data = [
{
"id": mv.id,
"multivariate_feature_option": {
"id": mv.multivariate_feature_option_id,
"value": mv.multivariate_feature_option.value,
},
"percentage_allocation": mv.percentage_allocation,
}
for mv in (multivariate_feature_state_values or [])
]

# TODO: refactor to use a serializer / schema
data = {
data: dict[str, typing.Any] = {
"feature": {
"id": feature.id,
"created_date": feature.created_date.strftime("%Y-%m-%dT%H:%M:%S.%fZ"),
Expand All @@ -671,6 +685,7 @@ def generate_webhook_feature_state_data(
"feature_segment": None,
"enabled": enabled,
"feature_state_value": value,
"multivariate_feature_state_values": mv_values_data,
}
if feature_segment:
data["feature_segment"] = {
Expand Down
47 changes: 36 additions & 11 deletions api/features/tasks.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import logging
from typing import Any

from task_processor.decorators import (
register_task_handler,
)

from environments.models import Webhook
from features.models import Feature, FeatureState
from features.multivariate.models import MultivariateFeatureStateValue
from webhooks.constants import WEBHOOK_DATETIME_FORMAT
from webhooks.tasks import (
call_environment_webhooks,
Expand Down Expand Up @@ -41,7 +43,7 @@ def trigger_feature_state_change_webhooks( # type: ignore[no-untyped-def]
new_state = (
None
if event_type == WebhookEventType.FLAG_DELETED
else _get_feature_state_webhook_data(instance) # type: ignore[no-untyped-call]
else _get_feature_state_webhook_data(instance)
)
data = {"new_state": new_state, "changed_by": changed_by, "timestamp": timestamp}
previous_state = _get_previous_state(instance, history_instance, event_type)
Expand All @@ -68,33 +70,56 @@ def _get_previous_state(
event_type: WebhookEventType,
) -> dict: # type: ignore[type-arg]
if event_type == WebhookEventType.FLAG_DELETED:
return _get_feature_state_webhook_data(instance) # type: ignore[no-untyped-call,no-any-return]
return _get_feature_state_webhook_data(instance)
if history_instance and history_instance.prev_record:
return _get_feature_state_webhook_data( # type: ignore[no-untyped-call,no-any-return]
return _get_feature_state_webhook_data(
history_instance.prev_record.instance, previous=True
)
return None # type: ignore[return-value]


def _get_feature_state_webhook_data(feature_state, previous=False): # type: ignore[no-untyped-def]
# TODO: fix circular imports and use serializers instead.
feature_state_value = (
feature_state.get_feature_state_value()
if not previous
else feature_state.previous_feature_state_value
)
def _get_feature_state_webhook_data(
feature_state: FeatureState,
previous: bool = False,
) -> dict[str, Any]:
if previous:
value = feature_state.previous_feature_state_value
mv_values = _get_previous_multivariate_values(feature_state)
else:
value = feature_state.get_feature_state_value()
mv_values = list(
feature_state.multivariate_feature_state_values.select_related(
"multivariate_feature_option"
).all()
)

assert feature_state.environment is not None
return Webhook.generate_webhook_feature_state_data(
feature_state.feature,
environment=feature_state.environment,
enabled=feature_state.enabled,
value=feature_state_value,
value=value,
identity_id=feature_state.identity_id,
identity_identifier=getattr(feature_state.identity, "identifier", None),
feature_segment=feature_state.feature_segment,
multivariate_feature_state_values=mv_values,
)


def _get_previous_multivariate_values(
feature_state: FeatureState,
) -> list[MultivariateFeatureStateValue]:
"""Get previous multivariate values from history."""
mv_values: list[MultivariateFeatureStateValue] = []
for mv in MultivariateFeatureStateValue.objects.filter(
feature_state_id=feature_state.id
).select_related("multivariate_feature_option"):
history = mv.history.first()
if history and history.prev_record:
mv_values.append(history.prev_record.instance)
return mv_values


@register_task_handler()
def delete_feature(feature_id: int) -> None:
Feature.objects.get(pk=feature_id).delete()
151 changes: 141 additions & 10 deletions api/tests/integration/features/featurestate/test_webhooks.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,54 @@
import json

import pytest
import responses
from django.urls import reverse
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
from pytest_mock import MockerFixture
from rest_framework import status
from rest_framework.test import APIClient

WEBHOOK_URL = "https://example.com/webhook"


@pytest.fixture
def environment_webhook(
admin_client: APIClient,
environment: int,
environment_api_key: str,
) -> str:
"""Create an environment webhook via API and return its URL."""
url = reverse(
"api-v1:environments:environment-webhooks-list",
args=[environment_api_key],
)
response = admin_client.post(url, data={"url": WEBHOOK_URL, "enabled": True})
assert response.status_code == status.HTTP_201_CREATED
return WEBHOOK_URL


@pytest.fixture
def organisation_webhook(
admin_client: APIClient,
organisation: int,
) -> str:
"""Create an organisation webhook via API and return its URL."""
url = reverse(
"api-v1:organisations:organisation-webhooks-list",
args=[organisation],
)
response = admin_client.post(url, data={"url": WEBHOOK_URL, "enabled": True})
assert response.status_code == status.HTTP_201_CREATED
return WEBHOOK_URL


@responses.activate
def test_update_segment_override__webhook_payload_has_correct_previous_and_new_values(
admin_client: APIClient,
environment: int,
feature: int,
segment: int,
mocker: MockerFixture,
environment_webhook: str,
) -> None:
"""
Test for issue #6050: Webhook payload shows incorrect previous_state values
Expand All @@ -26,6 +63,8 @@ def test_update_segment_override__webhook_payload_has_correct_previous_and_new_v
old_value = 0
new_value = 1

responses.add(responses.POST, environment_webhook, status=200)

# First create a feature_segment
feature_segment_url = reverse("api-v1:features:feature-segment-list")
feature_segment_data = {
Expand Down Expand Up @@ -56,11 +95,8 @@ def test_update_segment_override__webhook_payload_has_correct_previous_and_new_v
assert create_response.status_code == status.HTTP_201_CREATED
segment_override_id = create_response.json()["id"]

# Mock call_environment_webhooks to capture the actual payload
mock_call_environment_webhooks = mocker.patch(
"features.tasks.call_environment_webhooks"
)
mocker.patch("features.tasks.call_organisation_webhooks")
# Clear responses from create operation
responses.calls.reset()

# When - update the segment override via API
url = reverse("api-v1:features:featurestates-detail", args=[segment_override_id])
Expand All @@ -78,12 +114,107 @@ def test_update_segment_override__webhook_payload_has_correct_previous_and_new_v
# Then
assert response.status_code == status.HTTP_200_OK

# Verify webhook was called
mock_call_environment_webhooks.delay.assert_called_once()
webhook_args = mock_call_environment_webhooks.delay.call_args.kwargs["args"]
webhook_payload = webhook_args[1] # (environment_id, data, event_type)
# Verify webhook was called with correct payload
assert len(responses.calls) == 1
webhook_payload = json.loads(responses.calls[0].request.body)["data"] # type: ignore[union-attr]

# Verify the payload has correct values
assert webhook_payload["new_state"]["feature_segment"] is not None
assert webhook_payload["new_state"]["feature_state_value"] == new_value
assert webhook_payload["previous_state"]["feature_state_value"] == old_value


@pytest.mark.parametrize(
"webhook",
[lazy_fixture("environment_webhook"), lazy_fixture("organisation_webhook")],
)
@responses.activate
def test_update_multivariate_percentage__webhook_payload_includes_multivariate_values(
admin_client: APIClient,
environment: int,
feature: int,
mv_option_50_percent: int,
mv_option_value: str,
webhook: str,
mocker: MockerFixture,
) -> None:
"""
Test for issue #6190: Webhook payloads do not include multivariate values.

When updating the percentage allocation of a multivariate option,
the webhook payload should include the multivariate_feature_state_values
with their percentage allocations.
"""
# Given
# Get the feature state for this environment
feature_states_url = reverse("api-v1:features:featurestates-list")
feature_states_response = admin_client.get(
f"{feature_states_url}?environment={environment}&feature={feature}"
)
assert feature_states_response.status_code == status.HTTP_200_OK
feature_state = feature_states_response.json()["results"][0]
feature_state_id = feature_state["id"]

# Get current multivariate feature state values
assert len(feature_state["multivariate_feature_state_values"]) == 1
mv_fs_value = feature_state["multivariate_feature_state_values"][0]
old_percentage = mv_fs_value["percentage_allocation"]
new_percentage = 75

responses.add(responses.POST, webhook, status=200)

# When
# update only the multivariate percentage allocation
url = reverse("api-v1:features:featurestates-detail", args=[feature_state_id])
data = {
"id": feature_state_id,
"enabled": feature_state["enabled"],
"feature_state_value": feature_state["feature_state_value"],
"environment": environment,
"feature": feature,
"multivariate_feature_state_values": [
{
"id": mv_fs_value["id"],
"multivariate_feature_option": mv_fs_value[
"multivariate_feature_option"
],
"percentage_allocation": new_percentage,
}
],
}
admin_client.put(url, data=json.dumps(data), content_type="application/json")

# Then
# `FLAG_UPDATED`` webhook was called
# (should be the last sent event)
last_call = responses.calls[-1]
assert not isinstance(last_call, list)
webhook_payload = json.loads(last_call.request.body)
assert webhook_payload["event_type"] == "FLAG_UPDATED"

# the payload includes multivariate values
event_data = webhook_payload["data"]

assert "multivariate_feature_state_values" in event_data["new_state"]
assert "multivariate_feature_state_values" in event_data["previous_state"]

assert event_data["new_state"]["multivariate_feature_state_values"] == [
{
"id": mocker.ANY,
"multivariate_feature_option": {
"id": mv_option_50_percent,
"value": mv_option_value,
},
"percentage_allocation": new_percentage,
},
]
assert event_data["previous_state"]["multivariate_feature_state_values"] == [
{
"id": mocker.ANY,
"multivariate_feature_option": {
"id": mv_option_50_percent,
"value": mv_option_value,
},
"percentage_allocation": old_percentage,
},
]
19 changes: 19 additions & 0 deletions api/webhooks/webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
from core.signing import sign_payload
from environments.models import Environment, Webhook
from features.models import Feature
from features.multivariate.models import (
MultivariateFeatureOption,
MultivariateFeatureStateValue,
)
from organisations.models import OrganisationWebhook
from projects.models import ( # type: ignore[attr-defined]
Organisation,
Expand Down Expand Up @@ -328,6 +332,19 @@ def generate_environment_sample_webhook_data() -> dict[str, Any]:
),
)

mv_option = MultivariateFeatureOption(
id=1,
feature=feature,
default_percentage_allocation=50,
type="unicode",
string_value="variant_a",
)
mv_state_value = MultivariateFeatureStateValue(
id=1,
multivariate_feature_option=mv_option,
percentage_allocation=50,
)

data = {
"changed_by": "user@domain.com",
"timestamp": "2021-06-18T07:50:26.595298Z",
Expand All @@ -338,6 +355,7 @@ def generate_environment_sample_webhook_data() -> dict[str, Any]:
value="feature_state_value",
identity_id=1,
identity_identifier="test_identity",
multivariate_feature_state_values=[mv_state_value],
),
"previous_state": Webhook.generate_webhook_feature_state_data(
feature=feature,
Expand All @@ -346,6 +364,7 @@ def generate_environment_sample_webhook_data() -> dict[str, Any]:
value="old_feature_state_value",
identity_id=1,
identity_identifier="test_identity",
multivariate_feature_state_values=[mv_state_value],
),
}

Expand Down
Loading
Loading