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: deleting change requests with change sets throws 500 error #4439

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
10 changes: 8 additions & 2 deletions api/features/workflows/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,20 @@ def prevent_change_request_delete_if_committed(self) -> None:

@property
def live_from(self) -> datetime | None:
# Note: a change request can only have one of either
# feature_states, change_sets or environment_feature_versions

# First we check if there are feature states associated with the change request
# and, if so, we return the live_from of the feature state with the earliest
# live_from.
if first_feature_state := self.feature_states.order_by("live_from").first():
return first_feature_state.live_from

# Then we do the same for environment feature versions. Note that a change request
# can not have feature states and environment feature versions.
# Then we check the change sets.
elif first_change_set := self.change_sets.order_by("live_from").first():
return first_change_set.live_from

# Finally, we do the same for environment feature versions.
elif first_environment_feature_version := self.environment_feature_versions.order_by(
"live_from"
).first():
Comment on lines 271 to 282
Copy link
Member

Choose a reason for hiding this comment

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

Just a design question — are we going to deprecate feature_states and environment_feature_versions in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to, for sure, yep!

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import json
from datetime import timedelta

import freezegun
import pytest
from django.contrib.sites.models import Site
from django.utils import timezone
Expand All @@ -16,7 +18,10 @@
from audit.related_object_type import RelatedObjectType
from environments.models import Environment
from features.models import Feature, FeatureState
from features.versioning.models import EnvironmentFeatureVersion
from features.versioning.models import (
EnvironmentFeatureVersion,
VersionChangeSet,
)
from features.versioning.versioning_service import get_environment_flags_list
from features.workflows.core.exceptions import (
CannotApproveOwnChangeRequest,
Expand Down Expand Up @@ -731,3 +736,40 @@ def test_committing_change_request_with_environment_feature_versions_creates_pub
related_object_type=RelatedObjectType.EF_VERSION.name,
log=ENVIRONMENT_FEATURE_VERSION_PUBLISHED_MESSAGE % feature.name,
).exists()


def test_change_request_live_from_for_change_request_with_change_set(
feature: Feature,
environment_v2_versioning: Environment,
admin_user: FFAdminUser,
) -> None:
# Given
change_request = ChangeRequest.objects.create(
title="Test CR",
environment=environment_v2_versioning,
user=admin_user,
)
VersionChangeSet.objects.create(
change_request=change_request,
feature=feature,
feature_states_to_update=json.dumps(
[
{
"feature_segment": None,
"enabled": True,
"feature_state_value": {
"type": "unicode",
"string_value": "updated",
},
}
]
),
)

# When
now = timezone.now()
with freezegun.freeze_time(now):
change_request.commit(admin_user)

# Then
assert change_request.live_from == now
Loading