diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index 9f996f60..19c2754b 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -2,7 +2,7 @@ from django.conf import settings from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Model, NOT_PROVIDED, DateTimeField +from django.db.models import Model, NOT_PROVIDED, DateTimeField, ForeignKey from django.utils import timezone from django.utils.encoding import smart_text @@ -134,6 +134,14 @@ def model_instance_diff(old, new): old_value = get_field_value(old, field) new_value = get_field_value(new, field) + # Handle cases where a models ForeignKey field is saved using the + # '*_id' field and not the ForeignKey field itself. In this situation, + # the fields will appear to be identical when comparing the objects, + # but different when comparing the '*_id' fields. + if isinstance(field, ForeignKey) and old_value == new_value: + old_value = smart_text(getattr(old, field.attname, None)) + new_value = smart_text(getattr(new, field.attname, None)) + if old_value != new_value: diff[field.name] = (smart_text(old_value), smart_text(new_value)) diff --git a/src/auditlog_tests/models.py b/src/auditlog_tests/models.py index a0b114eb..5a0d32b0 100644 --- a/src/auditlog_tests/models.py +++ b/src/auditlog_tests/models.py @@ -66,7 +66,9 @@ class RelatedModel(models.Model): A model with a foreign key. """ - related = models.ForeignKey(to='self', on_delete=models.CASCADE) + related = models.ForeignKey( + to='self', on_delete=models.CASCADE, + null=True, blank=True) history = AuditlogHistoryField() diff --git a/src/auditlog_tests/tests.py b/src/auditlog_tests/tests.py index fb1a6e3d..12f0c57e 100644 --- a/src/auditlog_tests/tests.py +++ b/src/auditlog_tests/tests.py @@ -102,6 +102,55 @@ def setUp(self): self.obj = ProxyModel.objects.create(text='I am not what you think.') +class RelatedModelTest(TestCase): + """ + Test the behaviour of relationships. + """ + def setUp(self): + self.obj = RelatedModel.objects.create() + self.rel_obj = RelatedModel.objects.create(related=self.obj) + + def test_related(self): + # Creation of 'obj' should create single history record. + self.assertEqual(LogEntry.objects.get_for_object(self.obj).count(), 1) + # There is no 'related' object (None) so no history on it. + self.assertEqual(LogEntry.objects.get_for_object(self.obj.related).count(), 0) + + # Creation of 'rel_obj' should create single history record. + self.assertEqual(LogEntry.objects.get_for_object(self.rel_obj).count(), 1) + # The 'related' object is 'obj' and should have a single record as well. + self.assertEqual(LogEntry.objects.get_for_object(self.rel_obj.related).count(), 1) + + # Set a 'related' object. + self.obj.related = self.rel_obj + self.obj.save() + + # The 'obj' model now has two history records. + self.assertEqual(LogEntry.objects.get_for_object(self.obj).count(), 2) + # The 'related' object is now 'rel_obj' and has a single history record. + self.assertEqual(LogEntry.objects.get_for_object(self.obj.related).count(), 1) + + # Wipe out the relationship by changing the '*_id' field. + self.obj.related_id = None + self.obj.save() + self.obj.refresh_from_db() + + # The 'obj' model now has three history records. + self.assertEqual(LogEntry.objects.get_for_object(self.obj).count(), 3) + # The 'related' object is now 'None' so no history records. + self.assertEqual(LogEntry.objects.get_for_object(self.obj.related).count(), 0) + + # Set the relationship back using the '*_id' field. + self.obj.related_id = self.rel_obj.id + self.obj.save() + self.obj.refresh_from_db() + + # The 'obj' model now has four history records. + self.assertEqual(LogEntry.objects.get_for_object(self.obj).count(), 4) + # The 'related' object is now 'rel_obj' and has a single history record. + self.assertEqual(LogEntry.objects.get_for_object(self.obj.related).count(), 1) + + class ManyRelatedModelTest(TestCase): """ Test the behaviour of a many-to-many relationship.