Skip to content

Commit 196ebf3

Browse files
authored
fix: Flagsmith Environment Webhook incorrect payload values (#6646)
1 parent ee321ac commit 196ebf3

7 files changed

Lines changed: 442 additions & 37 deletions

File tree

api/audit/signals.py

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from audit.related_object_type import RelatedObjectType
1010
from audit.serializers import AuditLogListSerializer
1111
from audit.services import get_audited_instance_from_audit_log_record
12-
from features.models import FeatureState
12+
from features.models import FeatureState, FeatureStateValue
1313
from features.signals import feature_state_change_went_live
1414
from integrations.common.models import IntegrationsModel
1515
from integrations.datadog.datadog import DataDogWrapper
@@ -212,21 +212,30 @@ def send_audit_log_event_to_slack(sender, instance, **kwargs): # type: ignore[n
212212
@receiver(post_save, sender=AuditLog)
213213
@track_only([RelatedObjectType.FEATURE_STATE])
214214
def send_feature_flag_went_live_signal(sender, instance, **kwargs): # type: ignore[no-untyped-def]
215-
feature_state = get_audited_instance_from_audit_log_record(instance)
216-
if not isinstance(feature_state, FeatureState):
215+
audited_instance = get_audited_instance_from_audit_log_record(instance)
216+
217+
# Handle both FeatureState and FeatureStateValue audit logs
218+
# FeatureStateValue changes also have related_object_type=FEATURE_STATE
219+
if isinstance(audited_instance, FeatureStateValue):
220+
feature_state = audited_instance.feature_state
221+
elif isinstance(audited_instance, FeatureState):
222+
feature_state = audited_instance
223+
else:
217224
return
218225

219226
if feature_state.is_scheduled:
220227
return # This is handled by audit.tasks.create_feature_state_went_live_audit_log
221228

222-
feature_state_change_went_live.send(instance)
229+
feature_state_change_went_live.send(feature_state, audit_log=instance)
223230

224231

225232
@receiver(feature_state_change_went_live)
226-
def send_audit_log_event_to_sentry(sender: AuditLog, **kwargs: Any) -> None:
233+
def send_audit_log_event_to_sentry(
234+
sender: FeatureState, audit_log: AuditLog, **kwargs: Any
235+
) -> None:
227236
try:
228237
sentry_configuration = SentryChangeTrackingConfiguration.objects.get(
229-
environment=sender.environment,
238+
environment=audit_log.environment,
230239
deleted_at__isnull=True,
231240
)
232241
except SentryChangeTrackingConfiguration.DoesNotExist:
@@ -237,4 +246,35 @@ def send_audit_log_event_to_sentry(sender: AuditLog, **kwargs: Any) -> None:
237246
secret=sentry_configuration.secret,
238247
)
239248

240-
_track_event_async(sender, sentry_change_tracking) # type: ignore[no-untyped-call]
249+
_track_event_async(audit_log, sentry_change_tracking) # type: ignore[no-untyped-call]
250+
251+
252+
@receiver(feature_state_change_went_live)
253+
def trigger_feature_state_change_webhooks(
254+
sender: FeatureState,
255+
**kwargs: Any,
256+
) -> None:
257+
"""
258+
Trigger FLAG_UPDATED webhooks when a feature state change goes live.
259+
260+
Triggered from AuditLog post_save. Fetches a fresh feature state from the
261+
database to ensure we get the latest data (including FeatureStateValue),
262+
since drf-writable-nested saves the parent before nested objects.
263+
"""
264+
from features import tasks
265+
266+
# Fetch fresh data from the database to ensure we have the latest state
267+
# This is necessary because:
268+
# 1. drf-writable-nested saves FeatureState before FeatureStateValue
269+
# 2. The history record's instance may have stale cached values
270+
try:
271+
fresh_feature_state = FeatureState.objects.get(id=sender.id)
272+
except FeatureState.DoesNotExist:
273+
# Skip deleted feature states - handled in views
274+
return
275+
276+
# Skip versioned environments - handled by trigger_update_version_webhooks
277+
if fresh_feature_state.environment_feature_version_id:
278+
return
279+
280+
tasks.trigger_feature_state_change_webhooks(fresh_feature_state)

api/features/signals.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,7 @@
11
import logging
22

3-
from django.db.models.signals import post_save
4-
from django.dispatch import Signal, receiver
5-
6-
# noinspection PyUnresolvedReferences
7-
from .models import FeatureState
8-
from .tasks import trigger_feature_state_change_webhooks
3+
from django.dispatch import Signal
94

105
logger = logging.getLogger(__name__)
116

127
feature_state_change_went_live = Signal()
13-
14-
15-
@receiver(post_save, sender=FeatureState)
16-
def trigger_feature_state_change_webhooks_signal(instance, **kwargs): # type: ignore[no-untyped-def]
17-
if instance.environment_feature_version_id or instance.deleted_at:
18-
return
19-
trigger_feature_state_change_webhooks(instance)
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import json
2+
3+
from django.urls import reverse
4+
from pytest_mock import MockerFixture
5+
from rest_framework import status
6+
from rest_framework.test import APIClient
7+
8+
9+
def test_update_segment_override__webhook_payload_has_correct_previous_and_new_values(
10+
admin_client: APIClient,
11+
environment: int,
12+
feature: int,
13+
segment: int,
14+
mocker: MockerFixture,
15+
) -> None:
16+
"""
17+
Test for issue #6050: Webhook payload shows incorrect previous_state values
18+
when updating a segment override value via the API.
19+
20+
The bug occurs because:
21+
1. The webhook signal fires on FeatureState.post_save
22+
2. drf-writable-nested saves the parent (FeatureState) before nested (FeatureStateValue)
23+
3. So the webhook captures stale values
24+
"""
25+
# Given
26+
old_value = 0
27+
new_value = 1
28+
29+
# First create a feature_segment
30+
feature_segment_url = reverse("api-v1:features:feature-segment-list")
31+
feature_segment_data = {
32+
"feature": feature,
33+
"segment": segment,
34+
"environment": environment,
35+
}
36+
feature_segment_response = admin_client.post(
37+
feature_segment_url,
38+
data=json.dumps(feature_segment_data),
39+
content_type="application/json",
40+
)
41+
assert feature_segment_response.status_code == status.HTTP_201_CREATED
42+
feature_segment_id = feature_segment_response.json()["id"]
43+
44+
# Create segment override with initial value via API
45+
create_url = reverse("api-v1:features:featurestates-list")
46+
create_data = {
47+
"enabled": False,
48+
"feature_state_value": {"type": "int", "integer_value": old_value},
49+
"environment": environment,
50+
"feature": feature,
51+
"feature_segment": feature_segment_id,
52+
}
53+
create_response = admin_client.post(
54+
create_url, data=json.dumps(create_data), content_type="application/json"
55+
)
56+
assert create_response.status_code == status.HTTP_201_CREATED
57+
segment_override_id = create_response.json()["id"]
58+
59+
# Mock call_environment_webhooks to capture the actual payload
60+
mock_call_environment_webhooks = mocker.patch(
61+
"features.tasks.call_environment_webhooks"
62+
)
63+
mocker.patch("features.tasks.call_organisation_webhooks")
64+
65+
# When - update the segment override via API
66+
url = reverse("api-v1:features:featurestates-detail", args=[segment_override_id])
67+
data = {
68+
"enabled": False,
69+
"feature_state_value": {"type": "int", "integer_value": new_value},
70+
"environment": environment,
71+
"feature": feature,
72+
"feature_segment": feature_segment_id,
73+
}
74+
response = admin_client.put(
75+
url, data=json.dumps(data), content_type="application/json"
76+
)
77+
78+
# Then
79+
assert response.status_code == status.HTTP_200_OK
80+
81+
# Verify webhook was called
82+
mock_call_environment_webhooks.delay.assert_called_once()
83+
webhook_args = mock_call_environment_webhooks.delay.call_args.kwargs["args"]
84+
webhook_payload = webhook_args[1] # (environment_id, data, event_type)
85+
86+
# Verify the payload has correct values
87+
assert webhook_payload["new_state"]["feature_segment"] is not None
88+
assert webhook_payload["new_state"]["feature_state_value"] == new_value
89+
assert webhook_payload["previous_state"]["feature_state_value"] == old_value

0 commit comments

Comments
 (0)