Ruby on Rails application controller

Posted on

Problem

My application controller keeps growing with small helper methods that I use in both my controller and views, is it okay? or what do you recommend doing?

class ApplicationController < ActionController::Base
  protect_from_forgery with: :exception

  helper_method :resource_name, :resource, :devise_mapping, :resource_class, :is_admin?, :is_representative?, :is_shipper?,
    :is_agent?, :is_operation_completed?, :is_fcl_exw?, :is_fcl_exw_info_stage_completed?, :is_fcl_exw_info_requested?, :is_fcl_exw_info_confirmed?,
    :is_pricing_representative?, :is_fcl_exw_quotation_confirmed?



  # Roles detection helpers
  def is_admin?
    current_role == 'admin' ? true : false
  end

  def is_representative?
    current_role == 'representative' ? true : false
  end

  def is_shipper?
    current_role == 'shipper' ? true : false
  end

  def is_agent?
    current_role == 'agent' ? true : false
  end

  def is_pricing_representative?
    current_role == 'pricing_representative' ? true : false
  end



  # Devise helpers
  def resource_name
    :user
  end

  def resource
    @resource ||= User.new
  end

  def resource_class
    User
  end

  def devise_mapping
    @devise_mapping ||= Devise.mappings[:user]
  end


  # Operation helpers
  def is_operation_completed?(operation_id)
    GeneralCargoInfo.find_by(operation_id: operation_id).nil? ? false : true
  end

  # FCL-EXW helpers
  def is_fcl_exw_info_stage_completed?(operation_id)
    FclExwCargoInfo.find_by(operation_id: operation_id).nil? ? false : true
  end

  def is_fcl_exw?(operation_id)
    Operation.find(operation_id).modality == 'FCL - EXW' ? true : false
  end

  def is_fcl_exw_info_requested?(operation_id)
    Operation.find(operation_id).fcl_exw_info_requested
  end

  def is_fcl_exw_info_confirmed?(operation_id)
    Operation.find(operation_id).fcl_exw_info_confirmed
  end

  def is_fcl_exw_quotation_confirmed?(operation_id)
    Operation.find(operation_id).fcl_exw_quotation_confirmed
  end

  private
    def current_role
        current_role = current_user.nil? ? 'Not logged in' : Role.find(current_user.role_id).name 
    end

    def require_new_operation_permission
      check_permission('representative', 'agent')
    end

    def check_permission(*roles)
      unless roles.include? current_role
        flash[:alert] = "Access denied"
        redirect_back(fallback_location: authenticated_root_path)
      end
    end


end

Solution

There is no need to use the prefix is with every method if your method return type is boolean then simply use ? at the end of the method name
I replace all the role methods name from is_role? with role? ,i.e is_admin? to admin? etc.
instead of nil? Should use present?

class ApplicationController < ActionController::Base
  protect_from_forgery with: :exception

  helper_method :resource_name, :resource, :devise_mapping, :resource_class, :is_operation_completed?, :is_fcl_exw?, :is_fcl_exw_info_stage_completed?, :is_fcl_exw_info_requested?, :is_fcl_exw_info_confirmed?,:is_fcl_exw_quotation_confirmed?

  ROLES = %w[admin representative shipper agent pricing_representative]
  ROLES.each do |role|
    define_method("#{role}?") { current_role == role }
    helper_method "#{role}?".to_sym
  end

  # Devise helpers
  def resource_name
    :user
  end

  def resource
    @resource ||= User.new
  end

  def resource_class
    User
  end

  def devise_mapping
    @devise_mapping ||= Devise.mappings[:user]
  end


  # Operation helpers
  def is_operation_completed?(operation_id)
    GeneralCargoInfo.find_by(operation_id: operation_id).present?
  end

  # FCL-EXW helpers
  def is_fcl_exw_info_stage_completed?(operation_id)
    FclExwCargoInfo.find_by(operation_id: operation_id).present?
  end

  def is_fcl_exw?(operation_id)
    Operation.find(operation_id).modality == 'FCL - EXW'
  end

  def is_fcl_exw_info_requested?(operation_id)
    Operation.find(operation_id).fcl_exw_info_requested
  end

  def is_fcl_exw_info_confirmed?(operation_id)
    Operation.find(operation_id).fcl_exw_info_confirmed
  end

  def is_fcl_exw_quotation_confirmed?(operation_id)
    Operation.find(operation_id).fcl_exw_quotation_confirmed
  end

  private
    def current_role
        current_role = current_user.nil? ? 'Not logged in' : Role.find(current_user.role_id).name 
    end

    def require_new_operation_permission
      check_permission('representative', 'agent')
    end

    def check_permission(*roles)
      unless roles.include? current_role
        flash[:alert] = "Access denied"
        redirect_back(fallback_location: authenticated_root_path)
      end
    end
end

Note: remove is with other methods as well also if you not consume this helper methods with any of the controllers then move them to view helper

You can move helper methods to ApplicationHelper and simply include it in Application controller, so your code will look like:

ApplicationController:

class ApplicationController < ActionController::Base
  include ApplicationHelper
  protect_from_forgery with: :exception

  private
    def current_role
        current_role = current_user.nil? ? 'Not logged in' : Role.find(current_user.role_id).name 
    end

    def require_new_operation_permission
      check_permission('representative', 'agent')
    end

    def check_permission(*roles)
      unless roles.include? current_role
        flash[:alert] = "Access denied"
        redirect_back(fallback_location: authenticated_root_path)
      end
    end
end

ApplicationHelper:

module ApplicationHelper
  # Roles detection helpers
  def is_admin?
    current_role == 'admin' ? true : false
  end

  def is_representative?
    current_role == 'representative' ? true : false
  end

  def is_shipper?
    current_role == 'shipper' ? true : false
  end

  def is_agent?
    current_role == 'agent' ? true : false
  end

  def is_pricing_representative?
    current_role == 'pricing_representative' ? true : false
  end

  # Devise helpers
  def resource_name
    :user
  end

  def resource
    @resource ||= User.new
  end

  def resource_class
    User
  end

  def devise_mapping
    @devise_mapping ||= Devise.mappings[:user]
  end


  # Operation helpers
  def is_operation_completed?(operation_id)
    GeneralCargoInfo.find_by(operation_id: operation_id).nil? ? false : true
  end

  # FCL-EXW helpers
  def is_fcl_exw_info_stage_completed?(operation_id)
    FclExwCargoInfo.find_by(operation_id: operation_id).nil? ? false : true
  end

  def is_fcl_exw?(operation_id)
    Operation.find(operation_id).modality == 'FCL - EXW' ? true : false
  end

  def is_fcl_exw_info_requested?(operation_id)
    Operation.find(operation_id).fcl_exw_info_requested
  end

  def is_fcl_exw_info_confirmed?(operation_id)
    Operation.find(operation_id).fcl_exw_info_confirmed
  end

  def is_fcl_exw_quotation_confirmed?(operation_id)
    Operation.find(operation_id).fcl_exw_quotation_confirmed
  end
end

Leave a Reply

Your email address will not be published. Required fields are marked *