From e63a8a4bfa372c955d7178cc516335ca5740f72d Mon Sep 17 00:00:00 2001
From: Raphael <raphael@joindiaspora.com>
Date: Tue, 14 Dec 2010 12:11:15 -0800
Subject: [PATCH] Take contact.person query out of the views, only query people
 once in hashes_for_aspects

---
 app/controllers/aspects_controller.rb           | 14 +++++++++-----
 app/helpers/aspects_helper.rb                   |  5 -----
 app/views/aspects/_aspect.haml                  |  6 +++---
 app/views/aspects/_edit_aspect_pane.html.haml   |  4 ++--
 app/views/aspects/manage.html.haml              |  8 ++++----
 app/views/aspects/show.html.haml                |  2 +-
 app/views/requests/_manage_aspect_contacts.haml |  6 +++---
 app/views/shared/_contact_list.html.haml        |  6 +++---
 spec/controllers/aspects_controller_spec.rb     | 17 +++++++++--------
 9 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb
index d9118c9b8f..d6704f63b1 100644
--- a/app/controllers/aspects_controller.rb
+++ b/app/controllers/aspects_controller.rb
@@ -166,13 +166,17 @@ class AspectsController < ApplicationController
   end
 
   def hashes_for_aspects aspects, contacts, opts = {}
+    contact_hashes = hashes_for_contacts contacts
     aspects.map do |a|
       hash = {:aspect => a}
-      aspect_contacts = contacts.select{|c|
-          c.aspect_ids.include?(a.id)}
-      hash[:contact_count] = aspect_contacts.count
-      person_ids = aspect_contacts.map{|c| c.person_id}
-      hash[:people] = Person.all({:id.in => person_ids, :fields => [:profile]}.merge(opts))
+      aspect_contact_hashes = contact_hashes.select{|c|
+          c[:contact].aspect_ids.include?(a.id)}
+      hash[:contact_count] = aspect_contact_hashes.count
+      if opts[:limit]
+        hash[:contacts] = aspect_contact_hashes.slice(0,opts[:limit])
+      else
+        hash[:contacts] = aspect_contact_hashes
+      end
       hash
     end
   end
diff --git a/app/helpers/aspects_helper.rb b/app/helpers/aspects_helper.rb
index 0642c27d73..ad3b9ab488 100644
--- a/app/helpers/aspects_helper.rb
+++ b/app/helpers/aspects_helper.rb
@@ -30,10 +30,5 @@ module AspectsHelper
       remove_from_aspect_button(aspect_id, contact.person.id)
     end
   end
-
-  def contact_link(contact)
-    person = contact.person 
-    link_to person.name, person
-  end
 end
 
diff --git a/app/views/aspects/_aspect.haml b/app/views/aspects/_aspect.haml
index 8c0f229bb6..a6008f3096 100644
--- a/app/views/aspects/_aspect.haml
+++ b/app/views/aspects/_aspect.haml
@@ -4,6 +4,6 @@
     %span
       = t('contacts', :count => contact_count)
 
-  - if contact_count > 0 
-    - for person in people
-      = person_image_link(person)
+  - if contact_count > 0
+    - for hash in contacts
+      = person_image_link(hash[:person])
diff --git a/app/views/aspects/_edit_aspect_pane.html.haml b/app/views/aspects/_edit_aspect_pane.html.haml
index 0c8ecd43e0..f05410c0ae 100644
--- a/app/views/aspects/_edit_aspect_pane.html.haml
+++ b/app/views/aspects/_edit_aspect_pane.html.haml
@@ -6,9 +6,9 @@
   = include_javascripts :aspects
 
 #edit_aspect_pane
-  - if @contacts.count > 0 
+  - if @contacts.count > 0
     %h4= t('.add_existing')
-    = render 'shared/contact_list', :aspect_id => aspect.id, :contacts => contacts, :manage => defined?(manage)
+    = render 'shared/contact_list', :aspect_id => aspect.id, :contact_hashes => contacts, :manage => defined?(manage)
 
   = render 'shared/add_contact', :aspect_id => aspect.id
 
diff --git a/app/views/aspects/manage.html.haml b/app/views/aspects/manage.html.haml
index 012b66594b..b57b4cf3d0 100644
--- a/app/views/aspects/manage.html.haml
+++ b/app/views/aspects/manage.html.haml
@@ -51,17 +51,17 @@
             %li!= remove_link(hash[:aspect])
 
         %ul.dropzone{:data=>{:aspect_id=>hash[:aspect].id}}
-          -for person in hash[:people]
-            %li.person{:data=>{:guid=>person.id, :aspect_id=>hash[:aspect].id}}
+          -for contact_hash in hash[:contacts]
+            %li.person{:data=>{:guid=>contact_hash[:person].id, :aspect_id=>hash[:aspect].id}}
               .delete
                 .x
                   X
                 .circle
-              = link_to person_image_tag(person), person
+              = link_to person_image_tag(contact_hash[:person]), contact_hash[:person]
           .draggable_info
             =t('.drag_to_add')
 
           .fancybox_content
             %div{:id => "manage_aspect_contacts_pane_#{hash[:aspect].id}"}
-              = render "requests/manage_aspect_contacts", :aspect_name => hash[:aspect].name, :aspect_id => hash[:aspect].id, :manage => true
+              = render "requests/manage_aspect_contacts", :aspect => hash[:aspect], :manage => true, :contact_hashes => hash[:contacts]
 
diff --git a/app/views/aspects/show.html.haml b/app/views/aspects/show.html.haml
index da066c2b08..b1de218739 100644
--- a/app/views/aspects/show.html.haml
+++ b/app/views/aspects/show.html.haml
@@ -15,7 +15,7 @@
 
 .span-8.append-1
   = render 'aspects/aspect_contacts', :contacts => @aspect_contacts, :aspect => @aspect
-  = render 'aspects/edit_aspect_pane', :contacts => @contacts, :aspect => @aspect
+  = render 'aspects/edit_aspect_pane', :contacts => @aspect_contacts, :aspect => @aspect
 
 
 .span-15.last
diff --git a/app/views/requests/_manage_aspect_contacts.haml b/app/views/requests/_manage_aspect_contacts.haml
index e72f1bfc89..2d0dd0fa6d 100644
--- a/app/views/requests/_manage_aspect_contacts.haml
+++ b/app/views/requests/_manage_aspect_contacts.haml
@@ -6,10 +6,10 @@
   .modal_title_bar
     %h4
       = t('.manage_within')
-      %i= aspect_name
+      %i= aspect.name
   .span-6.append-1.last
     %h3= t('.existing')
-    = render 'shared/contact_list', :aspect_id => aspect_id, :contacts => @contacts, :manage => defined?(manage)
+    = render 'shared/contact_list', :aspect_id => aspect.id, :contact_hashes => contact_hashes, :manage => defined?(manage)
 
   .span-7.last
-    = render 'shared/add_contact', :aspect_id => aspect_id
+    = render 'shared/add_contact', :aspect_id => aspect.id
diff --git a/app/views/shared/_contact_list.html.haml b/app/views/shared/_contact_list.html.haml
index bdaffb741c..2507741054 100644
--- a/app/views/shared/_contact_list.html.haml
+++ b/app/views/shared/_contact_list.html.haml
@@ -6,10 +6,10 @@
 .contact_list
   = search_field_tag :contact_search, "", :class => 'contact_list_search', :results => 5, :placeholder => t('.all_contacts')
   %ul
-    - for contact in contacts
+    - for hash in contact_hashes
       %li
         %span.name
-          = contact_link contact
+          = link_to hash[:person].name, hash[:person]
         .right
-          = aspect_membership_button(aspect_id, contact)
+          = aspect_membership_button(aspect_id, hash[:contact])
 
diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb
index 24bf67a419..0175a3eb93 100644
--- a/spec/controllers/aspects_controller_spec.rb
+++ b/spec/controllers/aspects_controller_spec.rb
@@ -190,19 +190,20 @@ describe AspectsController do
       @hashes.length.should == 2
       @hash[:aspect].should == @aspect
     end
-    it 'has a contact count' do
+    it 'has a contact_count' do
       @hash[:contact_count].should == @aspect.contacts.count
     end
-    it 'has people' do
-      desired_people = @aspect.contacts.map{|c| c.person.id}
-      gotten_people = @hash[:people].map{|p| p.id}
-      gotten_people.each{|p| desired_people.should include p}
+    it 'takes a limit on contacts returned' do
+      @hash[:contacts].count.should == 9
     end
-    it 'takes a limit on people returned' do
-      @hash[:people].length.should == 9
+    it 'has a person in each hash' do
+      @aspect.contacts.map{|c| c.person}.include?(@hash[:contacts].first[:person]).should be_true
     end
     it "does not return the rsa key" do
-      @hash[:people].first.serialized_public_key.should be_nil
+      @hash[:contacts].first[:person].serialized_public_key.should be_nil
+    end
+    it 'has a contact in each hash' do
+      @aspect.contacts.include?(@hash[:contacts].first[:contact]).should be_true
     end
   end
 
-- 
GitLab