Code

Opened 6 years ago

Closed 6 years ago

#5913 closed (fixed)

Newforms ModelChoiceField and ModelMultipleChoiceField accepting invalid choices when cleaning

Reported by: cohcheto@… Owned by: nobody
Component: Forms Version: master
Severity: Keywords: newforms, clean
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In this fields clean() method there is no validation if the returned object is actually in the valid choices if the queryset is filtered somehow

class ModelChoiceField(ModelChoiceField):
    ...
    def clean(self, value):
        Field.clean(self, value)
        if value in ('', None):
            return None
        try:
            value = self.queryset.model._default_manager.get(pk=value)
        except self.queryset.model.DoesNotExist:
            raise ValidationError(ugettext(u'Select a valid choice. That choice is not one of the available choices.'))
        return value

Here we check only if there is an instance of that model with pk=value, but not if this object is a valid choice given the queryset
Here is an example lets say we have a model TestModel with attribute test_attribute which is BooleanField:

>>> from django.newforms.models import ModelChoiceField
>>> from testapp.models import TestModel
>>> ins_1 = TestModel._default_manager.create(test_attribute=True)
>>> ins_2 = TestModel._default_manager.create(test_attribute=False)
>>> field = ModelChoiceField(queryset=TestModel._default_manager.filter(test_attribute=True))
>>> field.clean(ins_1._get_pk_value())
<TestModel: test_attribute - True> # or something like that
>>> field.clean(ins_2._get_pk_value())
<TestModel: test_attribute - False> # this is not a valid choice but it is cleaned like it is

I provide a patch its simple but probably not the best option its for ModelChoiceField only but its the same for the multiple field

Attachments (1)

model_choice_field.patch (595 bytes) - added by cohcheto@… 6 years ago.
Patch

Download all attachments as: .zip

Change History (4)

Changed 6 years ago by cohcheto@…

Patch

comment:1 Changed 6 years ago by brosner

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed

This falls under the same functionality that #4787 is wanting to give to ModelChoiceField and ModelMultipleChoiceField. Marking as duplicate. Perhaps you can write a better patch that includes this and attach it there?

comment:2 Changed 6 years ago by gwilson

  • Resolution duplicate deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Unreviewed to Accepted

Let's keep the issues separate please.

comment:3 Changed 6 years ago by gwilson

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

(In [6670]) Fixed #4787, #5913 -- Updating the queryset on a ModelChoiceField or ModelMultipleChoiceField now updates its widget's choices. The clean methods for ModelChoiceField and ModelMultipleChoiceField were changed to only allow choices in the specified queryset (instead of allowing all choices returned by the queryset model's default manager).

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.