From 1fe0b167af3611cca79e8a8092ee929312193c6f Mon Sep 17 00:00:00 2001 From: Jacob Rief Date: Sat, 8 Apr 2023 22:10:17 +0200 Subject: [PATCH] Fixed #34473 -- Fixed step validation for form fields with non-zero minimum value. --- django/core/validators.py | 31 ++++++++++++++++++- django/forms/fields.py | 4 ++- docs/ref/forms/fields.txt | 12 +++++-- docs/ref/validators.txt | 11 +++++-- docs/releases/5.0.txt | 4 ++- .../field_tests/test_decimalfield.py | 19 ++++++++++++ .../field_tests/test_floatfield.py | 12 +++++++ .../field_tests/test_integerfield.py | 16 ++++++++++ tests/validators/tests.py | 28 +++++++++++++++++ 9 files changed, 129 insertions(+), 8 deletions(-) diff --git a/django/core/validators.py b/django/core/validators.py index 6c622f57887..0f6515f6578 100644 --- a/django/core/validators.py +++ b/django/core/validators.py @@ -397,8 +397,37 @@ class StepValueValidator(BaseValidator): message = _("Ensure this value is a multiple of step size %(limit_value)s.") code = "step_size" + def __init__(self, limit_value, message=None, offset=None): + super().__init__(limit_value, message) + if offset is not None: + self.message = _( + "Ensure this value is a multiple of step size %(limit_value)s, " + "starting from %(offset)s, e.g. %(offset)s, %(valid_value1)s, " + "%(valid_value2)s, and so on." + ) + self.offset = offset + + def __call__(self, value): + if self.offset is None: + super().__call__(value) + else: + cleaned = self.clean(value) + limit_value = ( + self.limit_value() if callable(self.limit_value) else self.limit_value + ) + if self.compare(cleaned, limit_value): + offset = cleaned.__class__(self.offset) + params = { + "limit_value": limit_value, + "offset": offset, + "valid_value1": offset + limit_value, + "valid_value2": offset + 2 * limit_value, + } + raise ValidationError(self.message, code=self.code, params=params) + def compare(self, a, b): - return not math.isclose(math.remainder(a, b), 0, abs_tol=1e-9) + offset = 0 if self.offset is None else self.offset + return not math.isclose(math.remainder(a - offset, b), 0, abs_tol=1e-9) @deconstructible diff --git a/django/forms/fields.py b/django/forms/fields.py index d759da71abe..b8316079a3b 100644 --- a/django/forms/fields.py +++ b/django/forms/fields.py @@ -316,7 +316,9 @@ class IntegerField(Field): if min_value is not None: self.validators.append(validators.MinValueValidator(min_value)) if step_size is not None: - self.validators.append(validators.StepValueValidator(step_size)) + self.validators.append( + validators.StepValueValidator(step_size, offset=min_value) + ) def to_python(self, value): """ diff --git a/docs/ref/forms/fields.txt b/docs/ref/forms/fields.txt index 4084ae78d5d..aa0a7a84441 100644 --- a/docs/ref/forms/fields.txt +++ b/docs/ref/forms/fields.txt @@ -562,7 +562,9 @@ For each field, we describe the default widget used if you don't specify .. attribute:: step_size - Limit valid inputs to an integral multiple of ``step_size``. + Limit valid inputs to an integral multiple of ``step_size``. If + ``min_value`` is also provided, it's added as an offset to determine if + the step size matches. ``DurationField`` ----------------- @@ -695,7 +697,9 @@ For each field, we describe the default widget used if you don't specify .. attribute:: step_size - Limit valid inputs to an integral multiple of ``step_size``. + Limit valid inputs to an integral multiple of ``step_size``. If + ``min_value`` is also provided, it's added as an offset to determine if + the step size matches. ``GenericIPAddressField`` ------------------------- @@ -831,7 +835,9 @@ For each field, we describe the default widget used if you don't specify .. attribute:: step_size - Limit valid inputs to an integral multiple of ``step_size``. + Limit valid inputs to an integral multiple of ``step_size``. If + ``min_value`` is also provided, it's added as an offset to determine if + the step size matches. ``JSONField`` ------------- diff --git a/docs/ref/validators.txt b/docs/ref/validators.txt index a091d20dbb0..8dd90e0e61b 100644 --- a/docs/ref/validators.txt +++ b/docs/ref/validators.txt @@ -340,9 +340,16 @@ to, or in lieu of custom ``field.clean()`` methods. ``StepValueValidator`` ---------------------- -.. class:: StepValueValidator(limit_value, message=None) +.. class:: StepValueValidator(limit_value, message=None, offset=None) Raises a :exc:`~django.core.exceptions.ValidationError` with a code of ``'step_size'`` if ``value`` is not an integral multiple of ``limit_value``, which can be a float, integer or decimal value or a - callable. + callable. When ``offset`` is set, the validation occurs against + ``limit_value`` plus ``offset``. For example, for + ``StepValueValidator(3, offset=1.4)`` valid values include ``1.4``, + ``4.4``, ``7.4``, ``10.4``, and so on. + + .. versionchanged:: 5.0 + + The ``offset`` argument was added. diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index fb446fcd7a7..1eb21190b7f 100644 --- a/docs/releases/5.0.txt +++ b/docs/releases/5.0.txt @@ -367,7 +367,9 @@ Utilities Validators ~~~~~~~~~~ -* ... +* The new ``offset`` argument of + :class:`~django.core.validators.StepValueValidator` allows specifying an + offset for valid values. .. _backwards-incompatible-5.0: diff --git a/tests/forms_tests/field_tests/test_decimalfield.py b/tests/forms_tests/field_tests/test_decimalfield.py index 4e24d553f3d..9d26bc88b0c 100644 --- a/tests/forms_tests/field_tests/test_decimalfield.py +++ b/tests/forms_tests/field_tests/test_decimalfield.py @@ -152,6 +152,25 @@ class DecimalFieldTest(FormFieldAssertionsMixin, SimpleTestCase): with self.assertRaisesMessage(ValidationError, msg): f.clean("1.1") + def test_decimalfield_step_size_min_value(self): + f = DecimalField( + step_size=decimal.Decimal("0.3"), + min_value=decimal.Decimal("-0.4"), + ) + self.assertWidgetRendersTo( + f, + '', + ) + msg = ( + "Ensure this value is a multiple of step size 0.3, starting from -0.4, " + "e.g. -0.4, -0.1, 0.2, and so on." + ) + with self.assertRaisesMessage(ValidationError, msg): + f.clean("1") + self.assertEqual(f.clean("0.2"), decimal.Decimal("0.2")) + self.assertEqual(f.clean(2), decimal.Decimal(2)) + self.assertEqual(f.step_size, decimal.Decimal("0.3")) + def test_decimalfield_scientific(self): f = DecimalField(max_digits=4, decimal_places=2) with self.assertRaisesMessage(ValidationError, "Ensure that there are no more"): diff --git a/tests/forms_tests/field_tests/test_floatfield.py b/tests/forms_tests/field_tests/test_floatfield.py index 276520f04de..77b404102c1 100644 --- a/tests/forms_tests/field_tests/test_floatfield.py +++ b/tests/forms_tests/field_tests/test_floatfield.py @@ -84,6 +84,18 @@ class FloatFieldTest(FormFieldAssertionsMixin, SimpleTestCase): self.assertEqual(-1.26, f.clean("-1.26")) self.assertEqual(f.step_size, 0.02) + def test_floatfield_step_size_min_value(self): + f = FloatField(step_size=0.02, min_value=0.01) + msg = ( + "Ensure this value is a multiple of step size 0.02, starting from 0.01, " + "e.g. 0.01, 0.03, 0.05, and so on." + ) + with self.assertRaisesMessage(ValidationError, msg): + f.clean("0.02") + self.assertEqual(f.clean("2.33"), 2.33) + self.assertEqual(f.clean("0.11"), 0.11) + self.assertEqual(f.step_size, 0.02) + def test_floatfield_widget_attrs(self): f = FloatField(widget=NumberInput(attrs={"step": 0.01, "max": 1.0, "min": 0.0})) self.assertWidgetRendersTo( diff --git a/tests/forms_tests/field_tests/test_integerfield.py b/tests/forms_tests/field_tests/test_integerfield.py index 1361b5cc9e8..a76c2fd5084 100644 --- a/tests/forms_tests/field_tests/test_integerfield.py +++ b/tests/forms_tests/field_tests/test_integerfield.py @@ -126,6 +126,22 @@ class IntegerFieldTest(FormFieldAssertionsMixin, SimpleTestCase): self.assertEqual(12, f.clean("12")) self.assertEqual(f.step_size, 3) + def test_integerfield_step_size_min_value(self): + f = IntegerField(step_size=3, min_value=-1) + self.assertWidgetRendersTo( + f, + '', + ) + msg = ( + "Ensure this value is a multiple of step size 3, starting from -1, e.g. " + "-1, 2, 5, and so on." + ) + with self.assertRaisesMessage(ValidationError, msg): + f.clean("9") + self.assertEqual(f.clean("2"), 2) + self.assertEqual(f.clean("-1"), -1) + self.assertEqual(f.step_size, 3) + def test_integerfield_localized(self): """ A localized IntegerField's widget renders to a text input without any diff --git a/tests/validators/tests.py b/tests/validators/tests.py index 02bee30ac13..af08a785fe1 100644 --- a/tests/validators/tests.py +++ b/tests/validators/tests.py @@ -451,11 +451,39 @@ TEST_DATA = [ (StepValueValidator(3), 1, ValidationError), (StepValueValidator(3), 8, ValidationError), (StepValueValidator(3), 9, None), + (StepValueValidator(2), 4, None), + (StepValueValidator(2, offset=1), 3, None), + (StepValueValidator(2, offset=1), 4, ValidationError), (StepValueValidator(0.001), 0.55, None), (StepValueValidator(0.001), 0.5555, ValidationError), + (StepValueValidator(0.001, offset=0.0005), 0.5555, None), + (StepValueValidator(0.001, offset=0.0005), 0.555, ValidationError), (StepValueValidator(Decimal(0.02)), 0.88, None), (StepValueValidator(Decimal(0.02)), Decimal(0.88), None), (StepValueValidator(Decimal(0.02)), Decimal(0.77), ValidationError), + (StepValueValidator(Decimal(0.02), offset=Decimal(0.01)), Decimal(0.77), None), + (StepValueValidator(Decimal(2.0), offset=Decimal(0.1)), Decimal(0.1), None), + ( + StepValueValidator(Decimal(0.02), offset=Decimal(0.01)), + Decimal(0.88), + ValidationError, + ), + (StepValueValidator(Decimal("1.2"), offset=Decimal("2.2")), Decimal("3.4"), None), + ( + StepValueValidator(Decimal("1.2"), offset=Decimal("2.2")), + Decimal("1.2"), + ValidationError, + ), + ( + StepValueValidator(Decimal("-1.2"), offset=Decimal("2.2")), + Decimal("1.1"), + ValidationError, + ), + ( + StepValueValidator(Decimal("-1.2"), offset=Decimal("2.2")), + Decimal("1.0"), + None, + ), (URLValidator(EXTENDED_SCHEMES), "file://localhost/path", None), (URLValidator(EXTENDED_SCHEMES), "git://example.com/", None), (