Inconsistent validation for implicit ManyRelatedField on ModelSerializer

[leifurhauks]

failing test: #4936

When using a ModelSerializer for a model with a ManyToMany field, and applying required=False to that field in extra_kwargs, omitting the ManyToManyField in the data results in a validation error ("This list may not be empty.") for form data requests (including from the browsable api form), but not for JSON requests.

On closer inspection, I found that when using the serializer directly, passing a dict, the validation error doesn't appear; whereas when passing a QueryDict it does.

Checklist

• I have verified that that issue exists against the master branch of Django REST framework.
• I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• This is not a usage question. (Those should be directed to the discussion group instead.)
• This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• I have reduced the issue to the simplest possible case.
• I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

I created a toy project to reproduce the bug.

Expected behavior

The validation shouldn't differ between form data requests and JSON requests. Because the field has the option required=False, omitting the field shouldn't prevent deserialization; thus the JSON behaviour is correct.

Actual behavior

When passing the data in a dict, omitting the many to many field ('tags' in the toy project linked above), the data passes validation; whereas passing a QueryDict (or making a request with form data rather than json) results in a validation error with message "This list may not be empty."

[xordoquy]

Duplicating my response on the PR:
The Model's field needs to have blank=True. You'll notice that the serializer doesn't complain about the lack of the field. It complains about the field being blank.
At first sight, this is a side effect of forms that can't set a value to null.

[leifurhauks]

My understanding is that that the required=False argument should be sufficient. Setting blank=True on a model field is supposed to make the field not required for validation; setting required=False on the serializer has the same purpose.

In fact, if I declare the field explicitly with required=False I don't encounter this error:

tags = serializers.PrimaryKeyRelatedField(many=True, required=False, queryset=Tag.objects)

Also, if you add print(data_querydict) to that test, you'll notice that the querydict doesn't contain an empty list for that field, the field is omitted.

[xordoquy]

Reopening for further investigations

[leifurhauks]

On closer inspection, I think I was looking at this kind of backward. As you mentioned, @xordoquy , the html forms in the browsable API don't support setting a list field to null or omitting it; if no items in the multi select widget are selected, the field is deserialized as an empty list. To be omitted, I guess the field would need to be dynamically removed from the DOM with javascript.

So I guess the real issue is, why is an empty list valid when setting required=False on an explicitly declared field, but not on a field with {'required': False} in the extra_kwargs? Should required=False imply that an empty list is valid, or not?

[xordoquy]

So I guess the real issue is, why is an empty list valid when setting required=False on an explicitly declared field, but not on a field with {'required': False} in the extra_kwargs? Should required=False imply that an empty list is valid, or not?

That's the reason why I reopened the issue ;)

[gabomasi]

Any solution?