Skip to content

Conversation

@aaronrussell
Copy link
Contributor

I have created a rails-5 branch with new gemset, and all existing tests are green. You may want to merge this in to a separate rails-5 branch as I'm not claiming this is 100% problem free with Rails 5, but this PR fixes the issue I raised in #220, I've removed a few deprecation warnings from your test suite, and all existing tests are passing.

Ref #220, I have refactored how the default_tree_scope creates a scope. Rather than building it off _ct.model_class (which will always be the base class when STI is used), the method now explicitly accepts a scope on which to build the new scope. This ensures if hash_tree is appended to the end of a scope chain then all the existing conditions are included in the new scope.

As an aside, I don't actually know why this was working in Rails 4.2 - I suspect a bug in the internals of activerecord which is now fixed in 5.

…enerated. The scope should be generated from the existing chained scope, rather than built from `model_class`.
…ssociation to reload is now deprecated and will be removed in Rails 5.1
@seuros seuros self-assigned this Jun 20, 2016
@seuros seuros merged commit e5d65bb into ClosureTree:master Jun 20, 2016
@mceachen
Copy link
Collaborator

Thank you! 🎖
On Jun 20, 2016 5:52 AM, "Abdelkader Boudih" notifications@github.com
wrote:

Merged #221 #221.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#221 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AANNOcIlO394T4RPe3Dk8beErfcm4cTlks5qNo0GgaJpZM4I5mpi
.

@aaronrussell aaronrussell mentioned this pull request Jul 11, 2016
@bhtabor
Copy link

bhtabor commented Sep 24, 2016

Does this fix work for activerecord-5.0.0 too? I see that it has been tested against activerecord-5.0.0.rc1 but I still see hash_tree not using the STI scope on activerecord-5.0.0 Or is it just me?

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.

4 participants