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

Add role inheritance to attr_accessible #5699

Closed
wants to merge 2 commits into from
Closed

Add role inheritance to attr_accessible #5699

wants to merge 2 commits into from

Conversation

larzconwell
Copy link
Contributor

I noticed when making pretty complex applications with a lot of attributes and roles, that the attr_accessible list gets copied a lot, and just isn't very DRY. Adding an inheritance option to attr_accessible enables developers to clearly see the attributes being inherited to the role.


self._accessible_attributes = accessible_attributes_configs.dup

Array(role).each do |name|
self._accessible_attributes[name] = self.accessible_attributes(name) + args
role = inherited_role || name # Use name role if inherited_role doesn't exist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From your example above you pass both :as => :admin and :inherits => :default, but here you are overwriting role if inherited_role was passed. Doesn't that make it meaningless to pass the :as option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, because you still need the :as to access the role. I tested the overwriting in a live application to make sure it didn't mess anything up, and it works perfectly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it's not overwriting any original roles, the Array(role).each is looping through all the given attributes for that role, then if there is an inherited role it will gather those attributes as well, then the + args below will gather the current roles attributes.

@dmathieu
Copy link
Contributor

I like the idea. Not very much the API though.
What about having something like this :

attr_accessible :title, as: [:admin, :user, :default]

This would avoid adding a new parameter and allow multiple inheritance.

@erichmenge
Copy link

I don't think much can be done with this until after the details on the strong_parameters implementation are hashed out.

I believe the model assignment security is going to be factored out into a gem at some point, IIRC.

@larzconwell
Copy link
Contributor Author

@dmathieu Actually that's already available and I didn't know that at the time I wrote this up.

I'll close this since you can already do something similar by just using an array in the as option.

Thanks for the comments!

vijaydev pushed a commit that referenced this pull request Jul 7, 2012
Related to the request #5699 - #5699 and
not documented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants