django-rest-framework-json-api icon indicating copy to clipboard operation
django-rest-framework-json-api copied to clipboard

Resolve resource type in serializer's meta class

Open sliverc opened this issue 3 years ago • 8 comments

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:

  • move logic of get_resource_type_from_serializer to serializer's meta class
  • improve error handling (better error message).

sliverc avatar Nov 30 '21 17:11 sliverc

See discussion #1017 for more details.

sliverc avatar Nov 30 '21 18:11 sliverc

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.

jokiefer avatar Dec 01 '21 14:12 jokiefer

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.

sliverc avatar Dec 02 '21 17:12 sliverc

Ok - i'll do it quickly.

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

jokiefer avatar Dec 02 '21 18:12 jokiefer

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.

sliverc avatar Dec 02 '21 19:12 sliverc

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.

SafaAlfulaij avatar Dec 31 '21 15:12 SafaAlfulaij

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

jokiefer avatar Dec 31 '21 17:12 jokiefer

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.

SafaAlfulaij avatar Jan 04 '22 18:01 SafaAlfulaij