From cc3601d3b9a93e401f1f5c709a96e67ad25fdd4f Mon Sep 17 00:00:00 2001 From: Shubham Date: Sun, 8 Mar 2026 20:56:25 +0530 Subject: [PATCH] [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 --- openwisp_utils/fields.py | 10 ++-- tests/test_project/tests/test_model.py | 69 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/openwisp_utils/fields.py b/openwisp_utils/fields.py index eaf747d63..73d798103 100644 --- a/openwisp_utils/fields.py +++ b/openwisp_utils/fields.py @@ -48,15 +48,15 @@ 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.""" diff --git a/tests/test_project/tests/test_model.py b/tests/test_project/tests/test_model.py index bb46a1dbb..0cbcf9e7e 100644 --- a/tests/test_project/tests/test_model.py +++ b/tests/test_project/tests/test_model.py @@ -2,6 +2,9 @@ from django.core.exceptions import ValidationError from django.db import connection +from django.db.migrations.autodetector import MigrationAutodetector +from django.db.migrations.loader import MigrationLoader +from django.db.migrations.questioner import NonInteractiveMigrationQuestioner from django.test import TestCase from ..models import Book, OrganizationRadiusSettings, Project, Shelf @@ -180,3 +183,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): + test_cases = [ + ("FallbackBooleanChoiceField", OrganizationRadiusSettings, "is_active"), + ("FallbackCharField", OrganizationRadiusSettings, "greeting_text"), + ("FallbackDecimalField", Book, "price"), + ("FallbackPositiveIntegerField", Shelf, "books_count"), + ("Plain field without fallback", Shelf, "name"), + ] + for field_type, model, field_name in test_cases: + with self.subTest(field_type): + field = model._meta.get_field(field_name) + name, path, args, kwargs = field.deconstruct() + self.assertNotIn("fallback", kwargs) + + def test_fallback_field_no_migration_on_fallback_change(self): + 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 + ] + name, path, orig_args, orig_kwargs = live_field.deconstruct() + orig_kwargs["fallback"] = new_fallback_by_field[field_name] + recorded_state.models[(app_label, model_name)].fields[field_name] = ( + live_field.__class__(*orig_args, **orig_kwargs) + ) + + changes = MigrationAutodetector( + recorded_state, + current_state, + NonInteractiveMigrationQuestioner(), + ).changes(graph=loader.graph) + self.assertEqual(changes, {}) + + def test_fallback_field_clone_preserves_fallback(self): + test_cases = [ + ("FallbackBooleanChoiceField", OrganizationRadiusSettings, "is_active"), + ( + "FallbackCharChoiceField", + OrganizationRadiusSettings, + "is_first_name_required", + ), + ("FallbackDecimalField", Book, "price"), + ("FallbackPositiveIntegerField", Shelf, "books_count"), + ("FallbackTextField", OrganizationRadiusSettings, "extra_config"), + ("FallbackURLField", OrganizationRadiusSettings, "password_reset_url"), + ("FallbackCharField", OrganizationRadiusSettings, "greeting_text"), + ] + for field_type, model, field_name in test_cases: + with self.subTest(field_type): + field = model._meta.get_field(field_name) + cloned = field.clone() + self.assertEqual(cloned.fallback, field.fallback)