Ruby on Rails syntax

This is one of those questions that you just know is going to make you feel stupid when you know the answer, but I just can't get this thing working so here goes. In railstutorial.org I'm trying to solve exercise 5 at the end of chapter 10 but the following code is faulty, and every variation I try (using 'unless' for example) does not work. I simply want to execute two lines of code if the current user is not an admin user. Where am I going wrong?

def destroy
  if !current_user.admin?
    User.find(params[:id]).destroy
    flash[:success] = "User destroyed."
  end
  redirect_to users_path
end

EDIT: I was confusing the current user with the user to be deleted. I am using Aditya's code, which I'm sure is correct, but now I am getting a 'can't run tests' message suggesting there is a syntax error in my tests (below).

describe "as an admin user" do

  before(:each) do
    admin = Factory(:user, :email => "admin@example.com", :admin => true)
    test_sign_in(admin)
  end

  it "should not allow an admin user to destroy himself" do
    lambda do
      delete :destr开发者_开发技巧oy, :id => admin
    end.should_not change(User, :count)
  end

end

---

def destroy
  if current_user.admin? 
    # If the current user IS admin
    User.find(params[:id]).destroy # Find the user and destroy it
    flash[:success] = "User destroyed." # Set the flash
  end
  redirect_to users_path # Go to users.
end

Your if condition does not make sense. It was saying if the user is NOT an admin, destroy. Whereas i would imagine it would be that only admins can destroy users. :)

EDIT: After reading Exercise 5 of Chapter 10 of the Railstutorial.org, I get a clearer idea. :) Modify the destroy action to prevent admin users from destroying themselves.

I know this might be cheating but this is SO and we suckers are here to help!

def destroy
  user = User.find(params[:id])
  if user != current_user
    user.destroy
    flash[:notice] = "User destroyed".
  else
    flash[:error] = "Suicide is immoral."
  end
  redirect_to users_path
end

Yet another edit, ONLY admins are not allowed to fall on the sword.

def destroy
  user = User.find(params[:id]) # Find the victim
  if (user == current_user) && (current_user.admin?)
    flash[:error] = "Suicide is immoral."
  else
    user.destroy
    flash[:notice] = "User destroyed".
  end
  redirect_to users_path
end

---

What exactly is not working?

Check if a user exists with the id param that you are passing. User.find returns nil if no user with the given id is found. Calling destroy on nil is going to throw an error.

---

Since the code you wrote does what you think it does, then the error is in User#admin?. To check it, open rails console and find the current user manually. Is it really an admin, what does the database say? Call admin? by hand and see what it returns. If the user is admin according to the database, then the error is in the implementation of the admin?-method.

---

Regarding the edit by the OP, including the rspec test, the 'admin' variable should not be local, but an instance variable (@admin):

describe "as an admin user" do
  before(:each) do
    @admin = Factory(:user, :email => "admin@example.com", :admin => true)
    test_sign_in(@admin)
  end

  it "should not destroy himself" do
    lambda do
      delete :destroy, :id => @admin
    end.should_not change(User, :count)
  end
end

Furthermore, I've made some other modifications to the code of the controller:

...
before_filter :authenticate_admin, :only => :destroy
...
def destroy
  user = User.find(params[:id])
  if user != current_user
    user.destroy
    flash[:success] = t(:user_deleted_message)
  else
    flash[:error] = t(:user_not_deleted_message)
  end
  redirect_to users_path
end
...
private
  def authenticate_admin
    if current_user.nil?
      redirect_to(signin_path)
    else
      redirect_to(root_path) unless current_user.admin?
    end
  end
...

I hope it helps somebody else looking for a solution to the exercise from the tutorial.

---

I asked the same question. I used

def destroy
      User.find(params[:id]).destroy unless current_user.admin? 
      flash[:success] = "User destroyed."
      redirect_to users_path

    end

a little funky, but it's short!