From 4601ed2f3aebc545024287e318494ce32bc5803f Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 15 Feb 2009 13:33:09 +0000
Subject: [PATCH] Replaces Enumeration.get_values and Enumeration.default with
 named scopes.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2472 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/documents_controller.rb    |  2 +-
 app/controllers/enumerations_controller.rb |  2 +-
 app/controllers/issues_controller.rb       |  8 +++----
 app/controllers/reports_controller.rb      |  4 ++--
 app/helpers/timelog_helper.rb              |  2 +-
 app/models/document.rb                     |  2 +-
 app/models/enumeration.rb                  | 26 ++++++++++++++++------
 app/models/issue.rb                        |  2 +-
 app/models/time_entry.rb                   |  2 +-
 app/views/documents/_form.rhtml            |  2 +-
 app/views/enumerations/list.rhtml          |  2 +-
 app/views/issues/bulk_edit.rhtml           |  2 +-
 lib/tasks/migrate_from_mantis.rake         |  2 +-
 lib/tasks/migrate_from_trac.rake           |  2 +-
 test/functional/issues_controller_test.rb  |  4 ++--
 test/unit/enumeration_test.rb              | 16 ++++++-------
 test/unit/issue_test.rb                    | 10 ++++-----
 17 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb
index 2d1c414c..96561414 100644
--- a/app/controllers/documents_controller.rb
+++ b/app/controllers/documents_controller.rb
@@ -54,7 +54,7 @@ class DocumentsController < ApplicationController
   end
   
   def edit
-    @categories = Enumeration::get_values('DCAT')
+    @categories = Enumeration.document_categories
     if request.post? and @document.update_attributes(params[:document])
       flash[:notice] = l(:notice_successful_update)
       redirect_to :action => 'show', :id => @document
diff --git a/app/controllers/enumerations_controller.rb b/app/controllers/enumerations_controller.rb
index 50521bab..6e136fba 100644
--- a/app/controllers/enumerations_controller.rb
+++ b/app/controllers/enumerations_controller.rb
@@ -85,7 +85,7 @@ class EnumerationsController < ApplicationController
         redirect_to :action => 'index'
       end
     end
-    @enumerations = Enumeration.get_values(@enumeration.opt) - [@enumeration]
+    @enumerations = Enumeration.values(@enumeration.opt) - [@enumeration]
   #rescue
   #  flash[:error] = 'Unable to delete enumeration'
   #  redirect_to :action => 'index'
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 557347bb..5efa1d64 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -100,7 +100,7 @@ class IssuesController < ApplicationController
     @journals.reverse! if User.current.wants_comments_in_reverse_order?
     @allowed_statuses = @issue.new_statuses_allowed_to(User.current)
     @edit_allowed = User.current.allowed_to?(:edit_issues, @project)
-    @priorities = Enumeration::get_values('IPRI')
+    @priorities = Enumeration.priorities
     @time_entry = TimeEntry.new
     respond_to do |format|
       format.html { render :template => 'issues/show.rhtml' }
@@ -153,7 +153,7 @@ class IssuesController < ApplicationController
         return
       end		
     end	
-    @priorities = Enumeration::get_values('IPRI')
+    @priorities = Enumeration.priorities
     render :layout => !request.xhr?
   end
   
@@ -163,7 +163,7 @@ class IssuesController < ApplicationController
   
   def edit
     @allowed_statuses = @issue.new_statuses_allowed_to(User.current)
-    @priorities = Enumeration::get_values('IPRI')
+    @priorities = Enumeration.priorities
     @edit_allowed = User.current.allowed_to?(:edit_issues, @project)
     @time_entry = TimeEntry.new
     
@@ -403,7 +403,7 @@ class IssuesController < ApplicationController
       @assignables << @issue.assigned_to if @issue && @issue.assigned_to && !@assignables.include?(@issue.assigned_to)
     end
     
-    @priorities = Enumeration.get_values('IPRI').reverse
+    @priorities = Enumeration.priorities.reverse
     @statuses = IssueStatus.find(:all, :order => 'position')
     @back = request.env['HTTP_REFERER']
     
diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb
index bfe51577..f4594139 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -37,7 +37,7 @@ class ReportsController < ApplicationController
       render :template => "reports/issue_report_details"
     when "priority"
       @field = "priority_id"
-      @rows = Enumeration::get_values('IPRI')
+      @rows = Enumeration.priorities
       @data = issues_by_priority
       @report_title = l(:field_priority)
       render :template => "reports/issue_report_details"   
@@ -68,7 +68,7 @@ class ReportsController < ApplicationController
     else
       @trackers = @project.trackers
       @versions = @project.versions.sort
-      @priorities = Enumeration::get_values('IPRI')
+      @priorities = Enumeration.priorities
       @categories = @project.issue_categories
       @assignees = @project.members.collect { |m| m.user }
       @authors = @project.members.collect { |m| m.user }
diff --git a/app/helpers/timelog_helper.rb b/app/helpers/timelog_helper.rb
index f068b4db..f43bce0b 100644
--- a/app/helpers/timelog_helper.rb
+++ b/app/helpers/timelog_helper.rb
@@ -27,7 +27,7 @@ module TimelogHelper
   end
   
   def activity_collection_for_select_options
-    activities = Enumeration::get_values('ACTI')
+    activities = Enumeration.activities
     collection = []
     collection << [ "--- #{l(:actionview_instancetag_blank_option)} ---", '' ] unless activities.detect(&:is_default)
     activities.each { |a| collection << [a.name, a.id] }
diff --git a/app/models/document.rb b/app/models/document.rb
index 2ec99fe0..f78c15e9 100644
--- a/app/models/document.rb
+++ b/app/models/document.rb
@@ -31,7 +31,7 @@ class Document < ActiveRecord::Base
   
   def after_initialize
     if new_record?
-      self.category ||= Enumeration.default('DCAT')
+      self.category ||= Enumeration.document_categories.default
     end
   end
 end
diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb
index e4b080be..d466940b 100644
--- a/app/models/enumeration.rb
+++ b/app/models/enumeration.rb
@@ -26,17 +26,29 @@ class Enumeration < ActiveRecord::Base
 
   # Single table inheritance would be an option
   OPTIONS = {
-    "IPRI" => {:label => :enumeration_issue_priorities, :model => Issue, :foreign_key => :priority_id},
-    "DCAT" => {:label => :enumeration_doc_categories, :model => Document, :foreign_key => :category_id},
-    "ACTI" => {:label => :enumeration_activities, :model => TimeEntry, :foreign_key => :activity_id}
+    "IPRI" => {:label => :enumeration_issue_priorities, :model => Issue, :foreign_key => :priority_id, :scope => :priorities},
+    "DCAT" => {:label => :enumeration_doc_categories, :model => Document, :foreign_key => :category_id, :scope => :document_categories},
+    "ACTI" => {:label => :enumeration_activities, :model => TimeEntry, :foreign_key => :activity_id, :scope => :activities}
   }.freeze
   
-  def self.get_values(option)
-    find(:all, :conditions => {:opt => option}, :order => 'position')
+  # Creates a named scope for each type of value. The scope has a +default+ method
+  # that returns the default value, or nil if no value is set as default.
+  # Example:
+  #   Enumeration.priorities
+  #   Enumeration.priorities.default
+  OPTIONS.each do |k, v|
+    next unless v[:scope]
+    named_scope v[:scope], :conditions => { :opt => k }, :order => 'position' do
+      def default
+        find(:first, :conditions => { :is_default => true })
+      end
+    end
   end
   
-  def self.default(option)  
-    find(:first, :conditions => {:opt => option, :is_default => true}, :order => 'position')
+  named_scope :values, lambda {|opt| { :conditions => { :opt => opt }, :order => 'position' } } do
+    def default
+      find(:first, :conditions => { :is_default => true })
+    end
   end
 
   def option_name
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 618c5597..46a28708 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -63,7 +63,7 @@ class Issue < ActiveRecord::Base
     if new_record?
       # set default values for new records only
       self.status ||= IssueStatus.default
-      self.priority ||= Enumeration.default('IPRI')
+      self.priority ||= Enumeration.priorities.default
     end
   end
   
diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb
index f10b179d..4cb76441 100644
--- a/app/models/time_entry.rb
+++ b/app/models/time_entry.rb
@@ -37,7 +37,7 @@ class TimeEntry < ActiveRecord::Base
 
   def after_initialize
     if new_record? && self.activity.nil?
-      if default_activity = Enumeration.default('ACTI')
+      if default_activity = Enumeration.activities.default
         self.activity_id = default_activity.id
       end
     end
diff --git a/app/views/documents/_form.rhtml b/app/views/documents/_form.rhtml
index d45e295b..12a16f5d 100644
--- a/app/views/documents/_form.rhtml
+++ b/app/views/documents/_form.rhtml
@@ -2,7 +2,7 @@
 <div class="box">
 <!--[form:document]-->
 <p><label for="document_category_id"><%=l(:field_category)%></label>
-<%= select('document', 'category_id', Enumeration.get_values('DCAT').collect {|c| [c.name, c.id]}) %></p>
+<%= select('document', 'category_id', Enumeration.document_categories.collect {|c| [c.name, c.id]}) %></p>
 
 <p><label for="document_title"><%=l(:field_title)%> <span class="required">*</span></label>
 <%= text_field 'document', 'title', :size => 60 %></p>
diff --git a/app/views/enumerations/list.rhtml b/app/views/enumerations/list.rhtml
index 7f3886b4..c4a9af27 100644
--- a/app/views/enumerations/list.rhtml
+++ b/app/views/enumerations/list.rhtml
@@ -3,7 +3,7 @@
 <% Enumeration::OPTIONS.each do |option, params| %>
 <h3><%= l(params[:label]) %></h3>
 
-<% enumerations = Enumeration.get_values(option) %>
+<% enumerations = Enumeration.values(option) %>
 <% if enumerations.any? %>
 <table class="list">
 <% enumerations.each do |enumeration| %>
diff --git a/app/views/issues/bulk_edit.rhtml b/app/views/issues/bulk_edit.rhtml
index acda6a65..21411686 100644
--- a/app/views/issues/bulk_edit.rhtml
+++ b/app/views/issues/bulk_edit.rhtml
@@ -13,7 +13,7 @@
 <%= select_tag('status_id', "<option value=\"\">#{l(:label_no_change_option)}</option>" + options_from_collection_for_select(@available_statuses, :id, :name)) %></label>
 <% end %>
 <label><%= l(:field_priority) %>: 
-<%= select_tag('priority_id', "<option value=\"\">#{l(:label_no_change_option)}</option>" + options_from_collection_for_select(Enumeration.get_values('IPRI'), :id, :name)) %></label>
+<%= select_tag('priority_id', "<option value=\"\">#{l(:label_no_change_option)}</option>" + options_from_collection_for_select(Enumeration.priorities, :id, :name)) %></label>
 <label><%= l(:field_category) %>: 
 <%= select_tag('category_id', content_tag('option', l(:label_no_change_option), :value => '') +
                                 content_tag('option', l(:label_none), :value => 'none') +
diff --git a/lib/tasks/migrate_from_mantis.rake b/lib/tasks/migrate_from_mantis.rake
index bf3c32cc..facaef04 100644
--- a/lib/tasks/migrate_from_mantis.rake
+++ b/lib/tasks/migrate_from_mantis.rake
@@ -40,7 +40,7 @@ task :migrate_from_mantis => :environment do
                         90 => closed_status    # closed
                         }
                         
-      priorities = Enumeration.get_values('IPRI')
+      priorities = Enumeration.priorities
       DEFAULT_PRIORITY = priorities[2]
       PRIORITY_MAPPING = {10 => priorities[1], # none
                           20 => priorities[1], # low
diff --git a/lib/tasks/migrate_from_trac.rake b/lib/tasks/migrate_from_trac.rake
index 02d92130..9b245cd1 100644
--- a/lib/tasks/migrate_from_trac.rake
+++ b/lib/tasks/migrate_from_trac.rake
@@ -37,7 +37,7 @@ namespace :redmine do
                           'closed' => closed_status
                           }
 
-        priorities = Enumeration.get_values('IPRI')
+        priorities = Enumeration.priorities
         DEFAULT_PRIORITY = priorities[0]
         PRIORITY_MAPPING = {'lowest' => priorities[0],
                             'low' => priorities[0],
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index cc1c7740..25100f9a 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -640,7 +640,7 @@ class IssuesControllerTest < Test::Unit::TestCase
            :id => 1,
            :issue => { :status_id => 2, :assigned_to_id => 3 },
            :notes => 'Assigned to dlopper',
-           :time_entry => { :hours => '', :comments => '', :activity_id => Enumeration.get_values('ACTI').first }
+           :time_entry => { :hours => '', :comments => '', :activity_id => Enumeration.activities.first }
     end
     assert_redirected_to :action => 'show', :id => '1'
     issue.reload
@@ -676,7 +676,7 @@ class IssuesControllerTest < Test::Unit::TestCase
       post :edit,
            :id => 1,
            :notes => '2.5 hours added',
-           :time_entry => { :hours => '2.5', :comments => '', :activity_id => Enumeration.get_values('ACTI').first }
+           :time_entry => { :hours => '2.5', :comments => '', :activity_id => Enumeration.activities.first }
     end
     assert_redirected_to :action => 'show', :id => '1'
     
diff --git a/test/unit/enumeration_test.rb b/test/unit/enumeration_test.rb
index 4da3f094..c192cee8 100644
--- a/test/unit/enumeration_test.rb
+++ b/test/unit/enumeration_test.rb
@@ -38,7 +38,7 @@ class EnumerationTest < Test::Unit::TestCase
   end
   
   def test_default
-    e = Enumeration.default('IPRI')
+    e = Enumeration.priorities.default
     assert e.is_a?(Enumeration)
     assert e.is_default?
     assert_equal 'Normal', e.name
@@ -47,31 +47,31 @@ class EnumerationTest < Test::Unit::TestCase
   def test_create
     e = Enumeration.new(:opt => 'IPRI', :name => 'Very urgent', :is_default => false)
     assert e.save
-    assert_equal 'Normal', Enumeration.default('IPRI').name
+    assert_equal 'Normal', Enumeration.priorities.default.name
   end
   
   def test_create_as_default
     e = Enumeration.new(:opt => 'IPRI', :name => 'Very urgent', :is_default => true)
     assert e.save
-    assert_equal e, Enumeration.default('IPRI')
+    assert_equal e, Enumeration.priorities.default
   end
   
   def test_update_default
-    e = Enumeration.default('IPRI')
+    e = Enumeration.priorities.default
     e.update_attributes(:name => 'Changed', :is_default => true)
-    assert_equal e, Enumeration.default('IPRI')
+    assert_equal e, Enumeration.priorities.default
   end
   
   def test_update_default_to_non_default
-    e = Enumeration.default('IPRI')
+    e = Enumeration.priorities.default
     e.update_attributes(:name => 'Changed', :is_default => false)
-    assert_nil Enumeration.default('IPRI')
+    assert_nil Enumeration.priorities.default
   end
   
   def test_change_default
     e = Enumeration.find_by_name('Urgent')
     e.update_attributes(:name => 'Urgent', :is_default => true)
-    assert_equal e, Enumeration.default('IPRI')
+    assert_equal e, Enumeration.priorities.default
   end
   
   def test_destroy_with_reassign
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 89ec3c6d..bfe29f55 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -27,14 +27,14 @@ class IssueTest < Test::Unit::TestCase
            :time_entries
 
   def test_create
-    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'test_create', :description => 'IssueTest#test_create', :estimated_hours => '1:30')
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.priorities.first, :subject => 'test_create', :description => 'IssueTest#test_create', :estimated_hours => '1:30')
     assert issue.save
     issue.reload
     assert_equal 1.5, issue.estimated_hours
   end
   
   def test_create_minimal
-    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'test_create')
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.priorities.first, :subject => 'test_create')
     assert issue.save
     assert issue.description.nil?
   end
@@ -110,7 +110,7 @@ class IssueTest < Test::Unit::TestCase
   end
   
   def test_category_based_assignment
-    issue = Issue.create(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'Assignment test', :description => 'Assignment test', :category_id => 1)
+    issue = Issue.create(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.priorities.first, :subject => 'Assignment test', :description => 'Assignment test', :category_id => 1)
     assert_equal IssueCategory.find(1).assigned_to, issue.assigned_to
   end
   
@@ -126,7 +126,7 @@ class IssueTest < Test::Unit::TestCase
   
   def test_should_close_duplicates
     # Create 3 issues
-    issue1 = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'Duplicates test', :description => 'Duplicates test')
+    issue1 = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :priority => Enumeration.priorities.first, :subject => 'Duplicates test', :description => 'Duplicates test')
     assert issue1.save
     issue2 = issue1.clone
     assert issue2.save
@@ -153,7 +153,7 @@ class IssueTest < Test::Unit::TestCase
   
   def test_should_not_close_duplicated_issue
     # Create 3 issues
-    issue1 = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'Duplicates test', :description => 'Duplicates test')
+    issue1 = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :priority => Enumeration.priorities.first, :subject => 'Duplicates test', :description => 'Duplicates test')
     assert issue1.save
     issue2 = issue1.clone
     assert issue2.save