From 19c8cb66554d5fc66776c96e78bcd2210ae8ac67 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 3 Dec 2025 17:51:05 +0100 Subject: [PATCH 1/7] fix: clear-feature-states-cr-when-migrating-to-v2 --- api/features/versioning/tasks.py | 5 +- .../core/test_unit_workflows_models.py | 53 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/api/features/versioning/tasks.py b/api/features/versioning/tasks.py index 086b9c291cb6..8a8cb996fd9a 100644 --- a/api/features/versioning/tasks.py +++ b/api/features/versioning/tasks.py @@ -106,7 +106,10 @@ def _create_initial_feature_versions(environment: "Environment"): # type: ignor feature_states__in=latest_feature_states ) - latest_feature_states.update(environment_feature_version=ef_version) + latest_feature_states.update( + environment_feature_version=ef_version, + change_request=None, + ) related_feature_segments.update(environment_feature_version=ef_version) scheduled_feature_states = FeatureState.objects.filter( diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index 9d875e36dfe3..f9e6e5127e28 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -9,6 +9,7 @@ from flag_engine.segments.constants import EQUAL, PERCENTAGE_SPLIT from freezegun.api import FrozenDateTimeFactory from pytest_mock import MockerFixture +from features.versioning.tasks import enable_v2_versioning from audit.constants import ( CHANGE_REQUEST_APPROVED_MESSAGE, @@ -1030,3 +1031,55 @@ def test_delete_organisation_with_committed_change_request( # Then assert organisation.deleted_at is not None + + +def test_enable_v2_versioning_clears_change_request_from_feature_states( + project: Project, + environment: Environment, + feature: Feature, + admin_user: FFAdminUser, +) -> None: + # Given - Existing committed CR and one scheduled CR + pre_existing_cr = ChangeRequest.objects.create( + environment=environment, + title="immediate", + user=admin_user, + ) + FeatureState.objects.create( + environment=environment, + feature=feature, + change_request=pre_existing_cr, + version=None, + ) + pre_existing_cr.commit(admin_user) + + scheduled_cr = ChangeRequest.objects.create( + environment=environment, + title="scheduled", + user=admin_user, + ) + FeatureState.objects.create( + environment=environment, + feature=feature, + change_request=scheduled_cr, + live_from=timezone.now() + timedelta(days=2), + version=None, + ) + scheduled_cr.commit(admin_user) + + # When - Enable v2 versioning + enable_v2_versioning(environment_id=environment.id) + + environment.refresh_from_db() + assert environment.use_v2_feature_versioning is True + + # Then - CR must have been cleared from feature states + feature_states_with_efv = FeatureState.objects.filter( + environment=environment, + environment_feature_version__isnull=False, + ) + assert feature_states_with_efv.exists() + assert not feature_states_with_efv.filter(change_request__isnull=False).exists() + + project.delete() + assert project.deleted_at is not None From 664fe4a7fe28b3a64852823a15b2e08b9d5d181c Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 3 Dec 2025 17:51:30 +0100 Subject: [PATCH 2/7] fix: lint --- .../unit/features/workflows/core/test_unit_workflows_models.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index f9e6e5127e28..8ca3c465df02 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -9,7 +9,6 @@ from flag_engine.segments.constants import EQUAL, PERCENTAGE_SPLIT from freezegun.api import FrozenDateTimeFactory from pytest_mock import MockerFixture -from features.versioning.tasks import enable_v2_versioning from audit.constants import ( CHANGE_REQUEST_APPROVED_MESSAGE, @@ -27,7 +26,7 @@ EnvironmentFeatureVersion, VersionChangeSet, ) -from features.versioning.tasks import publish_version_change_set +from features.versioning.tasks import enable_v2_versioning, publish_version_change_set from features.versioning.versioning_service import get_environment_flags_list from features.workflows.core.exceptions import ( CannotApproveOwnChangeRequest, From 7d70d990e5f1b4b90104588a038156a339c0e3a8 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 4 Dec 2025 17:59:47 +0100 Subject: [PATCH 3/7] fix: skip-audit-log-for-soft-deleted-change-request --- api/features/versioning/tasks.py | 5 +- api/features/workflows/core/models.py | 9 ++- .../core/test_unit_workflows_models.py | 61 ++++++------------- 3 files changed, 26 insertions(+), 49 deletions(-) diff --git a/api/features/versioning/tasks.py b/api/features/versioning/tasks.py index 8a8cb996fd9a..086b9c291cb6 100644 --- a/api/features/versioning/tasks.py +++ b/api/features/versioning/tasks.py @@ -106,10 +106,7 @@ def _create_initial_feature_versions(environment: "Environment"): # type: ignor feature_states__in=latest_feature_states ) - latest_feature_states.update( - environment_feature_version=ef_version, - change_request=None, - ) + latest_feature_states.update(environment_feature_version=ef_version) related_feature_segments.update(environment_feature_version=ef_version) scheduled_feature_states = FeatureState.objects.filter( diff --git a/api/features/workflows/core/models.py b/api/features/workflows/core/models.py index 04957a52a1af..d32b9315dc2b 100644 --- a/api/features/workflows/core/models.py +++ b/api/features/workflows/core/models.py @@ -18,6 +18,7 @@ LifecycleModelMixin, hook, ) +from django_lifecycle.conditions import WhenFieldValueIs, WhenFieldValueIsNot from audit.constants import ( CHANGE_REQUEST_APPROVED_MESSAGE, @@ -192,7 +193,13 @@ def set_project_from_environment(self): # type: ignore[no-untyped-def] self.project_id = self.environment.project_id # type: ignore[union-attr] @hook(AFTER_CREATE, when="committed_at", is_not=None) - @hook(AFTER_SAVE, when="committed_at", is_not=None) + @hook( + AFTER_SAVE, + condition=( + WhenFieldValueIsNot("committed_at", value=None) + & WhenFieldValueIs("deleted_at", value=None) + ), + ) def create_audit_log_for_related_feature_state(self): # type: ignore[no-untyped-def] for feature_state in self.feature_states.all(): if self.committed_at < feature_state.live_from: # type: ignore[operator] diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index 8ca3c465df02..2fdd76a306e8 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -1032,53 +1032,26 @@ def test_delete_organisation_with_committed_change_request( assert organisation.deleted_at is not None -def test_enable_v2_versioning_clears_change_request_from_feature_states( - project: Project, - environment: Environment, - feature: Feature, - admin_user: FFAdminUser, +def test_soft_delete_committed_change_request_does_not_trigger_audit_log( + change_request_no_required_approvals: ChangeRequest, + mocker: MockerFixture, ) -> None: - # Given - Existing committed CR and one scheduled CR - pre_existing_cr = ChangeRequest.objects.create( - environment=environment, - title="immediate", - user=admin_user, - ) - FeatureState.objects.create( - environment=environment, - feature=feature, - change_request=pre_existing_cr, - version=None, - ) - pre_existing_cr.commit(admin_user) + # Given + committer = FFAdminUser.objects.create(email="committer@example.com") + change_request_no_required_approvals.commit(committer) - scheduled_cr = ChangeRequest.objects.create( - environment=environment, - title="scheduled", - user=admin_user, + mock_went_live_task = mocker.patch( + "features.workflows.core.models.create_feature_state_went_live_audit_log" ) - FeatureState.objects.create( - environment=environment, - feature=feature, - change_request=scheduled_cr, - live_from=timezone.now() + timedelta(days=2), - version=None, + mock_updated_task = mocker.patch( + "features.workflows.core.models.create_feature_state_updated_by_change_request_audit_log" ) - scheduled_cr.commit(admin_user) - - # When - Enable v2 versioning - enable_v2_versioning(environment_id=environment.id) - environment.refresh_from_db() - assert environment.use_v2_feature_versioning is True - - # Then - CR must have been cleared from feature states - feature_states_with_efv = FeatureState.objects.filter( - environment=environment, - environment_feature_version__isnull=False, - ) - assert feature_states_with_efv.exists() - assert not feature_states_with_efv.filter(change_request__isnull=False).exists() + # When + environment = change_request_no_required_approvals.environment + environment.delete() + change_request_no_required_approvals.delete() - project.delete() - assert project.deleted_at is not None + # Then + mock_went_live_task.delay.assert_not_called() + mock_updated_task.delay.assert_not_called() From 068bd97ccba608f95b35b7a62416fca702980614 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 4 Dec 2025 17:00:03 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../unit/features/workflows/core/test_unit_workflows_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index 2fdd76a306e8..9aed72fa2826 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -26,7 +26,7 @@ EnvironmentFeatureVersion, VersionChangeSet, ) -from features.versioning.tasks import enable_v2_versioning, publish_version_change_set +from features.versioning.tasks import publish_version_change_set from features.versioning.versioning_service import get_environment_flags_list from features.workflows.core.exceptions import ( CannotApproveOwnChangeRequest, From bf74c4993c7edb090813957b18edf309f7151808 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 4 Dec 2025 18:14:39 +0100 Subject: [PATCH 5/7] fix: improved-test --- api/features/workflows/core/models.py | 6 +-- .../core/test_unit_workflows_models.py | 42 +++++++++++++++---- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/api/features/workflows/core/models.py b/api/features/workflows/core/models.py index d32b9315dc2b..51659f6dc59d 100644 --- a/api/features/workflows/core/models.py +++ b/api/features/workflows/core/models.py @@ -18,7 +18,7 @@ LifecycleModelMixin, hook, ) -from django_lifecycle.conditions import WhenFieldValueIs, WhenFieldValueIsNot +from django_lifecycle.conditions import WhenFieldValueIs, WhenFieldValueIsNot # type: ignore[import-untyped] from audit.constants import ( CHANGE_REQUEST_APPROVED_MESSAGE, @@ -196,8 +196,8 @@ def set_project_from_environment(self): # type: ignore[no-untyped-def] @hook( AFTER_SAVE, condition=( - WhenFieldValueIsNot("committed_at", value=None) - & WhenFieldValueIs("deleted_at", value=None) + WhenFieldValueIsNot("committed_at", None) + & WhenFieldValueIs("deleted_at", None) ), ) def create_audit_log_for_related_feature_state(self): # type: ignore[no-untyped-def] diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index 2fdd76a306e8..0da615daeb45 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -1032,13 +1032,42 @@ def test_delete_organisation_with_committed_change_request( assert organisation.deleted_at is not None -def test_soft_delete_committed_change_request_does_not_trigger_audit_log( - change_request_no_required_approvals: ChangeRequest, +def test_delete_project_with_v2_versioning_does_not_trigger_audit_log( + project: Project, + environment: Environment, + feature: Feature, + admin_user: FFAdminUser, mocker: MockerFixture, ) -> None: # Given - committer = FFAdminUser.objects.create(email="committer@example.com") - change_request_no_required_approvals.commit(committer) + pre_existing_cr = ChangeRequest.objects.create( + environment=environment, + title="immediate", + user=admin_user, + ) + FeatureState.objects.create( + environment=environment, + feature=feature, + change_request=pre_existing_cr, + version=None, + ) + pre_existing_cr.commit(admin_user) + + scheduled_cr = ChangeRequest.objects.create( + environment=environment, + title="scheduled", + user=admin_user, + ) + FeatureState.objects.create( + environment=environment, + feature=feature, + change_request=scheduled_cr, + live_from=timezone.now() + timedelta(days=2), + version=None, + ) + scheduled_cr.commit(admin_user) + + enable_v2_versioning(environment_id=environment.id) mock_went_live_task = mocker.patch( "features.workflows.core.models.create_feature_state_went_live_audit_log" @@ -1048,10 +1077,9 @@ def test_soft_delete_committed_change_request_does_not_trigger_audit_log( ) # When - environment = change_request_no_required_approvals.environment - environment.delete() - change_request_no_required_approvals.delete() + project.delete() # Then + assert project.deleted_at is not None mock_went_live_task.delay.assert_not_called() mock_updated_task.delay.assert_not_called() From 0ce6dde949a71184aac7516c79b88a79d193c235 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 4 Dec 2025 17:15:07 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/features/workflows/core/models.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/features/workflows/core/models.py b/api/features/workflows/core/models.py index 51659f6dc59d..3d33f48e2f0b 100644 --- a/api/features/workflows/core/models.py +++ b/api/features/workflows/core/models.py @@ -18,7 +18,10 @@ LifecycleModelMixin, hook, ) -from django_lifecycle.conditions import WhenFieldValueIs, WhenFieldValueIsNot # type: ignore[import-untyped] +from django_lifecycle.conditions import ( # type: ignore[import-untyped] + WhenFieldValueIs, + WhenFieldValueIsNot, +) from audit.constants import ( CHANGE_REQUEST_APPROVED_MESSAGE, From e8f8cf8af35439e95c8ca2d73b40329d63945ae7 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 4 Dec 2025 18:18:31 +0100 Subject: [PATCH 7/7] fix: re-added-function-import --- .../unit/features/workflows/core/test_unit_workflows_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index 0652d3106979..0da615daeb45 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -26,7 +26,7 @@ EnvironmentFeatureVersion, VersionChangeSet, ) -from features.versioning.tasks import publish_version_change_set +from features.versioning.tasks import enable_v2_versioning, publish_version_change_set from features.versioning.versioning_service import get_environment_flags_list from features.workflows.core.exceptions import ( CannotApproveOwnChangeRequest,