Skip stringify if json (#355)

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changes
 
+#### Fixes
+
+- Fix inconsistent changes with JSONField ([#355](https://github.com/jazzband/django-auditlog/pull/355))
+
 ## 2.0.0 (2022-05-09)
 
 #### Improvements

auditlog/diff.py

@@ -1,6 +1,6 @@
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
-from django.db.models import NOT_PROVIDED, DateTimeField, Model
+from django.db.models import NOT_PROVIDED, DateTimeField, JSONField, Model
 from django.utils import timezone
 from django.utils.encoding import smart_str
 
@@ -58,20 +58,19 @@ def get_field_value(obj, field):
     :return: The value of the field as a string.
     :rtype: str
     """
-    if isinstance(field, DateTimeField):
-        # DateTimeFields are timezone-aware, so we need to convert the field
-        # to its naive form before we can accurately compare them for changes.
-        try:
+    try:
+        if isinstance(field, DateTimeField):
+            # DateTimeFields are timezone-aware, so we need to convert the field
+            # to its naive form before we can accurately compare them for changes.
             value = field.to_python(getattr(obj, field.name, None))
             if value is not None and settings.USE_TZ and not timezone.is_naive(value):
                 value = timezone.make_naive(value, timezone=timezone.utc)
-        except ObjectDoesNotExist:
-            value = field.default if field.default is not NOT_PROVIDED else None
-    else:
-        try:
+        elif isinstance(field, JSONField):
+            value = field.to_python(getattr(obj, field.name, None))
+        else:
             value = smart_str(getattr(obj, field.name, None))
-        except ObjectDoesNotExist:
-            value = field.default if field.default is not NOT_PROVIDED else None
+    except ObjectDoesNotExist:
+        value = field.default if field.default is not NOT_PROVIDED else None
 
     return value
 

auditlog_tests/models.py

@@ -230,6 +230,12 @@ class NoDeleteHistoryModel(models.Model):
     history = AuditlogHistoryField(delete_related=False)
 
 
+class JSONModel(models.Model):
+    json = models.JSONField(default=dict)
+
+    history = AuditlogHistoryField(delete_related=False)
+
+
 auditlog.register(AltPrimaryKeyModel)
 auditlog.register(UUIDPrimaryKeyModel)
 auditlog.register(ProxyModel)
@@ -244,3 +250,4 @@ auditlog.register(ChoicesFieldModel)
 auditlog.register(CharfieldTextfieldModel)
 auditlog.register(PostgresArrayFieldModel)
 auditlog.register(NoDeleteHistoryModel)
+auditlog.register(JSONModel)

auditlog_tests/tests.py

@@ -22,6 +22,7 @@ from auditlog_tests.models import (
     CharfieldTextfieldModel,
     ChoicesFieldModel,
     DateTimeFieldModel,
+    JSONModel,
     ManyRelatedModel,
     NoDeleteHistoryModel,
     PostgresArrayFieldModel,
@@ -970,3 +971,90 @@ class NoDeleteHistoryTest(TestCase):
             list(entries.values_list("action", flat=True)),
             [LogEntry.Action.CREATE, LogEntry.Action.UPDATE, LogEntry.Action.DELETE],
         )
+
+
+class JSONModelTest(TestCase):
+    def setUp(self):
+        self.obj = JSONModel.objects.create()
+
+    def test_update(self):
+        """Changes on a JSONField are logged correctly."""
+        # Get the object to work with
+        obj = self.obj
+
+        # Change something
+        obj.json = {
+            "quantity": "1",
+        }
+        obj.save()
+
+        # Check for log entries
+        self.assertEqual(
+            obj.history.filter(action=LogEntry.Action.UPDATE).count(),
+            1,
+            msg="There is one log entry for 'UPDATE'",
+        )
+
+        history = obj.history.get(action=LogEntry.Action.UPDATE)
+
+        self.assertJSONEqual(
+            history.changes,
+            '{"json": ["{}", "{\'quantity\': \'1\'}"]}',
+            msg="The change is correctly logged",
+        )
+
+    def test_update_with_no_changes(self):
+        """No changes are logged."""
+        first_json = {
+            "quantity": "1814",
+            "tax_rate": "17",
+            "unit_price": "144",
+            "description": "Method form.",
+            "discount_rate": "42",
+            "unit_of_measure": "bytes",
+        }
+        obj = JSONModel.objects.create(json=first_json)
+
+        # Change the order of the keys but not the values
+        second_json = {
+            "tax_rate": "17",
+            "description": "Method form.",
+            "quantity": "1814",
+            "unit_of_measure": "bytes",
+            "unit_price": "144",
+            "discount_rate": "42",
+        }
+        obj.json = second_json
+        obj.save()
+
+        # Check for log entries
+        self.assertEqual(
+            first_json,
+            second_json,
+            msg="dicts are the same",
+        )
+        self.assertEqual(
+            obj.history.filter(action=LogEntry.Action.UPDATE).count(),
+            0,
+            msg="There is no log entry",
+        )
+
+
+class ModelInstanceDiffTest(TestCase):
+    def test_when_field_doesnt_exit(self):
+        """No error is raised and the default is returned."""
+        first = SimpleModel(boolean=True)
+        second = SimpleModel()
+
+        # then boolean should be False, as we use the default value
+        # specified inside the model
+        del second.boolean
+
+        changes = model_instance_diff(first, second)
+
+        # Check for log entries
+        self.assertEqual(
+            changes,
+            {"boolean": ("True", "False")},
+            msg="ObjectDoesNotExist should be handled",
+        )