Redmine Refactor #143: Convert WikiController to use :id params

This refactoring is a major change to how Redmine’s WikiController is routed. It’s been using the params[:page] field to track which page in the wiki is being requested but that won’t work with Rails’ resource routing, which uses params[:id]. Since I’ve already refactored Redmine so that params[:project_id] is used instead of params[:id], I am now going to convert WikiController so it uses params[:id] instead of params[:page].

Before

1
2
3

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

# config/routes.rb
  map.with_options :controller => 'wiki' do |wiki_routes|
    wiki_routes.with_options :conditions => {:method => :get} do |wiki_views|
      wiki_views.connect 'projects/:project_id/wiki/export', :action => 'export'
      wiki_views.connect 'projects/:project_id/wiki/index', :action => 'index'
      wiki_views.connect 'projects/:project_id/wiki/date_index', :action => 'date_index'
      wiki_views.connect 'projects/:project_id/wiki/:page', :action => 'show', :page => nil
      wiki_views.connect 'projects/:project_id/wiki/:page/edit', :action => 'edit'
      wiki_views.connect 'projects/:project_id/wiki/:page/rename', :action => 'rename'
      wiki_views.connect 'projects/:project_id/wiki/:page/history', :action => 'history'
      wiki_views.connect 'projects/:project_id/wiki/:page/diff/:version/vs/:version_from', :action => 'diff'
      wiki_views.connect 'projects/:project_id/wiki/:page/annotate/:version', :action => 'annotate'
    end
 
    wiki_routes.connect 'projects/:project_id/wiki/:page/:action', 
      :action => /rename|preview|protect|add_attachment/,
      :conditions => {:method => :post}
 
    wiki_routes.connect 'projects/:project_id/wiki/:page/edit', :action => 'update', :conditions => {:method => :post}
 
    wiki_routes.connect 'projects/:project_id/wiki/:page', :action => 'destroy', :conditions => {:method => :delete}
  end

After

1
2
3

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

# config/routes.rb
  map.with_options :controller => 'wiki' do |wiki_routes|
    wiki_routes.with_options :conditions => {:method => :get} do |wiki_views|
      wiki_views.connect 'projects/:project_id/wiki/export', :action => 'export'
      wiki_views.connect 'projects/:project_id/wiki/index', :action => 'index'
      wiki_views.connect 'projects/:project_id/wiki/date_index', :action => 'date_index'
      wiki_views.connect 'projects/:project_id/wiki/:id', :action => 'show', :id => nil
      wiki_views.connect 'projects/:project_id/wiki/:id/edit', :action => 'edit'
      wiki_views.connect 'projects/:project_id/wiki/:id/rename', :action => 'rename'
      wiki_views.connect 'projects/:project_id/wiki/:id/history', :action => 'history'
      wiki_views.connect 'projects/:project_id/wiki/:id/diff/:version/vs/:version_from', :action => 'diff'
      wiki_views.connect 'projects/:project_id/wiki/:id/annotate/:version', :action => 'annotate'
    end
 
    wiki_routes.connect 'projects/:project_id/wiki/:id/:action', 
      :action => /rename|preview|protect|add_attachment/,
      :conditions => {:method => :post}
 
    wiki_routes.connect 'projects/:project_id/wiki/:id/edit', :action => 'update', :conditions => {:method => :post}
 
    wiki_routes.connect 'projects/:project_id/wiki/:id', :action => 'destroy', :conditions => {:method => :delete}
  end

I’m only showing the #show action here because all of the rest of the changes were similar.

Git tipped me off to a really bad smell with Redmine’s Wiki:

Showing 66 changed files with 281 additions and 281 deletions.

So I changed one parameter and 66 files had to change… something feels wrong here. If you look at the commit, every layer of the Redmine stack was changed because of this parameter change:

  • controllers – makes sense, since parameters are the inputs to controllers
  • helpers – okay
  • views – okay, links need to use the new parameters
  • routing – fine, Redmine has some complex routes
  • lib – starting to see some implementation leak out of the controller/view here
  • models – really?
  • translations – that’s just wrong

Once I’ve finished refactoring to restful routes, some of these smells might go away but I suspect their smelly friends will come back later and bite me.

Reference commit