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

Better notation for deferred fields? #97

Open
Rjevski opened this issue Jun 1, 2022 · 1 comment
Open

Better notation for deferred fields? #97

Rjevski opened this issue Jun 1, 2022 · 1 comment

Comments

@Rjevski
Copy link
Contributor

Rjevski commented Jun 1, 2022

Hello and hope you're well!

I wanted to raise a discussion on how deferred fields are currently defined and whether a less verbose approach could be supported?

At the moment my understanding is that expandable fields explicitly need to have their serializer (or field type) defined. This is fine for "true" expands (that warrant a separate serializer) but becomes unnecessarily verbose for fields on the same model - those only defined in fields and the ModelSerializer infers the actual field types at runtime from the model.

Given this serializer:

class MySerializer(serializers.ModelSerializer):
    class Meta:
        model = MyModel
        fields = ("id", "name", "description", "etc")

Let's say I wanted to have description and etc deferred - not rendered by default unless requested, currently I'd have to do this:

class MySerializer(serializers.ModelSerializer):
    class Meta:
        model = MyModel
        fields = ("id", "name")
        expandable_fields = {"description" serializers.CharField, "etc": serializers.CharField}

This requires explicitly listing out field classes for every field, a pretty tedious process.

In the codebase I'm currently working on we worked around this as follows:

class CustomFlexFieldsSerializerMixin(FlexFieldsSerializerMixin):
    """
    Overriding the FlexFieldsSerializerMixin to enable declaring of "default_fields"
    in the Serializer.Meta.
    This is a list of fields to be shown if no "fields" parameter is present.

    class Meta:
        default_fields = ["id", "name"]
    """

    def __init__(self, *args, **kwargs):
        """Set fields from Meta.default_fields if not provided in the parameters"""
        if (
            kwargs.get("context")
            and not kwargs["context"]["request"].query_params.getlist(FIELDS_PARAM)
            and not kwargs["context"]["request"].query_params.getlist(OMIT_PARAM)
        ):
            super().__init__(*args, **kwargs, fields=self._default_fields)
        else:
            super().__init__(*args, **kwargs)

    @property
    def _default_fields(self) -> dict:
        if hasattr(self, "Meta") and hasattr(self.Meta, "default_fields"):
            return self.Meta.default_fields
        return {}

Essentially the above approach sets the fields argument to Meta.default_fields (unless it's explicitly set within the context from the originating request) as if they were explicitly requested via the query string - this allows you to have deferrable fields with minimal changes to the serializer - just set default_fields and you're good to go.

We had a TODO in there to upstream this so I wanted to raise this discussion to see if there's a way we can merge our approaches so our custom override above is no longer required.

@rsinger86
Copy link
Owner

That's really clever.

I'm just not sure how to integrate this. This seems to focus on "make it easy to have a skinny default representation, but include other fields on demand", whereas I think most people use this to expand simple fields to full serializers.

So maybe it could be an optional mixin? One other thing I'm thinking is that the API might feel more familiar if we added a deferred_fields Meta attribute since people are used to fields acting as the default fields. But this would probably require a very different implementation.

class MySerializer(serializers.ModelSerializer):
    class Meta:
        model = MyModel
        fields = ("id", "name", "mentions")
        deferred_fields = ["description", "etc"]
        expandable_fields = {"mentions" MentionsSerializer}

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