fix: change request audit logs (#2527)

.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
 

.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
 

.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:

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"

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(

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)
 

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"]:

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

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(