django-json-api / django-rest-framework-json-api

JSON:API support for Django REST framework
https://django-rest-framework-json-api.readthedocs.org/
BSD 2-Clause "Simplified" License
1.18k stars 294 forks source link

Resolve resource type in serializer's meta class #1019

Open sliverc opened 2 years ago

sliverc commented 2 years ago

utils.get_resource_type_from_serializer is called several times throughout the DJA code base. When this method raises an AttributeError it is a configuration/code setup issue and not a runtime issue. Besides there is no error message describing want went wrong.

Goals:

sliverc commented 2 years ago

See discussion #1017 for more details.

jokiefer commented 2 years ago

i experiment with the meta class implementation at my fork

In principle that works - but i think we should rethink about the goal of moving the logic to the meta class creation, cause all serializers are configured on django start and at this point many tests have to become refactored. Maybe i can do that refactoring, but it will take a while.

The easier way would be just to add the detail information on the AttributeError.

sliverc commented 2 years ago

In terms of API moving the logic of calculating relation resource type. I understand that this is not a easy and simple task. So for a quick fix I am happy to receive a PR which adds details to the AttributeError. We can then leave this issue open for further refactoring later on.

jokiefer commented 2 years ago

Ok - i'll do it quickly.

Is there any program workflow cheatsheet? I would do a docs enhancement in a separated issue if not.

sliverc commented 2 years ago

As DJA is a DRF extension the worklow is the same. Not sure whether there is a sketch like this for DRF but I guess it would make most sense to add to the upstream Django REST framework doc.

Feel free to sketch something though and open a discussion about it.

SafaAlfulaij commented 2 years ago

While we are at this, what do you think if we allowed serializers themselves to define what kind of type they are, via a function? This would overwrite the attribute access. Something very similar to what DRF does with queryset/get_queryset and serializer_class/get_serializer_class in views. This would help for polymorphic relations, generic relations and custom non-ORM relations and serializers.

jokiefer commented 2 years ago

I think this would match the django common way. Many django porjects does it like that.

SafaAlfulaij commented 2 years ago

I'm also thinking if we make included_serializers and included_resources the same way. This also would help polymorphic relations, generic relations and custom non-ORM relations and serializers.