fix(audit): add details for override creation (#3359)

api/audit/models.py

@@ -1,4 +1,5 @@
 import typing
+from functools import cached_property
 from importlib import import_module
 
 from django.db import models
@@ -78,7 +79,7 @@ def environment_document_updated(self) -> bool:
         )
         return "send_environments_to_dynamodb" not in skip_signals_and_hooks
 
-    @property
+    @cached_property
     def history_record(self) -> typing.Optional[Model]:
         if not (self.history_record_class_path and self.history_record_id):
             # There are still AuditLog records that will not have this detail

api/audit/serializers.py

@@ -40,6 +40,7 @@ class AuditLogRetrieveSerializer(serializers.ModelSerializer):
     environment = EnvironmentSerializerLight()
     project = ProjectListSerializer()
     change_details = serializers.SerializerMethodField()
+    change_type = serializers.SerializerMethodField()
 
     class Meta:
         model = AuditLog
@@ -54,6 +55,7 @@ class Meta:
             "related_object_type",
             "is_system_event",
             "change_details",
+            "change_type",
         )
 
     @swagger_serializer_method(
@@ -69,6 +71,16 @@ def get_change_details(
 
         return []
 
+    def get_change_type(self, instance: AuditLog) -> str:
+        if not (history_record := instance.history_record):
+            return "UNKNOWN"
+
+        return {
+            "+": "CREATE",
+            "-": "DELETE",
+            "~": "UPDATE",
+        }.get(history_record.history_type)
+
 
 class AuditLogsQueryParamSerializer(serializers.Serializer):
     project = serializers.IntegerField(required=False)

api/core/models.py

@@ -4,6 +4,7 @@
 
 from django.db import models
 from django.db.models import Manager
+from django.forms import model_to_dict
 from django.http import HttpRequest
 from simple_history.models import HistoricalRecords, ModelChange
 from softdelete.models import SoftDeleteManager, SoftDeleteObject
@@ -54,6 +55,7 @@ class Meta:
 
 class _BaseHistoricalModel(models.Model):
     include_in_audit = True
+    _show_change_details_for_create = False
 
     master_api_key = models.ForeignKey(
         "api_keys.MasterAPIKey", blank=True, null=True, on_delete=models.DO_NOTHING
@@ -63,22 +65,32 @@ class Meta:
         abstract = True
 
     def get_change_details(self) -> typing.Optional[typing.List[ModelChange]]:
-        if not self.history_type == "~":
-            # we only return the change details for updates
-            return
-
-        return [
-            change
-            for change in self.diff_against(self.prev_record).changes
-            if change.field not in self._change_details_excluded_fields
-        ]
+        if self.history_type == "~":
+            return [
+                change
+                for change in self.diff_against(self.prev_record).changes
+                if change.field not in self._change_details_excluded_fields
+            ]
+        elif self.history_type == "+" and self._show_change_details_for_create:
+            return [
+                ModelChange(field_name=key, old_value=None, new_value=value)
+                for key, value in self.instance.to_dict().items()
+                if key not in self._change_details_excluded_fields
+            ]
+        elif self.history_type == "-":
+            # Ignore deletes because they get painful due to cascade deletes
+            # Maybe we can resolve this in the future but for now it's not
+            # critical.
+            return []
 
 
 def base_historical_model_factory(
     change_details_excluded_fields: typing.Sequence[str],
+    show_change_details_for_create: bool = False,
 ) -> typing.Type[_BaseHistoricalModel]:
     class BaseHistoricalModel(_BaseHistoricalModel):
         _change_details_excluded_fields = set(change_details_excluded_fields)
+        _show_change_details_for_create = show_change_details_for_create
 
         class Meta:
             abstract = True
@@ -102,6 +114,9 @@ class _AbstractBaseAuditableModel(models.Model):
     history_record_class_path = None
     related_object_type = None
 
+    to_dict_excluded_fields: typing.Sequence[str] = None
+    to_dict_included_fields: typing.Sequence[str] = None
+
     class Meta:
         abstract = True
 
@@ -150,6 +165,17 @@ def get_audit_log_related_object_type(self, history_instance) -> RelatedObjectTy
         """
         return self.related_object_type
 
+    def to_dict(self) -> dict[str, typing.Any]:
+        # by default, we exclude the id and any foreign key fields from the response
+        return model_to_dict(
+            instance=self,
+            fields=[
+                f.name
+                for f in self._meta.fields
+                if f.name != "id" and not f.related_model
+            ],
+        )
+
     def _get_environment(self) -> typing.Optional["Environment"]:
         """Return the related environment for this model."""
         return None
@@ -169,10 +195,16 @@ def get_history_user(
 def abstract_base_auditable_model_factory(
     historical_records_excluded_fields: typing.List[str] = None,
     change_details_excluded_fields: typing.Sequence[str] = None,
+    show_change_details_for_create: bool = False,
 ) -> typing.Type[_AbstractBaseAuditableModel]:
     class Base(_AbstractBaseAuditableModel):
         history = HistoricalRecords(
-            bases=[base_historical_model_factory(change_details_excluded_fields or [])],
+            bases=[
+                base_historical_model_factory(
+                    change_details_excluded_fields or [],
+                    show_change_details_for_create,
+                )
+            ],
             excluded_fields=historical_records_excluded_fields or [],
             get_user=get_history_user,
             inherit=True,

api/features/managers.py

@@ -1,9 +1,17 @@
 from __future__ import unicode_literals
 
+import typing
+
 from core.models import UUIDNaturalKeyManagerMixin
+from django.db.models import Q, QuerySet
+from django.utils import timezone
 from ordered_model.models import OrderedModelManager
 from softdelete.models import SoftDeleteManager
 
+if typing.TYPE_CHECKING:
+    from environments.models import Environment
+    from features.models import FeatureState
+
 
 class FeatureSegmentManager(UUIDNaturalKeyManagerMixin, OrderedModelManager):
     pass
@@ -14,7 +22,31 @@ class FeatureManager(UUIDNaturalKeyManagerMixin, SoftDeleteManager):
 
 
 class FeatureStateManager(UUIDNaturalKeyManagerMixin, SoftDeleteManager):
-    pass
+    def get_live_feature_states(
+        self, environment: "Environment", additional_filters: Q = None, **kwargs
+    ) -> QuerySet["FeatureState"]:
+        # TODO: replace additional_filters with just using kwargs in calling locations
+
+        now = timezone.now()
+
+        qs_filter = Q(environment=environment, deleted_at__isnull=True)
+        if environment.use_v2_feature_versioning:
+            qs_filter &= Q(
+                environment_feature_version__isnull=False,
+                environment_feature_version__published_at__isnull=False,
+                environment_feature_version__live_from__lte=now,
+            )
+        else:
+            qs_filter &= Q(
+                live_from__isnull=False,
+                live_from__lte=now,
+                version__isnull=False,
+            )
+
+        if additional_filters:
+            qs_filter &= additional_filters
+
+        return self.filter(qs_filter, **kwargs)
 
 
 class FeatureStateValueManager(UUIDNaturalKeyManagerMixin, SoftDeleteManager):

api/features/models.py

@@ -373,7 +373,11 @@ def _get_environment(self) -> "Environment":
 class FeatureState(
     SoftDeleteExportableModel,
     LifecycleModelMixin,
-    abstract_base_auditable_model_factory(["uuid"]),
+    abstract_base_auditable_model_factory(
+        historical_records_excluded_fields=["uuid"],
+        change_details_excluded_fields=["live_from", "version"],
+        show_change_details_for_create=True,
+    ),
 ):
     history_record_class_path = "features.models.HistoricalFeatureState"
     related_object_type = RelatedObjectType.FEATURE_STATE
@@ -846,6 +850,17 @@ def get_skip_create_audit_log(self) -> bool:
         return False
 
     def get_create_log_message(self, history_instance) -> typing.Optional[str]:
+        if (
+            history_instance.history_type == "+"
+            and (self.identity_id or self.feature_segment_id)
+            and self.enabled == self.get_environment_default().enabled
+        ):
+            # Don't create an Audit Log for overrides that are created which don't differ
+            # from the environment default. This likely means that an override was created
+            # for a remote config value, and hence there will be an AuditLog message
+            # created for the FeatureStateValue model change.
+            return
+
         if self.identity_id:
             return audit_helpers.get_identity_override_created_audit_message(self)
         elif self.feature_segment_id:
@@ -900,6 +915,21 @@ def get_extra_audit_log_kwargs(self, history_instance) -> dict:
 
         return kwargs
 
+    def get_environment_default(self) -> typing.Optional["FeatureState"]:
+        if self.feature_segment_id or self.identity_id:
+            return (
+                self.__class__.objects.get_live_feature_states(
+                    environment=self.environment,
+                    feature_id=self.feature_id,
+                    feature_segment_id__isnull=True,
+                    identity_id__isnull=True,
+                )
+                .order_by("-version", "-environment_feature_version__live_from")
+                .first()
+            )
+
+        return None
+
     def _get_environment(self) -> typing.Optional["Environment"]:
         return self.environment
 

api/features/versioning/versioning_service.py

@@ -38,15 +38,14 @@ def get_environment_flags_list(
     feature states. The logic to grab the latest version is then handled in python
     by building a dictionary. Returns a list of FeatureState objects.
     """
-    qs_filter = _build_environment_flags_qs_filter(
-        environment, feature_name, additional_filters
-    )
-
     additional_select_related_args = additional_select_related_args or tuple()
     additional_prefetch_related_args = additional_prefetch_related_args or tuple()
 
     feature_states = (
-        FeatureState.objects.select_related(
+        FeatureState.objects.get_live_feature_states(
+            environment=environment, additional_filters=additional_filters
+        )
+        .select_related(
             "environment",
             "feature",
             "feature_state_value",
@@ -55,9 +54,11 @@ def get_environment_flags_list(
             *additional_select_related_args,
         )
         .prefetch_related(*additional_prefetch_related_args)
-        .filter(qs_filter)
     )
 
+    if feature_name:
+        feature_states = feature_states.filter(feature__name__iexact=feature_name)
+
     # Build up a dictionary in the form
     # {(feature_id, feature_segment_id, identity_id): feature_state}
     # and only keep the latest version for each feature.
@@ -88,30 +89,3 @@ def get_current_live_environment_feature_version(
         .order_by("-live_from")
         .first()
     )
-
-
-def _build_environment_flags_qs_filter(
-    environment: "Environment", feature_name: str = None, additional_filters: Q = None
-) -> Q:
-    now = timezone.now()
-
-    qs_filter = Q(environment=environment, deleted_at__isnull=True)
-    if environment.use_v2_feature_versioning:
-        qs_filter &= Q(
-            environment_feature_version__isnull=False,
-            environment_feature_version__published_at__isnull=False,
-            environment_feature_version__live_from__lte=now,
-        )
-    else:
-        qs_filter &= Q(
-            live_from__isnull=False,
-            live_from__lte=now,
-            version__isnull=False,
-        )
-
-    if feature_name:
-        qs_filter &= Q(feature__name__iexact=feature_name)
-    if additional_filters:
-        qs_filter &= additional_filters
-
-    return qs_filter