is this safe?

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

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).

-Rob

Rob Biedenharn http://agileconsultingllc.com
Rob@AgileConsultingLLC.com

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.

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:

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.