Why does the order of the before_actions declaration matter?

Question

I am working through Hartl's Ruby on Rails Tutorial. When doing Chapter 11, I updated the before_actions in the UserController and did not think the order mattered. So mine looked like this:

class UsersController < ApplicationController
  before_action :correct_user,   only: [:edit, :update]
  before_action :admin_user,     only: :destroy
  before_action :signed_in_user,
            only: [:index, :edit, :update, :destroy, :following, :followers]

His looked like this:

class UsersController < ApplicationController
  before_action :signed_in_user,
            only: [:index, :edit, :update, :destroy, :following, :followers]
  before_action :correct_user,   only: [:edit, :update]
  before_action :admin_user,     only: :destroy

The only difference is the order of the statements.

When I run rspec on mine, I get four errors. But when I change the order, there are none. I don't see any dependence on the order of the declarations. I would like to know why the order matters.

That's my question... Here is more information about the code I am running.

Because I'm at the end of the book, there is a lot of code, so I will try to include the parts that I think could be relevant.

Two of the actions listed in the declarations are defined at the end of the same class:

class UsersController < ApplicationController

  before_action :correct_user,   only: [:edit, :update]
  before_action :admin_user,     only: :destroy
  before_action :signed_in_user,
            only: [:index, :edit, :update, :destroy, :following, :followers]

.
.
.
  private

  def user_params
    params.require(:user).permit(:name, :email, :password,
                                 :password_confirmation)
  end

  # Before filters

  def correct_user
    @user = User.find(params[:id])
    redirect_to(root_url) unless current_user?(@user)
  end

  def admin_user
    redirect_to(root_url) unless current_user.admin?
  end
end

The third one is defined in the SessionsHelper which is included in the ApplicationController, the parent class of UsersController:

class ApplicationController < ActionController::Base
  protect_from_forgery with: :exception
  include SessionsHelper
end

module SessionsHelper
.
.
.
  def current_user=(user)
    @current_user = user
  end

  def current_user
    remember_token = User.digest(cookies[:remember_token])
    @current_user ||= User.find_by(remember_token: remember_token)
  end

  def current_user?(user)
    user == current_user
  end

  def signed_in_user
    unless signed_in?
      store_location
      redirect_to signin_url, notice: "Please sign in."
    end
  end

  def signed_in?
    !current_user.nil?
  end
.
.
.

Here is the result if running rspec with the code ordered the way I did it:

>rspec spec
.............................................................................F.
....FF.............................................F...........................
.............

Failures:

  1) Authentication authorization for non-signed-in users when attempting to visit a protected page after signing in should render the desired protected page
     Failure/Error: expect(page).to have_title('Edit user')
       expected #has_title?("Edit user") to return true, got false
     # ./spec/requests/authentication_pages_spec.rb:68:in `block (6 levels) in <top (required)>'

  2) Authentication authorization for non-signed-in users in the Users controller visiting the edit page 
     Failure/Error: it { should have_title('Sign in') }
       expected #has_title?("Sign in") to return true, got false
     # ./spec/requests/authentication_pages_spec.rb:117:in `block (6 levels) in <top (required)>'

  3) Authentication authorization for non-signed-in users in the Users controller submitting to the update action 
     Failure/Error: specify { expect(response).to redirect_to(signin_path) }
       Expected response to be a redirect to <http://www.example.com/signin> but was a redirect to <http://www.example.com/>.
       Expected "http://www.example.com/signin" to be === "http://www.example.com/".
     # ./spec/requests/authentication_pages_spec.rb:122:in `block (6 levels) in <top (required)>'

  4) User pages as admin user should not be able to delete it's own account 
     Failure/Error: expect { delete user_path(admin) }.not_to change(User, :count)
     NoMethodError:
       undefined method `admin?' for nil:NilClass
     # ./app/controllers/users_controller.rb:95:in `admin_user'
     # ./spec/requests/user_pages_spec.rb:79:in `block (5 levels) in <top (required)>'
     # ./spec/requests/user_pages_spec.rb:79:in `block (4 levels) in <top (required)>'

Finished in 8.84 seconds
171 examples, 4 failures

Failed examples:

rspec ./spec/requests/authentication_pages_spec.rb:67 # Authentication authorization for non-signed-in users when attempting to visit a protected page after signing in should render the desired protected page
rspec ./spec/requests/authentication_pages_spec.rb:117 # Authentication authorization for non-signed-in users in the Users controller visiting the edit page 
rspec ./spec/requests/authentication_pages_spec.rb:122 # Authentication authorization for non-signed-in users in the Users controller submitting to the update action 
rspec ./spec/requests/user_pages_spec.rb:78 # User pages as admin user should not be able to delete it's own account 

Here are the failing tests in the authentication_pages_spec.rb:

describe "Authentication" do
  subject { page }
.
.
.
  describe "authorization" do

    describe "for non-signed-in users" do
      let(:user) { FactoryGirl.create(:user) }

      describe "when attempting to visit a protected page" do
        before do
          visit edit_user_path(user)
          sign_in(user)
        end

        describe "after signing in" do

          it "should render the desired protected page" do
            expect(page).to have_title('Edit user')
          end

.
.
.
      end
.
.
.
      describe "in the Users controller" do

        describe "visiting the edit page" do
          before { visit edit_user_path(user) }
          it { should have_title('Sign in') }
        end

        describe "submitting to the update action" do
          before { patch user_path(user) }
          specify { expect(response).to redirect_to(signin_path) }
        end
        describe "visiting the user index" do
          before { visit users_path }
          it { should have_title('Sign in') }
        end

        describe "visiting the following page" do
          before { visit following_user_path(user) }
          it { should have_title('Sign in') }
        end

        describe "visiting the followers page" do
          before { visit followers_user_path(user) }
          it { should have_title('Sign in') }
        end

      end
    end

Just to be complete, let me add that if I change the order of the before_action declarations to this:

  before_action :signed_in_user,
                only: [:index, :edit, :update, :destroy, :following, :followers]
  before_action :correct_user,   only: [:edit, :update]
  before_action :admin_user,     only: :destroy

All of the tests pass:

>rspec spec
..............................................................
..............................................................
...............................................

Finished in 8.82 seconds
171 examples, 0 failures

The code seems to work perfectly in the browser either way. The order of the declarations only affects the result of the tests. It took me a while to find this problem by going back over Hartl's code and making sure mine exactly reflected his. I found this order difference after a while and made the change, and suddenly my tests all passed. I've switched the order back and forth several times to make sure it is the order and only the order that is changing and that is making the difference. I'd like to understand so I'm not caught on a similiar problem when I am writing my own code rather than copying someone else's code.

Answer 1

It looks like in this controller, signed_in_user is acting as a guard to make sure that there is a valid current_user before proceeding (and if there isn't one, redirecting to the signin page, where one will be created).

Your fourth test gives this away - admin? is undefined because admin_user was called before signed_in_user. When signed_in_user is called first, admin_user won't be called unless there is actually a valid current_user to call admin? on. So there actually is a dependency on the order of filters here - signed_in_user must be called first, because it ensures that there is a valid current_user, and the other filters assume that this has happened already.

Answer 2

Obviously the order of before_action really matters.

If you don't use signed_in_user at the first, other actions (such as correct_user or admin_user method) may not have the current_user variable to use.

When you login first, your code may work well. But if you call actions before login, you will get error just like current_user is nil.