From d289619c757b52849002e0a78ecc4b45065cb8af Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 12 Feb 2012 14:54:30 +0000
Subject: [PATCH] Fixed potential error when sorting members without role
 (#10053).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8863 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/member.rb     | 12 +++++++++++-
 app/models/principal.rb  |  4 +++-
 test/unit/member_test.rb | 17 +++++++++++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/app/models/member.rb b/app/models/member.rb
index bddb54354..96e7c5c39 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -58,7 +58,17 @@ class Member < ActiveRecord::Base
 
   def <=>(member)
     a, b = roles.sort.first, member.roles.sort.first
-    a == b ? (principal <=> member.principal) : (a <=> b)
+    if a == b
+      if principal
+        principal <=> member.principal
+      else
+        1
+      end
+    elsif a
+      a <=> b
+    else
+      1
+    end
   end
 
   def deletable?
diff --git a/app/models/principal.rb b/app/models/principal.rb
index 77fe19fe0..5c4e17a8b 100644
--- a/app/models/principal.rb
+++ b/app/models/principal.rb
@@ -50,7 +50,9 @@ class Principal < ActiveRecord::Base
   end
 
   def <=>(principal)
-    if self.class.name == principal.class.name
+    if principal.nil?
+      -1
+    elsif self.class.name == principal.class.name
       self.to_s.downcase <=> principal.to_s.downcase
     else
       # groups after users
diff --git a/test/unit/member_test.rb b/test/unit/member_test.rb
index 783ec90b9..5fb8b30ac 100644
--- a/test/unit/member_test.rb
+++ b/test/unit/member_test.rb
@@ -106,6 +106,23 @@ class MemberTest < ActiveSupport::TestCase
     assert_nil category1.assigned_to_id
   end
 
+  def test_sort_without_roles
+    a = Member.new(:roles => [Role.first])
+    b = Member.new
+
+    assert_equal -1, a <=> b
+    assert_equal 1,  b <=> a
+  end
+
+  def test_sort_without_principal
+    role = Role.first
+    a = Member.new(:roles => [role], :principal => User.first)
+    b = Member.new(:roles => [role])
+
+    assert_equal -1, a <=> b
+    assert_equal 1,  b <=> a
+  end
+
   context "removing permissions" do
     setup do
       Watcher.delete_all("user_id = 9")