Redmine Refactor #129: Extract Method TimelogController#edit to TimelogController#new

In this refactoring I’m using extract method to split up the TimelogController#edit method. Right now it’s handling four separate actions:

  1. Display an empty form for a new TimeEntry
  2. Save a new TimeEntry
  3. Display a form for an existing TimeEntry
  4. Save an existing TimeEntry

Normally these would be separate RESTful Rails actions; #new, #create, #edit, and #update. I’m going to start with extracting the #new method from #edit.

Before

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

class TimelogController  [:edit, :destroy]
 
  def edit
    (render_403; return) if @time_entry && !@time_entry.editable_by?(User.current)
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
    @time_entry.attributes = params[:time_entry]
 
    call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
 
    if request.post? and @time_entry.save
      flash[:notice] = l(:notice_successful_update)
      redirect_back_or_default :action => 'index', :project_id => @time_entry.project
      return
    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

class TimelogController  [:new, :edit, :destroy]
 
  def new
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
    @time_entry.attributes = params[:time_entry]
 
    call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
    render :action => 'edit'
  end
 
  def edit
    (render_403; return) if @time_entry && !@time_entry.editable_by?(User.current)
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
    @time_entry.attributes = params[:time_entry]
 
    call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
 
    if request.post? and @time_entry.save
      flash[:notice] = l(:notice_successful_update)
      redirect_back_or_default :action => 'index', :project_id => @time_entry.project
      return
    end    
  end
end

Even after this refactoring, there isn’t any code that can be removed from #edit. It looks like once I do a few more method extractions I’ll be able to remove some code, but until then I need to duplicate the method body.

Reference commit