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