Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Nested serializer field required when required = False #2719

Closed
Anton-Shutik opened this issue Mar 19, 2015 · 19 comments
Closed

Nested serializer field required when required = False #2719

Anton-Shutik opened this issue Mar 19, 2015 · 19 comments

Comments

@Anton-Shutik
Copy link
Contributor

Hi there,

I'm on DRF 3.1.0 and Django 1.7.6 and have serializers like below:

class AddressSerializer(serializers.ModelSerializer):
    zip = serializers.RegexField(regex=r'^\d{5}(-\d{4})?$')
    addr1 = serializers.Field()

    class Meta:
        model = Address

class CustomerSerializer(serializers.ModelSerializer):
    ship_address = AddressSerializer(required=False)
    email = serializers.Field()

    class Meta:
        model = Customer

and it works fine when doing like this (usual dict):

CustomerSerializer(data={'email':'test@test.test'}).is_valid() # Gives True, which is ok

BUT it fails when serializer created with request.data in the view, because request.data has MergeDict type

from rest_framework.requests import MergeDict
cs=CustomerSerializer(data=MergeDict({'email':'test@test.test'})) 
cs.is_valid()# Gives False
cs.errors # {'ship_address': {'zip': [u'This field is required.'], 'addr1': [u'This field is required.']}}

I understand that 'zip' and 'addr1' required fields on Address, but in this case Address is not required at all.

Am I doing smth wrong or any ideas how to fix that ?

Thanks in advance

@tomchristie
Copy link
Member

Please send usage questions to the discussion group, IRC or stack overflow.
The issue tracker is for managing bugs, feature requests and other active work on the project.

Thanks! :)

@Anton-Shutik
Copy link
Contributor Author

@tomchristie This looks like a bug,
I also found a person with same problem here https://groups.google.com/forum/?fromgroups#!topic/django-rest-framework/ttq8PUAU424

@variable
Copy link

Having same problem in viewset

class CommentSerializer(serializers.ModelSerializer):
    created_by = UserSerializer(required=False)
    content = serializers.PrimaryKeyRelatedField(queryset=Content.objects.all(), required=False)

    class Meta:
        model = Comment

keeps saying the fields in created_by are required.

@variable
Copy link

Hi @tomchristie, just wondering if this will be fixed anytime soon?

@xordoquy
Copy link
Collaborator

@variable if you need to speed up the process the best way is to help us with a pull request.

@tomchristie
Copy link
Member

Unclear to me if this is a duplicate of #2919.

@tomchristie
Copy link
Member

Hi @variable

keeps saying the fields in created_by are required.

Could you expand on that? What's the error message, and what code is being run that causes it?

@tomchristie
Copy link
Member

Probably not a duplicate. First step is to write the most minimal possible failing test case.

@variable
Copy link

@tomchristie I am not able to reproduce in test case:

class CommentTestCase(TestCase):

    def test_comment(self):
        self.assertEqual(Comment.objects.count(), 0)
        user = User.objects.create_user('testuser', 'test@test.com', 'password')
        post = Post.objects.create(created_by=user)
        factory = APIRequestFactory()
        req = factory.post('/api/comments/comments/?content_uuid={}'.format(post.uuid), data={
            'message': 'abc'
        }, format='json')
        force_authenticate(req, user=user)
        CommentViewSet.as_view({'post': 'create'})(req)
        self.assertEqual(Comment.objects.count(), 1)

    def test_comment_1(self):
        self.assertEqual(Comment.objects.count(), 0)
        user = User.objects.create_user('testuser', 'test@test.com', 'password')
        post = Post.objects.create(created_by=user)
        client = APIClient()
        client.login(username='testuser', password='password')
        result = client.post('/api/comments/comments/?content_uuid={}'.format(post.uuid), data={
            'message': 'abc'
        }, format='json')
        self.assertEqual(Comment.objects.count(), 1)

The only difference is I am posting via Ajax post, does it make any difference?

    $.post('/api/comments/comments/?content_uuid='+uuid, {message: msg}, function(data){}, 'json');

@variable
Copy link

@tomchristie
I have added "read_only=True" to the created_by field now it works in ajax post, but it seems the read_only flag is trivial since "required=False"

@tomchristie
Copy link
Member

Given this "I am not able to reproduce in test case:" I'm going to assume this was a duplicate of #2919 (just closed now), and close this. More than happy to reconsider if anyone can expand on this with a failing example case.

@sandeepginside
Copy link

I m having exactly the same problem. As mentioned by Anton,

class AddressSerializer(serializers.ModelSerializer):
    class Meta:
        model = Address
        fields = ['house_number', 'street', 'area', 'locality', 'address_of']

class CustomerSerializer(serializers.ModelSerializer):
    ship_address = AddressSerializer(required=False)

    class Meta:
        model = Customer

In Address model Street is a required field.

I want to have the ship_address to be optional in CustomerSerializer.
Inspite of having the required=False flag, I m getting a validation error :

"ship_address": {
        "street": [
            "This field may not be blank."
        ]
    }

Please suggest...

@xordoquy
Copy link
Collaborator

The discussion group is the best place to take this point and other usage questions. Thanks!

@mihailb
Copy link

mihailb commented Sep 7, 2018

Looks like this issue was never fixed. Having the same problem as @Anton-Shutik and @sandeepginside and it is not a duplicate of #2919
@xordoquy @tomchristie Can this issue be reopened?

@rpkilby
Copy link
Member

rpkilby commented Sep 7, 2018

Hi @mihailb. The easiest place to start would be to turn #2719 (comment) into a failing test case/PR.

@francisjeanneau
Copy link

francisjeanneau commented Oct 12, 2018

Why is this closed? Looks like an important bug to me and can't see any info on that elsewhere. Is there a work around?

By using a custom name instead of the model field name and specifying the source parameter to the serializer, the problem goes away. ie:

user_details = UserSerializer(
    read_only=True,
    source='user',
)

instead of

user = UserSerializer(
    read_only=True,
)

which generates a HTTP 400 field required. Note that this prevents the use of the depth attribute in the serializer's Meta class.

Also, @tomchristie refers this issue in a reply in the mailing list, however no informations help to overcome the problem here. (https://groups.google.com/forum/#!topic/django-rest-framework/ttq8PUAU424)

@rpkilby
Copy link
Member

rpkilby commented Oct 12, 2018

Reopening, since @sandeepginside's example should be enough to create a test case.

@rpkilby rpkilby reopened this Oct 12, 2018
@beda42
Copy link

beda42 commented Aug 24, 2019

Just a hint for some of you experiencing this - I found that in my case it was caused by a unique_together constrain on the model which forced the field at hand to behave as required=True.
This case is documented in the docs here: https://www.django-rest-framework.org/api-guide/validators/#optional-fields

@therealak12
Copy link

May be this answer solves your problem:
allow_null

@encode encode locked and limited conversation to collaborators Mar 3, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

10 participants