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

Add $joinEager query param to fetch relations with JoinEagerAlgorithm #10

Merged
merged 2 commits into from
Jan 20, 2018

Conversation

cdelaorden
Copy link
Contributor

Hi, this makes a quick fix for #9

You can now use $joinEager param in the query object, and this relation will be eager loaded with SQL join, so you can actually filter based on related fields.

NOTE: At the moment it doesn't work too well with $select, because $select refers to fields in the Model table and joinEager will add all the related entities field so ambiguous name can occur.

This can however fixed easily by using the modify parameter in the relation, that allows to include specific fields for the related entity.

Pull request includes code, test, and added --fix to your eslint config so spaces and such are automatically fixed. For tests, I added a new Employee model that belongs to existing Company. That DB setup for tests could probably be extracted into a helper file, but I didn't want to move too many parts around. :)

What do you think? :)

@NotAmaan
Copy link
Collaborator

@mcchrish Would you mind reviewing this? Its currently not possible to use eager otherwise. Should also fix #8

@mcchrish
Copy link
Member

@cdelaorden @NotAmaan thanks for taking a look at this.

It's been awhile since I've used this lib for a project but feel free to extend and send PRs when you see an improvement can be done.

I can see as well that feathers have moved on to v3. Looking forward for this moved along side it.

@mcchrish mcchrish merged commit 4bb46e1 into feathersjs-ecosystem:master Jan 20, 2018
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.

None yet

3 participants