Refactoring code inside a Ruby Proc in a rails_admin custom action

Question

A little background:

My app has tools which have permission_levels. There are User's that have authorization for some tools and not others. The admin panel is built using the rails_admin gem.

If the admin is editing a User and that User has access to tools that the admin does not, these tools do not show up in the edit view (this is desired). However, when the admin updates the User, the User loses the associations to tools and permission_levels that were not shown.

To combat that, I overrode the #edit action inside RailsAdmin. The resulting code is a bit of a mess, and I'd like to refactor by pulling out related code into methods, but the section is wrapped inside a Proc.

How can I pull out the code below into methods? Specifically, the register_instance_option :controller section.

.
.
.
module Actions
  class Edit < RailsAdmin::Config::Actions::Base

    register_instance_option :member? do
      true
    end

    register_instance_option :route_fragment do
      'edit'
    end

    register_instance_option :http_methods do
      [:get, :put]
    end

    register_instance_option :visible? do
      authorized?
    end

    register_instance_option :controller do
      Proc.new do
        if @object.class.base_class.name == 'User'
          if request.get?
            session[:tools] = (@object.tools - Tool.admined_by(current_user)).map(&:id)
            pl = session[:tools].map do |t|
              tl = Tool.find(t)
              tl.permission_levels
            end
            session[:permission_levels] = []
            pl.flatten.each do |p|
              session[:permission_levels] << p if @object.permission_levels.include?(p)
            end
            session[:permission_levels].map!(&:id)
          elsif request.put?
            duped_tools = session[:tools].dup
            params[:user][:tool_ids] = duped_tools.
              concat(params[:user][:tool_ids]).uniq!.delete_if { |id| id == "" }.
              map { |id| id.to_i }
            duped_permission_levels = session[:permission_levels].dup
            params[:user][:permission_level_ids] = duped_permission_levels.
              concat(params[:user][:permission_level_ids]).uniq!.delete_if { |id| id == "" }.
              map { |id| id.to_i }
            if @object.update_attributes(params.require(:user).permit!)
              session[:tools] = nil
              session[:permission_levels] = nil
              duped_session = nil
              flash[:success] = t("admin.flash.successful", :name => @model_config.label, :action => t("admin.actions.edit.done"))
              redirect_to index_path
            else
              flash[:error] = t("admin.flash.error", :name => @model_config.label, :action => t("admin.actions.edit.done"))
              redirect_path = back_or_index
            end
          end
        end
      end
    end

    register_instance_option :link_icon do
      'icon-pencil'
    end
  end
end
.
.
.

MrYoshiji · Accepted Answer · 2014-07-15T17:38:19.333

Here is several code improvements in order to use less lines and possibly better performances:

Use less lines:

# All `register_instance_option` can be refactored in a one-line bock:
register_instance_option :member? do
  true
end
# to
register_instance_option(:member?) { true }

Better use of Rails' methods:

if @object.class.base_class.name == 'User'
# can be changed to:
if @object.kind_of?(User) # will not work if @object is a Admin which inherits from User class

array_of_ids.delete_if { |id| id == "" }
# can be changed to:
array_of_ids.select(&:present?)

array = session[:tools].map do
  # your logic
end
array = array.flatten
# can be changed to:
array = session[:tools].flat_map do
  # your logic
end
# no need for a flatten (thanks to `.flat_map`)

session[:tools] = nil
session[:permission_levels] = nil
duped_session = nil
# can be changed to:
session[:tools] = session[:permission_levels] = duped_session = nil

I would have refactored your code into this: (feel free to comment this)

if @object.kind_of?(User)
  if request.get?
    session[:tools] = @object.tools.where('id NOT IN (?)', Tool.admined_by(current_user).pluck(:id)) # better performances
    session[:tools] = session[:tools].includes(:permission_levels).flat_map(&:permission_levels)
    session[:permission_levels] = []
    session[:tools].each do |p|
      session[:permission_levels] << p.id if @object.permission_levels.include?(p)
    end

  elsif request.put?
    duped_tools = session[:tools].dup
    params[:user][:tool_ids] = duped_tools.concat(params[:user][:tool_ids]).uniq.select(&:present?).map(&:to_i)
    duped_permission_levels = session[:permission_levels].dup
    params[:user][:permission_level_ids] = duped_permission_levels.concat(params[:user][:permission_level_ids]).uniq.select(&:present?).map(&:to_i)
    if @object.update_attributes(params.require(:user).permit!)
      session[:tools] = session[:permission_levels] = nil
      flash[:success] = t("admin.flash.successful", name: @model_config.label, name: t("admin.actions.edit.done"))
      redirect_to index_path
    else
      flash[:error] = t("admin.flash.error", name: @model_config.label, action: t("admin.actions.edit.done"))
      redirect_path = back_or_index
    end
  end
end

Refactoring code inside a Ruby Proc in a rails_admin custom action

1 Answers1

Use less lines:

Better use of Rails' methods: