Yet another dry question

Rails
1

One of these days I’ll figure this out, but in the meantime help me be a
better programmer by eliminating some excess code:

I’m trying to check to see if somebody trying to view/edit/update a
product is the owner. In my scaffold I have this code that works:

def edit
@owner = Product.find(params[:id].to_i)

  if @owner.user_id == @user.id
    @product = Product.find(params[:id])
  else
    flash[:notice] = 'Sorry, you're not the owner.'
    redirect_to :action => 'list'
  end

end

Ok, so to simplify it, I wrote a private method called confirm_user:

def confirm_owner
@owner = Product.find(params[:id].to_i)
unless @owner.user_id == @user.id
flash[:notice] = ‘Sorry, you’re not the owner.’
redirect_to :action => ‘list’
end
end

Now, how can I call this in my edit? I tried to use a before_filter
within my edit function but rails didn’t like that saying before_filter
is an undefined method:

def edit
before_filter :confirm_owner
@product = Product.find(params[:id])
end

Any suggestions on how to do this please?

2

Vince W. wrote:

One of these days I’ll figure this out, but in the meantime help me be a
better programmer by eliminating some excess code:

I’m trying to check to see if somebody trying to view/edit/update a
product is the owner. In my scaffold I have this code that works:

def edit
@owner = Product.find(params[:id].to_i)

  if @owner.user_id == @user.id
    @product = Product.find(params[:id])
  else
    flash[:notice] = 'Sorry, you're not the owner.'
    redirect_to :action => 'list'
  end

end

Ok, so to simplify it, I wrote a private method called confirm_user:

def confirm_owner
@owner = Product.find(params[:id].to_i)
unless @owner.user_id == @user.id
flash[:notice] = ‘Sorry, you’re not the owner.’
redirect_to :action => ‘list’
end
end

Now, how can I call this in my edit? I tried to use a before_filter
within my edit function but rails didn’t like that saying before_filter
is an undefined method:

def edit
before_filter :confirm_owner
@product = Product.find(params[:id])
end

Any suggestions on how to do this please?

In your controller, not in your action, try:

before_filter :confirm_owner, :only => [:view, :edit, :update]

That will constrain the filter to only the view, edit and update
actions. And you only need to say it once, not even in each action.

3

You’re actually 98% of the way there. All you’re missing is the place
to put the before_filter method call. It goes just below the Class
line of your Controller, not inside any of the Controller’s Actions,
as you have it in your email below.

Move the before_filter to the top of your Class definition, outside
all the actions, and you should be all set.

-Brian

4

What you are saying makes sense, and I’ve incorporated that into my
code:

class PostController < ApplicationController
include AuthenticatedSystem
before_filter :login_required,
:find_user,
:confirm_owner, :only => [:edit]

but when I put the line with my other before filters I get the error:

Called id for nil, which would mistakenly be 4 – if you really wanted
the id of nil, use object_id

I suspect it’s because in my confirm_owner I call
@owner = Product.find(params[:id].to_i

but the before filter hasn’t been passed any value for the product_id to
find. Does that make sense? So maybe the before_filter isn’t the way
to go?

-Vince

5

Vince W. wrote:

What you are saying makes sense, and I’ve incorporated that into my
code:

class PostController < ApplicationController
include AuthenticatedSystem
before_filter :login_required,
:find_user,
:confirm_owner, :only => [:edit]

but when I put the line with my other before filters I get the error:

Called id for nil, which would mistakenly be 4 – if you really wanted
the id of nil, use object_id

I suspect it’s because in my confirm_owner I call
@owner = Product.find(params[:id].to_i

but the before filter hasn’t been passed any value for the product_id to
find. Does that make sense? So maybe the before_filter isn’t the way
to go?

-Vince

I suspect its actually exploding on this line:

unless @owner.user_id == @user.id

Because you havent assigned anything to @user yet.

6

Alex W. wrote:

I suspect its actually exploding on this line:

unless @owner.user_id == @user.id

Because you havent assigned anything to @user yet.

I thought of that actually so I had added a definition for @ user in my
confirm_user method:

private
def confirm_owner
@user = User.find(session[:user].to_i)
@owner = Product.find(params[:id].to_i)
unless @owner.user_id == @user.id
flash[:notice] = ‘You are not the owner of this product.’
redirect_to :action => ‘list’
end
end

Still have the same problem though.

7

Vince W. wrote:

Alex W. wrote:

I suspect its actually exploding on this line:

unless @owner.user_id == @user.id

Because you havent assigned anything to @user yet.

I thought of that actually so I had added a definition for @ user in my
confirm_user method:

private
def confirm_owner
@user = User.find(session[:user].to_i)
@owner = Product.find(params[:id].to_i)
unless @owner.user_id == @user.id
flash[:notice] = ‘You are not the owner of this product.’
redirect_to :action => ‘list’
end
end

Still have the same problem though.

Well there is only one spot your are calling the method “id” and that is
on the @user object. Are you sure session[:user] has something in when
you execute this?

try this:

def confirm_owner
@user = User.find(session[:user])
@owner = Product.find(params[:id])
unless @user && @owner.user_id == @user.id
flash[:notice] = ‘You are not the owner of this product.’
redirect_to :action => ‘list’
end
end

Since you can’t be the owner if you are not logged in, the unless
statement first checks to see if @user is set. If @user is nil, the
second condition does not need to be executed, and we do the redirect.
If @user is a real user, we then check to see if that user is the owner.

Also the to_i is not require for the find method, it’ll take a number as
a string just fine.

8

Alex W. wrote:

try this:

def confirm_owner
@user = User.find(session[:user])
@owner = Product.find(params[:id])
unless @user && @owner.user_id == @user.id
flash[:notice] = ‘You are not the owner of this product.’
redirect_to :action => ‘list’
end
end

Same result…

I’m pretty sure it’s getting nil b/c it’s not passing a product id

-Vince

9

Vince W. wrote:

Alex W. wrote:

try this:

def confirm_owner
@user = User.find(session[:user])
@owner = Product.find(params[:id])
unless @user && @owner.user_id == @user.id
flash[:notice] = ‘You are not the owner of this product.’
redirect_to :action => ‘list’
end
end

Ok, I did some more hacking around on this. Turns out the nil error is
because of the before_filter confirm_owner :only => [:edit] line
when I remove the :only => [:edit] part I get an error saying Couldn’t
find Product without an ID.

I have no idea why I get a nil error when I try to use only or except in
the before filter but regardless I think I’m going to have to cut/paste
this code in a few different places because of product_id issue.

10

Vince W. wrote:

Vince W. wrote:

Alex W. wrote:

try this:

def confirm_owner
@user = User.find(session[:user])
@owner = Product.find(params[:id])
unless @user && @owner.user_id == @user.id
flash[:notice] = ‘You are not the owner of this product.’
redirect_to :action => ‘list’
end
end

Ok, I did some more hacking around on this. Turns out the nil error is
because of the before_filter confirm_owner :only => [:edit] line
when I remove the :only => [:edit] part I get an error saying Couldn’t
find Product without an ID.

I have no idea why I get a nil error when I try to use only or except in
the before filter but regardless I think I’m going to have to cut/paste
this code in a few different places because of product_id issue.

params should be available in before_filters, try sticking the value of
params[:id] in your session with “session[:debug] = params[:id]” and
looking at its value on the error page. If its nil, then you are not
successfully passing the value from the url into your action, and you
may want to look at your routes.