RolifyCommunity / rolify

Role management library with resource scoping
https://rolifycommunity.github.io/rolify/
MIT License
3.17k stars 405 forks source link

suggestion: move code injected into User to a method and inject the method instead #26

Closed hazah closed 12 years ago

hazah commented 12 years ago

Maybe it's just me, but I've always favored the declarative approach rather then the procedural one when drawing up a specification. What are your thoughts in regards to moving the injected text out of the model and into its own method? That gives you two things, first, a cleaner User model :). Second, and I think more importantly, it allows you to have finer control over the monkey patching that takes place. Currently, since all the code is dumped into the user's lap, you have to be very careful by not adding anything to the scheme. Hiding it behind a method will give you more flexibility on how the User model will get augmented.

turn this:


inject_into_class(model_path, user_cname.camelize) do
   " include Rolify::Roles\n" +
   " #{'# ' if !options[:dynamic_shortcuts]}extend Rolify::Dynamic\n" +
   " has_and_belongs_to_many :roles#{", :class_name => \"" + role_cname.camelize + "\"" if role_cname != "Role"}, :join_table => :#{user_cname.tableize + "_" + role_cname.tableize}\n"
end

into this:


inject_into_class(model_path, user_cname.camelize) do
   "rolify"
end

def self.rolify option = { :role_cname => 'Role', :dynamic => false }

   include Rolify::Roles
   extend Rolify::Dynamic if options[:dynamic]
   has_and_belongs_to_many :roles, :class_name => options[:role_cname].constantize, :join_table => :"#{self.class.to_s.tabletize}_#{options[:role_cname].tableize}"

end

Thoughts?

EppO commented 12 years ago

I like it. it's pretty neat indeed. I changed many times the design of the module and it would make easier future upgrades. putting that in the v3 roadmap items.

thanks for your feedback, very appreciated.

hazah commented 12 years ago

No prob. I'm beginning to commit to this gem for my purposes, so I am interested in its evolution :).

EppO commented 12 years ago

Still need to extend the User class before calling rolify method. Will see if I can extend ActiveRecord or Mongoid to avoid that.

hazah commented 12 years ago

I favour the ActiveSupport.on_load(:active_record) do ... end approach :).

EppO commented 12 years ago

For v3.0, I am introducing mongoid support, so I guess I have to find a way to handle both. For instance, having to explicitly extend the class works for both cases.

EppO commented 12 years ago

Apparently ActiveSupport.on_load(:mongoid) seems to work as well but mongoid object doesn't inherit from a base class like ActiveRecord objects, so maybe I will have to the extend Object class... still investigating

hazah commented 12 years ago

Hmm I believe the purpose of ActiveSupport is to handle the individual cases (that is they're supposed to abstract away that issue). I have not dealt with other ORM layers though. Let me know if you need help. I would not mind flexing my brain matter, but I have not yet dealt with other ORM layers before.

EppO commented 12 years ago

I think I will take the Kaminari's approach: https://github.com/amatsuda/kaminari/blob/master/lib/kaminari/hooks.rb