The Refactoring
Today I refactored the conditional of StuffToDo#conditions_for_available
using a little known library in Redmine called ARCondition
.
Before
1 2 3 4 5 6 7 8 9 10 11 12 13 |
# app/models/stuff_to_do.rb class StuffToDo < ActiveRecord::Base def self.conditions_for_available(filter_by, record_id) case filter_by when :user ["assigned_to_id = ? AND #{IssueStatus.table_name}.is_closed = ?", record_id, false ] when :status ["#{IssueStatus.table_name}.id = (?) AND #{IssueStatus.table_name}.is_closed = ?", record_id, false ] when :priority ["#{Enumeration.table_name}.id = (?) AND #{IssueStatus.table_name}.is_closed = ?", record_id, false ] end end end |
After
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 |
class StuffToDo < ActiveRecord::Base def self.conditions_for_available(filter_by, record_id) conditions_builder = ARCondition.new conditions_builder.add(["#{IssueStatus.table_name}.is_closed = ?", false ]) case filter_by when :user conditions_builder.add(["assigned_to_id = ?", record_id]) when :status conditions_builder.add(["#{IssueStatus.table_name}.id = (?)", record_id]) when :priority conditions_builder.add(["#{Enumeration.table_name}.id = (?)", record_id]) end conditions_builder.conditions end end |
Review
Using ARCondition
, I was able split up how the conditions where being generated. So instead of creating three similar Arrays, I can create separate Arrays for each part of the conditions. Redmine uses this pattern in several places where the conditions need to be generated dynamically.
There is one more refactoring I want to try from the flay report, by combining the Redmine core patches to Issue
and Project
.