Conversation
closes #26
@gssbzn Do you feel this is mergable now? It sure seems to work. Are there any noteworthy differences to #29? /CC @seanfcarroll |
@bbonislawski and @gssbzn given you have solved the same problem, do you think the two approaches should be merged? Alternatively vote for one of the approaches and then perhaps write some tests? I see there is a test in this PR, but I'd be happy seeing a few more. That being said, there are a lot of people waiting on this in the Ransack and ActiveAdmin communities. If no-one is willing to spend time on tests it can be merged right away, but we might regrets that approach. Thoughts? |
@seanfcarroll @halo I did the fixes and changes based on the new 5.2 API for queries but wanted to test Ransack/ActiveAdmin to check nothing breaks. Sadly I've been busy at work this week but should have some free time this monday. If someone is able to check ransack referencing my branch over the weekend and provide some feedback I'll be thankful |
@gssbzn Thank you for your work. Are you not experiencing any of the problems listed in #26 (comment)? If I use @Petercopter can you provide a stacktrace for your |
I fixed this:
As there was a change on the Rails API not addressed on #29, the rails team now pass an extra argument to track table aliases I'll need to check @Petercopter error |
@seanfcarroll I suggest that you merge this PR for the following reasons:
Thank you and Merry Christmas! ;) |
Thanks for your efforts @halo ... fingers crossed! |
That was fast, haha! 😆 Will you release it to Rubygems? |
Yes, will do. |
FYI - I'm just waiting on gem release permissions. Will be released ASAP. |
end | ||
|
||
def build_constraint(klass, table, key, foreign_table, foreign_key) | ||
if reflection.polymorphic? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this works as expected?
JoinAssociation#build_constraint
was moved into Reflection
at rails/rails@442c15f.
Initial work to bring support to rails 5.2
closes #26