diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a20307..c63e855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changes +#### Breaking Changes + +- feat: Change `LogEntry.change` field type to `JSONField` rather than `TextField`. This change include a migration that may take time to run depending on the number of records on your `LogEntry` table ([#407](https://github.com/jazzband/django-auditlog/pull/407)) + ## Next Release #### Improvements diff --git a/auditlog/migrations/0015_alter_logentry_changes.py b/auditlog/migrations/0015_alter_logentry_changes.py new file mode 100644 index 0000000..6743b89 --- /dev/null +++ b/auditlog/migrations/0015_alter_logentry_changes.py @@ -0,0 +1,18 @@ +# Generated by Django 4.0 on 2022-08-04 15:41 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("auditlog", "0014_logentry_cid"), + ] + + operations = [ + migrations.AlterField( + model_name="logentry", + name="changes", + field=models.JSONField(null=True, verbose_name="change message"), + ), + ] diff --git a/auditlog/models.py b/auditlog/models.py index d5b04ff..8f98fce 100644 --- a/auditlog/models.py +++ b/auditlog/models.py @@ -26,13 +26,15 @@ class LogEntryManager(models.Manager): Custom manager for the :py:class:`LogEntry` model. """ - def log_create(self, instance, **kwargs): + def log_create(self, instance, force_log: bool = False, **kwargs): """ Helper method to create a new log entry. This method automatically populates some fields when no explicit value is given. :param instance: The model instance to log a change for. :type instance: Model + :param force_log: Create a LogEntry even if no changes exist. + :type force_log: bool :param kwargs: Field overrides for the :py:class:`LogEntry` object. :return: The new log entry or `None` if there were no changes. :rtype: LogEntry @@ -42,7 +44,7 @@ class LogEntryManager(models.Manager): changes = kwargs.get("changes", None) pk = self._get_pk_value(instance) - if changes is not None: + if changes is not None or force_log: kwargs.setdefault( "content_type", ContentType.objects.get_for_model(instance) ) @@ -350,7 +352,7 @@ class LogEntry(models.Model): action = models.PositiveSmallIntegerField( choices=Action.choices, verbose_name=_("action"), db_index=True ) - changes = models.TextField(blank=True, verbose_name=_("change message")) + changes = models.JSONField(null=True, verbose_name=_("change message")) actor = models.ForeignKey( to=settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, @@ -399,10 +401,7 @@ class LogEntry(models.Model): """ :return: The changes recorded in this log entry as a dictionary object. """ - try: - return json.loads(self.changes) or {} - except ValueError: - return {} + return self.changes or {} @property def changes_str(self, colon=": ", arrow=" \u2192 ", separator="; "): diff --git a/auditlog/receivers.py b/auditlog/receivers.py index 561013e..fefa2cd 100644 --- a/auditlog/receivers.py +++ b/auditlog/receivers.py @@ -1,4 +1,3 @@ -import json from functools import wraps from django.conf import settings @@ -115,7 +114,8 @@ def _create_log_entry( LogEntry.objects.log_create( instance, action=action, - changes=json.dumps(changes), + changes=changes, + force_log=force_log, ) except BaseException as e: error = e diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index c747582..04f3975 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -105,9 +105,9 @@ class SimpleModelTest(TestCase): obj.save() def check_update_log_entry(self, obj, history): - self.assertJSONEqual( + self.assertDictEqual( history.changes, - '{"boolean": ["False", "True"]}', + {"boolean": ["False", "True"]}, msg="The change is correctly logged", ) @@ -120,9 +120,9 @@ class SimpleModelTest(TestCase): obj.save(update_fields=["boolean"]) # This implicitly asserts there is only one UPDATE change since the `.get` would fail otherwise. - self.assertJSONEqual( + self.assertDictEqual( obj.history.get(action=LogEntry.Action.UPDATE).changes, - '{"boolean": ["False", "True"]}', + {"boolean": ["False", "True"]}, msg=( "Object modifications that are not saved to DB are not logged " "when using the `update_fields`." @@ -153,9 +153,9 @@ class SimpleModelTest(TestCase): obj.integer = 1 obj.boolean = True obj.save(update_fields=None) - self.assertJSONEqual( + self.assertDictEqual( obj.history.get(action=LogEntry.Action.UPDATE).changes, - '{"boolean": ["False", "True"], "integer": ["None", "1"]}', + {"boolean": ["False", "True"], "integer": ["None", "1"]}, msg="The 2 fields changed are correctly logged", ) @@ -537,9 +537,9 @@ class SimpleIncludeModelTest(TestCase): obj.text = "Newer text" obj.save(update_fields=["text", "label"]) - self.assertJSONEqual( + self.assertDictEqual( obj.history.get(action=LogEntry.Action.UPDATE).changes, - '{"label": ["Initial label", "New label"]}', + {"label": ["Initial label", "New label"]}, msg="Only the label was logged, regardless of multiple entries in `update_fields`", ) @@ -1395,7 +1395,27 @@ class DiffMsgTest(TestCase): return LogEntry.objects.log_create( SimpleModel.objects.create(), # doesn't affect anything action=action, - changes=json.dumps(changes), + changes=changes, + ) + + def test_change_msg_create_when_exceeds_max_len(self): + log_entry = self._create_log_entry( + LogEntry.Action.CREATE, + { + "Camelopardalis": [None, "Giraffe"], + "Capricornus": [None, "Sea goat"], + "Equuleus": [None, "Little horse"], + "Horologium": [None, "Clock"], + "Microscopium": [None, "Microscope"], + "Reticulum": [None, "Net"], + "Telescopium": [None, "Telescope"], + }, + ) + + self.assertEqual( + self.admin.msg_short(log_entry), + "7 changes: Camelopardalis, Capricornus, Equuleus, Horologium, " + "Microscopium, ..", ) def test_changes_msg_delete(self): @@ -1569,9 +1589,9 @@ class JSONModelTest(TestCase): history = obj.history.get(action=LogEntry.Action.UPDATE) - self.assertJSONEqual( + self.assertDictEqual( history.changes, - '{"json": ["{}", "{\'quantity\': \'1\'}"]}', + {"json": ["{}", "{'quantity': '1'}"]}, msg="The change is correctly logged", ) @@ -1910,7 +1930,7 @@ class TestAccessLog(TestCase): self.assertEqual( log_entry.action, LogEntry.Action.ACCESS, msg="Action is 'ACCESS'" ) - self.assertEqual(log_entry.changes, "null") + self.assertIsNone(log_entry.changes) self.assertEqual(log_entry.changes_dict, {})