Redmine Refactor #132: Convert Timelogs to REST Resource

Now that TimelogController has most of the REST actions it needs, I’m able to convert it to a REST resource.

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

# config/routes.rb
  map.connect 'time_entries/:id/edit', :action => 'edit', :controller => 'timelog'
  map.connect 'time_entries/:id', :action => 'update', :controller => 'timelog', :conditions => {:method => :put}
  map.connect 'projects/:project_id/time_entries/new', :action => 'new', :controller => 'timelog'
  map.connect 'projects/:project_id/issues/:issue_id/time_entries/new', :action => 'new', :controller => 'timelog'
 
  map.with_options :controller => 'timelog' do |timelog|
    timelog.connect 'projects/:project_id/time_entries', :action => 'index'
 
    timelog.with_options :action => 'index', :conditions => {:method => :get}  do |time_details|
      time_details.connect 'time_entries'
      time_details.connect 'time_entries.:format'
      time_details.connect 'issues/:issue_id/time_entries'
      time_details.connect 'issues/:issue_id/time_entries.:format'
      time_details.connect 'projects/:project_id/time_entries.:format'
      time_details.connect 'projects/:project_id/issues/:issue_id/time_entries'
      time_details.connect 'projects/:project_id/issues/:issue_id/time_entries.:format'
    end
    timelog.connect 'projects/:project_id/time_entries/report', :controller => 'time_entry_reports', :action => 'report'
    timelog.with_options :controller => 'time_entry_reports', :action => 'report',:conditions => {:method => :get} do |time_report|
      time_report.connect 'time_entries/report'
      time_report.connect 'time_entries/report.:format'
      time_report.connect 'projects/:project_id/time_entries/report.:format'
    end
 
    timelog.with_options :action => 'new', :conditions => {:method => :get} do |time_edit|
      time_edit.connect 'issues/:issue_id/time_entries/new'
    end
 
    timelog.connect 'projects/:project_id/timelog/edit', :action => 'create', :conditions => {:method => :post}
    timelog.connect 'time_entries/:id/destroy', :action => 'destroy', :conditions => {:method => :post}
  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

# config/routes.rb
  map.connect 'projects/:project_id/time_entries/report', :controller => 'time_entry_reports', :action => 'report'
  map.with_options :controller => 'time_entry_reports', :action => 'report',:conditions => {:method => :get} do |time_report|
    time_report.connect 'time_entries/report'
    time_report.connect 'time_entries/report.:format'
    time_report.connect 'projects/:project_id/time_entries/report.:format'
  end
 
  # TODO: wasteful since this is also nested under issues, projects, and projects/issues
  map.resources :time_entries, :controller => 'timelog'
 
  map.resources :issues, :member => { :edit => :post }, :collection => {} do |issues|
    issues.resources :time_entries, :controller => 'timelog'
  end
 
  map.resources :issues, :path_prefix => '/projects/:project_id', :collection => { :create => :post } do |issues|
    issues.resources :time_entries, :controller => 'timelog'
  end
 
  map.resources :projects, :member => {
    :copy => [:get, :post],
    :settings => :get,
    :modules => :post,
    :archive => :post,
    :unarchive => :post
  } do |project|
    # ...
    project.resources :time_entries, :controller => 'timelog', :path_prefix => 'projects/:project_id'
  end

This was one of the more difficult refactorings I’ve done, because of all the potential ways a timelog can be accessed.

Time Entry Reports

1
2
3
4
5
6

  map.connect 'projects/:project_id/time_entries/report', :controller => 'time_entry_reports', :action => 'report'
  map.with_options :controller => 'time_entry_reports', :action => 'report',:conditions => {:method => :get} do |time_report|
    time_report.connect 'time_entries/report'
    time_report.connect 'time_entries/report.:format'
    time_report.connect 'projects/:project_id/time_entries/report.:format'
  end

First, I extracted the routing for the TimeEntryReportsController. This was embedded in the map.with_options section before but there was no reason to leave it in there with the resource.

Base Time Entries

1

  map.resources :time_entries, :controller => 'timelog'

Next I converted the base time entries at the root of Redmine to a resource. This is the standard “/time_entries” and “/time_entries/1234” routing that is common with Rails apps.

Issue Time Entries

1
2
3

  map.resources :issues, :member => { :edit => :post }, :collection => {} do |issues|
    issues.resources :time_entries, :controller => 'timelog'
  end

Then I needed to handle issue specific routing of time entries, like “/issues/100/time_entries” and “/issues/100/time_entries/1234”. For this I defined a nested resource under the :issues resource.

Project Time Entries

1
2
3
4
5
6
7
8
9
10

  map.resources :projects, :member => {
    :copy => [:get, :post],
    :settings => :get,
    :modules => :post,
    :archive => :post,
    :unarchive => :post
  } do |project|
    # ...
    project.resources :time_entries, :controller => 'timelog', :path_prefix => 'projects/:project_id'
  end

I also needed to handle project specific routing like, “/projects/my-project/time_entries” and “/projects/my-project/time_entries/1234”. So underneath the :projects resource I added another nested resource, just like I did for :issues.

Project and Issue Time Entries

1
2
3

  map.resources :issues, :path_prefix => '/projects/:project_id', :collection => { :create => :post } do |issues|
    issues.resources :time_entries, :controller => 'timelog'
  end

Finally I needed to handle both project and issue specific routing, like “/projects/my-project/issues/100/time_entries” and “/projects/my-project/issues/100/time_entries/1234”. For this I used a nested resource underneath the other :issues resource. Since that :issues resource is using a :path_prefix, it will pick up the project part of the url that I need.

The good news is that I now have enough of Redmine converted to REST routes that I can start to refactor the nested routes using the :shallow option. Hopefully this will really clean up the routing and make it easy to start using resources as links (e.g. issues_path, project_issues_path(@project)). This will also make adding a REST API much easier now that some of the controllers follow a standard format.

Reference commit