Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for #25980: ModelMultipleChoiceField does not properly clean QuerySets #5873

Closed
wants to merge 1 commit into from

Conversation

karyon
Copy link
Contributor

@karyon karyon commented Dec 24, 2015

f = forms.ModelMultipleChoiceField(queryset=Writer.objects.all(), disabled=True, required=False)
f.clean([person1.pk])
f.clean(Writer.objects.none())
f.clean(Writer.objects.all()) # used to throw a ValidationError: ['Enter a list of values.']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"used to" in an older version of Django? Did you find the commit where the behavior changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh sorry, so that fails right now, with the current 1.9.x branch. i don't know when it got introduced, i can search for that if you want. i guess this got exposed with the introduction of the disabled attribute, as with it the initial value is passed unprocessed into clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the clean method seems to handle only lists and tuples of PKs and false-y values since it was created (somewhere in 2007).

@timgraham
Copy link
Member

Can you give a higher level example? I tested with a form like this and didn't encounter any problems:

class Form(forms.ModelForm):
    choices = forms.ModelMultipleChoiceField(queryset=Choice.objects.all(), disabled=True, required=False)

    class Meta:
        model = Question
        fields = '__all__'

@karyon
Copy link
Contributor Author

karyon commented Dec 28, 2015

you have to provide an initial value that is not False-y. this will be passed to the clean method as-is on disabled fields, which i simulated in the test. i can provide a test using a complete form later.

@timgraham
Copy link
Member

I tested with a Question which had an existing many-to-many relation for choices, but I think I understand the problem now. You passed a queryset as the form's initial value. The documentation says, "Validates that every id in the given list of values exists in the queryset." so I think you should be providing a list of ids for the initial values instead of a queryset.

@timgraham
Copy link
Member

Closing since this test isn't meant to merged.

@timgraham timgraham closed this Dec 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants