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
54 changes: 47 additions & 7 deletions api/audit/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from audit.related_object_type import RelatedObjectType
from audit.serializers import AuditLogListSerializer
from audit.services import get_audited_instance_from_audit_log_record
from features.models import FeatureState
from features.models import FeatureState, FeatureStateValue
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 @@ -212,21 +212,30 @@ def send_audit_log_event_to_slack(sender, instance, **kwargs): # type: ignore[n
@receiver(post_save, sender=AuditLog)
@track_only([RelatedObjectType.FEATURE_STATE])
def send_feature_flag_went_live_signal(sender, instance, **kwargs): # type: ignore[no-untyped-def]
feature_state = get_audited_instance_from_audit_log_record(instance)
if not isinstance(feature_state, FeatureState):
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):
feature_state = audited_instance.feature_state
elif isinstance(audited_instance, FeatureState):
feature_state = audited_instance
else:
return

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

feature_state_change_went_live.send(instance)
feature_state_change_went_live.send(feature_state, audit_log=instance)


@receiver(feature_state_change_went_live)
def send_audit_log_event_to_sentry(sender: AuditLog, **kwargs: Any) -> None:
def send_audit_log_event_to_sentry(
sender: FeatureState, audit_log: AuditLog, **kwargs: Any
) -> None:
try:
sentry_configuration = SentryChangeTrackingConfiguration.objects.get(
environment=sender.environment,
environment=audit_log.environment,
deleted_at__isnull=True,
)
except SentryChangeTrackingConfiguration.DoesNotExist:
Expand All @@ -237,4 +246,35 @@ def send_audit_log_event_to_sentry(sender: AuditLog, **kwargs: Any) -> None:
secret=sentry_configuration.secret,
)

_track_event_async(sender, sentry_change_tracking) # type: ignore[no-untyped-call]
_track_event_async(audit_log, sentry_change_tracking) # type: ignore[no-untyped-call]


@receiver(feature_state_change_went_live)
def trigger_feature_state_change_webhooks(
sender: FeatureState,
**kwargs: Any,
) -> None:
"""
Trigger FLAG_UPDATED webhooks when a feature state change goes live.

Triggered from AuditLog post_save. Fetches a fresh feature state from the
database to ensure we get the latest data (including FeatureStateValue),
since drf-writable-nested saves the parent before nested objects.
"""
from features import tasks

# Fetch fresh data from the database to ensure we have the latest state
# This is necessary because:
# 1. drf-writable-nested saves FeatureState before FeatureStateValue
# 2. The history record's instance may have stale cached values
try:
fresh_feature_state = FeatureState.objects.get(id=sender.id)
except FeatureState.DoesNotExist:
# Skip deleted feature states - handled in views
return

# Skip versioned environments - handled by trigger_update_version_webhooks
if fresh_feature_state.environment_feature_version_id:
return

tasks.trigger_feature_state_change_webhooks(fresh_feature_state)
14 changes: 1 addition & 13 deletions api/features/signals.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
import logging

from django.db.models.signals import post_save
from django.dispatch import Signal, receiver

# noinspection PyUnresolvedReferences
from .models import FeatureState
from .tasks import trigger_feature_state_change_webhooks
from django.dispatch import Signal

logger = logging.getLogger(__name__)

feature_state_change_went_live = Signal()


@receiver(post_save, sender=FeatureState)
def trigger_feature_state_change_webhooks_signal(instance, **kwargs): # type: ignore[no-untyped-def]
if instance.environment_feature_version_id or instance.deleted_at:
return
trigger_feature_state_change_webhooks(instance)
89 changes: 89 additions & 0 deletions api/tests/integration/features/featurestate/test_webhooks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import json

from django.urls import reverse
from pytest_mock import MockerFixture
from rest_framework import status
from rest_framework.test import APIClient


def test_update_segment_override__webhook_payload_has_correct_previous_and_new_values(
admin_client: APIClient,
environment: int,
feature: int,
segment: int,
mocker: MockerFixture,
) -> None:
"""
Test for issue #6050: Webhook payload shows incorrect previous_state values
when updating a segment override value via the API.

The bug occurs because:
1. The webhook signal fires on FeatureState.post_save
2. drf-writable-nested saves the parent (FeatureState) before nested (FeatureStateValue)
3. So the webhook captures stale values
"""
# Given
old_value = 0
new_value = 1

# First create a feature_segment
feature_segment_url = reverse("api-v1:features:feature-segment-list")
feature_segment_data = {
"feature": feature,
"segment": segment,
"environment": environment,
}
feature_segment_response = admin_client.post(
feature_segment_url,
data=json.dumps(feature_segment_data),
content_type="application/json",
)
assert feature_segment_response.status_code == status.HTTP_201_CREATED
feature_segment_id = feature_segment_response.json()["id"]

# Create segment override with initial value via API
create_url = reverse("api-v1:features:featurestates-list")
create_data = {
"enabled": False,
"feature_state_value": {"type": "int", "integer_value": old_value},
"environment": environment,
"feature": feature,
"feature_segment": feature_segment_id,
}
create_response = admin_client.post(
create_url, data=json.dumps(create_data), content_type="application/json"
)
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")

# When - update the segment override via API
url = reverse("api-v1:features:featurestates-detail", args=[segment_override_id])
data = {
"enabled": False,
"feature_state_value": {"type": "int", "integer_value": new_value},
"environment": environment,
"feature": feature,
"feature_segment": feature_segment_id,
}
response = admin_client.put(
url, data=json.dumps(data), content_type="application/json"
)

# 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 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
Loading
Loading