Skip to content

Commit e2da62d

Browse files
committed
Fix an audit trail race condition when deleting segment overrides
1 parent 1c67f96 commit e2da62d

File tree

2 files changed

+49
-7
lines changed

2 files changed

+49
-7
lines changed

api/features/models.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -962,9 +962,9 @@ def get_create_log_message(self, history_instance) -> typing.Optional[str]: # t
962962

963963
return audit_helpers.get_environment_feature_state_created_audit_message(self)
964964

965-
def get_update_log_message(self, history_instance) -> typing.Optional[str]: # type: ignore[no-untyped-def]
965+
def get_update_log_message(self, history_instance: "FeatureState") -> str | None:
966966
if self.change_request and self.is_scheduled:
967-
live_from: datetime.datetime = timezone.localtime(self.live_from)
967+
live_from = timezone.localtime(self.live_from)
968968
return FEATURE_STATE_SCHEDULED_TO_UPDATE_MESSAGE % (
969969
self.feature.name,
970970
self.change_request.title,
@@ -975,11 +975,15 @@ def get_update_log_message(self, history_instance) -> typing.Optional[str]: # t
975975
self.feature.name,
976976
self.identity.identifier,
977977
)
978-
elif self.feature_segment:
979-
return SEGMENT_FEATURE_STATE_UPDATED_MESSAGE % (
980-
self.feature.name,
981-
self.feature_segment.segment.name,
982-
)
978+
if self.feature_segment_id:
979+
try:
980+
return SEGMENT_FEATURE_STATE_UPDATED_MESSAGE % (
981+
self.feature.name,
982+
self.feature_segment.segment.name, # type: ignore[union-attr]
983+
)
984+
except FeatureSegment.DoesNotExist:
985+
# Cascade-deleted from segment overrides
986+
return None
983987
return FEATURE_STATE_UPDATED_MESSAGE % self.feature.name
984988

985989
def get_delete_log_message(self, history_instance) -> typing.Optional[str]: # type: ignore[no-untyped-def]

api/tests/unit/audit/test_unit_audit_tasks.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,44 @@ def test_create_audit_log_from_historical_record_creates_audit_log_with_correct_
225225
)
226226

227227

228+
def test_create_audit_log_from_historical_record__cascade_deleted_feature_segment__does_nothing(
229+
admin_user: FFAdminUser,
230+
feature: Feature,
231+
segment: Segment,
232+
environment: Environment,
233+
mocker: MockerFixture,
234+
) -> None:
235+
"""https://github.com/Flagsmith/flagsmith/issues/6792"""
236+
# Given
237+
feature_segment = FeatureSegment.objects.create(
238+
environment=environment,
239+
feature=feature,
240+
segment=segment,
241+
)
242+
feature_state = FeatureState.objects.create(
243+
environment=environment,
244+
feature=feature,
245+
feature_segment=feature_segment,
246+
)
247+
feature_state.enabled = not feature_state.enabled
248+
feature_state.save() # creates a "~" historical record
249+
history_instance = feature_state.history.filter(history_type="~").first()
250+
assert history_instance is not None
251+
feature_segment.delete() # cascade-deletes the FeatureState
252+
get_update_log_message = mocker.spy(FeatureState, "get_update_log_message")
253+
254+
# When
255+
create_audit_log_from_historical_record(
256+
history_instance.history_id,
257+
admin_user.id,
258+
feature_state.history_record_class_path,
259+
)
260+
261+
# Then
262+
assert get_update_log_message.call_count == 1
263+
assert get_update_log_message.spy_return is None
264+
265+
228266
def test_create_segment_priorities_changed_audit_log(
229267
admin_user: FFAdminUser,
230268
feature_segment: FeatureSegment,

0 commit comments

Comments
 (0)