diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fbce33b..7404b0f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ #### 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)) +- 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))([#495](https://github.com/jazzband/django-auditlog/pull/495)) #### Improvements diff --git a/auditlog/management/commands/auditlogmigratejson.py b/auditlog/management/commands/auditlogmigratejson.py index f03304b8..2a4ad78b 100644 --- a/auditlog/management/commands/auditlogmigratejson.py +++ b/auditlog/management/commands/auditlogmigratejson.py @@ -1,6 +1,7 @@ +from math import ceil + from django.conf import settings from django.core.management.base import BaseCommand -from django.core.paginator import Paginator from auditlog.models import LogEntry @@ -13,7 +14,8 @@ def add_arguments(self, parser): "-d", "--database", default=None, - help="If provided, we will use native db operations.", + help="If provided, the script will use native db operations. " + "Otherwise, it will use LogEntry.objects.bulk_create", dest="db", type=str, choices=["postgres", "mysql", "oracle"], @@ -21,8 +23,9 @@ def add_arguments(self, parser): parser.add_argument( "-b", "--bactch-size", - default=None, - help="Split the migration into multiple batches", + default=500, + help="Split the migration into multiple batches. If 0, then no batching will be done. " + "When passing a -d/database, the batch value will be ignored.", dest="batch_size", type=int, ) @@ -83,12 +86,13 @@ def _apply_django_migration(_logs) -> int: return len(updated) logs = self.get_logs() + if not batch_size: return _apply_django_migration(logs) total_updated = 0 - for page in Paginator(logs, batch_size): - total_updated += _apply_django_migration(page.object_list) + for _ in range(ceil(logs.count() / batch_size)): + total_updated += _apply_django_migration(self.get_logs()[:batch_size]) return total_updated def migrate_using_sql(self, database): diff --git a/auditlog/migrations/0015_alter_logentry_changes.py b/auditlog/migrations/0015_alter_logentry_changes.py index 6e2e54c3..27299875 100644 --- a/auditlog/migrations/0015_alter_logentry_changes.py +++ b/auditlog/migrations/0015_alter_logentry_changes.py @@ -24,7 +24,7 @@ def two_step_migrations() -> List: migrations.AddField( model_name="logentry", name="changes_text", - field=models.TextField(null=True, verbose_name="change message"), + field=models.TextField(blank=True, verbose_name="text change message"), ), migrations.AlterField( model_name="logentry", @@ -40,6 +40,4 @@ class Migration(migrations.Migration): ("auditlog", "0014_logentry_cid"), ] - atomic = not settings.AUDITLOG_TWO_STEP_MIGRATION - operations = [*two_step_migrations()] diff --git a/auditlog/models.py b/auditlog/models.py index 59ec66cf..85e3db15 100644 --- a/auditlog/models.py +++ b/auditlog/models.py @@ -357,7 +357,7 @@ class Action: action = models.PositiveSmallIntegerField( choices=Action.choices, verbose_name=_("action"), db_index=True ) - changes_text = models.TextField(null=True, verbose_name=_("change message")) + changes_text = 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, diff --git a/auditlog_tests/test_two_step_json_migration.py b/auditlog_tests/test_two_step_json_migration.py index 904c39a6..94a5ddec 100644 --- a/auditlog_tests/test_two_step_json_migration.py +++ b/auditlog_tests/test_two_step_json_migration.py @@ -1,5 +1,4 @@ import json -import unittest from io import StringIO from unittest.mock import patch @@ -35,8 +34,8 @@ def test_use_text_changes_first(self): class AuditlogMigrateJsonTest(TestCase): - def make_logentry(self, text="I am a simple model."): - model = SimpleModel.objects.create(text=text) + def make_logentry(self): + model = SimpleModel.objects.create(text="I am a simple model.") log_entry: LogEntry = model.history.first() log_entry.changes_text = json.dumps(log_entry.changes) log_entry.changes = None @@ -73,7 +72,7 @@ def test_using_django(self): log_entry = self.make_logentry() # Act - outbuf, errbuf = self.call_command() + outbuf, errbuf = self.call_command("-b=0") log_entry.refresh_from_db() # Assert @@ -85,42 +84,10 @@ def test_using_django_batched(self): log_entry_1 = self.make_logentry() log_entry_2 = self.make_logentry() - # Act - outbuf, errbuf = self.call_command("-b=2") - log_entry_1.refresh_from_db() - log_entry_2.refresh_from_db() - - # Assert - self.assertEqual(errbuf, "") - self.assertIsNotNone(log_entry_1.changes) - self.assertIsNotNone(log_entry_2.changes) - - @unittest.skip("Flaky test descriped below.") - def test_using_django_batched_with_batch_size_1(self): - """ - Weird things happen when I use b=1. - When I click debug, the test works, but when I click run, it fails. - - What happens when I click run is: - 1. printing the logs variable returns all 3 logs in the correct order - 2. the last page's `page.object_list` is an empty queryset. - 3. page 1 has l1 and page 2 has l3 (no l2) - - Changing the batch size to 2, page 1 has l1 and l2, and page 2 is empty. - Could this be an issue with django's paginator? I don't know. - Anyways, I don't think this is an issue as users can run the script twice and it should work. - """ - - # Arrange - log_entry_1 = self.make_logentry("l1") - log_entry_2 = self.make_logentry("l2") - log_entry_3 = self.make_logentry("l3") - # Act outbuf, errbuf = self.call_command("-b=1") log_entry_1.refresh_from_db() log_entry_2.refresh_from_db() - log_entry_3.refresh_from_db() # Assert self.assertEqual(errbuf, "")