Skip to content

Commit 9094867

Browse files
authored
fix(Webhooks): Correct previous_state for scheduled feature updates (#6740)
1 parent 3b1db2d commit 9094867

2 files changed

Lines changed: 158 additions & 2 deletions

File tree

api/features/tasks.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,51 @@ def _get_previous_state(
6868
instance: FeatureState,
6969
history_instance: HistoricalFeatureState,
7070
event_type: WebhookEventType,
71-
) -> dict: # type: ignore[type-arg]
71+
) -> dict[str, Any] | None:
7272
if event_type == WebhookEventType.FLAG_DELETED:
7373
return _get_feature_state_webhook_data(instance)
74+
75+
# Change requests create a new FeatureState with its own history
76+
if instance.change_request_id is not None:
77+
previous_fs = _get_previous_feature_state_for_change_request(instance)
78+
if previous_fs:
79+
return _get_feature_state_webhook_data(previous_fs)
80+
return None
81+
7482
if history_instance and history_instance.prev_record:
7583
return _get_feature_state_webhook_data(
7684
history_instance.prev_record.instance, previous=True
7785
)
78-
return None # type: ignore[return-value]
86+
return None
87+
88+
89+
def _get_previous_feature_state_for_change_request(
90+
instance: FeatureState,
91+
) -> FeatureState | None:
92+
"""Find the previous live FeatureState for a change request (legacy versioning)."""
93+
result: FeatureState | None = (
94+
FeatureState.objects.exclude(
95+
change_request_id=instance.change_request_id,
96+
)
97+
.filter(
98+
environment_id=instance.environment_id,
99+
feature_id=instance.feature_id,
100+
feature_segment=instance.feature_segment,
101+
identity=instance.identity,
102+
version__isnull=False,
103+
live_from__lt=instance.live_from,
104+
)
105+
.order_by("-live_from")
106+
.select_related(
107+
"feature",
108+
"environment",
109+
"feature_state_value",
110+
"feature_segment",
111+
"identity",
112+
)
113+
.first()
114+
)
115+
return result
79116

80117

81118
def _get_feature_state_webhook_data(

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,37 @@
11
import json
2+
from collections.abc import Generator
3+
from typing import Any
24

35
import pytest
46
import responses
57
from django.urls import reverse
8+
from freezegun import freeze_time
69
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
710
from pytest_mock import MockerFixture
811
from rest_framework import status
912
from rest_framework.test import APIClient
1013

14+
from audit.tasks import create_feature_state_went_live_audit_log
15+
from features.models import FeatureState
16+
from features.workflows.core.models import ChangeRequest
17+
from users.models import FFAdminUser
18+
1119
WEBHOOK_URL = "https://example.com/webhook"
1220

1321

22+
def _extract_webhook_payloads(event_type: str | None) -> Generator[dict[str, Any]]:
23+
"""Extract webhook payloads from responses cache"""
24+
return (
25+
payload
26+
for payload in (
27+
json.loads(r.request.body)
28+
for r in responses.calls
29+
if r.request.url == WEBHOOK_URL
30+
)
31+
if event_type is None or payload["event_type"] == event_type
32+
)
33+
34+
1435
@pytest.fixture
1536
def environment_webhook(
1637
admin_client: APIClient,
@@ -218,3 +239,101 @@ def test_update_multivariate_percentage__webhook_payload_includes_multivariate_v
218239
"percentage_allocation": old_percentage,
219240
},
220241
]
242+
243+
244+
@pytest.mark.parametrize(
245+
"webhook",
246+
[lazy_fixture("environment_webhook"), lazy_fixture("organisation_webhook")],
247+
)
248+
@responses.activate
249+
def test_update_feature_live__legacy_versioning__webhook_payload_has_correct_previous_and_new_states(
250+
admin_client: APIClient,
251+
environment: int,
252+
feature: int,
253+
webhook: str,
254+
) -> None:
255+
# Given
256+
feature_state_json = admin_client.get(
257+
f"/api/v1/features/featurestates/?environment={environment}&feature={feature}"
258+
).json()["results"][0]
259+
feature_state_id = feature_state_json["id"]
260+
previous_state = feature_state_json["enabled"]
261+
previous_value = feature_state_json["feature_state_value"]["string_value"]
262+
263+
# When
264+
responses.add(responses.POST, webhook, status=200)
265+
response = admin_client.put(
266+
f"/api/v1/features/featurestates/{feature_state_id}/",
267+
data={
268+
**feature_state_json,
269+
"feature_state_value": {"string_value": "new_value"},
270+
"enabled": True,
271+
},
272+
format="json",
273+
)
274+
assert response.status_code == 200
275+
276+
# Then
277+
payload = list(_extract_webhook_payloads("FLAG_UPDATED"))[-1]
278+
assert payload["data"]["previous_state"]["enabled"] is previous_state
279+
assert payload["data"]["previous_state"]["feature_state_value"] == previous_value
280+
assert payload["data"]["new_state"]["enabled"] is True
281+
assert payload["data"]["new_state"]["feature_state_value"] == "new_value"
282+
283+
284+
@pytest.mark.parametrize(
285+
"webhook",
286+
[lazy_fixture("environment_webhook"), lazy_fixture("organisation_webhook")],
287+
)
288+
@responses.activate
289+
def test_update_feature_scheduled__legacy_versioning__webhook_payload_has_correct_previous_and_new_states(
290+
admin_user: FFAdminUser,
291+
environment: int,
292+
feature: int,
293+
webhook: str,
294+
) -> None:
295+
"""Covers https://github.com/Flagsmith/flagsmith/issues/2063"""
296+
# Given
297+
previous_feature_state = FeatureState.objects.get(feature_id=feature)
298+
previous_state = previous_feature_state.enabled
299+
previous_value = previous_feature_state.get_feature_state_value()
300+
301+
# Simulate scheduled update in the frontend
302+
with freeze_time("2048-02-29T02:00:00+0000"):
303+
# POST create-change-request
304+
change_request = ChangeRequest.objects.create(
305+
environment=previous_feature_state.environment,
306+
title="Scheduled update",
307+
user=admin_user,
308+
)
309+
new_feature_state = FeatureState.objects.create(
310+
environment=previous_feature_state.environment,
311+
feature=previous_feature_state.feature,
312+
change_request=change_request,
313+
enabled=True,
314+
live_from="2048-02-29T06:00:00+0000",
315+
version=None,
316+
)
317+
new_feature_state.feature_state_value.string_value = "new_value"
318+
new_feature_state.feature_state_value.save()
319+
320+
with freeze_time("2048-02-29T02:00:01+0000"):
321+
# PUT workflows:change-requests-detail pk={change_request.id}
322+
new_feature_state.save()
323+
new_feature_state.feature_state_value.save()
324+
325+
responses.calls.reset()
326+
327+
# When
328+
responses.add(responses.POST, webhook, status=200)
329+
change_request.commit(committed_by=admin_user)
330+
assert not any(_extract_webhook_payloads("FLAG_UPDATED"))
331+
with freeze_time("2048-02-29T06:00:00+0000"):
332+
create_feature_state_went_live_audit_log(new_feature_state.id)
333+
334+
# Then
335+
payload = list(_extract_webhook_payloads("FLAG_UPDATED"))[-1]
336+
assert payload["data"]["previous_state"]["enabled"] is previous_state
337+
assert payload["data"]["previous_state"]["feature_state_value"] == previous_value
338+
assert payload["data"]["new_state"]["enabled"] is True
339+
assert payload["data"]["new_state"]["feature_state_value"] == "new_value"

0 commit comments

Comments
 (0)