Redmine Refactor #123: Split #add method in UsersController to #add and #create

The next set of refactorings will be done to the UsersController. There are a few people wanting to add a REST API for Redmine Users, so I want to make the controller is refactored before we start adding new features. UsersController is in pretty good shape, it only has two non-standard methods and two methods that handle both GET and POST.

Starting with the methods at the top of the class (because it’s an easy way to refactor), the #add method is handling the form rendering (GET) and saving a new user (POST). So I’ll use split method to break this up into separate methods, #add and #create.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17

class UsersController  Setting.default_language)
    else
      @user = User.new(params[:user])
      @user.admin = params[:user][:admin] || false
      @user.login = params[:user][:login]
      @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless @user.auth_source_id
      if @user.save
        Mailer.deliver_account_information(@user, params[:password]) if params[:send_information]
        flash[:notice] = l(:notice_successful_create)
        redirect_to(params[:continue] ? {:controller => 'users', :action => 'add'} : 
                                        {:controller => 'users', :action => 'edit', :id => @user})
        return
      end
    end
    @auth_sources = AuthSource.find(:all)
  end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22

class UsersController  Setting.default_language)
    @auth_sources = AuthSource.find(:all)
  end
 
  verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
  def create
    @user = User.new(params[:user])
    @user.admin = params[:user][:admin] || false
    @user.login = params[:user][:login]
    @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless @user.auth_source_id
    if @user.save
      Mailer.deliver_account_information(@user, params[:password]) if params[:send_information]
      flash[:notice] = l(:notice_successful_create)
      redirect_to(params[:continue] ? {:controller => 'users', :action => 'add'} : 
                                      {:controller => 'users', :action => 'edit', :id => @user})
      return
    else
      @auth_sources = AuthSource.find(:all)
      render :action => 'add'
    end
  end
end

In order to make this split method work, I needed to use verify to make sure only POST requests get to #create and I had to add an else case to handle failed saves. Next I’ll need to rename #add since that’s not the standard method name for RESTful Rails.

Reference commit