diff --git a/CHANGELOG.md b/CHANGELOG.md index c63e855..6576baf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ #### Fixes +- fix: Foreign key values are used to check for changes in related fields instead of object representations. When changes are detected, the foreign key value is persisted in `LogEntry.changes` field instead of object representations. ([#420](https://github.com/jazzband/django-auditlog/pull/420)) - fix: Display `created` timestamp in server timezone ([#404](https://github.com/jazzband/django-auditlog/pull/404)) - fix: Handle port in `remote_addr` ([#417](https://github.com/jazzband/django-auditlog/pull/417)) - fix: Handle the error with AttributeError: 'OneToOneRel' error occur during a `PolymorphicModel` has relation with other models ([#429](https://github.com/jazzband/django-auditlog/pull/429)) diff --git a/auditlog/diff.py b/auditlog/diff.py index 0d25c86..cf62504 100644 --- a/auditlog/diff.py +++ b/auditlog/diff.py @@ -74,6 +74,10 @@ def get_field_value(obj, field): value = django_timezone.make_naive(value, timezone=timezone.utc) elif isinstance(field, JSONField): value = field.to_python(getattr(obj, field.name, None)) + elif (field.one_to_one or field.many_to_one) and hasattr(field, "rel_class"): + value = smart_str( + getattr(obj, field.get_attname(), None), strings_only=True + ) else: value = smart_str(getattr(obj, field.name, None)) except ObjectDoesNotExist: diff --git a/auditlog/models.py b/auditlog/models.py index 8f98fce..066fe1c 100644 --- a/auditlog/models.py +++ b/auditlog/models.py @@ -2,7 +2,7 @@ import ast import json from copy import deepcopy from datetime import timezone -from typing import Any, Dict, List +from typing import Any, Dict, List, Union from dateutil import parser from dateutil.tz import gettz @@ -10,7 +10,11 @@ from django.conf import settings from django.contrib.contenttypes.fields import GenericRelation from django.contrib.contenttypes.models import ContentType from django.core import serializers -from django.core.exceptions import FieldDoesNotExist +from django.core.exceptions import ( + FieldDoesNotExist, + ObjectDoesNotExist, + ValidationError, +) from django.db import DEFAULT_DB_ALIAS, models from django.db.models import Q, QuerySet from django.utils import formats @@ -491,6 +495,9 @@ class LogEntry(models.Model): value = formats.localize(value) except ValueError: pass + elif field_type in ["ForeignKey", "OneToOneField"]: + value = self._get_changes_display_for_fk_field(field, value) + # check if length is longer than 140 and truncate with ellipsis if len(value) > 140: value = f"{value[:140]}..." @@ -502,6 +509,31 @@ class LogEntry(models.Model): changes_display_dict[verbose_name] = values_display return changes_display_dict + def _get_changes_display_for_fk_field( + self, field: Union[models.ForeignKey, models.OneToOneField], value: Any + ) -> str: + """ + :return: A string representing a given FK value and the field to which it belongs + """ + # Return "None" if the FK value is "None". + if value == "None": + return value + + # Attempt to convert given value to the PK type for the related model + try: + pk_value = field.related_model._meta.pk.to_python(value) + # ValidationError will handle legacy values where string representations were + # stored rather than PKs. This will also handle cases where the PK type is + # changed between the time the LogEntry is created and this method is called. + except ValidationError: + return value + # Attempt to return the string representation of the object + try: + return smart_str(field.related_model.objects.get(pk=pk_value)) + # ObjectDoesNotExist will be raised if the object was deleted. + except ObjectDoesNotExist: + return f"Deleted '{field.related_model.__name__}' ({value})" + class AuditlogHistoryField(GenericRelation): """ diff --git a/auditlog_tests/models.py b/auditlog_tests/models.py index 1a6c5a2..10a5503 100644 --- a/auditlog_tests/models.py +++ b/auditlog_tests/models.py @@ -22,6 +22,9 @@ class SimpleModel(models.Model): history = AuditlogHistoryField() + def __str__(self): + return self.text + class AltPrimaryKeyModel(models.Model): """ diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 04f3975..4b46773 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -1684,6 +1684,142 @@ class ModelInstanceDiffTest(TestCase): ) +class TestRelatedDiffs(TestCase): + def setUp(self): + self.test_date = datetime.datetime(2022, 1, 1, 12, tzinfo=datetime.timezone.utc) + + def test_log_entry_changes_on_fk_object_update(self): + t1 = self.test_date + with freezegun.freeze_time(t1): + simple = SimpleModel.objects.create() + one_simple = SimpleModel.objects.create() + two_simple = SimpleModel.objects.create() + instance = RelatedModel.objects.create( + one_to_one=simple, related=one_simple + ) + + t2 = self.test_date + datetime.timedelta(days=20) + with freezegun.freeze_time(t2): + instance.related = two_simple + instance.save() + + log_one = instance.history.filter(timestamp=t1).first() + log_two = instance.history.filter(timestamp=t2).first() + self.assertTrue(isinstance(log_one, LogEntry)) + self.assertTrue(isinstance(log_two, LogEntry)) + + self.assertEqual(int(log_one.changes_dict["related"][1]), one_simple.id) + self.assertEqual(int(log_one.changes_dict["one_to_one"][1]), simple.id) + self.assertEqual(int(log_two.changes_dict["related"][1]), two_simple.id) + + def test_log_entry_changes_on_fk_id_update(self): + t1 = self.test_date + with freezegun.freeze_time(t1): + simple = SimpleModel.objects.create() + one_simple = SimpleModel.objects.create() + two_simple = SimpleModel.objects.create() + instance = RelatedModel.objects.create( + one_to_one_id=int(simple.id), related_id=int(one_simple.id) + ) + + t2 = self.test_date + datetime.timedelta(days=20) + with freezegun.freeze_time(t2): + instance.related_id = int(two_simple.id) + instance.save() + + log_one = instance.history.filter(timestamp=t1).first() + log_two = instance.history.filter(timestamp=t2).first() + self.assertTrue(isinstance(log_one, LogEntry)) + self.assertTrue(isinstance(log_two, LogEntry)) + + self.assertEqual(int(log_one.changes_dict["related"][1]), one_simple.id) + self.assertEqual(int(log_one.changes_dict["one_to_one"][1]), simple.id) + self.assertEqual(int(log_two.changes_dict["related"][1]), two_simple.id) + + def test_log_entry_create_fk_changes_to_string_objects_in_display_dict(self): + t1 = self.test_date + with freezegun.freeze_time(t1): + simple = SimpleModel.objects.create(text="Test Foo") + one_simple = SimpleModel.objects.create(text="Test Bar") + instance = RelatedModel.objects.create( + one_to_one=simple, related=one_simple + ) + + log_one = instance.history.filter(timestamp=t1).first() + self.assertTrue(isinstance(log_one, LogEntry)) + display_dict = log_one.changes_display_dict + self.assertEqual(display_dict["related"][1], "Test Bar") + self.assertEqual(display_dict["related"][0], "None") + self.assertEqual(display_dict["one to one"][1], "Test Foo") + + def test_log_entry_deleted_fk_changes_to_string_objects_in_display_dict(self): + t1 = self.test_date + with freezegun.freeze_time(t1): + simple = SimpleModel.objects.create(text="Test Foo") + one_simple = SimpleModel.objects.create(text="Test Bar") + one_simple_id = int(one_simple.id) + instance = RelatedModel.objects.create( + one_to_one=simple, related=one_simple + ) + + t2 = self.test_date + datetime.timedelta(days=20) + with freezegun.freeze_time(t2): + one_simple.delete() + + log_two = LogEntry.objects.filter(object_id=instance.id, timestamp=t2).first() + self.assertTrue(isinstance(log_two, LogEntry)) + display_dict = log_two.changes_display_dict + self.assertEqual( + display_dict["related"][0], f"Deleted 'SimpleModel' ({one_simple_id})" + ) + self.assertEqual(display_dict["related"][1], "None") + + def test_no_log_entry_created_on_related_object_string_update(self): + t1 = self.test_date + with freezegun.freeze_time(t1): + simple = SimpleModel.objects.create(text="Test Foo") + one_simple = SimpleModel.objects.create(text="Test Bar") + instance = RelatedModel.objects.create( + one_to_one=simple, related=one_simple + ) + + t2 = self.test_date + datetime.timedelta(days=20) + with freezegun.freeze_time(t2): + # Order is important. Without special FK handling, the arbitrary in memory + # changes to the (same) related object's signature result in a perceived + # update where no update has occurred. + one_simple.text = "Test Baz" + instance.save() + one_simple.save() + + # Assert that only one log for the instance was created + self.assertEqual(instance.history.all().count(), 1) + # Assert that two logs were created for the parent object + self.assertEqual(one_simple.history.all().count(), 2) + + def test_log_entry_created_if_obj_strings_are_same_for_two_objs(self): + """FK changes trigger update when the string representation is the same.""" + t1 = self.test_date + with freezegun.freeze_time(t1): + simple = SimpleModel.objects.create(text="Test Foo") + one_simple = SimpleModel.objects.create(text="Twinsies", boolean=True) + two_simple = SimpleModel.objects.create(text="Twinsies", boolean=False) + instance = RelatedModel.objects.create( + one_to_one=simple, related=one_simple + ) + + t2 = self.test_date + datetime.timedelta(days=20) + with freezegun.freeze_time(t2): + instance.related = two_simple + instance.save() + + self.assertEqual(instance.history.all().count(), 2) + log_create = instance.history.filter(timestamp=t1).first() + log_update = instance.history.filter(timestamp=t2).first() + self.assertEqual(int(log_create.changes_dict["related"][1]), one_simple.id) + self.assertEqual(int(log_update.changes_dict["related"][1]), two_simple.id) + + class TestModelSerialization(TestCase): def setUp(self): super().setUp()