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

Serializer mixin fields not getting picked up #4482

Closed
vesterbaek opened this issue Sep 12, 2016 · 10 comments
Closed

Serializer mixin fields not getting picked up #4482

vesterbaek opened this issue Sep 12, 2016 · 10 comments

Comments

@vesterbaek
Copy link

I would like to declare som common fields in a mixin class, used by subclasses of both serializer.Serializer and serializer.ModelSerializer. These fields, however, seems not to be picked up, even though defined in Meta.fields. Is this by design?

Repro:

class TestSerializerMixin:
    def test_mixin(self):
        class MyMixin(object):
            some_field = serializers.BooleanField()

        class MySerializer(MyMixin, serializers.Serializer):
            class Meta:
                fields = ['some_field']

        serializer = MySerializer(data={})
        serializer.is_valid()
        assert 'some_field' in serializer.errors

=>

    assert 'some_field' in serializer.errors
E   assert 'some_field' in {}
E    +  where {} = MySerializer(data={}):.errors
@xordoquy
Copy link
Collaborator

Fields are gathered on classes that inherit from serializers.
Your mixin has object as parent and thus break the field introspection.

@vesterbaek
Copy link
Author

vesterbaek commented Sep 12, 2016

Ok, see _declared_fields is populated in SerializerMetaclass. Would it be ok to use this class on the mixin to get the fields registered?

@xordoquy
Copy link
Collaborator

Should likely do the trick

@vesterbaek
Copy link
Author

It does:

class TestSerializerMixin:
    def test_mixin(self):
        @six.add_metaclass(serializers.SerializerMetaclass)
        class MyMixin(object):
            some_field = serializers.BooleanField()

        class MySerializer(MyMixin, serializers.Serializer):
            class Meta:
                fields = ['some_field2']

        serializer = MySerializer(data={})
        serializer.is_valid()
        assert 'some_field' in serializer.errors

I more asking if this from a framework design perspective is ok or if the meta class is considered internal.

@xordoquy
Copy link
Collaborator

The metaclass is considered as internal since it's not documented. However, it's unlikely to change.

@rpkilby
Copy link
Member

rpkilby commented Sep 12, 2016

Why do

@six.add_metaclass(serializers.SerializerMetaclass)
class MyMixin(object):

when you can just do

class MyMixin(serializers.Serializer):

especially since the former is not technically a public API? Is there something I'm missing? ModelSerializer inherits from Serializer.

@vesterbaek
Copy link
Author

vesterbaek commented Sep 12, 2016

I'm using this for both Serializer and ModelSerializer. Not sure about the consequences for multiple inheritance like that. Guess it should work if the mixin class is listed after the ModelSerializer

@xordoquy
Copy link
Collaborator

Using Serializer instead of just the meta may mess with heritage if some intermediate mixins do override some members.

@ThatsAMorais
Copy link

I know one thing that could be problematic when inheriting serializers.Serializer is having to add definitions for all abstract methods of Serializer in said mixin. I used the metaclass and was successful in achieving what I wanted.

@erkanImperiatec
Copy link

erkanImperiatec commented Aug 24, 2022

Hello, I know there's quite a gap between when this issue has been handled, but referencing what @vesterbaek suggested in his test case:

@six.add_metaclass(serializers.SerializerMetaclass)
        class MyMixin(object):
            some_field = serializers.BooleanField()

        class MySerializer(MyMixin, serializers.Serializer):
            class Meta:
                fields = ['some_field2']

Is this still the way to go if we want to make generic fields or groups of fields that we want to reuse across different serializers with specific settings that implement the same logic based on these given fields @xordoquy ?

Is doing this good practice, even if not documented ? Else, is there another way to declaring serializers that is similar to the way DRF views are composable with mixins ?

I would also like to do this for Django Models, but that is not in the scope of DRF thus i won't focus on it here, but i think the way to do it is django models with abstract = True

to illustrate, let me show what i mean by showing the piece of logic i want implement based on @vesterbaek 's code:

@six.add_metaclass(serializers.SerializerMetaclass)
        class CheckDatesIfTrueMixin(object): # I named it that way so you understand the point of those fields
            bool_field = serializers.BooleanField()
            begin_date = serializers.BooleanField()
            end_date = serializers.BooleanField()
            # these fields are intertwined, they work together, and i have a custom validators class that is based on the 3 of them

        class MySerializer(CheckDatesIfTrueMixin, serializers.ModelSerializer):
            class Meta:
                model = XYZ 
                fields = "__all__"
                validators = [
                    CustomValidator # this contains some validation logic based on 'bool_field', 'begin_date', and 'end_date'
                ]
                
# xyz/models.py

from django.db import models

class XYZ(models.Model):
    ... # other fields
    bool_field = models.BooleanField(blank=False, null=False, default=False)
    begin_date = models.DateField(blank=True, null=True)
    end_date = models.DateField(blank=True, null=True)

I want to use this logic across multiple serializers/models , is this suitable ? / doable in any other way ? / not possible ?

Thanks in advance,

Hope someone can answer despite the multiple year gap between comments :)

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

5 participants