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

Reverse the relationship names in includes #203

Open
schtibe opened this issue Feb 8, 2016 · 6 comments
Open

Reverse the relationship names in includes #203

schtibe opened this issue Feb 8, 2016 · 6 comments
Labels

Comments

@schtibe
Copy link
Contributor

schtibe commented Feb 8, 2016

I am not a 100% sure about this, but when all resource names are dasherized and pluralized, shouldn't the included names be plural and dasherized too?

By included names I mean:
http://jsonapi.org/format/#fetching-includes

Right now, I retrieve data like this: /api/v1/vehicle-checks?include=calendar_block, which shows the mix in naming style: the ressource is dasherized and pluralized, the included ressource is not.

This request works, and ``?include=calendar-blocks` does not, which is obvious when looking at serializers.py lines 75 - 80.

So my question is, shouldn't some reversing occur to match the naming style, similar to how the output is changed?

@jerel
Copy link
Member

jerel commented Feb 8, 2016

I feel like that would be reasonable. Perhaps using the JSON_API_FORMAT_RELATION_KEYS setting to reverse it to snake_case?

@schtibe
Copy link
Contributor Author

schtibe commented Feb 9, 2016

I suspect that it has to have an option to tell how to reverse the names... in case someone isn't following the pep8 standards.

Something like JSON_API_PARSE_RELATION_KEYS

@schtibe
Copy link
Contributor Author

schtibe commented Feb 11, 2016

Another way to solve this would be to use different names in the included_serializers.. although I am not quite sure if the names there have to match the field names!?

@jerel
Copy link
Member

jerel commented Feb 11, 2016

I think parsing it like you're doing in the PR is the right direction. The serializer doesn't strike me as the right way to stick this but I'm open to a counter argument

@jesushernandez
Copy link

@schtibe @jerel I just found myself in your same situation :). However I have a question: should includes refer always to the resource_name of the resource you want to include? or should they refer to the field name you defined in the serializer?

Let me explain with some dummy (incomplete) code:

class Car(object):
   pass

class Passenger(object):
  passenger = ForeignKey(Car, related_name='human-passengers')


class PassengerSerializer(serializer.ModelSerializer):
   class Meta:
       model = Passenger

class CarSerializer(serializer.ModelSerializer):
   passengers = ResourceRelatedField(many=True, source='human-passengers', read_only=True)

   include_serializers = dict(passengers=PassengerSerializer)
   class Meta:
       model = Car

class Passenger(viewsets.ReadOnlyModelViewSet):
   resource_name = 'human-passengers'
   serializer_class = PassengerSerializer

class Car(viewsets.ReadOnlyModelViewSet):
   serializer_class = CarSerializer

Now If I did GET /cars/1/?include=human-passengers I'd expect the view to return the human-passengers resource included in the response (but it won't).

At the moment, to get this I'd have to do GET /cars/1/?include=passengers, because passengers matches the field name in CarSerializer.

Acording to:
http://jsonapi.org/format/#document-resource-objects
http://jsonapi.org/format/#fetching-includes

What you put in the includes should be the whatever the type key says. In this case, the type key for the Passenger resource should be human-passengers (because DRF jsonapi allows you to explicitely set resource_name).

@sliverc sliverc added the bug label Jun 8, 2018
@sliverc
Copy link
Member

sliverc commented Oct 11, 2021

This is a fairly old issue and there might be some confusion. So to clarify the specification states that the include query param are relationship field names and not resource names (see https://jsonapi.org/format/#fetching-includes).

So simply running the include name through the utils.undo_format_field_name will do it. There is already some underscoring of include names happening here but it does not take into consideration that includes maybe nested (e.g. include=author-bio.author-type)

This issue is related to #871 Maybe fixing both issue in one PR might be considered if the unformatting could happen in the utility method utils.get_included_resources).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants