Skip to content

Commit

Permalink
fix(versioning): prevent FeatureSegment from writing audit log on del…
Browse files Browse the repository at this point in the history
…ete when v2 versioning enabled (#4088)
  • Loading branch information
matthewelwell authored Jun 3, 2024
1 parent 29d78ca commit 60c0748
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
5 changes: 5 additions & 0 deletions api/features/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,11 @@ def to_id_priority_tuple_pairs(
def get_audit_log_related_object_id(self, history_instance) -> int:
return self.feature_id

def get_skip_create_audit_log(self) -> bool:
# Don't create audit logs when deleting feature segments using versioning
# v2 as we rely on the version history instead.
return self.environment_feature_version_id is not None

def get_delete_log_message(self, history_instance) -> typing.Optional[str]:
return SEGMENT_FEATURE_STATE_DELETED_MESSAGE % (
self.feature.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
from rest_framework import status
from rest_framework.test import APIClient

from audit.constants import SEGMENT_FEATURE_STATE_DELETED_MESSAGE
from audit.models import AuditLog
from audit.related_object_type import RelatedObjectType
from environments.models import Environment
from environments.permissions.constants import (
MANAGE_SEGMENT_OVERRIDES,
Expand Down Expand Up @@ -596,3 +599,43 @@ def test_get_feature_segments_only_returns_latest_version(
response_json = response.json()
assert response_json["count"] == 1
assert response_json["results"][0]["id"] == feature_segment_v2.id


def test_delete_feature_segment_does_not_create_audit_log_for_versioning_v2(
feature: Feature,
segment: Segment,
feature_segment: FeatureSegment,
segment_featurestate: FeatureState,
environment_v2_versioning: Environment,
staff_client: APIClient,
with_environment_permissions: WithEnvironmentPermissionsCallable,
with_project_permissions: WithProjectPermissionsCallable,
) -> None:
# Given
with_project_permissions([VIEW_PROJECT])
with_environment_permissions([MANAGE_SEGMENT_OVERRIDES, VIEW_ENVIRONMENT])

# we first need to create a new version so that we can modify the feature segment
# that is generated as part of the new version
version_2 = EnvironmentFeatureVersion.objects.create(
environment=environment_v2_versioning, feature=feature
)
version_2_feature_segment = FeatureSegment.objects.get(
feature=feature, segment=segment, environment_feature_version=version_2
)

url = reverse(
"api-v1:features:feature-segment-detail", args=[version_2_feature_segment.id]
)

# When
response = staff_client.delete(url)

# Then
assert response.status_code == status.HTTP_204_NO_CONTENT

assert not AuditLog.objects.filter(
related_object_type=RelatedObjectType.FEATURE.name,
related_object_id=feature.id,
log=SEGMENT_FEATURE_STATE_DELETED_MESSAGE % (feature.name, segment.name),
).exists()

0 comments on commit 60c0748

Please sign in to comment.