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

Possible bug #20

Open
pySilver opened this issue Sep 16, 2014 · 8 comments
Open

Possible bug #20

pySilver opened this issue Sep 16, 2014 · 8 comments

Comments

@pySilver
Copy link

Hi!

Here is a possible bug:
https://github.com/estebistec/drf-compound-fields/blob/master/drf_compound_fields/fields.py#L105

Shouldn't it be(?):

self.list_field = ListField(item_field, *args, **kwargs)

Otherwise validators may report required field error for some circumstances. Consider example:

myfield = ListOrItemField(IntegerField(), required=False, blank=True)

so, it woud result into:

[] -> Error myfield is required

@estebistec
Copy link
Owner

so in your example, a list value would be optional but an integer value would be required?

@pySilver
Copy link
Author

@estebistec yeah well thats not clear from my example.

here is a serializer:

class MySerializer(Serializer):
    authors = ListOrItemField(
        PrimaryKeyRelatedField(
            queryset=Authors.objects.all(),
            error_messages={
                'does_not_exist': _(u"Invalid Account: %s")
            },
        ),
        write_only=True,
        required=False,
        blank=True
    )

So, since requred=False, authors field expected to accept empty values, such as empty list. Thats how DRF would behave. But in reality it would behave like DRF if empty value won't be passed by api client at all. Otherwise it would result in error:

{
    "authors": ['Field `authors` is required', ]
}

This is happening because undelying ListField did not receive _args and *_kwargs that were defined for original authors field, so it fails while validating field value at DRF side:

rest_framework.fields.WritableField#validate:

def validate(self, value):
        if value in validators.EMPTY_VALUES and self.required:
            raise ValidationError(self.error_messages['required'])

And this problem exists just for ListOrItemField, since ListField normally constructs field using _args and *_kwargs passed from serializer.

@estebistec
Copy link
Owner

Okay, I see. I'll poke at a fix this evening. Thanks for reporting it!

@estebistec
Copy link
Owner

Sorry, i haven't forgotten this. Simply passing args and kwargs didn't fix this like I'd hoped, so I have to dig deeper.

@pySilver
Copy link
Author

Hm, that worked for me just fine. So passing empty list as value to a ListOrItemField did not result in required error. However passing an empty string would result in some other error – but this is how DRF work (and will work until 3.0)

@estebistec
Copy link
Owner

@pySilver have you moved to DRF 3.0 yet? I'm thinking of upgrading this lib for it and if you're game I would ensure this bug is fixed or not present in that upgrade.

@pySilver
Copy link
Author

@estebistec nop, our app is too big to make an easy upgrade to 3.0 :/

@estebistec
Copy link
Owner

Please see #27

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

No branches or pull requests

2 participants