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

Changes from update_attributes to assignment #56

Closed
wants to merge 1 commit into from

Conversation

vesan
Copy link

@vesan vesan commented Oct 21, 2012

Marks position column as not attr_accessible (like it is in many apps)
and sets the column by directly setting the attribute.

Fixes issue #50

I don't know if there was a reason for using update_attributes!, I couldn't
find any. This patch adds set_list_position public method. It could also
be private but there are few tests that used update_attributes! which I
converted to use the new set_list_position method.

This way of fixing the problem doesn't make acts_as_list dependent of
attr_accessible like #51
which is nice considering strong_parameters seems to be the future.

Marks position column as not attr_accessible (like it is in many apps) 
and sets the column by directly setting the attribute.

Fixes issue brendon#50
@itsNikolay
Copy link

Good pull request!
Voting for merge it.
Without the request, I getting brainblowing issue when I use:

attr_accessible :position, as: :admin

@swanandp
Copy link
Contributor

This is good, thanks!

@swanandp
Copy link
Contributor

This got merged with #60.

@vesan
Copy link
Author

vesan commented Nov 23, 2012

Thanks!

@vesan vesan closed this Nov 23, 2012
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