From a8306f9f374affcf81ff2acc2671a20beff9399b Mon Sep 17 00:00:00 2001
From: Raphael <raphael@joindiaspora.com>
Date: Wed, 26 Jan 2011 14:05:06 -0800
Subject: [PATCH] don't select duplicate people

---
 app/models/contact.rb       |  6 +--
 spec/models/contact_spec.rb | 90 +++++++++++++++++++++----------------
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/app/models/contact.rb b/app/models/contact.rb
index 332699d167..53a21b442b 100644
--- a/app/models/contact.rb
+++ b/app/models/contact.rb
@@ -27,14 +27,14 @@ class Contact < ActiveRecord::Base
   end
 
   def contacts
-    t_p = Person.arel_table
+    people = Person.arel_table
     incoming_aspects = Aspect.joins(:contacts).where(
       :user_id => self.person.owner_id,
       :contacts_visible => true,
-      :contacts => {:person_id => self.user.person.id})
+      :contacts => {:person_id => self.user.person.id}).select('`aspects`.id')
     incoming_aspect_ids = incoming_aspects.map{|a| a.id}
     similar_contacts = Person.joins(:contacts => :aspect_memberships).where(
-      :aspect_memberships => {:aspect_id => incoming_aspect_ids}).where(t_p[:id].not_eq(self.user.person.id))
+      :aspect_memberships => {:aspect_id => incoming_aspect_ids}).where(people[:id].not_eq(self.user.person.id)).select('DISTINCT `people`.*')
   end
   private
   def not_contact_for_self
diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb
index 51cd86551e..1fa36dbe45 100644
--- a/spec/models/contact_spec.rb
+++ b/spec/models/contact_spec.rb
@@ -47,6 +47,58 @@ describe Contact do
     end
   end
 
+  describe '#contacts' do
+    before do
+      @alice = alice
+      @bob = bob
+      @eve = eve
+      @bob.aspects.create(:name => 'next')
+      @people1 = []
+      @people2 = []
+
+      1.upto(5) do
+        person = Factory(:person)
+        bob.activate_contact(person, bob.aspects.first)
+        @people1 << person
+      end
+      1.upto(5) do
+        person = Factory(:person)
+        bob.activate_contact(person, bob.aspects.last)
+        @people2 << person
+      end
+    #eve <-> bob <-> alice
+    end
+    context 'on a contact for a local user' do
+      before do
+        @contact = @alice.contact_for(@bob.person)
+      end
+      it "returns the target local user's contacts that are in the same aspect" do
+        @contact.contacts.map{|p| p.id}.should == [@eve.person].concat(@people1).map{|p| p.id}
+      end
+      it 'returns nothing if contacts_visible is false in that aspect' do
+        asp = @bob.aspects.first
+        asp.contacts_visible = false
+        asp.save
+        @contact.contacts.should == []
+      end
+      it 'returns no duplicate contacts' do
+        [@alice, @eve].each {|c| @bob.add_contact_to_aspect(@bob.contact_for(c.person), @bob.aspects.last)}
+        contact_ids = @contact.contacts.map{|p| p.id}
+        contact_ids.uniq.should == contact_ids
+      end
+    end
+
+    context 'on a contact for a remote user' do
+      before do
+        @contact = @bob.contact_for @people1.first
+      end
+      it 'returns an empty array' do
+        @contact.contacts.should == []
+      end
+    end
+
+  end
+
 
   context 'requesting' do
     before do
@@ -58,44 +110,6 @@ describe Contact do
       @contact.person = @person
     end
 
-    describe '#contacts' do
-      before do
-        @alice = alice
-        @bob = bob
-        @eve = eve
-        @bob.aspects.create(:name => 'next')
-        @people1 = []
-        @people2 = []
-
-        1.upto(5) do
-          person = Factory(:person)
-          bob.activate_contact(person, bob.aspects.first)
-          @people1 << person
-        end
-        1.upto(5) do
-          person = Factory(:person)
-          bob.activate_contact(person, bob.aspects.last)
-          @people2 << person
-        end
-      #eve <-> bob <-> alice
-      end
-      context 'on a contact for a local user' do
-        before do
-          @contact = @alice.contact_for(@bob.person)
-        end
-        it "returns the target local user's contacts that are in the same aspect" do
-          @contact.contacts.map{|p| p.id}.should == [@eve.person].concat(@people1).map{|p| p.id}
-        end
-        it 'returns nothing if contacts_visible is false in that aspect' do
-          asp = @bob.aspects.first
-          asp.contacts_visible = false
-          asp.save
-          @contact.contacts.should == []
-        end
-      end
-
-    end
-
     describe '#generate_request' do
       it 'makes a request' do
         @contact.stub(:user).and_return(@user)
-- 
GitLab