The Refactoring
I started on a larger refactoring of the issue_report_details method today but found a potential security bug so I threw it away. It's better to be secure than refactored :)
So instead, I performed a common Rails refactoring for removing duplicate code: extracting some code to a before filter.
Before
# app/controllers/reports_controller.rb
class ReportsController < ApplicationController
menu_item :issues
before_filter :find_project, :authorize
def issue_report
@statuses = IssueStatus.find(:all, :order => 'position')
# ...
end
def issue_report_details
@statuses = IssueStatus.find(:all, :order => 'position')
# ...
end
end
After
# app/controllers/reports_controller.rb
class ReportsController < ApplicationController
menu_item :issues
before_filter :find_project, :authorize, :find_issue_statuses
def issue_report
# ...
end
def issue_report_details
# ...
end
private
def find_issue_statuses
@statuses = IssueStatus.find(:all, :order => 'position')
end
end
Review
This is a good refactoring when your controller actions need some data but that data isn't their core responsibility. For example the find_project and authorize before filters in Redmine will find the current project, user, and check that the permissions allow access. These don't have anything to do with the actions, but they are still required.
