diff --git a/app/models/role.rb b/app/models/role.rb index ee59ce905..c8a1028da 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -26,11 +26,11 @@ class Role < ActiveRecord::Base ['own', :label_issues_visibility_own] ] - scope :sorted, {:order => 'builtin, position'} - scope :givable, { :conditions => "builtin = 0", :order => 'position' } + scope :sorted, order("#{table_name}.builtin ASC, #{table_name}.position ASC") + scope :givable, order("#{table_name}.position ASC").where(:builtin => 0) scope :builtin, lambda { |*args| - compare = 'not' if args.first == true - { :conditions => "#{compare} builtin = 0" } + compare = (args.first == true ? 'not' : '') + where("#{compare} builtin = 0") } before_destroy :check_deletable @@ -87,7 +87,15 @@ class Role < ActiveRecord::Base end def <=>(role) - role ? position <=> role.position : -1 + if role + if builtin == role.builtin + position <=> role.position + else + builtin <=> role.builtin + end + else + -1 + end end def to_s @@ -134,7 +142,7 @@ class Role < ActiveRecord::Base # Find all the roles that can be given to a project member def self.find_all_givable - find(:all, :conditions => {:builtin => 0}, :order => 'position') + Role.givable.all end # Return the builtin 'non member' role. If the role doesn't exist, @@ -165,7 +173,7 @@ private end def self.find_or_create_system_role(builtin, name) - role = first(:conditions => {:builtin => builtin}) + role = where(:builtin => builtin).first if role.nil? role = create(:name => name, :position => 0) do |r| r.builtin = builtin diff --git a/app/models/workflow.rb b/app/models/workflow.rb index 5c1ef5dea..36b4c7df8 100644 --- a/app/models/workflow.rb +++ b/app/models/workflow.rb @@ -25,8 +25,8 @@ class Workflow < ActiveRecord::Base # Returns workflow transitions count by tracker and role def self.count_by_tracker_and_role counts = connection.select_all("SELECT role_id, tracker_id, count(id) AS c FROM #{Workflow.table_name} GROUP BY role_id, tracker_id") - roles = Role.find(:all, :order => 'builtin, position') - trackers = Tracker.find(:all, :order => 'position') + roles = Role.sorted.all + trackers = Tracker.sorted.all result = [] trackers.each do |tracker| diff --git a/test/unit/role_test.rb b/test/unit/role_test.rb index 1c2cdcfe7..2ef20f4d5 100644 --- a/test/unit/role_test.rb +++ b/test/unit/role_test.rb @@ -20,6 +20,19 @@ require File.expand_path('../../test_helper', __FILE__) class RoleTest < ActiveSupport::TestCase fixtures :roles, :workflows + def test_sorted_scope + assert_equal Role.all.sort, Role.sorted.all + end + + def test_givable_scope + assert_equal Role.all.reject(&:builtin?).sort, Role.givable.all + end + + def test_builtin_scope + assert_equal Role.all.select(&:builtin?).sort, Role.builtin(true).all.sort + assert_equal Role.all.reject(&:builtin?).sort, Role.builtin(false).all.sort + end + def test_copy_workflows source = Role.find(1) assert_equal 90, source.workflows.size @@ -57,6 +70,10 @@ class RoleTest < ActiveSupport::TestCase assert_equal 'Non membre', Role.non_member.name end + def test_find_all_givable + assert_equal Role.all.reject(&:builtin?).sort, Role.find_all_givable + end + context "#anonymous" do should "return the anonymous role" do role = Role.anonymous