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

Fix Sparse fieldsets to work with relationships #389

Closed
wants to merge 1 commit into from

Conversation

mrhardikjoshi
Copy link
Contributor

@mrhardikjoshi mrhardikjoshi commented Sep 14, 2021

Fixes #301

The sparse fieldsets is not filtering relationships. Currently if we specify any attribute and/or relationship in fields param, it works correctly on attributes but the not on relationships. It returns all the relationships, ignoring fields param. According to jsonapi format both attribute and relationship should be supported. This PR fixes that.

In case of fields param having only attribute(s) in them, this change will restrict all relationships as well because of the following line from jsonapi format doc (fields represent both attributes and relationships)

If a client requests a restricted set of fields for a given resource type, an endpoint MUST NOT include additional fields in resource objects of that type in its response.

I have not changed include section because of the following specification from the jsonapi format doc.

The only exception to the full linkage requirement is when relationship fields that would otherwise contain linkage data are excluded via sparse fieldsets

So in case of having a relationship mentioned in include and sparse fieldsets having some atttribute, this will still return the relationship but the linkage won't be there.

@smai-f
Copy link

smai-f commented May 25, 2022

Just being the messenger from the Graphiti Slack so future perusers are aware...

image

https://graphiti-api.slack.com/archives/CJLJFRQFK/p1633956166007200?thread_ts=1632930343.006800&cid=CJLJFRQFK

@jkeen jkeen added the wontfix label Feb 27, 2024
@jkeen
Copy link
Collaborator

jkeen commented Feb 27, 2024

I'm getting up to speed here and cleaning up some things and am going to have to trust the wisdom of @richmolj and close this PR until I'm more convinced that this is a real issue.

@jkeen jkeen closed this Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fields parameter is not considering relationship names as values
3 participants