From c100f8bfdde89ab9a6760757f003a08d2914ba83 Mon Sep 17 00:00:00 2001
From: Maxwell Salzberg <maxwell@joindiaspora.com>
Date: Fri, 27 Jan 2012 02:02:18 -0800
Subject: [PATCH] clean up Person scopes, and re-use one in contact controller
 to deprecate a controller method

---
 app/controllers/contacts_controller.rb | 17 +++--------------
 app/models/contact.rb                  |  6 ++++++
 app/models/person.rb                   | 14 ++++++++++----
 lib/stream/aspect.rb                   |  2 +-
 spec/lib/stream/aspect_spec.rb         |  2 +-
 5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/app/controllers/contacts_controller.rb b/app/controllers/contacts_controller.rb
index a35902b81..d42a8288d 100644
--- a/app/controllers/contacts_controller.rb
+++ b/app/controllers/contacts_controller.rb
@@ -19,17 +19,14 @@ class ContactsController < ApplicationController
         current_user.contacts.receiving
       end
     end
+    @contacts = @contacts.for_a_stream(params[:page])
 
     respond_to do |format|
-      format.html { @contacts = sort_and_paginate_profiles(@contacts) }
-      format.mobile { @contacts = sort_and_paginate_profiles(@contacts) }
       format.json {
-        @people = Person.for_json.joins(:contacts => :aspect_memberships).
-          where(:contacts => { :user_id => current_user.id },
-                :aspect_memberships => { :aspect_id => params[:aspect_ids] })
-
+        @people = Person.all_from_aspects(params[:aspect_ids], current_user).for_json
         render :json => @people.to_json
       }
+      format.any{}
     end
   end
 
@@ -43,12 +40,4 @@ class ContactsController < ApplicationController
     @people = Person.community_spotlight
   end
 
-  private
-
-  def sort_and_paginate_profiles contacts
-    contacts.
-      includes(:aspects, :person => :profile).
-      order('profiles.last_name ASC').
-      paginate(:page => params[:page], :per_page => 25)
-  end
 end
diff --git a/app/models/contact.rb b/app/models/contact.rb
index 574a11dc1..cd2f5f243 100644
--- a/app/models/contact.rb
+++ b/app/models/contact.rb
@@ -37,6 +37,12 @@ class Contact < ActiveRecord::Base
     where(:receiving => true)
   }
 
+  scope :for_a_stream, lambda { |page|
+    includes(:aspects, :person => :profile).
+        order('profiles.last_name ASC').
+        paginate(:page => page, :per_page => 25)
+  }
+
   scope :only_sharing, lambda {
     sharing.where(:receiving => false)
   }
diff --git a/app/models/person.rb b/app/models/person.rb
index 5509b6554..a056decc2 100644
--- a/app/models/person.rb
+++ b/app/models/person.rb
@@ -67,12 +67,18 @@ class Person < ActiveRecord::Base
   # @note user is passed in here defensively
   scope :all_from_aspects, lambda { |aspect_ids, user|
     joins(:contacts => :aspect_memberships).
-         where(:contacts => {:user_id => user.id},
-               :aspect_memberships => {:aspect_id => aspect_ids}).
-         select("DISTINCT people.*")
+         where(:contacts => {:user_id => user.id}, :aspect_memberships => {:aspect_id => aspect_ids})
   }
 
-  scope :in_aspects, lambda{|aspect_ids| joins(:contacts => {:aspect_memberships => :aspect}).where(Aspect.arel_table[:id].in(aspect_ids))}
+  scope :unique_from_aspects, lambda{ |aspect_ids, user|
+    all_from_aspects(aspect_ids, user).select('DISTINCT people.*')
+  }
+
+  #not defensive
+  scope :in_aspects, lambda { |aspect_ids|
+    joins(:contacts => :aspect_memberships).
+        where(:contacts => { :aspect_memberships => {:aspect_id => aspect_ids}})
+  }
 
   scope :profile_tagged_with, lambda{|tag_name| joins(:profile => :tags).where(:profile => {:tags => {:name => tag_name}}).where('profiles.searchable IS TRUE') }
 
diff --git a/lib/stream/aspect.rb b/lib/stream/aspect.rb
index 1af899132..349fdc809 100644
--- a/lib/stream/aspect.rb
+++ b/lib/stream/aspect.rb
@@ -48,7 +48,7 @@ class Stream::Aspect < Stream::Base
 
   # @return [ActiveRecord::Association<Person>] AR association of people within stream's given aspects
   def people
-    @people ||= Person.all_from_aspects(aspect_ids, user).includes(:profile)
+    @people ||= Person.unique_from_aspects(aspect_ids, user).includes(:profile)
   end
 
   # @return [String] URL
diff --git a/spec/lib/stream/aspect_spec.rb b/spec/lib/stream/aspect_spec.rb
index 45f26fdf1..7072fb8fd 100644
--- a/spec/lib/stream/aspect_spec.rb
+++ b/spec/lib/stream/aspect_spec.rb
@@ -92,7 +92,7 @@ describe Stream::Aspect do
       stream = Stream::Aspect.new(alice, [])
 
       stream.stub(:aspect_ids).and_return(aspect_ids)
-      Person.should_receive(:all_from_aspects).with(stream.aspect_ids, alice).and_return(stub(:includes => :profile))
+      Person.should_receive(:unique_from_aspects).with(stream.aspect_ids, alice).and_return(stub(:includes => :profile))
       stream.people
     end
   end