Code

Opened 2 years ago

Closed 15 months ago

#17147 closed Bug (needsinfo)

DecimalField causes inline formsets to disappear

Reported by: leith.john@… Owned by: jcd
Component: Forms Version: 1.2
Severity: Normal Keywords: admin formset
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a client who typed non-numeric characters in a DecimalField (for price). When he saved the form returned but the inline formset had vanished.

I think when the characters came to the DecimalField, it raised an exception which caused the inline to crash and thus disappear.

in django.forms.fields:

    def to_python(self, value):
        """
        Validates that the input is a decimal number. Returns a Decimal
        instance. Returns None for empty values. Ensures that there are no more
        than max_digits in the number, and no more than decimal_places digits
        after the decimal point.
        """
        if value in validators.EMPTY_VALUES:
            return None
        if self.localize:
            value = formats.sanitize_separators(value)
        value = smart_str(value).strip()
        try:
            value = Decimal(value)
        except DecimalException:
            # >>> i think this is the culprit, should it throw validationerrors here?
            # about line 292
            raise ValidationError(self.error_messages['invalid'])
        return value

I know my client shouldn't have put characters in there of course, but it's very possible to accidentally put in wrong data in a form field. I think the decimal field should display an error rather than raising an exception at this point.

I'm using this for now:

        class SafeDecimalField(forms.DecimalField):
            def to_python(self, value):
                try:
                    return super(SafeDecimalField, self).to_python(value)
                except Exception, ex:
                    return Decimal("0.0")

Attachments (0)

Change History (5)

comment:1 Changed 2 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Raising a ValidationError is actually the way to return an error message to the user.

Note that models.fields.DecimalField.to_python() catches a different error (decimal.InvalidOperation). Maybe the form field should also catch decimal.InvalidOperation instead of DecimalException.

Could you please provide a minimal test case reproducing the problem you're facing?

comment:2 Changed 2 years ago by julien

  • Triage Stage changed from Unreviewed to Accepted

Accepting at least on the basis that models.fields.DecimalField.to_python() and form.fields.DecimalField.to_python() should have a more consolidated way of handling exceptions. Again, a test demonstrating an actual use case would also be useful.

comment:3 follow-up: Changed 2 years ago by jcd

  • Owner changed from nobody to jcd
  • Status changed from new to assigned

I'm having trouble reproducing a text input that doesn't trigger a validation error.

The decimal module has certain flags that occur in non-normal circumstances. The flags raise exceptions (of the same name) if "traps" are set for them. By default, traps are set for InvalidOperation, DivisionByZero and Overflow. All of the flags are subclasses of DecimalException.

InvalidOperation is what happens when a string is passed to Decimal that doesn't represent a number. So I thought perhaps the trap was being turned off on InvalidOperation. But without the trap, an InvalidOperation produces a Decimal('NaN') as the documentation specifies, and Decimal('NaN') triggers the same ValidationError, so that isn't the issue.

Overflow only seems to occur when math happens, not when numbers are instantiated from strings. Decimal will happily create an object for me with an exponent greater than emax, but if I try to add to it, I get an Overflow

    >>> import decimal
    >>> decimal.getcontext()
    >>> import decimal
    >>> decimal.getcontext()
    Context(prec=28, rounding=ROUND_HALF_EVEN, Emin=-999999999, Emax=999999999, capitals=1, flags=[], traps=[Overflow, DivisionByZero, InvalidOperation])
    >>> decimal.getcontext().Emax=10
    >>> d = decimal.Decimal('1e9')
    >>> d
    Decimal('1E+9')
    >>> d * 100
    Traceback (most recent call last):
      ...
    decimal.Overflow: above Emax
    >>> d = decimal.Decimal('1E11')
    >>> d
    Decimal('1E+11')
    >>> 

It seems as though InvalidOperation is the only kind of DecimalException that you would hit when instantiating a Decimal object from a string. Thus, it's hard to see how you would not hit the ValidationError. A few questions for the reporter of this bug:

  1. What input is causing this error?
  2. Does your application alter the decimal context in any way?
  3. What is the result of print decimal.getcontext() right before the form is validated?
  4. Is your form doing anything special with the DecimalField? Custom validation or cleaning?

Thanks.

comment:4 in reply to: ↑ 3 Changed 2 years ago by anonymous

I've tried to produce a test case that would be helpful for you, but after creating arbitrary models to simulate the problem everything was working perfectly. I think i must conclude that the problem is somewhere in my code then (or the satchmo base that i'm using). I'll check it out and let you know if i can find anything on it.

comment:5 Changed 15 months ago by claudep

  • Resolution set to needsinfo
  • Status changed from assigned to closed

Closed as needinfo until a test case can demonstrate the issue.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.