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

ManyToMany not validated when sending form-data instead of JSON #2249

Closed
dadoeyad opened this issue Dec 10, 2014 · 9 comments
Closed

ManyToMany not validated when sending form-data instead of JSON #2249

dadoeyad opened this issue Dec 10, 2014 · 9 comments

Comments

@dadoeyad
Copy link

Hello,

In 3.0
I have a model serializer and the model have required ManyToMany fields.
When I'm sending data as application/json it's validating that some of the ManyToMany fields aren't present and it throws the correct errors.
But when I send them using multipart/form-data it's not validating the ManyToMany fields. It would actually create the object without any ManyToMany fields.
Not sure if this behavior is intended or not.

@tomchristie
Copy link
Member

Hiya, thanks for raising this.

Could you give some more detailed information. What do we need to do to replicate this? What 'errors' are being raised for JSON but not for form-data? Are you able to reproduce in a test case?

@dadoeyad
Copy link
Author

There you go.
https://github.com/dadoeyad/rest_manytomany

if you try with JSON POST to /api/properties/ with only friendly_name you'll get the error "images": ["This field is required."]
But if you post with form-data with only friendly_name the object is created.

@tomchristie
Copy link
Member

I'd believe what you're seeing is because on the form submission the images key is being included, it just happens that you haven't selected any items, so it's a (valid) empty list.

eg. try overriding .create and doing this:

def create(self, request, *args, **kwargs):
    print request.data
    return super(MyViewClass, self).create(request, *args, **kwargs)

So you can see what data is being submitted.

Welcome to reopen the issue if you think that's not correct or needs further explanation.

We could consider an allow_empty=False flag if that's what you're looking for, but I'm not sure if it's something we'd want by default or not.

@dadoeyad
Copy link
Author

I printed the data in the create method in my view.
It was {u'friendly_name': u'friendly_name'} no images present.

But I also tried. Printing in the serializer`s create method.

def create(self, validated_data):
        print validated_data
        return super(PropertyCreateSerializer, self).create(validated_data)

and that gave me {'images': [], 'friendly_name': u'friendly_name'}

@tomchristie
Copy link
Member

Ah apologies, what you're seeing is slightly different that how I described it before.

multipart/form-data cannot represent an empty set of selections, so when nothing is seen on an HTML input for a list of selections we have to assume that an empty list has been submitted.

Hope that helps explain what you're seeing. Don't believe there's any way around that. Follow up question is do you want to allow an empty list as a valid submission or not?

@dadoeyad
Copy link
Author

I get it now.

To answer your question. No I don't want to allow empty lists.
I'm actually building this for an app. So the data won't be sent from HTML inputs nor was I testing it in HTML inputs. Even if you do curl -v -include --form friendly_name=1 http://127.0.0.1:8000/api/properties/ the object is created.

So yes I need the data to be validated correctly. Both when I don't send the images and also when I send an empty list. and I think it makes more sense this way, right?

@tomchristie
Copy link
Member

Related issue #2250 now created. In the meantime you could use a validate_images method on the serializer class, eg:

def validate_images(self, data):
    if not data:
        raise serializers.ValidationError('May not be empty')
   return data

@tomchristie
Copy link
Member

Thanks for working through the issue. Hope this helps.

@dadoeyad
Copy link
Author

Thanks a lot!

And if you did decide to add the allow_empty flag. I'd very much like to contribute to DRF :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants