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

Fix attr_accessible handling #75

Closed
wants to merge 2 commits into from
Closed

Conversation

ahacking
Copy link

Closure Tree now works for me on Rails4 with attr_accessible

The class_eval used for building the new hierarchy class was not working as intended.

The accessible_attributes.present? check always returns false when building the new hierarchy class since the whitelist is always empty.

@mceachen
Copy link
Collaborator

The test is failing with ruby 1.9.3:

/home/travis/build/mceachen/closure_tree/lib/closure_tree/support.rb:31:in `hierarchy_class_for_model': uninitialized constant Activodel::ForbiddenAttributesProtection (NameError)
    from /home/travis/build/mceachen/closure_tree/lib/closure_tree/support.rb:30:in `class_eval'
    from /home/travis/build/mceachen/closure_tree/lib/closure_tree/support.rb:30:in `hierarchy_class_for_model'

Note the weird "Activodel"

@ahacking
Copy link
Author

Thats a strange error for sure, it worked for me, ruby -v reports:
ruby 1.9.3p194 (2012-04-20 revision 35410) [x86_64-darwin10.8.0]

Closure tree doesn't work with attr_accessible and AR 4.0.0 as it stands currently, can you just apply the changes in support_flags.rb or do you need another PR?

@mceachen
Copy link
Collaborator

I partially cherry-picked out support_flags in ff8331e and added some documentation in a1d8076. I can push a new version once Travis goes green.

@ahacking
Copy link
Author

Sounds great. Thanks!

@rubiii
Copy link

rubiii commented Sep 17, 2013

@mceachen just wanted to report that the change you cherry-picked seems to work fine.

@mceachen
Copy link
Collaborator

mceachen commented Oct 6, 2013

This got pushed into v4.2.8

@mceachen mceachen closed this Oct 6, 2013
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