I am using an eval so I dont have to check avery case of a permission. def folder_permission(folder_id, per) permissions = self.user_folder_permissions.find_by_folder_id(folder_id) return true unless permissions.blank? or not eval("permissions."+per) return false end This function is called from private method in file controller. def authorize_creating unless session[:user].folder_permision(params[:folder_id],"can_create") flash.now[:folder_error] = "You don''t have create permissions for this folder." redirect_to :controller => ''file'', :action => ''error'' and return false end end So am I safe to use this, is this efficient for ruby? thanks PAco --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On Mar 26, 9:11 pm, "Paco Viramontes" <kidpo...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> I am using an eval so I dont have to check avery case of a permission. > > def folder_permission(folder_id, per) > permissions = self.user_folder_permissions.find_by_folder_id(folder_id) > return true unless permissions.blank? or not eval("permissions."+per) > return false > endI''d just write permissions.send(per) rather than use the eval. I don''t think you''re opening yourself up to anything super bad, but it seems like a big sledgehammer being used for a rather small nut. Fred --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On Mar 26, 2008, at 5:16 PM, Frederick Cheung wrote:> On Mar 26, 9:11 pm, "Paco Viramontes" <kidpo...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> I am using an eval so I dont have to check avery case of a >> permission. >> >> def folder_permission(folder_id, per) >> permissions = >> self.user_folder_permissions.find_by_folder_id(folder_id) >> return true unless permissions.blank? or not >> eval("permissions."+per) >> return false >> end > > I''d just write permissions.send(per) rather than use the eval. I don''t > think you''re opening yourself up to anything super bad, but it seems > like a big sledgehammer being used for a rather small nut. > > FredGo with .send(per), but doesn''t the structure of that hurt your head?! def folder_permission(folder_id, per) if permission = self.user_folder_permissions.find_by_folder_id(folder_id) permission.send(per) rescue false else false end end Having that "unless A or not B" in there has got to be confusing (or it will be a month from now). -Rob Rob Biedenharn http://agileconsultingllc.com Rob-xa9cJyRlE0mWcWVYNo9pwxS2lgjeYSpx@public.gmane.org --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On Wed, Mar 26, 2008 at 5:58 PM, Rob Biedenharn <Rob-GBZH0y1GwQfnZcttdmLDtcI/UQi/AW5J@public.gmane.org> wrote:> > On Mar 26, 2008, at 5:16 PM, Frederick Cheung wrote: > > On Mar 26, 9:11 pm, "Paco Viramontes" <kidpo...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> I am using an eval so I dont have to check avery case of a > >> permission. > >> > >> def folder_permission(folder_id, per) > >> permissions > >> self.user_folder_permissions.find_by_folder_id(folder_id) > >> return true unless permissions.blank? or not > >> eval("permissions."+per) > >> return false > >> end > > > > I''d just write permissions.send(per) rather than use the eval. I don''t > > think you''re opening yourself up to anything super bad, but it seems > > like a big sledgehammer being used for a rather small nut. > > > > Fred > > > Go with .send(per), but doesn''t the structure of that hurt your head?! > > > def folder_permission(folder_id, per) > if permission > > self.user_folder_permissions.find_by_folder_id(folder_id) > permission.send(per) rescue false > else > false > end > end > > Having that "unless A or not B" in there has got to be confusing (or > it will be a month from now).I''d consider a more radical refactoring (all untested of course): in the models (I''m assuming the folder_permission method is in the User model based on the controller code, and there''s a Permissions model): class Permissions < ActiveRecord::Base class NoPermissions < Permissions def can_not false end [:can_create, :can_read, :can_write].each {|meth| alias_method(meth, :can_not)} end def Permissions.none @no_permissions ||= NoPermissions.new end end class User < ActiveRecord::Base def permissions_for_folder(folder_id) user_folder_permissions.find_by_folder_id(folder_id) || Permissions.none end And the controller method becomes: def authorize_creating unless session[:user]. permissions_for_folder(params[:folder_id]).can_create) flash.now[:folder_error] = "You don''t have create permissions for this folder." redirect_to :controller => ''file'', :action => ''error'' and return false end end I might also think about renaming the can_xx methods to something like allows_xxx? or permits_xxx? which would read a bit better I think. -- Rick DeNatale My blog on Ruby http://talklikeaduck.denhaven2.com/ --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Ok, thanks I like your aproach Rick but I dont quite get what your are doung there in the permissions model. My model actually looks like this: ------ # == Schema Information # Schema version: 21 # # Table name: user_folder_permissions # # id :integer(11) not null, primary key # folder_id :integer(11) default(0) # user_id :integer(11) default(0) # can_create :boolean(1) # can_read :boolean(1) # can_update :boolean(1) # can_delete :boolean(1) # class UserFolderPermission < ActiveRecord::Base belongs_to :user belongs_to :folder end -------- Also if the permissions_for_folder receives folder_id why do you call it like this? permissions_for_folder(params[:folder_id]).can_create) On Fri, Mar 28, 2008 at 6:23 AM, Rick DeNatale <rick.denatale-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> > On Wed, Mar 26, 2008 at 5:58 PM, Rob Biedenharn > <Rob-GBZH0y1GwQfnZcttdmLDtcI/UQi/AW5J@public.gmane.org> wrote: > > > > On Mar 26, 2008, at 5:16 PM, Frederick Cheung wrote: > > > On Mar 26, 9:11 pm, "Paco Viramontes" <kidpo...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > >> I am using an eval so I dont have to check avery case of a > > >> permission. > > >> > > >> def folder_permission(folder_id, per) > > >> permissions > > >> self.user_folder_permissions.find_by_folder_id(folder_id) > > >> return true unless permissions.blank? or not > > >> eval("permissions."+per) > > >> return false > > >> end > > > > > > I''d just write permissions.send(per) rather than use the eval. I > don''t > > > think you''re opening yourself up to anything super bad, but it seems > > > like a big sledgehammer being used for a rather small nut. > > > > > > Fred > > > > > > Go with .send(per), but doesn''t the structure of that hurt your head?! > > > > > > def folder_permission(folder_id, per) > > if permission > > > > self.user_folder_permissions.find_by_folder_id(folder_id) > > permission.send(per) rescue false > > else > > false > > end > > end > > > > Having that "unless A or not B" in there has got to be confusing (or > > it will be a month from now). > > I''d consider a more radical refactoring (all untested of course): > > in the models (I''m assuming the folder_permission method is in the > User model based on the controller code, and there''s a Permissions > model): > > class Permissions < ActiveRecord::Base > > class NoPermissions < Permissions > def can_not > false > end > [:can_create, :can_read, :can_write].each {|meth| > alias_method(meth, :can_not)} > end > > def Permissions.none > @no_permissions ||= NoPermissions.new > end > end > > class User < ActiveRecord::Base > > def permissions_for_folder(folder_id) > user_folder_permissions.find_by_folder_id(folder_id) || > Permissions.none > end > > And the controller method becomes: > > def authorize_creating > unless session[:user]. > permissions_for_folder(params[:folder_id]).can_create) > flash.now[:folder_error] = "You don''t have create permissions > for this folder." > redirect_to :controller => ''file'', :action => ''error'' and return > false > end > end > > I might also think about renaming the can_xx methods to something like > allows_xxx? or permits_xxx? which would read a bit better I think. > > -- > Rick DeNatale > > My blog on Ruby > http://talklikeaduck.denhaven2.com/ > > > >--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On Apr 1, 1:49 pm, "Paco Viramontes" <kidpo...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> Ok, thanks I like your aproach Rick but I dont quite get what your are doung > there in the permissions model. My model actually looks like this: > ------ > # == Schema Information > # Schema version: 21 > # > # Table name: user_folder_permissions > # > # id :integer(11) not null, primary key > # folder_id :integer(11) default(0) > # user_id :integer(11) default(0) > # can_create :boolean(1) > # can_read :boolean(1) > # can_update :boolean(1) > # can_delete :boolean(1) > # > > class UserFolderPermission < ActiveRecord::Base > belongs_to :user > belongs_to :folder > end > > -------- > Also if the permissions_for_folder receives folder_id why do you call it > like this? > permissions_for_folder(params[:folder_id]).can_create)Because params[:folder_id] contains the folder_id ? Given your schema above, it does seem like you want to use Rick''s approach of 1) get the permissions object, 2) ask it about the particular permission permission_for(...).can_create? What''s the point in having those methods if you''re going to bend over backwards to call them via eval or send? You may want to consider caching the retrieved permission object also. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---