Redmine Refactor #113: Convert FilesController to resource

With yesterday’s split method on FilesController complete, I’m now ready to convert FilesController to become a full RESTful resource under projects.

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

# config/routes.rb
  map.resources :projects, :member => {
    :copy => [:get, :post],
    :settings => :get,
    :modules => :post,
    :archive => :post,
    :unarchive => :post
  } do |project|
    project.resource :project_enumerations, :as => 'enumerations', :only => [:update, :destroy]
  end
 
  # TODO: port to be part of the resources route(s)
  map.with_options :controller => 'projects' do |project_mapper|
    project_mapper.with_options :conditions => {:method => :get} do |project_views|
      project_views.connect 'projects/:id/files', :controller => 'files', :action => 'index'
      project_views.connect 'projects/:id/files/new', :controller => 'files', :action => 'new'
      project_views.connect 'projects/:id/settings/:tab', :controller => 'projects', :action => 'settings'
      project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new'
    end
 
    project_mapper.with_options :conditions => {:method => :post} do |project_actions|
      project_actions.connect 'projects/:id/files/new', :controller => 'files', :action => 'create'
    end
  end
 
1
2
3

class FilesController  'files', :action => 'index', :id => @project
  end
end

After

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

# config/routes.rb
  map.resources :projects, :member => {
    :copy => [:get, :post],
    :settings => :get,
    :modules => :post,
    :archive => :post,
    :unarchive => :post
  } do |project|
    project.resource :project_enumerations, :as => 'enumerations', :only => [:update, :destroy]
    project.resources :files, :only => [:index, :new, :create]
  end
 
  # TODO: port to be part of the resources route(s)
  map.with_options :controller => 'projects' do |project_mapper|
    project_mapper.with_options :conditions => {:method => :get} do |project_views|
      project_views.connect 'projects/:id/settings/:tab', :controller => 'projects', :action => 'settings'
      project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new'
    end
  end
 
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

class FilesController < ApplicationController
  menu_item :files
 
  before_filter :find_project_by_project_id
  before_filter :authorize
 
  helper :sort
  include SortHelper
 
  def index
    # ...
  end
 
  def new
    # ...
  end
 
  def create
    container = (params[:version_id].blank? ? @project : @project.versions.find_by_id(params[:version_id]))
    attachments = Attachment.attach_files(container, params[:attachments])
    render_attachment_warning_if_needed(container)
 
    if !attachments.empty? && Setting.notified_events.include?('file_added')
      Mailer.deliver_attachments_added(attachments[:files])
    end
    redirect_to project_files_path(@project)
  end
end

This refactoring let me remove some code in the routes but the big help comes from being able to use the Rails url helpers and setting up the controller for a major rewrite.

Now instead of writing the FilesController paths like:

1
2
3
4
5

redirect_to :controller => 'files', :action => 'index', :id => @project
 
link_to_if_authorized l(:label_attachment_new), {:controller => 'files', :action => 'new', :id => @project}, :class => 'icon icon-add'
 
form_tag({ :action => 'create', :id => @project }, :multipart => true, :class => "tabular")

I can use project_files_path now:

1
2
3
4
5

redirect_to project_files_path(@project)
 
link_to_if_authorized l(:label_attachment_new), new_project_file_path(@project), :class => 'icon icon-add'
 
form_tag(project_files_path(@project), :multipart => true, :class => "tabular")

The other major benefit of this refactoring is that I can now start to think about the proposed merge for Redmine’s Files and Documents modules. I think this merge will be a great addition for Redmine, but the existing code needs to be cleaned up in order to plan how to approach it.

Reference commit