Skip to content

[fix]: Prevent FallbackMixin from generating spurious migrations #1231 #604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shivsubh wants to merge 1 commit into
base: master
Choose a base branch
from
+7 −2
Open

[fix]: Prevent FallbackMixin from generating spurious migrations #1231 #604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions openwisp_utils/fields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,13 @@ 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 from_db_value(self, value, expression, connection):
Expand Down
6 changes: 6 additions & 0 deletions tests/test_project/tests/test_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,9 @@ 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):
field = OrganizationRadiusSettings._meta.get_field("is_active")
name, path, args, kwargs = field.deconstruct()
self.assertNotIn("fallback", kwargs)
Comment on lines +184 to +187
Copy link

@coderabbitai coderabbitai bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider testing multiple field types for better coverage.

The test only verifies FallbackBooleanChoiceField. While the deconstruct() logic is in FallbackMixin (inherited by all subclasses), testing additional field types would provide more confidence, especially given different parent classes (CharField, DecimalField, PositiveIntegerField, etc.) may have their own deconstruct() behavior.

♻️ Suggested test expansion 
 def test_fallback_field_deconstruct(self):
-    field = OrganizationRadiusSettings._meta.get_field("is_active")
-    name, path, args, kwargs = field.deconstruct()
-    self.assertNotIn("fallback", kwargs)
+    """Verify fallback is not included in deconstruct() for all field types."""
+    test_cases = [
+        (OrganizationRadiusSettings, "is_active"),  # FallbackBooleanChoiceField
+        (OrganizationRadiusSettings, "is_first_name_required"),  # FallbackCharChoiceField
+        (OrganizationRadiusSettings, "greeting_text"),  # FallbackCharField
+        (OrganizationRadiusSettings, "password_reset_url"),  # FallbackURLField
+        (OrganizationRadiusSettings, "extra_config"),  # FallbackTextField
+        (Shelf, "books_count"),  # FallbackPositiveIntegerField
+        (Book, "price"),  # FallbackDecimalField
+    ]
+    for model, field_name in test_cases:
+        with self.subTest(model=model.__name__, field=field_name):
+            field = model._meta.get_field(field_name)
+            name, path, args, kwargs = field.deconstruct()
+            self.assertNotIn("fallback", kwargs)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/test_project/tests/test_model.py` around lines 184 - 187, Expand
test_fallback_field_deconstruct to cover other field types that use
FallbackMixin: fetch additional fields from OrganizationRadiusSettings via
OrganizationRadiusSettings._meta.get_field for examples such as a CharField,
DecimalField, and PositiveIntegerField (use the actual field names present on
the model), call field.deconstruct() for each, and assert that "fallback" is not
present in the returned kwargs; ensure you reference the existing test name
test_fallback_field_deconstruct and the model field retrieval pattern so the new
assertions mirror the existing is_active check.


Loading