Opened 15 years ago

Closed 13 years ago

Last modified 13 years ago

#11496 closed (fixed)

max_value validation is inconsistant

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

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 13 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 by James Bennett, 15 years ago

Description: modified (diff)

(formatting)

comment:2 by Karen Tracey, 15 years ago

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.

in reply to:  2 comment:3 by anonymous, 15 years ago

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 by anonymous, 15 years ago

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 by empty, 14 years ago

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 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:7 by Adam Vandenberg, 13 years ago

Has patch: set
Version: 1.0SVN

by Adam Vandenberg, 13 years ago

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 by Tim Graham, 13 years ago

Component: FormsDocumentation

comment:9 by Tim Graham, 13 years ago

Resolution: fixed
Status: newclosed

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

comment:10 by Tim Graham, 13 years ago

(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