Opened 6 years ago

Closed 4 years ago

Last modified 4 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 ubernostrum)

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 adamv 5 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 6 years ago by ubernostrum

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

(formatting)

comment:2 follow-up: Changed 6 years ago by kmtracey

  • milestone 1.0.3 deleted

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 6 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 6 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 5 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 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:7 Changed 5 years ago by adamv

  • Has patch set
  • Version changed from 1.0 to SVN

Changed 5 years ago by adamv

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

comment:8 Changed 4 years ago by timo

  • Component changed from Forms to Documentation

comment:9 Changed 4 years ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:10 Changed 4 years ago by timo

(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