diff --git a/docs/developer/custom-fields.rst b/docs/developer/custom-fields.rst index 88d393cb..8553cb8a 100644 --- a/docs/developer/custom-fields.rst +++ b/docs/developer/custom-fields.rst @@ -22,12 +22,12 @@ openwisp modules. This field extends Django's `BooleanField `_ and provides additional functionality for handling choices with a fallback -value. The field will use the **fallback value** whenever the field is set -to ``None``. +value. + +.. include:: ../partials/fallback-fields.rst This field is particularly useful when you want to present a choice -between enabled and disabled options, with an additional "Default" option -that reflects the fallback value. +between enabled and disabled options. .. code-block:: python @@ -38,9 +38,6 @@ that reflects the fallback value. class MyModel(models.Model): is_active = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, fallback=app_settings.IS_ACTIVE_FALLBACK, ) @@ -50,8 +47,9 @@ that reflects the fallback value. This field extends Django's `CharField `_ and provides additional functionality for handling choices with a fallback -value. The field will use the **fallback value** whenever the field is set -to ``None``. +value. + +.. include:: ../partials/fallback-fields.rst .. code-block:: python @@ -62,8 +60,6 @@ to ``None``. class MyModel(models.Model): is_first_name_required = FallbackCharChoiceField( - null=True, - blank=True, max_length=32, choices=( ("disabled", _("Disabled")), @@ -81,8 +77,7 @@ This field extends Django's `CharField provides additional functionality for handling text fields with a fallback value. -It allows populating the form with the fallback value when the actual -value is set to ``null`` in the database. +.. include:: ../partials/fallback-fields.rst .. code-block:: python @@ -93,8 +88,6 @@ value is set to ``null`` in the database. class MyModel(models.Model): greeting_text = FallbackCharField( - null=True, - blank=True, max_length=200, fallback=app_settings.GREETING_TEXT, ) @@ -107,8 +100,7 @@ This field extends Django's `URLField provides additional functionality for handling URL fields with a fallback value. -It allows populating the form with the fallback value when the actual -value is set to ``null`` in the database. +.. include:: ../partials/fallback-fields.rst .. code-block:: python @@ -119,8 +111,6 @@ value is set to ``null`` in the database. class MyModel(models.Model): password_reset_url = FallbackURLField( - null=True, - blank=True, max_length=200, fallback=app_settings.DEFAULT_PASSWORD_RESET_URL, ) @@ -133,8 +123,7 @@ This extends Django's `TextField and provides additional functionality for handling text fields with a fallback value. -It allows populating the form with the fallback value when the actual -value is set to ``null`` in the database. +.. include:: ../partials/fallback-fields.rst .. code-block:: python @@ -145,8 +134,6 @@ value is set to ``null`` in the database. class MyModel(models.Model): extra_config = FallbackTextField( - null=True, - blank=True, max_length=200, fallback=app_settings.EXTRA_CONFIG, ) @@ -159,8 +146,7 @@ This extends Django's `PositiveIntegerField and provides additional functionality for handling positive integer fields with a fallback value. -It allows populating the form with the fallback value when the actual -value is set to ``null`` in the database. +.. include:: ../partials/fallback-fields.rst .. code-block:: python @@ -171,7 +157,29 @@ value is set to ``null`` in the database. class MyModel(models.Model): count = FallbackPositiveIntegerField( - blank=True, - null=True, fallback=app_settings.DEFAULT_COUNT, ) + +``openwisp_utils.fields.FallbackDecimalField`` +---------------------------------------------- + +This extends Django's `DecimalField +`_ +and provides additional functionality for handling decimal fields with a +fallback value. + +.. include:: ../partials/fallback-fields.rst + +.. code-block:: python + + from django.db import models + from openwisp_utils.fields import FallbackDecimalField + from myapp import settings as app_settings + + + class MyModel(models.Model): + price = FallbackDecimalField( + max_digits=4, + decimal_places=2, + fallback=app_settings.DEFAULT_PRICE, + ) diff --git a/docs/developer/other-utilities.rst b/docs/developer/other-utilities.rst index 2c26766e..de0bb548 100644 --- a/docs/developer/other-utilities.rst +++ b/docs/developer/other-utilities.rst @@ -27,12 +27,6 @@ Which use respectively ``AutoCreatedField``, ``AutoLastModifiedField`` from ``model_utils.fields`` (self-updating fields providing the creation date-time and the last modified date-time). -``openwisp_utils.base.FallBackModelMixin`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Model mixin that implements ``get_field_value`` method which can be used -to get value of fallback fields. - REST API Utilities ------------------ diff --git a/docs/partials/fallback-fields.rst b/docs/partials/fallback-fields.rst new file mode 100644 index 00000000..507b99f6 --- /dev/null +++ b/docs/partials/fallback-fields.rst @@ -0,0 +1,6 @@ +.. note:: + + - The field will return the **fallback value** whenever is set to + ``None``. + - Setting the same value as the **fallback value** will save ``None`` + (NULL) in the database. diff --git a/openwisp_utils/base.py b/openwisp_utils/base.py index f2576a16..7264667f 100644 --- a/openwisp_utils/base.py +++ b/openwisp_utils/base.py @@ -23,12 +23,3 @@ class TimeStampedEditableModel(UUIDModel): class Meta: abstract = True - - -class FallbackModelMixin(object): - def get_field_value(self, field_name): - value = getattr(self, field_name) - field = self._meta.get_field(field_name) - if value is None and hasattr(field, 'fallback'): - return field.fallback - return value diff --git a/openwisp_utils/fields.py b/openwisp_utils/fields.py index fb3ee180..dcc66372 100644 --- a/openwisp_utils/fields.py +++ b/openwisp_utils/fields.py @@ -1,7 +1,9 @@ from django import forms from django.db.models.fields import ( + BLANK_CHOICE_DASH, BooleanField, CharField, + DecimalField, PositiveIntegerField, TextField, URLField, @@ -39,30 +41,48 @@ def __init__( class FallbackMixin(object): + """Returns the fallback value when the value of the field is falsy (None or ''). + + If the value of the field is equal to the fallback value, then the + field will save `None` in the database. + """ + def __init__(self, *args, **kwargs): - self.fallback = kwargs.pop('fallback', None) - super().__init__(*args, **kwargs) + self.fallback = kwargs.pop('fallback') + 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) - -class FallbackFromDbValueMixin: - """Returns the fallback value when empty. - - Returns the fallback value when the value of the field is falsy (None - or ''). It does not set the field's value to "None" when the value is - equal to the fallback value. This allows overriding of the value when - a user knows that the default will get changed. - """ - def from_db_value(self, value, expression, connection): + """Called when fetching value from the database.""" if value is None: return self.fallback return value + def get_db_prep_value(self, value, connection, prepared=False): + """Called when saving value in the database.""" + value = super().get_db_prep_value(value, connection, prepared) + if value == self.fallback: + return None + return value + + def get_default(self): + """Returns the fallback value for the default. + + The default is set to `None` on field initialization to ensure + that the default value in the database schema is `NULL` instead of + a non-null value (fallback value). Returning the fallback value + here also sets the initial value of the field to the fallback + value in admin add forms, similar to how Django handles default + values. + """ + return self.fallback + class FalsyValueNoneMixin: """Stores None instead of empty strings. @@ -87,16 +107,12 @@ def clean(self, value, model_instance): class FallbackBooleanChoiceField(FallbackMixin, BooleanField): def formfield(self, **kwargs): - default_value = _('Enabled') if self.fallback else _('Disabled') kwargs.update( { - "form_class": forms.NullBooleanField, + "form_class": forms.BooleanField, 'widget': forms.Select( - choices=[ - ( - '', - _('Default') + f' ({default_value})', - ), + choices=BLANK_CHOICE_DASH + + [ (True, _('Enabled')), (False, _('Disabled')), ] @@ -107,14 +123,6 @@ def formfield(self, **kwargs): class FallbackCharChoiceField(FallbackMixin, CharField): - def get_choices(self, **kwargs): - for choice, value in self.choices: - if choice == self.fallback: - default = value - break - kwargs.update({'blank_choice': [('', _('Default') + f' ({default})')]}) - return super().get_choices(**kwargs) - def formfield(self, **kwargs): kwargs.update( { @@ -124,52 +132,36 @@ def formfield(self, **kwargs): return super().formfield(**kwargs) -class FallbackPositiveIntegerField( - FallbackMixin, FallbackFromDbValueMixin, PositiveIntegerField -): +class FallbackPositiveIntegerField(FallbackMixin, PositiveIntegerField): pass -class FallbackCharField( - FallbackMixin, FalsyValueNoneMixin, FallbackFromDbValueMixin, CharField -): - """Implements fallback logic. - - Populates the form with the fallback value if the value is set to NULL - in the database. - """ +class FallbackCharField(FallbackMixin, FalsyValueNoneMixin, CharField): + """Populates the form with the fallback value if the value is set to null in the database.""" pass -class FallbackURLField( - FallbackMixin, FalsyValueNoneMixin, FallbackFromDbValueMixin, URLField -): - """Implements fallback logic. - - Populates the form with the fallback value if the value is set to NULL - in the database. - """ +class FallbackURLField(FallbackMixin, FalsyValueNoneMixin, URLField): + """Populates the form with the fallback value if the value is set to null in the database.""" pass -class FallbackTextField( - FallbackMixin, FalsyValueNoneMixin, FallbackFromDbValueMixin, TextField -): - """Implements fallback logic. - - Populates the form with the fallback value if the value is set to NULL - in the database. - """ +class FallbackTextField(FallbackMixin, FalsyValueNoneMixin, TextField): + """Populates the form with the fallback value if the value is set to null in the database.""" def formfield(self, **kwargs): - kwargs.update({'form_class': FallbackTextFormField}) + kwargs.update( + { + 'form_class': forms.CharField, + 'widget': forms.Textarea( + attrs={'rows': 2, 'cols': 34, 'style': 'width:auto'} + ), + } + ) return super().formfield(**kwargs) -class FallbackTextFormField(forms.CharField): - def widget_attrs(self, widget): - attrs = super().widget_attrs(widget) - attrs.update({'rows': 2, 'cols': 34, 'style': 'width:auto'}) - return attrs +class FallbackDecimalField(FallbackMixin, DecimalField): + pass diff --git a/openwisp_utils/metric_collection/tests/test_models.py b/openwisp_utils/metric_collection/tests/test_models.py index 859256b8..bf88e3bf 100644 --- a/openwisp_utils/metric_collection/tests/test_models.py +++ b/openwisp_utils/metric_collection/tests/test_models.py @@ -26,7 +26,6 @@ def setUp(self): # The post_migrate signal creates the first OpenwispVersion object # and uses the actual modules installed in the Python environment. # This would cause tests to fail when other modules are also installed. - # import ipdb; ipdb.set_trace() OpenwispVersion.objects.update( module_version={ 'OpenWISP Version': '23.0.0a', diff --git a/tests/test_project/migrations/0008_book_price.py b/tests/test_project/migrations/0008_book_price.py new file mode 100644 index 00000000..8637c5ba --- /dev/null +++ b/tests/test_project/migrations/0008_book_price.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.7 on 2024-07-24 15:20 + +from django.db import migrations +import openwisp_utils.fields + + +class Migration(migrations.Migration): + dependencies = [ + ("test_project", "0007_radiusaccounting_start_time_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="book", + name="price", + field=openwisp_utils.fields.FallbackDecimalField( + blank=True, + decimal_places=2, + default=None, + fallback=20.0, + max_digits=4, + null=True, + ), + ), + ] diff --git a/tests/test_project/models.py b/tests/test_project/models.py index 1b0ae101..7db9aa49 100644 --- a/tests/test_project/models.py +++ b/tests/test_project/models.py @@ -1,23 +1,19 @@ from django.core.exceptions import ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ -from openwisp_utils.base import ( - FallbackModelMixin, - KeyField, - TimeStampedEditableModel, - UUIDModel, -) +from openwisp_utils.base import KeyField, TimeStampedEditableModel, UUIDModel from openwisp_utils.fields import ( FallbackBooleanChoiceField, FallbackCharChoiceField, FallbackCharField, + FallbackDecimalField, FallbackPositiveIntegerField, FallbackTextField, FallbackURLField, ) -class Shelf(FallbackModelMixin, TimeStampedEditableModel): +class Shelf(TimeStampedEditableModel): TYPES = ( ('HORROR', 'HORROR'), ('FANTASY', 'FANTASY'), @@ -36,8 +32,6 @@ class Shelf(FallbackModelMixin, TimeStampedEditableModel): _("Type of book"), choices=TYPES, null=True, blank=True, max_length=50 ) books_count = FallbackPositiveIntegerField( - blank=True, - null=True, fallback=21, verbose_name=_("Number of books"), ) @@ -69,6 +63,7 @@ class Book(TimeStampedEditableModel): name = models.CharField(_('name'), max_length=64) author = models.CharField(_('author'), max_length=64) shelf = models.ForeignKey('test_project.Shelf', on_delete=models.CASCADE) + price = FallbackDecimalField(max_digits=4, decimal_places=2, fallback=20.0) def __str__(self): return self.name @@ -100,16 +95,11 @@ class RadiusAccounting(models.Model): ) -class OrganizationRadiusSettings(FallbackModelMixin, models.Model): +class OrganizationRadiusSettings(models.Model): is_active = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, fallback=False, ) is_first_name_required = FallbackCharChoiceField( - null=True, - blank=True, max_length=32, choices=( ('disabled', _('Disabled')), @@ -119,20 +109,14 @@ class OrganizationRadiusSettings(FallbackModelMixin, models.Model): fallback='disabled', ) greeting_text = FallbackCharField( - null=True, - blank=True, max_length=200, fallback='Welcome to OpenWISP!', ) password_reset_url = FallbackURLField( - null=True, - blank=True, max_length=200, fallback='http://localhost:8000/admin/password_change/', ) extra_config = FallbackTextField( - null=True, - blank=True, max_length=200, fallback='no data', ) diff --git a/tests/test_project/tests/test_admin.py b/tests/test_project/tests/test_admin.py index 74eb4e94..361afc7a 100644 --- a/tests/test_project/tests/test_admin.py +++ b/tests/test_project/tests/test_admin.py @@ -510,7 +510,7 @@ def test_organization_radius_settings_admin(self): self.assertContains( response, '', html=True, @@ -519,8 +519,8 @@ def test_organization_radius_settings_admin(self): self.assertContains( response, '', html=True, @@ -555,20 +555,14 @@ def test_organization_radius_settings_admin(self): response = self.client.post(url, payload, follow=True) self.assertEqual(response.status_code, 200) org_rad_settings.refresh_from_db() - self.assertEqual(org_rad_settings.get_field_value('is_active'), False) + self.assertEqual(org_rad_settings.is_active, False) + self.assertEqual(org_rad_settings.is_first_name_required, 'allowed') + self.assertEqual(org_rad_settings.greeting_text, 'Greeting text') self.assertEqual( - org_rad_settings.get_field_value('is_first_name_required'), 'allowed' - ) - self.assertEqual( - org_rad_settings.get_field_value('greeting_text'), 'Greeting text' - ) - self.assertEqual( - org_rad_settings.get_field_value('password_reset_url'), + org_rad_settings.password_reset_url, 'http://localhost:8000/admin/password_change/', ) - self.assertEqual( - org_rad_settings.get_field_value('extra_config'), 'no data' - ) + self.assertEqual(org_rad_settings.extra_config, 'no data') @patch( 'openwisp_utils.admin_theme.system_info.settings.INSTALLED_APPS', diff --git a/tests/test_project/tests/test_model.py b/tests/test_project/tests/test_model.py index 8ec6a4c4..a40fbf67 100644 --- a/tests/test_project/tests/test_model.py +++ b/tests/test_project/tests/test_model.py @@ -4,7 +4,7 @@ from django.db import connection from django.test import TestCase -from ..models import OrganizationRadiusSettings, Project, Shelf +from ..models import Book, OrganizationRadiusSettings, Project, Shelf from . import CreateMixin @@ -29,6 +29,7 @@ def test_key_validator(self): class TestFallbackFields(CreateMixin, TestCase): org_radius_settings_model = OrganizationRadiusSettings shelf_model = Shelf + book_model = Book def test_fallback_field_falsy_values(self): org_rad_settings = self._create_org_radius_settings() @@ -60,8 +61,10 @@ def test_fallback_boolean_choice_field(self): with self.subTest('Test is_active set to None'): org_rad_settings.is_active = None + org_rad_settings.save(update_fields=['is_active']) + org_rad_settings.refresh_from_db(fields=['is_active']) # Ensure fallback value is returned - self.assertEqual(org_rad_settings.get_field_value('is_active'), False) + self.assertEqual(org_rad_settings.is_active, False) with self.subTest('Test fallback value changed'): with patch.object( @@ -72,21 +75,25 @@ def test_fallback_boolean_choice_field(self): True, ): org_rad_settings.is_active = None - self.assertEqual(org_rad_settings.get_field_value('is_active'), True) + org_rad_settings.save(update_fields=['is_active']) + org_rad_settings.refresh_from_db(fields=['is_active']) + self.assertEqual(org_rad_settings.is_active, True) with self.subTest('Test overriding default value'): org_rad_settings.is_active = True - self.assertEqual(org_rad_settings.get_field_value('is_active'), True) + org_rad_settings.save(update_fields=['is_active']) + org_rad_settings.refresh_from_db(fields=['is_active']) + self.assertEqual(org_rad_settings.is_active, True) def test_fallback_char_choice_field(self): org_rad_settings = self._create_org_radius_settings() with self.subTest('Test is_first_name_required set to None'): org_rad_settings.is_first_name_required = None + org_rad_settings.save(update_fields=['is_first_name_required']) + org_rad_settings.refresh_from_db(fields=['is_first_name_required']) # Ensure fallback value is returned - self.assertEqual( - org_rad_settings.get_field_value('is_first_name_required'), 'disabled' - ) + self.assertEqual(org_rad_settings.is_first_name_required, 'disabled') with self.subTest('Test fallback value changed'): with patch.object( @@ -97,24 +104,28 @@ def test_fallback_char_choice_field(self): 'mandatory', ): org_rad_settings.is_first_name_required = None + org_rad_settings.save(update_fields=['is_first_name_required']) + org_rad_settings.refresh_from_db(fields=['is_first_name_required']) self.assertEqual( - org_rad_settings.get_field_value('is_first_name_required'), + org_rad_settings.is_first_name_required, 'mandatory', ) with self.subTest('Test overriding default value'): org_rad_settings.is_first_name_required = 'allowed' - self.assertEqual( - org_rad_settings.get_field_value('is_first_name_required'), 'allowed' - ) + org_rad_settings.save(update_fields=['is_first_name_required']) + org_rad_settings.refresh_from_db(fields=['is_first_name_required']) + self.assertEqual(org_rad_settings.is_first_name_required, 'allowed') def test_fallback_positive_integer_field(self): shelf = self._create_shelf() with self.subTest('Test books_count set to None'): shelf.books_count = None + shelf.save(update_fields=['books_count']) + shelf.refresh_from_db(fields=['books_count']) # Ensure fallback value is returned - self.assertEqual(shelf.get_field_value('books_count'), 21) + self.assertEqual(shelf.books_count, 21) with self.subTest('Test fallback value changed'): with patch.object( @@ -125,11 +136,47 @@ def test_fallback_positive_integer_field(self): 32, ): shelf.books_count = None + shelf.save(update_fields=['books_count']) + shelf.refresh_from_db(fields=['books_count']) self.assertEqual( - shelf.get_field_value('books_count'), + shelf.books_count, 32, ) with self.subTest('Test overriding default value'): shelf.books_count = 56 - self.assertEqual(shelf.get_field_value('books_count'), 56) + shelf.save(update_fields=['books_count']) + shelf.refresh_from_db(fields=['books_count']) + self.assertEqual(shelf.books_count, 56) + + def test_fallback_decimal_field(self): + book = self._create_book(shelf=self._create_shelf()) + + with self.subTest('Test price set to None'): + book.price = None + book.save(update_fields=['price']) + book.refresh_from_db(fields=['price']) + # Ensure fallback value is returned + self.assertEqual(book.price, 20.0) + + with self.subTest('Test fallback value changed'): + with patch.object( + # The fallback value is set on project startup, hence + # it also requires mocking. + book._meta.get_field('price'), + 'fallback', + 32, + ): + book.price = None + book.save(update_fields=['price']) + book.refresh_from_db(fields=['price']) + self.assertEqual( + book.price, + 32.0, + ) + + with self.subTest('Test overriding default value'): + book.price = 56 + book.save(update_fields=['price']) + book.refresh_from_db(fields=['price']) + self.assertEqual(book.price, 56)