Disabled ModelMultipleChoiceField does not validate anymore

[karyon]

i have the following test:

    def test_foo(self):
        # Course has an m2m to degrees
        course = mommy.make(Course, semester=mommy.make(Semester), degrees=[mommy.make(Degree)])

        # get_form_data_from_instance creates a dict with all the attributes. contains "degrees: [4]"
        form_data = get_form_data_from_instance(ContributorCourseForm, course)

        # ContributorCourseForm is a simple ModelForm on Course
        form = ContributorCourseForm(form_data, instance=course)
        self.assertTrue(form.is_valid())
        
        # recreate form
        form = ContributorCourseForm(form_data, instance=course)
        # disable degrees field
        form.fields['degrees'].disabled = True
        self.assertTrue(form.is_valid())

the second assert fails. form.errors says "degrees: Enter a list of values." which doesn't make sense, since 1. it does have a degree (the form renders correctly with that degree selected), and 2. this shouldn't happen just because the field is disabled.

this showed up in 3744fc1666c41aeb4ed44f31605bafdb413b42bc, the fix for https://code.djangoproject.com/ticket/26917#comment:1

likely related: https://code.djangoproject.com/ticket/25980, same error message when providing a queryset as initial value to a disabled ModelMultipleChoiceField.

[Tim Graham]

Can you provide a test case for Django's test suite rather than your own project? Is this some change in Django 1.10 from 1.9? I thought we reverted to the 1.9 version of the code in a5f85d891b51d7ceb4f9e422e3e4f5c741062288.

[karyon]

i'll try. this test does not fail with the current 1.9 branch.

[karyon]

here are two tests:

# in class ModelMultipleChoiceFieldTests:

def test_disabled_model_multiple_choice_field(self):
    class WriterForm(forms.Form):
        persons = forms.ModelMultipleChoiceField(queryset=Writer.objects.all())

    person1 = Writer.objects.create(name="Person 1")
    form = WriterForm(data={'persons': [person1.pk]})
    self.assertTrue(form.is_valid())
    form = WriterForm(data={'persons': [person1.pk]})
    form.fields['persons'].disabled = True
    self.assertTrue(form.is_valid())

def test_disabled_model_multiple_choice_field2(self):
    class ArticleCategoriesForm(forms.ModelForm):
        categories = forms.ModelMultipleChoiceField(Category.objects.all(), required=False)

        class Meta:
            model = Article
            fields = ['categories']

    category1 = Category.objects.create(name="uiae")
    article1 = Article.objects.create(
                pub_date=datetime.date(1988, 1, 4),
                writer=Writer.objects.create(name='Test writer'),
                # categories= [category1.pk], # throws a warning
            )
    article1.categories.set([category1.pk]) # without this, the test passes
    # article1.categories = [category1.pk] # throws a warning

    # instance must be set, otherwise test passes
    form = ArticleCategoriesForm(data={'categories': [category1.pk]}, instance=article1)
    self.assertTrue(form.is_valid())
    form = ArticleCategoriesForm(data={'categories': [category1.pk]}, instance=article1)
    form.fields['categories'].disabled = True
    self.assertTrue(form.is_valid())

both fail at the second assert, the first one with "this field is required" (setting it to required=False or disabled=False makes the test pass), the second one with "please enter a list of values", like the original report.

i'm not sure what the protocol is, should i put the code somewhere else?

unrelated: note the warnings when assigning article.categories. the warning was "Direct assignment to the reverse side of a related set is deprecated due to the implicit save() that happens. Use article_set.set() instead." although i was not assigning to the reverse side. should i file a separate bug?

[Tim Graham]

From what I see, test_disabled_model_multiple_choice_field fails on 1.9.x as well as 1.10.x so it doesn't seem to be a correct regression test.

After applying 3744fc1666c41aeb4ed44f31605bafdb413b42bc to different points along the 1.10.x branch, I bisected the failure of test_disabled_model_multiple_choice_field2 to ded502024191053475bac811d365ac29dca1db61. I'll continue investigation later if no one else does.

Good spot on the deprecation message. Here's a ​PR to correct it.

[Tim Graham]

Let me know if this ​PR solves your failures.

[karyon]

yes it does, thanks!

the first test was not fixed by this. should i create a new bug for that?

[Tim Graham]

The first failure looks expected because disabled fields ignore what's in data and instead use what's in initial or instance.

[karyon]

i see, thanks for the explanation.

[Tim Graham <timograham@…>]

In 4e86168:

Fixed #26970 -- Fixed crash with disabled ModelMultipleChoiceField.

[Tim Graham <timograham@…>]

In c6e5878:

[1.10.x] Fixed #26970 -- Fixed crash with disabled ModelMultipleChoiceField.

Backport of 4e861682904744b0ea3ead8552513c6f1a826c5a from master