Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32949 closed Bug (fixed)

decimal.InvalidOperation error on DecimalField form field

Reported by: yakimka Owned by: yakimka
Component: Forms Version: 3.2
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a form with DecimalField and max_value argument:

class OrderForm(forms.ModelForm):
    sum = DecimalField(max_value=12)

    class Meta:
        model = Order
        fields = ['sum']

# model
class Order(models.Model):
    sum = models.DecimalField(
        'Sum',
        max_digits=18,
        decimal_places=2,
        default=0
    )

If I pass "NaN" value to this form it will fail with decimal.InvalidOperation error.

https://github.com/django/django/pull/14645

Change History (8)

comment:2 by Carlton Gibson, 3 years ago

Needs documentation: set
Patch needs improvement: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks for the report.

This is a regression in cc3d24d7d577f174937a0744d886c4c7123cfa85.

The #7777 originally added handling of NaN and Inf in DecimalField (in 692fd7da5e5bfe1533c40f94999ea42f9b986356).
It looked redundant but wasn't because MaxValueValidator does not handle Decimal('NaN').

Here's a minimal diff showing that:

diff --git a/tests/validators/tests.py b/tests/validators/tests.py
index e39d0e3a1c..42665c6ccb 100644
--- a/tests/validators/tests.py
+++ b/tests/validators/tests.py
@@ -214,6 +214,7 @@ TEST_DATA = [
 
     (MaxValueValidator(0), 1, ValidationError),
     (MaxValueValidator(NOW), NOW + timedelta(days=1), ValidationError),
+    (MaxValueValidator(Decimal('NaN')), 1, ValidationError),
 
     (MinValueValidator(-10), -10, None),
     (MinValueValidator(-10), 10, None),

Results in:

======================================================================
ERROR: test_validators (validators.tests.TestValidators) [MaxValueValidator] (value=1)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "...django/tests/validators/tests.py", line 355, in test_validators
    validator(value)
  File "...django/django/core/validators.py", line 358, in __call__
    if self.compare(cleaned, limit_value):
  File "...django/django/core/validators.py", line 383, in compare
    return a > b
decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>]

You PR here should (partially?) revert cc3d24d7d577f174937a0744d886c4c7123cfa85 (see #31806 and the PR for discussion and related commits).

Add max_value to the existing test_enter_a_numer_error() case:

diff --git a/tests/forms_tests/field_tests/test_decimalfield.py b/tests/forms_tests/field_tests/test_decimalfield.py
index c8b044722e..04c555c4ad 100644
--- a/tests/forms_tests/field_tests/test_decimalfield.py
+++ b/tests/forms_tests/field_tests/test_decimalfield.py
@@ -49,7 +49,7 @@ class DecimalFieldTest(FormFieldAssertionsMixin, SimpleTestCase):
         self.assertIsNone(f.min_value)
 
     def test_enter_a_number_error(self):
-        f = DecimalField(max_digits=4, decimal_places=2)
+        f = DecimalField(max_value=1, max_digits=4, decimal_places=2)
         values = (

It looks as if test test_value_placeholder_with_decimal_field (forms_tests.tests.test_validators.ValidatorCustomMessageTests) will need adjusting.

A release note in 3.2.6.txt will be needed.
Good spot.

Version 0, edited 3 years ago by Carlton Gibson (next)

comment:3 by Jacob Walls, 3 years ago

Needs documentation: unset
Owner: changed from nobody to yakimka
Status: newassigned

comment:4 by Carlton Gibson, 3 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:5 by Carlton Gibson <carlton.gibson@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In c542d0a:

Fixed #32949 -- Restored invalid number handling in DecimalField.validate().

DecimalField must itself validate() values, such as NaN, which cannot be
passed to validators, such as MaxValueValidator, during the
run_validators() phase.

Regression in cc3d24d7d577f174937a0744d886c4c7123cfa85.

comment:6 by Carlton Gibson <carlton.gibson@…>, 3 years ago

In 1346381:

[3.2.x] Fixed #32949 -- Restored invalid number handling in DecimalField.validate().

DecimalField must itself validate() values, such as NaN, which cannot be
passed to validators, such as MaxValueValidator, during the
run_validators() phase.

Regression in cc3d24d7d577f174937a0744d886c4c7123cfa85.

Backport of c542d0a07237033225c1d57337ca9474a00648f2 from main

comment:7 by Carlton Gibson <carlton.gibson@…>, 3 years ago

In 012f38f:

Refs #32949 -- Adjusted release note wording.

comment:8 by Carlton Gibson <carlton.gibson@…>, 3 years ago

In f4cf86f8:

[3.2.x] Refs #32949 -- Adjusted release note wording.

Backport of 012f38f9594b35743e9ab231757b7b62db638323 from main

Note: See TracTickets for help on using tickets.
Back to Top