Serializer resource_name for nested objects is not used

[mkohram]

Nested serializers don't use resource_name for the type. In the example below the type for the child would show something like nestedmodels.

class NestedSerializer(serializers.ModelSerializer):
    resource_name = "pretty_nested"
    class Meta:
        model = NestedModel

class ParentSerializer(serializers.ModelSerializer):
    nested = NestedSerializer(source='some_field', many=True)
    class Meta:
        model = ParentModel

Looking at the code it looks like in utils.py the only function that performs the correct checks to get the type is get_resource_name(). All the other functions simply do something like relation_type = inflection.pluralize(relation_model.__name__).lower() without checking for a resource_name on the serializer as the docs suggest. I could submit a PR if it makes sense to put these checks in place everywhere. Ideally a function like get_related_resource_type() should act as a getter for type so that we don't see these inconsistencies.

[topaxi]

We just stumbled upon the relation_type too, we use Ember.js for our frontend and create models based on the type property.

It would be very beneficial to us, if we could configure how the relation_type is formatted, very similar to JSON_API_FORMAT_KEYS. Just lowercasing the model name seems to remove information on the actual model name, a dasherized, underscored or camelcased type would be much better.

Otherwise we're forced to use "single worded" models.

[jsenecal]

Thanks for the suggestion @topaxi - I'm quite swamped however lately so unless @jerel has some time to spare, we are open to PRs ;)

[jerel]

This should be fixed by #76 when it is merged.

[mkohram]

#76 does centralize the formatting but still relation_model.__name__ is being used everywhere disregarding resource_name on serializers. The docs say

You may manually set the resource_name property on views or serializers to specify the type key in the json output.

For example:
https://github.com/django-json-api/django-rest-framework-json-api/blob/3498201af79dad06c497ffbc3f128985c21cade7/rest_framework_json_api/utils.py#l298

The serializer resource_name is not looked for when creating the type string.

[jerel]

Thanks for the comment on relation_model.__name__. I'll close this and we'll add resource_name capabilities on the serializer as part of fixing #77

[mkohram]

Sweet. Let me know if I can help!

[jsenecal]

Since this issue is not fixed by 64f8a6b I'll reopen this as a reminder that the reported problem still exists.

[jsenecal]

@mkohram could you please test code from branch feature/compound_documents to see if it fixes this issue?

[mkohram]

I'm afraid not. Looking through the code I believe extract_relationships is one of the culprits. For example here:

django-rest-framework-json-api/rest_framework_json_api/utils.py

Line 381 in 66c1eef

('type', nested_resource_instance_type),

...
        if isinstance(field, ListSerializer):
            relation_data = list()

            serializer_data = resource.get(field_name)
            resource_instance_queryset = list(relation_instance_or_manager.all())
            if isinstance(serializer_data, list):
                for position in range(len(serializer_data)):
                    nested_resource_instance = resource_instance_queryset[position]
                    # something along the lines of 
                    # nested_resource_instance_type = field.child.Meta.resource_name
                    nested_resource_instance_type = get_resource_type_from_instance(nested_resource_instance)
                    relation_data.append(OrderedDict([
                        ('type', nested_resource_instance_type),
                        ('id', encoding.force_text(nested_resource_instance.pk))
                    ]))

                data.update({field_name: {'data': relation_data}})
                continue

A check on field.child.Meta.resource_name is needed before resorting to getting the type from the queryset.

Other isinstance() statements in that sequence need similar resource_name checks.

Also my relationships don't make it into the included on feature/compound_documents for some reason.

[mkohram]

actually looking closer, looks like the extract_included() function already does this!

            serializer = field.child
            relation_type = get_resource_type_from_serializer(serializer)

We just need that up in extract_relationships() as well!

[jsenecal]

@mkohram I know, this is what I changed in the branch (if you check the diff of the latest commit) II had forgot the lines in extract_relationships though. (Oups)

[jsenecal]

@mkohram would you mind testing again please ?

[mkohram]

OK. One more ...

If the field is defined as ResourceRelatedField then this line returns the wrong type in the data. The type is correct in the included section though. Stumbled on this one on accident.
I'm generally getting terrified of format_relation_name(get_resource_type_from_instance(instance)). I searched the repo and found it in a couple more places too (here and here). Not sure if they are harmful or not, would put my bets on yes 😏.

[scottfisk]

@mkohram Since get_resource_type_from_instance uses the model instance solely so I believe a similar resource_name property would be required in the model meta class that takes precedence over __name__. I am happy to PR something like this but I assume there is a reason this project hasn't gone that route.

I have gotten around this issue for the most part by performing some long overdue renames to models which was trivial with RenameModel from django.

The last holdout use case is that my user model is called MyUser and I want the resource to be called users. My preferable fix is to be able to rename the model to User but Django wont let me do that because it is so intertwined with the contrib.auth app (maybe that is a separate issue that I should just hunker down to figure out).

[mkohram]

@scottfisk I haven't had the time to look too closely yet but I think get_resource_type_from_instance() should just check and make sure resource_name doesn't exist on the serializer (or view if possible) before resorting to the model name. Right? BTW, there is already a PR (#145). We can discuss code changes there.

[leifurhauks]

@mkohram you are correct, if the serializer or view has an attribute resource_name it will be used for type, otherwise the renderer will use the model name. If resource_name is defined on both the serializer and the view, currently the one on the view takes precedence.

[jsenecal]

@mkohram @scottfisk @leifurhauks - We still need help on this, PRs could be applied to pr #145 so we could all contribute.
TBH - I'm a bit lost on this one - We need more tests.

[jsenecal]

This should be fixed by the added feature in #145 - I'll close this now but feel free to reopen if the issue persists