Skip to content

Commit e477377

Browse files
committed
fix the issue
1 parent 183ea49 commit e477377

4 files changed

Lines changed: 95 additions & 40 deletions

File tree

api/audit/signals.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from audit.serializers import AuditLogListSerializer
1111
from audit.services import get_audited_instance_from_audit_log_record
1212
from features.models import FeatureState, FeatureStateValue
13+
from features.multivariate.models import MultivariateFeatureStateValue
1314
from features.signals import feature_state_change_went_live
1415
from integrations.common.models import IntegrationsModel
1516
from integrations.datadog.datadog import DataDogWrapper
@@ -214,9 +215,9 @@ def send_audit_log_event_to_slack(sender, instance, **kwargs): # type: ignore[n
214215
def send_feature_flag_went_live_signal(sender, instance, **kwargs): # type: ignore[no-untyped-def]
215216
audited_instance = get_audited_instance_from_audit_log_record(instance)
216217

217-
# Handle both FeatureState and FeatureStateValue audit logs
218-
# FeatureStateValue changes also have related_object_type=FEATURE_STATE
219-
if isinstance(audited_instance, FeatureStateValue):
218+
# Handle FeatureState, FeatureStateValue, and MultivariateFeatureStateValue audit logs
219+
# All these types have related_object_type=FEATURE_STATE
220+
if isinstance(audited_instance, (FeatureStateValue, MultivariateFeatureStateValue)):
220221
feature_state = audited_instance.feature_state
221222
elif isinstance(audited_instance, FeatureState):
222223
feature_state = audited_instance

api/environments/models.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,8 @@ def generate_webhook_feature_state_data(
638638
identity_id: int | str | None = None,
639639
identity_identifier: str | None = None,
640640
feature_segment: FeatureSegment | None = None,
641+
multivariate_feature_state_values: list[MultivariateFeatureStateValue]
642+
| None = None,
641643
) -> dict: # type: ignore[type-arg]
642644
if (identity_id or identity_identifier) and not (
643645
identity_id and identity_identifier
@@ -647,8 +649,20 @@ def generate_webhook_feature_state_data(
647649
if (identity_id and identity_identifier) and feature_segment:
648650
raise ValueError("Cannot provide identity information and feature segment")
649651

652+
mv_values_data = [
653+
{
654+
"id": mv.id,
655+
"multivariate_feature_option": {
656+
"id": mv.multivariate_feature_option_id,
657+
"value": mv.multivariate_feature_option.value,
658+
},
659+
"percentage_allocation": mv.percentage_allocation,
660+
}
661+
for mv in (multivariate_feature_state_values or [])
662+
]
663+
650664
# TODO: refactor to use a serializer / schema
651-
data = {
665+
data: dict[str, typing.Any] = {
652666
"feature": {
653667
"id": feature.id,
654668
"created_date": feature.created_date.strftime("%Y-%m-%dT%H:%M:%S.%fZ"),
@@ -671,6 +685,7 @@ def generate_webhook_feature_state_data(
671685
"feature_segment": None,
672686
"enabled": enabled,
673687
"feature_state_value": value,
688+
"multivariate_feature_state_values": mv_values_data,
674689
}
675690
if feature_segment:
676691
data["feature_segment"] = {

api/features/tasks.py

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import logging
2+
from typing import Any
23

34
from task_processor.decorators import (
45
register_task_handler,
56
)
67

78
from environments.models import Webhook
89
from features.models import Feature, FeatureState
10+
from features.multivariate.models import MultivariateFeatureStateValue
911
from webhooks.constants import WEBHOOK_DATETIME_FORMAT
1012
from webhooks.tasks import (
1113
call_environment_webhooks,
@@ -41,7 +43,7 @@ def trigger_feature_state_change_webhooks( # type: ignore[no-untyped-def]
4143
new_state = (
4244
None
4345
if event_type == WebhookEventType.FLAG_DELETED
44-
else _get_feature_state_webhook_data(instance) # type: ignore[no-untyped-call]
46+
else _get_feature_state_webhook_data(instance)
4547
)
4648
data = {"new_state": new_state, "changed_by": changed_by, "timestamp": timestamp}
4749
previous_state = _get_previous_state(instance, history_instance, event_type)
@@ -68,33 +70,55 @@ def _get_previous_state(
6870
event_type: WebhookEventType,
6971
) -> dict: # type: ignore[type-arg]
7072
if event_type == WebhookEventType.FLAG_DELETED:
71-
return _get_feature_state_webhook_data(instance) # type: ignore[no-untyped-call,no-any-return]
73+
return _get_feature_state_webhook_data(instance)
7274
if history_instance and history_instance.prev_record:
73-
return _get_feature_state_webhook_data( # type: ignore[no-untyped-call,no-any-return]
75+
return _get_feature_state_webhook_data(
7476
history_instance.prev_record.instance, previous=True
7577
)
7678
return None # type: ignore[return-value]
7779

7880

79-
def _get_feature_state_webhook_data(feature_state, previous=False): # type: ignore[no-untyped-def]
80-
# TODO: fix circular imports and use serializers instead.
81-
feature_state_value = (
82-
feature_state.get_feature_state_value()
83-
if not previous
84-
else feature_state.previous_feature_state_value
85-
)
81+
def _get_feature_state_webhook_data(
82+
feature_state: FeatureState,
83+
previous: bool = False,
84+
) -> dict[str, Any]:
85+
if previous:
86+
value = feature_state.previous_feature_state_value
87+
mv_values = _get_previous_multivariate_values(feature_state)
88+
else:
89+
value = feature_state.get_feature_state_value()
90+
mv_values = list(feature_state.multivariate_feature_state_values.all())
8691

92+
assert feature_state.environment is not None
8793
return Webhook.generate_webhook_feature_state_data(
8894
feature_state.feature,
8995
environment=feature_state.environment,
9096
enabled=feature_state.enabled,
91-
value=feature_state_value,
97+
value=value,
9298
identity_id=feature_state.identity_id,
9399
identity_identifier=getattr(feature_state.identity, "identifier", None),
94100
feature_segment=feature_state.feature_segment,
101+
multivariate_feature_state_values=mv_values,
95102
)
96103

97104

105+
def _get_previous_multivariate_values(
106+
feature_state: FeatureState,
107+
) -> list[MultivariateFeatureStateValue]:
108+
"""Get previous multivariate values from history."""
109+
mv_values: list[MultivariateFeatureStateValue] = []
110+
for mv in MultivariateFeatureStateValue.objects.filter(
111+
feature_state_id=feature_state.id
112+
):
113+
history = mv.history.first()
114+
if history and history.prev_record:
115+
mv_values.append(history.prev_record.instance)
116+
else:
117+
# No previous record, use current value
118+
mv_values.append(mv)
119+
return mv_values
120+
121+
98122
@register_task_handler()
99123
def delete_feature(feature_id: int) -> None:
100124
Feature.objects.get(pk=feature_id).delete()

api/tests/integration/features/featurestate/test_webhooks.py

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import responses
55
from django.urls import reverse
66
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
7+
from pytest_mock import MockerFixture
78
from rest_framework import status
89
from rest_framework.test import APIClient
910

@@ -133,7 +134,9 @@ def test_update_multivariate_percentage__webhook_payload_includes_multivariate_v
133134
environment: int,
134135
feature: int,
135136
mv_option_50_percent: int,
137+
mv_option_value: str,
136138
webhook: str,
139+
mocker: MockerFixture,
137140
) -> None:
138141
"""
139142
Test for issue #6190: Webhook payloads do not include multivariate values.
@@ -143,8 +146,6 @@ def test_update_multivariate_percentage__webhook_payload_includes_multivariate_v
143146
with their percentage allocations.
144147
"""
145148
# Given
146-
responses.add(responses.POST, webhook, status=200)
147-
148149
# Get the feature state for this environment
149150
feature_states_url = reverse("api-v1:features:featurestates-list")
150151
feature_states_response = admin_client.get(
@@ -160,7 +161,10 @@ def test_update_multivariate_percentage__webhook_payload_includes_multivariate_v
160161
old_percentage = mv_fs_value["percentage_allocation"]
161162
new_percentage = 75
162163

163-
# When - update only the multivariate percentage allocation
164+
responses.add(responses.POST, webhook, status=200)
165+
166+
# When
167+
# update only the multivariate percentage allocation
164168
url = reverse("api-v1:features:featurestates-detail", args=[feature_state_id])
165169
data = {
166170
"id": feature_state_id,
@@ -178,28 +182,39 @@ def test_update_multivariate_percentage__webhook_payload_includes_multivariate_v
178182
}
179183
],
180184
}
181-
response = admin_client.put(
182-
url, data=json.dumps(data), content_type="application/json"
183-
)
185+
admin_client.put(url, data=json.dumps(data), content_type="application/json")
184186

185187
# Then
186-
assert response.status_code == status.HTTP_200_OK
187-
188-
# Verify webhook was called
189-
assert len(responses.calls) >= 1
190-
webhook_payload = json.loads(responses.calls[0].request.body)["data"] # type: ignore[union-attr]
191-
192-
# Verify the payload includes multivariate values
193-
# This currently fails - issue #6190
194-
assert "multivariate_feature_state_values" in webhook_payload["new_state"]
195-
assert "multivariate_feature_state_values" in webhook_payload["previous_state"]
196-
197-
new_mv_values = webhook_payload["new_state"]["multivariate_feature_state_values"]
198-
previous_mv_values = webhook_payload["previous_state"][
199-
"multivariate_feature_state_values"
188+
# `FLAG_UPDATED`` webhook was called
189+
# (should be the last sent event)
190+
last_call = responses.calls[-1]
191+
assert not isinstance(last_call, list)
192+
webhook_payload = json.loads(last_call.request.body)
193+
assert webhook_payload["event_type"] == "FLAG_UPDATED"
194+
195+
# the payload includes multivariate values
196+
event_data = webhook_payload["data"]
197+
198+
assert "multivariate_feature_state_values" in event_data["new_state"]
199+
assert "multivariate_feature_state_values" in event_data["previous_state"]
200+
201+
assert event_data["new_state"]["multivariate_feature_state_values"] == [
202+
{
203+
"id": mocker.ANY,
204+
"multivariate_feature_option": {
205+
"id": mv_option_50_percent,
206+
"value": mv_option_value,
207+
},
208+
"percentage_allocation": new_percentage,
209+
},
210+
]
211+
assert event_data["previous_state"]["multivariate_feature_state_values"] == [
212+
{
213+
"id": mocker.ANY,
214+
"multivariate_feature_option": {
215+
"id": mv_option_50_percent,
216+
"value": mv_option_value,
217+
},
218+
"percentage_allocation": old_percentage,
219+
},
200220
]
201-
202-
assert len(new_mv_values) == 1
203-
assert len(previous_mv_values) == 1
204-
assert new_mv_values[0]["percentage_allocation"] == new_percentage
205-
assert previous_mv_values[0]["percentage_allocation"] == old_percentage

0 commit comments

Comments
 (0)