diff --git a/.github/workflows/api-deploy-production-ecs.yml b/.github/workflows/api-deploy-production-ecs.yml index 2111fa48d567..e5cda76b76a1 100644 --- a/.github/workflows/api-deploy-production-ecs.yml +++ b/.github/workflows/api-deploy-production-ecs.yml @@ -39,7 +39,7 @@ jobs: aws_identity_migration_task_role_arn: arn:aws:iam::084060095745:role/task-exec-role-741a7e3 aws_task_definitions_directory_path: infrastructure/aws/production flagsmith_saml_revision: v1.1.0 - flagsmith_workflows_revision: v1.2.4 + flagsmith_workflows_revision: v1.2.5 flagsmith_auth_controller_revision: v0.0.1 flagsmith_rbac_revision: v0.2.0 diff --git a/.github/workflows/api-deploy-staging-ecs.yml b/.github/workflows/api-deploy-staging-ecs.yml index 325ba526b7ef..fbd064525187 100644 --- a/.github/workflows/api-deploy-staging-ecs.yml +++ b/.github/workflows/api-deploy-staging-ecs.yml @@ -39,7 +39,7 @@ jobs: aws_identity_migration_task_role_arn: arn:aws:iam::302456015006:role/task-exec-role-6fb76f6 aws_task_definitions_directory_path: infrastructure/aws/staging flagsmith_saml_revision: v1.1.0 - flagsmith_workflows_revision: v1.2.4 + flagsmith_workflows_revision: v1.2.5 flagsmith_auth_controller_revision: v0.0.1 flagsmith_rbac_revision: v0.2.0 diff --git a/.github/workflows/platform-docker-publish-all-features-image.yml b/.github/workflows/platform-docker-publish-all-features-image.yml index acc430fbf0d3..659811bc4b66 100644 --- a/.github/workflows/platform-docker-publish-all-features-image.yml +++ b/.github/workflows/platform-docker-publish-all-features-image.yml @@ -8,7 +8,7 @@ on: env: FLAGSMITH_SAML_REVISION: v1.1.0 FLAGSMITH_RBAC_REVISION: v0.2.0 - FLAGSMITH_WORKFLOWS_REVISION: v1.2.4 + FLAGSMITH_WORKFLOWS_REVISION: v1.2.5 jobs: build-dockerhub: diff --git a/api/audit/constants.py b/api/audit/constants.py index 40fb4037250f..e639e1d0f45e 100644 --- a/api/audit/constants.py +++ b/api/audit/constants.py @@ -55,6 +55,6 @@ CHANGE_REQUEST_CREATED_MESSAGE = "Change Request: %s created" CHANGE_REQUEST_APPROVED_MESSAGE = "Change Request: %s approved" CHANGE_REQUEST_COMMITTED_MESSAGE = "Change Request: %s committed" - +CHANGE_REQUEST_DELETED_MESSAGE = "Change Request: %s deleted" DATETIME_FORMAT = "%d/%m/%Y %H:%M:%S" diff --git a/api/audit/tasks.py b/api/audit/tasks.py index a116a4ce5ad7..2418071ec91c 100644 --- a/api/audit/tasks.py +++ b/api/audit/tasks.py @@ -84,16 +84,19 @@ def create_audit_log_from_historical_record( environment, project = instance.get_environment_and_project() + related_object_id = instance.get_audit_log_related_object_id(history_instance) + related_object_type = instance.get_audit_log_related_object_type(history_instance) + + if not related_object_id: + return + log_message = { "+": instance.get_create_log_message, "-": instance.get_delete_log_message, "~": instance.get_update_log_message, }[history_instance.history_type](history_instance) - related_object_id = instance.get_audit_log_related_object_id(history_instance) - related_object_type = instance.get_audit_log_related_object_type(history_instance) - - if not (log_message and related_object_id): + if not log_message: return AuditLog.objects.create( diff --git a/api/features/models.py b/api/features/models.py index efe83b0f43ad..cc6b06b9d6b2 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -544,7 +544,7 @@ def is_live(self) -> bool: @property def is_scheduled(self) -> bool: - return self.live_from > timezone.now() + return self.live_from and self.live_from > timezone.now() def clone( self, @@ -833,14 +833,11 @@ def is_more_recent_live_from(self, other: "FeatureState") -> bool: and other.live_from is None ) - def get_create_log_message(self, history_instance) -> typing.Optional[str]: - if self.change_request_id and not self.change_request.committed_at: - # change requests create feature states that may not ever go live, - # since we already include the change requests in the audit log - # we don't want to create separate audit logs for the associated - # feature states - return + @property + def belongs_to_uncommited_change_request(self) -> bool: + return self.change_request_id and not self.change_request.committed_at + def get_create_log_message(self, history_instance) -> typing.Optional[str]: if self.identity_id: return audit_helpers.get_identity_override_created_audit_message(self) elif self.feature_segment_id: @@ -854,9 +851,6 @@ def get_create_log_message(self, history_instance) -> typing.Optional[str]: return audit_helpers.get_environment_feature_state_created_audit_message(self) def get_update_log_message(self, history_instance) -> typing.Optional[str]: - if self.change_request_id and not self.change_request.committed_at: - return - if self.identity: return IDENTITY_FEATURE_STATE_UPDATED_MESSAGE % ( self.feature.name, @@ -886,6 +880,15 @@ def get_delete_log_message(self, history_instance) -> typing.Optional[str]: # Account for cascade deletes return None + def get_audit_log_related_object_id(self, history_instance) -> int: + # Change requests can create, update, or delete feature states that may never go live, + # since we already include the change requests in the audit log + # we don't want to create separate audit logs for the associated + # feature states + if self.belongs_to_uncommited_change_request: + return None + return super().get_audit_log_related_object_id(history_instance) + def get_extra_audit_log_kwargs(self, history_instance) -> dict: kwargs = super().get_extra_audit_log_kwargs(history_instance) diff --git a/api/features/multivariate/models.py b/api/features/multivariate/models.py index 94be11061c27..a87a532f0055 100644 --- a/api/features/multivariate/models.py +++ b/api/features/multivariate/models.py @@ -150,6 +150,9 @@ def get_update_log_message(self, history_instance) -> typing.Optional[str]: return f"Multivariate value changed for feature '{feature.name}'." def get_audit_log_related_object_id(self, history_instance) -> int: + if self.feature_state.belongs_to_uncommited_change_request: + return None + return self.feature_state.feature_id def _get_environment(self) -> typing.Optional["Environment"]: diff --git a/api/features/workflows/core/models.py b/api/features/workflows/core/models.py index 0dae2e5198ea..7bd9c8b5f156 100644 --- a/api/features/workflows/core/models.py +++ b/api/features/workflows/core/models.py @@ -26,6 +26,7 @@ CHANGE_REQUEST_APPROVED_MESSAGE, CHANGE_REQUEST_COMMITTED_MESSAGE, CHANGE_REQUEST_CREATED_MESSAGE, + CHANGE_REQUEST_DELETED_MESSAGE, ) from audit.related_object_type import RelatedObjectType from audit.tasks import ( @@ -122,6 +123,9 @@ def commit(self, committed_by: "FFAdminUser"): def get_create_log_message(self, history_instance) -> typing.Optional[str]: return CHANGE_REQUEST_CREATED_MESSAGE % self.title + def get_delete_log_message(self, history_instance) -> typing.Optional[str]: + return CHANGE_REQUEST_DELETED_MESSAGE % self.title + def get_update_log_message(self, history_instance) -> typing.Optional[str]: if ( history_instance.prev_record diff --git a/api/tests/unit/features/test_unit_features_models.py b/api/tests/unit/features/test_unit_features_models.py index 30fed30a4c33..fab13868ee5e 100644 --- a/api/tests/unit/features/test_unit_features_models.py +++ b/api/tests/unit/features/test_unit_features_models.py @@ -135,7 +135,7 @@ def test_creating_a_feature_with_defaults_does_not_set_defaults_if_disabled( assert not feature_state.get_feature_state_value() -def test_feature_state_get_create_log_message_returns_nothing_if_uncommitted_change_request( +def test_feature_state_get_audit_log_related_object_id_returns_nothing_if_uncommitted_change_request( environment, feature, admin_user, mocker ): # Given @@ -150,12 +150,12 @@ def test_feature_state_get_create_log_message_returns_nothing_if_uncommitted_cha ) # When - message = feature_state.get_create_log_message( + related_object_id = feature_state.get_audit_log_related_object_id( mocker.MagicMock(id="history_instance") ) # history instance is irrelevant here # Then - assert message is None + assert related_object_id is None @pytest.mark.parametrize(