Redmine Refactor #109: Extract method ProjectsController#edit to #update

Using extract method on the ProjectsController, I split the #edit method into #edit and #update.

This is starting to become a standard refactoring for me now while I convert everything over to RESTful Rails. I’m even thinking about naming this refactoring “split method” or “split action”.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32

class ProjectsController  [:create, :edit, :archive, :unarchive, :destroy] do |controller|
    if controller.request.post?
      controller.send :expire_action, :controller => 'welcome', :action => 'robots.txt'
    end
  end
 
  # Edit @project
  def edit
    if request.get?
    else
      @project.attributes = params[:project]
      if validate_parent_id && @project.save
        @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
        respond_to do |format|
          format.html { 
            flash[:notice] = l(:notice_successful_update)
            redirect_to :action => 'settings', :id => @project
          }
          format.xml  { head :ok }
        end
      else
        respond_to do |format|
          format.html { 
            settings
            render :action => 'settings'
          }
          format.xml  { render :xml => @project.errors, :status => :unprocessable_entity }
        end
      end
    end
  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
23
24
25
26
27
28
29
30
31
32
33
34

class ProjectsController  [:create, :edit, :update, :archive, :unarchive, :destroy] do |controller|
    if controller.request.post?
      controller.send :expire_action, :controller => 'welcome', :action => 'robots.txt'
    end
  end
 
  # TODO: convert to PUT only
  verify :method => [:post, :put], :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
 
  def edit
  end
 
  def update
    @project.attributes = params[:project]
    if validate_parent_id && @project.save
      @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
      respond_to do |format|
        format.html { 
          flash[:notice] = l(:notice_successful_update)
          redirect_to :action => 'settings', :id => @project
        }
        format.xml  { head :ok }
      end
    else
      respond_to do |format|
        format.html { 
          settings
          render :action => 'settings'
        }
        format.xml  { render :xml => @project.errors, :status => :unprocessable_entity }
      end
    end
  end
end

Something that is important to remember when splitting actions is to make sure you check which HTTP verbs can access an action. I don’t want GET or DELETE to reach #update, so I used the verify method to make sure that only POST or PUT methods can reach it. Once I setup the projects resource in the routes.rb, Rails will handle this for me. But until then I need to be safe and explicitly define what is allowed.

Reference commit