diff --git a/auditlog/management/commands/auditlogmigratejson.py b/auditlog/management/commands/auditlogmigratejson.py index db27ef6..44871a8 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 import CommandError, CommandParser from django.core.management.base import BaseCommand from auditlog.models import LogEntry @@ -8,21 +9,30 @@ from auditlog.models import LogEntry class Command(BaseCommand): help = "Migrates changes from changes_text to json changes." + requires_migrations_checks = True - def add_arguments(self, parser): - parser.add_argument( + def add_arguments(self, parser: CommandParser): + group = parser.add_argument_group() + group.add_argument( + "--check", + action="store_true", + help="Just check the status of the migration", + dest="check", + ) + group.add_argument( "-d", "--database", default=None, + metavar="The database engine", help="If provided, the script will use native db operations. " - "Otherwise, it will use LogEntry.objects.bulk_create", + "Otherwise, it will use LogEntry.objects.bulk_update", dest="db", type=str, choices=["postgres", "mysql", "oracle"], ) - parser.add_argument( + group.add_argument( "-b", - "--bactch-size", + "--batch-size", 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.", @@ -33,18 +43,23 @@ class Command(BaseCommand): def handle(self, *args, **options): database = options["db"] batch_size = options["batch_size"] + check = options["check"] - if not self.check_logs(): + if (not self.check_logs()) or check: return if database: result = self.migrate_using_sql(database) self.stdout.write( - f"Updated {result} records using native database operations." + self.style.SUCCESS( + f"Updated {result} records using native database operations." + ) ) else: result = self.migrate_using_django(batch_size) - self.stdout.write(f"Updated {result} records using django operations.") + self.stdout.write( + self.style.SUCCESS(f"Updated {result} records using django operations.") + ) self.check_logs() @@ -54,11 +69,12 @@ class Command(BaseCommand): self.stdout.write(f"There are {count} records that needs migration.") return True - self.stdout.write("All records are have been migrated.") + self.stdout.write(self.style.SUCCESS("All records have been migrated.")) if settings.AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT: - self.stdout.write( - "You can now set AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT to False." + var_msg = self.style.WARNING( + "AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT" ) + self.stdout.write(f"You can now set {var_msg} to False.") return False @@ -72,17 +88,25 @@ class Command(BaseCommand): import json updated = [] + errors = [] for log in _logs: try: log.changes = json.loads(log.changes_text) except ValueError: - self.stderr.write( - f"ValueError was raised while migrating the log with id {log.id}." - ) + errors.append(log.id) else: updated.append(log) LogEntry.objects.bulk_update(updated, fields=["changes"]) + if errors: + self.stderr.write( + self.style.ERROR( + f"ValueError was raised while converting the logs with these ids into json." + f"They where not be included in this migration batch." + f"\n" + f"{errors}" + ) + ) return len(updated) logs = self.get_logs() @@ -107,8 +131,8 @@ class Command(BaseCommand): if database == "postgres": return postgres() - else: - self.stderr.write( - "Not yet implemented. Run this management command without passing a -d/--database argument." - ) - return 0 + + raise CommandError( + f"Migrating the records using {database} is not implemented. " + f"Run this management command without passing a -d/--database argument." + ) diff --git a/auditlog_tests/test_two_step_json_migration.py b/auditlog_tests/test_two_step_json_migration.py index 8470cbf..5beb560 100644 --- a/auditlog_tests/test_two_step_json_migration.py +++ b/auditlog_tests/test_two_step_json_migration.py @@ -2,7 +2,7 @@ import json from io import StringIO from unittest.mock import patch -from django.core.management import call_command +from django.core.management import CommandError, call_command from django.test import TestCase, override_settings from auditlog.models import LogEntry @@ -52,7 +52,7 @@ class AuditlogMigrateJsonTest(TestCase): def test_nothing_to_migrate(self): outbuf, errbuf = self.call_command() - msg = "All records are have been migrated." + msg = "All records have been migrated." self.assertEqual(outbuf, msg) @override_settings(AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT=True) @@ -60,12 +60,25 @@ class AuditlogMigrateJsonTest(TestCase): outbuf, errbuf = self.call_command() msg = ( - "All records are have been migrated.\n" + "All records have been migrated.\n" "You can now set AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT to False." ) self.assertEqual(outbuf, msg) + def test_check(self): + # Arrange + log_entry = self.make_logentry() + + # Act + outbuf, errbuf = self.call_command("--check") + log_entry.refresh_from_db() + + # Assert + self.assertEqual("There are 1 records that needs migration.", outbuf) + self.assertEqual("", errbuf) + self.assertIsNone(log_entry.changes) + def test_using_django(self): # Arrange log_entry = self.make_logentry() @@ -125,14 +138,18 @@ class AuditlogMigrateJsonTest(TestCase): def test_native_unsupported(self): # Arrange log_entry = self.make_logentry() + msg = ( + "Migrating the records using oracle is not implemented. " + "Run this management command without passing a -d/--database argument." + ) # Act - outbuf, errbuf = self.call_command("-d=oracle") + with self.assertRaises(CommandError) as cm: + self.call_command("-d=oracle") log_entry.refresh_from_db() # Assert - msg = "Not yet implemented. Run this management command without passing a -d/--database argument." - self.assertEqual(errbuf, msg) + self.assertEqual(msg, cm.exception.args[0]) self.assertIsNone(log_entry.changes) def test_using_django_with_error(self): @@ -146,6 +163,11 @@ class AuditlogMigrateJsonTest(TestCase): log_entry.refresh_from_db() # Assert - msg = f"ValueError was raised while migrating the log with id {log_entry.id}." - self.assertEqual(errbuf, msg) + msg = ( + f"ValueError was raised while converting the logs with these ids into json." + f"They where not be included in this migration batch." + f"\n" + f"{[log_entry.id]}" + ) + self.assertEqual(msg, errbuf) self.assertIsNone(log_entry.changes)