Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: audit logs generation for feature state value #4525

Merged
merged 3 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
14 changes: 12 additions & 2 deletions api/features/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1085,12 +1085,22 @@
self.save()

def get_skip_create_audit_log(self) -> bool:
return self.feature_state.get_skip_create_audit_log()
try:
return self.feature_state.get_skip_create_audit_log()
except ObjectDoesNotExist:
return False

Check warning on line 1091 in api/features/models.py

View check run for this annotation

Codecov / codecov/patch

api/features/models.py#L1090-L1091

Added lines #L1090 - L1091 were not covered by tests

def get_update_log_message(self, history_instance) -> typing.Optional[str]:
fs = self.feature_state

changes = history_instance.diff_against(history_instance.prev_record).changes
# NOTE: We have some feature state values that were created before we started
# tracking history, resulting in no prev_record.

changes = (
history_instance.diff_against(history_instance.prev_record).changes
if history_instance.prev_record
else []
)
if (
len(changes) == 1
and changes[0].field == "string_value"
Expand Down
25 changes: 24 additions & 1 deletion api/tests/unit/features/test_unit_features_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
from environments.identities.models import Identity
from environments.models import Environment
from features.constants import ENVIRONMENT, FEATURE_SEGMENT, IDENTITY
from features.models import Feature, FeatureSegment, FeatureState
from features.models import (
Feature,
FeatureSegment,
FeatureState,
FeatureStateValue,
)
from features.versioning.models import EnvironmentFeatureVersion
from features.workflows.core.models import ChangeRequest
from projects.models import Project
Expand Down Expand Up @@ -692,6 +697,24 @@ def test_feature_state_value_get_skip_create_audit_log_if_environment_feature_ve
assert feature_state.feature_state_value.get_skip_create_audit_log() is True


def test_feature_state_value_get_skip_create_audit_log_for_deleted_feature_state(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the codecov report, it looks like this isn't actually testing the codepath that you expect.

feature: Feature,
):
# Give
feature_state = feature.feature_states.first()
feature_state_value = feature_state.feature_state_value

# When
feature_state.delete()

# Then
fsv_history_instance = FeatureStateValue.history.filter(
id=feature_state_value.id, history_type="-"
).first()

assert fsv_history_instance.instance.get_skip_create_audit_log() is False


@pytest.mark.parametrize(
"feature_segment_id, identity_id, expected_function_name",
(
Expand Down
33 changes: 33 additions & 0 deletions api/tests/unit/features/test_unit_features_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2322,6 +2322,39 @@ def test_cannot_update_environment_of_a_feature_state(
)


def test_update_feature_state_without_history_of_fsv(
admin_client_new: APIClient,
environment: Environment,
feature: Feature,
feature_state: FeatureState,
) -> None:
# Given
url = reverse(
"api-v1:environments:environment-featurestates-detail",
args=[environment.api_key, feature_state.id],
)
new_value = "new-value"

# Remove historical feature state value
feature_state.feature_state_value.history.all().delete()

data = {
"id": feature_state.id,
"feature_state_value": new_value,
"enabled": False,
"feature": feature.id,
"environment": environment.id,
"identity": None,
"feature_segment": None,
}
# When
response = admin_client_new.put(
url, data=json.dumps(data), content_type="application/json"
)

assert response.status_code == status.HTTP_200_OK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert response.status_code == status.HTTP_200_OK
# Then
assert response.status_code == status.HTTP_200_OK



def test_cannot_update_feature_of_a_feature_state(
admin_client_new: APIClient,
environment: Environment,
Expand Down
Loading