CODEWITHSUNDEEP
ruby-on-rails
Chris Mendla
Chris Mendla

Reputation: 1017

Ruby on Rails get check which page we are on

I am trying to exclude an action by a controller from one particular page - the signon page. I want the code to execute on every other page.

In my controller, I have:

def user_check
  @current_user ||= User.find(session[:user_id]) if session[:user_id]

  if (session[:user_id])
    @loginstatus = "You are logged in as: #{@current_user.name}"
  elseif current_page?('/signin') == true
     @loginstatus = "Please login to use this application"
  else
    redirect_to '/sessions/new', :error => "You are not logged in"
  end
end

The line that is giving me trouble is 

elseif current_page?('/signin') == true

I am calling this in application.html.erb with 

<%= user_check %>

When I do that, I get

pp/views/layouts/application.html.erb where line #94 raised: undefined method `current_page?' for # Rails.root: C:/Users/cmendla/RubymineProjects/Rl2

BUT - if I put the following in a XX.html.erb page, it works fine. 

<%= render(:partial => 'shared/alt_nav') if current_page?('/help') %>

What I don't understand is why the current_page? check works on the page but doesn't work inside the elseif in a def.

Upvotes: 1

Views: 849

Answers (2)

Richard Peck
Richard Peck

Reputation: 76784

Your code isn't very efficient - you're validating two completely (unrelated) sets of conditions -- whether a variable is declared and whether a user is visiting a page.

I would do the following:

#app/controllers/application_controller.rb
class ApplicationController < ActionController::Base
    before_filter :check_user

    private

    def check_user
       @current_user ||= User.find(session[:user_id]) if session[:user_id]

       if (session[:user_id])
          @loginstatus = "You are logged in as: #{@current_user.name}"
       else
          redirect_to '/sessions/new', :error => "You are not logged in"
       end
    end
end

#app/controllers/sessions_controller.rb
class SessionsController < ApplicationController
   skip_before_filter :check_user
end

This does two things:

1) It builds your conditional logic around one set of conditions.

2) It makes your logic action orientated

The main problem you have is calling your current_page? in conditional logic basically ties your hands very tightly in respect to any other part of your application. Rails applications are object orientated, and are expected to behave as such.

When you validate on whether a user has visited a page, you're neglecting the fact that later in your development, that "page" could do / mean something completely different. Not to mention you may wish to extend your functionality beyond that page (what about "registrations" etc?).

This means that when you call conditional logic as you are, you are best keeping it base-level simple (are they logged in?), and filtering that based on the action the user is performing.

For example...

#app/controllers/sessions_controller.rb
class SessionsController < ApplicationController
   skip_before_filter :check_user, except: :destroy #-> doesn't check user login for user login / registration; does for logout
end

Hopefully this helps.

Upvotes: 2

Patrice Gagnon
Patrice Gagnon

Reputation: 1464

Move the signon page to an other controller (another views sub-directory), change the route as well. That'll be the cleanest.

If you expect all the secured page to call user_check, the simplest is to have your signon mounted to an /unsecure/signon route, mapped to and unsecure_controller.rb that' doesn't call user_check.

Upvotes: 0

Related Questions