Redmine Refactor #107: Extract method #create from ProjectsController#add

I decided to start splitting the dual method actions in the ProjectsController (dual method: takes GET and POST requests to perform two separate actions). I wanted to merge some of the extra actions like #archive into these methods first but I think it will be easier once the dual methods are split up. I might even wait until after ProjectsController is converted to a REST resource before merging.

So I’m going to start on the #add method today. It’s used to render the form to add a new project as well as to receive the response from the form to create the project.

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
33
34
35
36
37
38
39
40
41
42
43

class ProjectsController  [ :index, :list, :add, :copy ]
  before_filter :authorize, :except => [ :index, :list, :add, :copy, :archive, :unarchive, :destroy]
  before_filter :authorize_global, :only => :add
  before_filter :require_admin, :only => [ :copy, :archive, :unarchive, :destroy ]
  accept_key_auth :index
 
  after_filter :only => [:add, :edit, :archive, :unarchive, :destroy] do |controller|
    if controller.request.post?
      controller.send :expire_action, :controller => 'welcome', :action => 'robots.txt'
    end
  end
 
  def add
    @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position")
    @trackers = Tracker.all
    @project = Project.new(params[:project])
    if request.get?
      @project.identifier = Project.next_identifier if Setting.sequential_project_identifiers?
      @project.trackers = Tracker.all
      @project.is_public = Setting.default_projects_public?
      @project.enabled_module_names = Setting.default_projects_modules
    else
      @project.enabled_module_names = params[:enabled_modules]
      if validate_parent_id && @project.save
        @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
        # Add current user as a project member if he is not admin
        unless User.current.admin?
          r = Role.givable.find_by_id(Setting.new_project_user_role_id.to_i) || Role.givable.first
          m = Member.new(:user => User.current, :roles => [r])
          @project.members < 'projects', :action => 'settings', :id => @project
          }
          format.xml  { head :created, :location => url_for(:controller => 'projects', :action => 'show', :id => @project.id) }
        end
      else
        respond_to do |format|
          format.html
          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
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49

class ProjectsController  [ :index, :list, :add, :create, :copy ]
  before_filter :authorize, :except => [ :index, :list, :add, :create, :copy, :archive, :unarchive, :destroy]
  before_filter :authorize_global, :only => [:add, :create]
  before_filter :require_admin, :only => [ :copy, :archive, :unarchive, :destroy ]
  accept_key_auth :index
 
  after_filter :only => [:create, :edit, :archive, :unarchive, :destroy] do |controller|
    if controller.request.post?
      controller.send :expire_action, :controller => 'welcome', :action => 'robots.txt'
    end
  end
 
  def add
    @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position")
    @trackers = Tracker.all
    @project = Project.new(params[:project])
 
    @project.identifier = Project.next_identifier if Setting.sequential_project_identifiers?
    @project.trackers = Tracker.all
    @project.is_public = Setting.default_projects_public?
    @project.enabled_module_names = Setting.default_projects_modules
  end
 
  def create
    @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position")
    @trackers = Tracker.all
    @project = Project.new(params[:project])
 
    @project.enabled_module_names = params[:enabled_modules]
    if validate_parent_id && @project.save
      @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
      # Add current user as a project member if he is not admin
      unless User.current.admin?
        r = Role.givable.find_by_id(Setting.new_project_user_role_id.to_i) || Role.givable.first
        m = Member.new(:user => User.current, :roles => [r])
        @project.members < 'projects', :action => 'settings', :id => @project
        }
        format.xml  { head :created, :location => url_for(:controller => 'projects', :action => 'show', :id => @project.id) }
      end
    else
      respond_to do |format|
        format.html { render :action => 'add' }
        format.xml  { render :xml => @project.errors, :status => :unprocessable_entity }
      end
    end
 
  end
 
end

There a five things that changed in this refactoring:

  1. I created a new method #create
  2. I copied the setup code for #add into #create
  3. I moved the HTTP POST code from #add into #create (false condition for if request.get?)
  4. I updated #create so the rendering uses the #add template
  5. I updated the routes so #add is connected to GET requests and #create is connect to POST requests

Reference commit