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

#270 - Return a query builder for has many associations instead of array #274

Conversation

webandtech
Copy link
Collaborator

This PR addresses issue #270 which is that has_many associations only return an array, not a query builder which can accept params for querying the relationship.

This does not change the behavior of has_one or belongs_to associations.

With this PR, the has_many association no longer executes the query immediately instead, it returns a query builder based on the association class that you may use to query the association, such as by calling '#all' to get a paginatable ResultSet or applying other params to the query.

Owner.properties
#=> #<JsonApi::Query::Builder:0x007fca7197e350...>
props = Owner.properties.all
#=> [#<Property:@attributes=...,#<Property:@attributes=...]
props.next_page
#=> [#<Property:@attributes=...,#<Property:@attributes=...]

@webandtech
Copy link
Collaborator Author

@jamesotron @chingor13 I know this changes the behavior significantly but seems like the best way to adhere to the spec and provide a full query interface such as the ability to apply params to the association query such as Owner.properties.select('name').all.

To maintain backward compatibility we could possibly introduce a configuration option to return the query builder and change the default functionality to return a paginatable ResultSet. Looking forward to your thoughts!

jsmestad added a commit to jsmestad/jsonapi-consumer that referenced this pull request Apr 13, 2018
@jsmestad
Copy link
Collaborator

@webandtech this is great! I pulled it into my fork along with a handful of the other outstanding PRs on this project jsonapi-consumer

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