Conversation
dgeb
reviewed
Jan 2, 2019
|
|
||
| def test_pictures_index_with_polymorphic_include_one_level | ||
| assert_cacheable_get :index, params: {include: 'imageable'} | ||
| get :index, params: {include: 'imageable'} |
Contributor
There was a problem hiding this comment.
revert to assert_cacheable_get for consistency
dgeb
reviewed
Jan 2, 2019
| merge(Book.not_banned) | ||
| }, | ||
| class_name: 'Book', | ||
| join_table: :book_authors |
* enables related resource filtering * introduces JoinTree to track the required joins for a request to avoid join collisions * simplifies the resource finder, but there's still some room for improvement * makes use of the rails 5 `left_join` method. For rails 4 will require an additional gem to add the functionality. This is tested with the `left_join` gem, though others could exist and work.
dgeb
approved these changes
Jan 3, 2019
Contributor
dgeb
left a comment
dgeb
left a comment
There was a problem hiding this comment.
Looks good now. I like the renamed relation_join_hash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This solves a nagging issue of how to handle tracking aliases for arbitrarily deep joins. This is needed to allow filtering based on the joined tables.
The aliases are determined by building up the required joins one level at a time and noting the nodes added to the relation. To handle joins that may be added twice the JoinTree class is introduced and allows the tracking of the required joins and handles consolidating duplicate joins, say from two filters on the same related table.
This also removes some custom sql that was used for left joins. This is now using the
left_joinmethod that's in rails 5. This will require a gem to add this functionality to rails 4 projects. Theleft_joingem has been tested, and seems to work well.Adds support for relationship filters.
All Submissions:
New Feature Submissions:
Bug fixes and Changes to Core Features:
Test Plan:
Reviewer Checklist: