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

Related models in the response not converting to json #18

Open
ChristopherBrownie opened this issue Jan 25, 2023 · 8 comments
Open

Related models in the response not converting to json #18

ChristopherBrownie opened this issue Jan 25, 2023 · 8 comments

Comments

@ChristopherBrownie
Copy link

How would we include related models through select_related() or prefetch_related() in the json output? The model_to_dict() method used in the encoder doesn't seem to include them. I'm realizing I almost need a DRF serializer lite setup.

Is this something the default encoder could handle (instead of needing to make a custom one) or would it be easier for me just to use something like values() to select properties?

@BrandonShar
Copy link
Collaborator

This has been a sticking point for me as well and I've typically ended up writing custom serializers (or using values for simpler cases) in my project to handle it.

I would love for the default encoder to handle it, but I'm a bit worried about straying too far from expected Django behavior, so I've hesitated so far. For reference, this isn't an issue for the Rails or Laravel adapters because those frameworks have built in ways to handle json encoding models.

How would your ideal api work here?

@ChristopherBrownie
Copy link
Author

ChristopherBrownie commented Jan 26, 2023

I really like the simplicity of how Laravel handles models to json serialization. For Django models that could look something like:

class Model():
    class Meta:
        serialized_fields = ('id', 'value_field', 'related_model')

I was surprised Django doesn't support something like that out of the box. I asked around on the Django discord and got responses like "Django isn't a rest api framework" and "make your own serializer [json encoder]". In my opinion a web framework is a super set of a rest api framework, thus would expect to have it included.

I was hesitant to use DRF as it seems like overkill for what I would use it for, but maybe it's the best solution we have?
Something like serpy seems more appropriate here as serializing is all I need it to do. This package may not be supported anymore though. Any other thoughts?

A slightly related issue I've run into is the lazy loading of related models. So instead of serializing a related model if it has been queried (Laravel behavior), DRF for example will lazy load them leading to n+1 issues. Here is the section in DRF docs mentioning it:
"Note: REST Framework does not attempt to automatically optimize querysets passed to serializers in terms of select_related and prefetch_related since it would be too much magic. A serializer with a field spanning an orm relation through its source attribute could require an additional database hit to fetch related objects from the database. It is the programmer's responsibility to optimize queries to avoid additional database hits which could occur while using such a serializer."

I've looked into disabling lazy loading globally in Django but doesn't seem it's supported. The disadvantage here is needing to know what data to include in the query for the serializer to match.

@BrandonShar
Copy link
Collaborator

I love the idea of that, but I think it would have enough edge cases that it's too complex for inertia to take on. I'm also on the same page where I think DRF is too complex for what I'm typically looking to do.

Serpy looks pretty similar to what I've manually done in the past: I generally created pretty simple serializer dataclasses and then pass them instances of models.

It would be great if we could figure out a better solution, but I don't think it's worth it to create a serializer library within inertia. Maybe the ecosystem just needs a new serializer library in general!

@dankuck how are you currently handling serializing relationships in your project?

@dankuck
Copy link

dankuck commented Jan 26, 2023

When you were working on it you used serializers and we've kept up that tradition

@ChristopherBrownie
Copy link
Author

ChristopherBrownie commented Jan 26, 2023

So it sounds like we will seemingly always want to use a serializer library or convert the models to dictionaries ourselves. Maybe that could be stated in the inertia-django doc somewhere, as data is treated the same way in adapters like laravel and RoR.
Not wanting to stray from Django behavior makes sense, the model_to_dict() behavior might already be doing that though. The InertiaJsonEncoder seems limited and borderline produces unexpected output in this case, whereas Django wouldn't encode any of it.

@BrandonShar
Copy link
Collaborator

Yeah that's a fair point. The InertiaJsonEncoder was intended to get people off the ground quickly, but maybe it's more hurt than help and it would be better to explicitly tell people they need to serialize on their own and let Django throw errors if they don't.

@ChristopherBrownie
Copy link
Author

I've personally switched to the DjangoJsonEncoder, and here is the serialization approach I've gone with:

class CustomModel(Model):
    # ...
    objects = BaseQuerySet.as_manager()

    def serialize(self, *relations) -> dict:
        data = {
            'id': self.id,
            'url': self.get_absolute_url(),
            'name': self.name,
        }
        if ('custom_related_model' in relations):
            # Conditionally include related model to avoid n+1 issues
            data['custom_related_model'] = self.custom_related_model.serialize()
        return data

class BaseQuerySet(QuerySet):
    def serialize(self) -> list:
        return [model.serialize() for model in self]

# Other QuerySets can be added for including related model info - for example...
def serialize_with_custom_related_model(self):
    return [model.serialize('custom_related_model') for model in self.select_related('custom_related_model')]
# This also ensures the relation is eager loaded in the query whenever it should have its data included

This usage ended up being surprisingly simple, and it gives full control over what is serialized and how its formatted. Alternatively, a library that can serialize a model to a dict could be used instead, such as DRF.

It's a little more verbose than the Laravel implementation, but it also reduces magic, which is important to Django.

Feel free to use any of this in the readme if you think it would help future newcomers!

@vsg24
Copy link

vsg24 commented Jun 10, 2023

This is bugging me as well, wish it was handled automatically

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

4 participants