feat(audit): add change details to AuditLog (#3218)

api/audit/models.py

@@ -79,9 +79,9 @@ def environment_document_updated(self) -> bool:
         return "send_environments_to_dynamodb" not in skip_signals_and_hooks
 
     @property
-    def history_record(self):
+    def history_record(self) -> typing.Optional[Model]:
         klass = self.get_history_record_model_class(self.history_record_class_path)
-        return klass.objects.get(id=self.history_record_id)
+        return klass.objects.filter(history_id=self.history_record_id).first()
 
     @property
     def environment_name(self) -> str:

api/audit/serializers.py

@@ -1,3 +1,6 @@
+import typing
+
+from drf_yasg.utils import swagger_serializer_method
 from rest_framework import serializers
 
 from audit.models import AuditLog
@@ -6,7 +9,7 @@
 from users.serializers import UserListSerializer
 
 
-class AuditLogSerializer(serializers.ModelSerializer):
+class AuditLogListSerializer(serializers.ModelSerializer):
     author = UserListSerializer()
     environment = EnvironmentSerializerLight()
     project = ProjectListSerializer()
@@ -26,6 +29,47 @@ class Meta:
         )
 
 
+class AuditLogChangeDetailsSerializer(serializers.Serializer):
+    field = serializers.ReadOnlyField()
+    old = serializers.ReadOnlyField()
+    new = serializers.ReadOnlyField()
+
+
+class AuditLogRetrieveSerializer(serializers.ModelSerializer):
+    author = UserListSerializer()
+    environment = EnvironmentSerializerLight()
+    project = ProjectListSerializer()
+    change_details = serializers.SerializerMethodField()
+
+    class Meta:
+        model = AuditLog
+        fields = (
+            "id",
+            "created_date",
+            "log",
+            "author",
+            "environment",
+            "project",
+            "related_object_id",
+            "related_object_type",
+            "is_system_event",
+            "change_details",
+        )
+
+    @swagger_serializer_method(
+        serializer_or_field=AuditLogChangeDetailsSerializer(many=True)
+    )
+    def get_change_details(
+        self, instance: AuditLog
+    ) -> typing.List[typing.Dict[str, typing.Any]]:
+        if history_record := instance.history_record:
+            return AuditLogChangeDetailsSerializer(
+                instance=history_record.get_change_details(), many=True
+            ).data
+
+        return []
+
+
 class AuditLogsQueryParamSerializer(serializers.Serializer):
     project = serializers.IntegerField(required=False)
     environments = serializers.ListField(

api/audit/signals.py

@@ -4,7 +4,7 @@
 from django.dispatch import receiver
 
 from audit.models import AuditLog, RelatedObjectType
-from audit.serializers import AuditLogSerializer
+from audit.serializers import AuditLogListSerializer
 from integrations.datadog.datadog import DataDogWrapper
 from integrations.dynatrace.dynatrace import DynatraceWrapper
 from integrations.new_relic.new_relic import NewRelicWrapper
@@ -16,7 +16,7 @@
 
 @receiver(post_save, sender=AuditLog)
 def call_webhooks(sender, instance, **kwargs):
-    data = AuditLogSerializer(instance=instance).data
+    data = AuditLogListSerializer(instance=instance).data
 
     if not (instance.project or instance.environment):
         logger.warning("Audit log without project or environment. Not sending webhook.")

api/audit/views.py

@@ -10,16 +10,21 @@
     OrganisationAuditLogPermissions,
     ProjectAuditLogPermissions,
 )
-from audit.serializers import AuditLogSerializer, AuditLogsQueryParamSerializer
+from audit.serializers import (
+    AuditLogListSerializer,
+    AuditLogRetrieveSerializer,
+    AuditLogsQueryParamSerializer,
+)
 from organisations.models import OrganisationRole
 
 
 @method_decorator(
     name="list",
     decorator=swagger_auto_schema(query_serializer=AuditLogsQueryParamSerializer()),
 )
-class _BaseAuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet):
-    serializer_class = AuditLogSerializer
+class _BaseAuditLogViewSet(
+    mixins.ListModelMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet
+):
     pagination_class = CustomPagination
 
     def get_queryset(self) -> QuerySet[AuditLog]:
@@ -46,6 +51,11 @@ def get_queryset(self) -> QuerySet[AuditLog]:
     def _get_base_filters(self) -> Q:
         return Q()
 
+    def get_serializer_class(self):
+        return {"retrieve": AuditLogRetrieveSerializer}.get(
+            self.action, AuditLogListSerializer
+        )
+
 
 class AllAuditLogViewSet(_BaseAuditLogViewSet):
     def _get_base_filters(self) -> Q:

api/core/models.py

@@ -5,7 +5,7 @@
 from django.db import models
 from django.db.models import Manager
 from django.http import HttpRequest
-from simple_history.models import HistoricalRecords
+from simple_history.models import HistoricalRecords, ModelChange
 from softdelete.models import SoftDeleteManager, SoftDeleteObject
 
 from audit.related_object_type import RelatedObjectType
@@ -52,7 +52,7 @@ class Meta:
         abstract = True
 
 
-class BaseHistoricalModel(models.Model):
+class _BaseHistoricalModel(models.Model):
     include_in_audit = True
 
     master_api_key = models.ForeignKey(
@@ -62,6 +62,29 @@ class BaseHistoricalModel(models.Model):
     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
+        ]
+
+
+def base_historical_model_factory(
+    change_details_excluded_fields: typing.Sequence[str],
+) -> typing.Type[_BaseHistoricalModel]:
+    class BaseHistoricalModel(_BaseHistoricalModel):
+        _change_details_excluded_fields = set(change_details_excluded_fields)
+
+        class Meta:
+            abstract = True
+
+    return BaseHistoricalModel
+
 
 class _AbstractBaseAuditableModel(models.Model):
     """
@@ -145,10 +168,11 @@ 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,
 ) -> typing.Type[_AbstractBaseAuditableModel]:
     class Base(_AbstractBaseAuditableModel):
         history = HistoricalRecords(
-            bases=[BaseHistoricalModel],
+            bases=[base_historical_model_factory(change_details_excluded_fields or [])],
             excluded_fields=historical_records_excluded_fields or [],
             get_user=get_history_user,
             inherit=True,

api/environments/models.py

@@ -65,7 +65,11 @@
 
 
 class Environment(
-    LifecycleModel, abstract_base_auditable_model_factory(), SoftDeleteObject
+    LifecycleModel,
+    abstract_base_auditable_model_factory(
+        change_details_excluded_fields=["updated_at"]
+    ),
+    SoftDeleteObject,
 ):
     history_record_class_path = "environments.models.HistoricalEnvironment"
     related_object_type = RelatedObjectType.ENVIRONMENT

api/tests/integration/audit/test_audit_logs.py

@@ -1,5 +1,8 @@
+import json
+
 from django.urls import reverse
 from rest_framework import status
+from rest_framework.test import APIClient
 
 
 def test_audit_logs_only_makes_two_queries(
@@ -17,3 +20,161 @@ def test_audit_logs_only_makes_two_queries(
 
     assert res.status_code == status.HTTP_200_OK
     assert res.json()["count"] == 3
+
+
+def test_retrieve_audit_log_for_environment_change(
+    admin_client: APIClient,
+    project: int,
+    environment_api_key: str,
+    environment_name: str,
+    environment: int,
+) -> None:
+    # Given
+    # we update the environment
+    update_environment_url = reverse(
+        "api-v1:environments:environment-detail",
+        args=[environment_api_key],
+    )
+    new_name = "some new name!"
+    data = {"name": new_name}
+    admin_client.patch(
+        update_environment_url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # we list the audit log to get an audit record that was created when we update the feature above
+    list_audit_log_url = reverse("api-v1:audit-list")
+    list_response = admin_client.get(list_audit_log_url)
+
+    # When
+    # We retrieve the record directly where we updated the environment
+    environment_update_result = next(
+        filter(
+            lambda r: r["log"].startswith("Environment updated"),
+            list_response.json()["results"],
+        )
+    )
+    retrieve_audit_log_url = reverse(
+        "api-v1:audit-detail", args=[environment_update_result["id"]]
+    )
+    retrieve_response = admin_client.get(retrieve_audit_log_url)
+    retrieve_response_json = retrieve_response.json()
+    assert len(retrieve_response_json["change_details"]) == 1
+    assert retrieve_response_json["change_details"][0]["field"] == "name"
+    assert retrieve_response_json["change_details"][0]["new"] == new_name
+    assert retrieve_response_json["change_details"][0]["old"] == environment_name
+
+
+def test_retrieve_audit_log_for_feature_state_enabled_change(
+    admin_client: APIClient,
+    environment_api_key: str,
+    environment: int,
+    feature: int,
+    feature_state: int,
+) -> None:
+    # Given
+    # we update the state and value of a feature in an environment
+    update_feature_state_url = reverse(
+        "api-v1:environments:environment-featurestates-detail",
+        args=[environment_api_key, feature_state],
+    )
+    data = {"enabled": True}
+    admin_client.patch(
+        update_feature_state_url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # we list the audit log to get an audit record that was created when we update the feature above
+    list_audit_log_url = reverse("api-v1:audit-list")
+    list_response = admin_client.get(list_audit_log_url)
+    list_results = list_response.json()["results"]
+
+    # When
+    # We retrieve the records directly where we updated the feature state
+    flag_state_update_result = next(
+        filter(
+            lambda r: r["log"].startswith("Flag state updated"),
+            list_results,
+        )
+    )
+    retrieve_audit_log_url = reverse(
+        "api-v1:audit-detail", args=[flag_state_update_result["id"]]
+    )
+    retrieve_response = admin_client.get(retrieve_audit_log_url)
+
+    # Then
+    retrieve_response_json = retrieve_response.json()
+    assert len(retrieve_response_json["change_details"]) == 1
+    assert retrieve_response_json["change_details"][0]["field"] == "enabled"
+    assert retrieve_response_json["change_details"][0]["new"] is True
+    assert retrieve_response_json["change_details"][0]["old"] is False
+
+
+def test_retrieve_audit_log_for_feature_state_value_change(
+    admin_client: APIClient,
+    environment_api_key: str,
+    environment: int,
+    feature: int,
+    feature_state: int,
+    default_feature_value: str,
+) -> None:
+    # Given
+    # we update the state and value of a feature in an environment
+    new_value = "foobar"
+    update_feature_state_url = reverse(
+        "api-v1:environments:environment-featurestates-detail",
+        args=[environment_api_key, feature_state],
+    )
+    data = {"feature_state_value": new_value}
+    admin_client.patch(
+        update_feature_state_url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # we list the audit log to get an audit record that was created when we update the feature above
+    list_audit_log_url = reverse("api-v1:audit-list")
+    list_response = admin_client.get(list_audit_log_url)
+    list_results = list_response.json()["results"]
+
+    # When
+    # We retrieve the records directly where we updated the feature state
+    flag_state_update_result = next(
+        filter(
+            lambda r: r["log"].startswith("Remote config value updated"),
+            list_results,
+        )
+    )
+    retrieve_audit_log_url = reverse(
+        "api-v1:audit-detail", args=[flag_state_update_result["id"]]
+    )
+    retrieve_response = admin_client.get(retrieve_audit_log_url)
+
+    # Then
+    retrieve_response_json = retrieve_response.json()
+    assert len(retrieve_response_json["change_details"]) == 1
+    assert retrieve_response_json["change_details"][0]["field"] == "string_value"
+    assert retrieve_response_json["change_details"][0]["new"] == new_value
+    assert retrieve_response_json["change_details"][0]["old"] == default_feature_value
+
+
+def test_retrieve_audit_log_does_not_include_change_details_for_non_update(
+    admin_client: APIClient, project: int, environment: str
+) -> None:
+    # Given
+    # we list the audit log to get an audit record that was created when
+    # the environment was created in the fixture
+    list_audit_log_url = reverse("api-v1:audit-list")
+    list_response = admin_client.get(list_audit_log_url)
+
+    environment_update_result = next(
+        filter(
+            lambda r: r["log"].startswith("New Environment created"),
+            list_response.json()["results"],
+        )
+    )
+
+    # When
+    retrieve_audit_log_url = reverse(
+        "api-v1:audit-detail", args=[environment_update_result["id"]]
+    )
+    retrieve_response = admin_client.get(retrieve_audit_log_url)
+
+    # Then
+    assert retrieve_response.json()["change_details"] == []