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:
- To show the form that’s used to upload a new file
- 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.