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 JSONApi sparse fieldset compliance for relationships #203

Merged
merged 2 commits into from
May 14, 2019

Conversation

leepfrog
Copy link

Changes:

Relevant section from the spec: "A client MAY request that an endpoint return only specific fields in the response on a per-type basis by including a fields[TYPE] parameter."

Copy link
Contributor

@jherdman jherdman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! A few notes on making this safe to release ASAP.

test/jsonapi/plugs/query_parser_test.exs Show resolved Hide resolved
lib/jsonapi/plugs/query_parser.ex Outdated Show resolved Hide resolved
test/jsonapi/plugs/query_parser_test.exs Outdated Show resolved Hide resolved
@leepfrog
Copy link
Author

Thx! Will make these changes and update this PR shortly.

{_, view} -> view
nil -> raise_invalid_field_names(type, view.type())
end
end

@spec is_field_valid_for_relationship({atom(), module()}, String.t()) :: boolean()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the simplest way I could think of to handle the deprecation, but happy to take suggestions if needed.

Andy Tran added 2 commits May 14, 2019 12:32
  instead of relationship name.

  see: https://jsonapi.org/format/#fetching-sparse-fieldsets

- query_parser_test: fix a typo in test relationship definition
Copy link
Member

@doomspork doomspork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this @leepfrog!

@doomspork doomspork merged commit 2e59cfa into beam-community:master May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants