Redmine Refactor #104: Move method from ProjectsController#add_file to FilesController#new

Since I created a new FilesController yesterday, I can now move another method over to it from the ProjectsController. ProjectsController#add_file is used for two things:

  1. To show the form that’s used to upload a new file
  2. To receive a new file upload

Since both of these are basic descriptions of what I’d expect a File resource to be, they fit perfectly on the FilesController.

Before

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

class ProjectsController  [:add_file]
 
  def add_file
    if request.post?
      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 :controller => 'files', :action => 'index', :id => @project
      return
    end
    @versions = @project.versions.sort
  end
end
 
1
2

class FilesController < ApplicationController
end

After

1
2
3
4

class ProjectsController < ApplicationController
  # No menu_item for :files
  # Removed add_file method
end
 
1
2
3
4
5
6

class FilesController  'files', :action => 'index', :id => @project
      return
    end
    @versions = @project.versions.sort
  end
end

As part of this refactor, I also added a TODO note for later. Since the method is used for two separate actions, it needs to be split into #new and #create actions to function like a Rails REST controller. This splitting is common in older Rails code that predates REST.

I could do this split in tomorrow’s refactor but I’m going to leave it for later and keep focusing on the ProjectsController. Right now Redmine would get more benefit from refactoring it’s really bad code smells than to polish this new section of code.

Reference commit