Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#11496 closed (fixed)

max_value validation is inconsistant

Reported by: bobbo717@… Owned by: nobody
Component: Documentation Version: master
Severity: Keywords: max_value
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by James Bennett)

I am attempting to use validation in the following modelform:

class BatchForm(ModelForm):
    amount = forms.DecimalField(max_digits=8,decimal_places=2,max_value=599999.99)
    class Meta:
        model = Batch
        exclude = ('submit_date','user')

Sometimes is_valid() returns true and sometimes it returns false with the amount 1212.

The error returnedis:

Ensure this value is less than or equal to 599999.99.

Is this a know issue?

Version Django-1.0.2-final Python 2.5.1 Mac OSX 10.5.7

Attachments (1)

decimal-fields.diff (517 bytes) - added by Adam Vandenberg 6 years ago.
Document that min/max need to be decimals. Use text from IntegerField to eliminated missing apostrophe on "fields".

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by James Bennett

Description: modified (diff)
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

(formatting)

comment:2 Changed 7 years ago by Karen Tracey

milestone: 1.0.3

Removing milestone as an unconfirmed problem should not block a bug-fix release. As for investigating this -- there's not enough information here to recreate as the Batch model upon which your model form is based is missing. Also you say "sometimes" you encounter this problem, so I'm guessing even with full information it will be hard to recreate. The code involved here is in django/forms/fields.py and the case where this error message is raised is straightforward:

        if self.max_value is not None and value > self.max_value:
            raise ValidationError(self.error_messages['max_value'] % self.max_value)

As a first step I would probably change that error message and the code here that constructs it to also print the value being tested against, as that might give a clue what has gone wrong in the cases where you are seeing the problem.

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

Replying to kmtracey:

Removing milestone as an unconfirmed problem should not block a bug-fix release. As for investigating this -- there's not enough information here to recreate as the Batch model upon which your model form is based is missing. Also you say "sometimes" you encounter this problem, so I'm guessing even with full information it will be hard to recreate. The code involved here is in django/forms/fields.py and the case where this error message is raised is straightforward:

        if self.max_value is not None and value > self.max_value:
            raise ValidationError(self.error_messages['max_value'] % self.max_value)

As a first step I would probably change that error message and the code here that constructs it to also print the value being tested against, as that might give a clue what has gone wrong in the cases where you are seeing the problem.

Sometimes was a definite understatement. I am able to reproduce within 10 submits of my form. I have removed that validation from the code and am now validating through the view itself.

From views.py

def processBatchForm(request):

if request.method == 'POST':

form = BatchForm(request.POST)
message = ""
if form.is_valid():

...

else:

message = message + "Data Validation Error! Please see errors."

prev_batches = Batch.objects.all()[:10]
live_val = getBatchValue();
return render_to_response('refunds/batch_list.html', {

'form': form,
'batch_history': prev_batches,
'live_val' : live_val,
'message' : message,
'user' : request.user
})

else:

print "Request Method not permitted."

from

comment:4 Changed 7 years ago by anonymous

TYPE_CHOICES = (

('T','Today Only'),
('P','Permanent'),
('B','Both Today and Permanent')

)

class Batch(models.Model):

submit_date = models.DateTimeField('Batch Time')
user = models.CharField(max_length=25)
amount = models.DecimalField(max_digits=8,decimal_places=2)
type = models.CharField(max_length=1,choices=TYPE_CHOICES,default='T')
class Meta:

ordering = -submit_date?

class Amount(models.Model):

value = models.DecimalField(max_digits=8,decimal_places=2)

class BatchForm(ModelForm):

amount = forms.DecimalField(max_digits=8,decimal_places=2)
class Meta:

model = Batch
exclude = ('submit_date','user')

comment:5 Changed 7 years ago by empty

The issue here is that the docs are not clear that max_value needs to be declared as a Decimal type. So the OP should specify it like:

import decimal

amount = forms.DecimalField(max_digits=8,decimal_places=2,max_value=decimal.Decimal('599999.99'))

The appropriate thing to do here is to correct the docs, or should I say actually document the use. But even better to just handle the conversion in Django so the developer can specify it either way.

comment:6 Changed 7 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

comment:7 Changed 6 years ago by Adam Vandenberg

Has patch: set
Version: 1.0SVN

Changed 6 years ago by Adam Vandenberg

Attachment: decimal-fields.diff added

Document that min/max need to be decimals. Use text from IntegerField to eliminated missing apostrophe on "fields".

comment:8 Changed 6 years ago by Tim Graham

Component: FormsDocumentation

comment:9 Changed 6 years ago by Tim Graham

Resolution: fixed
Status: newclosed

(In [15084]) Fixed #11496 - note that DecimalField max/min_value should be type(decimal.Decimal); thanks adamv.

comment:10 Changed 6 years ago by Tim Graham

(In [15085]) [1.2.X] Fixed #11496 - note that DecimalField max/min_value should be type(decimal.Decimal); thanks adamv.

Backport of r15084 from trunk.

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