[fix]: Prevent FallbackMixin from generating spurious migrations

The deconstruct() method was serializing the fallback kwarg into Django migration files. This caused new migrations to be generated whenever the fallback default value changed in settings, even though no actual database schema change had occurred.

The fix removes fallback from deconstruct() so Django no longer tracks it as part of the field migration state. fallback is also made optional in __init__ (defaulting to None) so existing migrations that omit the kwarg remain valid.

Fixes: #1231

Author: shivsubh

openwisp_utils/fields.py

@@ -48,16 +48,20 @@ class FallbackMixin(object):
     """
 
     def __init__(self, *args, **kwargs):
-        self.fallback = kwargs.pop("fallback")
+        self.fallback = kwargs.pop("fallback", None)
         opts = dict(blank=True, null=True, default=None)
         opts.update(kwargs)
         super().__init__(*args, **opts)
 
     def deconstruct(self):
         name, path, args, kwargs = super().deconstruct()
-        kwargs["fallback"] = self.fallback
         return (name, path, args, kwargs)
 
+    def clone(self):
+        obj = super().clone()
+        obj.fallback = self.fallback
+        return obj
+
     def from_db_value(self, value, expression, connection):
         """Called when fetching value from the database."""
         if value is None:

tests/test_project/tests/test_model.py

@@ -2,6 +2,7 @@
 
 from django.core.exceptions import ValidationError
 from django.db import connection
+from django.db.migrations.autodetector import MigrationAutodetector
 from django.test import TestCase
 
 from ..models import Book, OrganizationRadiusSettings, Project, Shelf
@@ -180,3 +181,69 @@ def test_fallback_decimal_field(self):
             book.save(update_fields=["price"])
             book.refresh_from_db(fields=["price"])
             self.assertEqual(book.price, 56)
+
+    def test_fallback_field_deconstruct(self):
+        with self.subTest("FallbackBooleanChoiceField"):
+            field = OrganizationRadiusSettings._meta.get_field("is_active")
+            name, path, args, kwargs = field.deconstruct()
+            self.assertNotIn("fallback", kwargs)
+        with self.subTest("FallbackCharField"):
+            field = OrganizationRadiusSettings._meta.get_field("greeting_text")
+            name, path, args, kwargs = field.deconstruct()
+            self.assertNotIn("fallback", kwargs)
+        with self.subTest("FallbackDecimalField"):
+            field = Book._meta.get_field("price")
+            name, path, args, kwargs = field.deconstruct()
+            self.assertNotIn("fallback", kwargs)
+        with self.subTest("FallbackPositiveIntegerField"):
+            field = Shelf._meta.get_field("books_count")
+            name, path, args, kwargs = field.deconstruct()
+            self.assertNotIn("fallback", kwargs)
+        with self.subTest("Plain field without fallback"):
+            field = Shelf._meta.get_field("name")
+            name, path, args, kwargs = field.deconstruct()
+            self.assertNotIn("fallback", kwargs)
+
+    def test_fallback_field_no_migration_on_fallback_change(self):
+        import importlib
+
+        from django.db.migrations.autodetector import MigrationAutodetector
+        from django.db.migrations.loader import MigrationLoader
+        from django.db.migrations.questioner import NonInteractiveMigrationQuestioner
+
+        loader = MigrationLoader(None, ignore_no_migrations=True)
+        current_state = loader.project_state()
+        recorded_state = current_state.clone()
+
+        new_fallback_by_field = {
+            "is_active": True,
+            "price": 99.0,
+            "books_count": 999,
+        }
+        field_specs = [
+            ("test_project", "organizationradiussettings", "is_active"),
+            ("test_project", "book", "price"),
+            ("test_project", "shelf", "books_count"),
+        ]
+        for app_label, model_name, field_name in field_specs:
+            live_field = current_state.models[(app_label, model_name)].fields[
+                field_name
+            ]
+            _, path, orig_args, orig_kwargs = live_field.deconstruct()
+            orig_kwargs["fallback"] = new_fallback_by_field[field_name]
+            module_path, class_name = path.rsplit(".", 1)
+            cls = getattr(importlib.import_module(module_path), class_name)
+            recorded_state.models[(app_label, model_name)].fields[field_name] = cls(
+                *orig_args, **orig_kwargs
+            )
+
+        changes = MigrationAutodetector(
+            recorded_state,
+            current_state,
+            NonInteractiveMigrationQuestioner(),
+        ).changes(graph=loader.graph)
+        self.assertEqual(
+            changes,
+            {},
+            msg="Changing the fallback value should not generate migrations",
+        )