From c27f05ed425189462c531c60debc48af74c63867 Mon Sep 17 00:00:00 2001
From: ilya <ilya@laptop.(none)>
Date: Fri, 22 Oct 2010 18:09:57 -0700
Subject: [PATCH] Initial refactor done, need to stop mapping user.friends to
 people so much

---
 app/controllers/application_controller.rb   |  2 +-
 app/controllers/aspects_controller.rb       |  2 +-
 app/controllers/people_controller.rb        |  3 ++-
 app/models/user.rb                          | 14 +++++++-------
 app/views/aspects/manage.html.haml          |  3 ++-
 app/views/people/show.html.haml             |  2 +-
 lib/diaspora/user/querying.rb               | 15 +++++++++------
 spec/controllers/aspects_controller_spec.rb | 11 ++++++-----
 spec/misc_spec.rb                           |  8 ++++----
 spec/models/aspect_spec.rb                  |  8 ++++----
 spec/models/user/invite_spec.rb             |  4 ++--
 11 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 137705df41..7a68afdf22 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base
 
       @aspects = current_user.aspects
       @aspects_dropdown_array = current_user.aspects.collect{|x| [x.to_s, x.id]}
-      @friends = current_user.friends
+      @friends = current_user.friends.map{|c| c.person}
     end
   end
 
diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb
index ddde8a7257..439be4461a 100644
--- a/app/controllers/aspects_controller.rb
+++ b/app/controllers/aspects_controller.rb
@@ -50,7 +50,7 @@ class AspectsController < ApplicationController
     unless @aspect
       render :file => "#{Rails.root}/public/404.html", :layout => false, :status => 404
     else
-      @friends = @aspect.people
+      @friends = @aspect.people.map{|c| c.person}
       @posts   = current_user.visible_posts( :by_members_of => @aspect ).paginate :per_page => 15, :order => 'created_at DESC'
       respond_with @aspect
     end
diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb
index 352a39c92e..e599775861 100644
--- a/app/controllers/people_controller.rb
+++ b/app/controllers/people_controller.rb
@@ -21,7 +21,8 @@ class PeopleController < ApplicationController
       render :file => "#{Rails.root}/public/404.html", :layout => false, :status => 404
     else
       @profile = @person.profile
-      @aspects_with_person = current_user.aspects_with_person(@person)
+      @contact = current_user.contact_for(@person)
+      @aspects_with_person = @contact.aspects if @contact
       @posts = current_user.visible_posts(:person_id => @person.id).paginate :page => params[:page], :order => 'created_at DESC'
       @latest_status_message = current_user.raw_visible_posts.find_all_by__type_and_person_id("StatusMessage", params[:id]).last
       @post_count = @posts.count
diff --git a/app/models/user.rb b/app/models/user.rb
index 697c38263f..39c2baef2d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -32,11 +32,11 @@ class User
   key :invites, Integer, :default => 5
   key :invitation_token, String
   key :invitation_sent_at, DateTime
-  key :inviter_ids, Array
-  key :friend_ids, Array
-  key :pending_request_ids, Array
-  key :visible_post_ids, Array
-  key :visible_person_ids, Array
+  key :inviter_ids, Array, :typecast => 'ObjectId' 
+  key :friend_ids, Array, :typecast => 'ObjectId' 
+  key :pending_request_ids, Array, :typecast => 'ObjectId' 
+  key :visible_post_ids, Array, :typecast => 'ObjectId' 
+  key :visible_person_ids, Array, :typecast => 'ObjectId' 
 
   key :invite_messages, Hash
 
@@ -135,11 +135,11 @@ class User
     aspect = Aspect.find(aspect_id)
     raise "Can not delete a person from an aspect you do not own" unless aspect.user == self
     contact = contact_for Person.find(person_id)
-    contact.aspects.delete aspect
+    contact.aspect_ids.delete aspect.id
     opts[:posts] ||= aspect.posts.all(:person_id => person_id)
     aspect.posts -= opts[:posts]
-    aspect.save
     contact.save
+    aspect.save
   end
 
   ######## Posting ########
diff --git a/app/views/aspects/manage.html.haml b/app/views/aspects/manage.html.haml
index 8ac04b893e..a49030d347 100644
--- a/app/views/aspects/manage.html.haml
+++ b/app/views/aspects/manage.html.haml
@@ -48,7 +48,8 @@
             %li!= remove_link(aspect)
 
         %ul.dropzone{:data=>{:aspect_id=>aspect.id}}
-          -for person in aspect.people
+          -for contact in aspect.people
+            -person = contact.person
             %li.person{:data=>{:guid=>person.id, :aspect_id=>aspect.id}}
               .delete
                 .x
diff --git a/app/views/people/show.html.haml b/app/views/people/show.html.haml
index a20026e4f1..931c231047 100644
--- a/app/views/people/show.html.haml
+++ b/app/views/people/show.html.haml
@@ -17,7 +17,7 @@
         %li
           %i= t(".last_seen",:how_long_ago => how_long_ago(@posts.first))
 
-      - if @person != current_user.person && current_user.friends.include?(@person)
+      - if @person != current_user.person && @contact
         %li
           %i= t(".friends_since",:how_long_ago => how_long_ago(@person))
         %li
diff --git a/lib/diaspora/user/querying.rb b/lib/diaspora/user/querying.rb
index f1fbc7704d..f0097a1724 100644
--- a/lib/diaspora/user/querying.rb
+++ b/lib/diaspora/user/querying.rb
@@ -23,14 +23,17 @@ module Diaspora
 
       def visible_person_by_id( id )
         id = id.to_id
-        return self.person if id == self.person.id
-        result = friends.first(:person_id => id).person
-        result = visible_people.detect{|x| x.id == id } unless result
-        result
+        if id == self.person.id
+          self.person
+        elsif friend = friends.first(:person_id => id)
+          friend.person
+        else
+          visible_people.detect{|x| x.id == id }
+        end
       end
 
-      def friends_not_in_aspect( aspect )
-        Person.all(:id.in => self.friend_ids, :id.nin => aspect.person_ids)
+      def friends_not_in_aspect( aspect ) 
+        Contact.all(:user_id => self.id, :aspect_ids.ne => aspect._id).map{|c| c.person}
       end
 
       def aspect_by_id( id )
diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb
index 425d3de4b0..e1695eb3fc 100644
--- a/spec/controllers/aspects_controller_spec.rb
+++ b/spec/controllers/aspects_controller_spec.rb
@@ -14,6 +14,7 @@ describe AspectsController do
     @user2   = Factory.create(:user)
     @aspect2 = @user2.aspect(:name => "party people")
     friend_users(@user,@aspect, @user2, @aspect2)
+    @contact = @user.contact_for(@user2.person)
     sign_in :user, @user
   end
 
@@ -21,7 +22,7 @@ describe AspectsController do
     it "assigns @friends to all the user's friends" do
       Factory.create :person
       get :index
-      assigns[:friends].should == @user.friends
+      assigns[:friends].should == @user.friends.map{|c| c.person}
     end
   end
 
@@ -75,20 +76,20 @@ describe AspectsController do
   describe "#add_to_aspect" do
     it 'adds the users to the aspect' do
       @aspect1.reload
-      @aspect1.people.include?(@user2.person).should be false
+      @aspect1.people.include?(@contact).should be false
       post 'add_to_aspect', {:friend_id => @user2.person.id, :aspect_id => @aspect1.id }
       @aspect1.reload
-      @aspect1.people.include?(@user2.person).should be true
+      @aspect1.people.include?(@contact).should be true
     end
   end 
   
   describe "#remove_from_aspect" do
     it 'adds the users to the aspect' do
       @aspect.reload
-      @aspect.people.include?(@user2.person).should be true
+      @aspect.people.include?(@contact).should be true
       post 'remove_from_aspect', {:friend_id => @user2.person.id, :aspect_id => @aspect1.id }
       @aspect1.reload
-      @aspect1.people.include?(@user2.person).should be false
+      @aspect1.people.include?(@contact).should be false
     end
   end
 end
diff --git a/spec/misc_spec.rb b/spec/misc_spec.rb
index 75580af7bf..fcad1a5e34 100644
--- a/spec/misc_spec.rb
+++ b/spec/misc_spec.rb
@@ -22,15 +22,15 @@ describe 'making sure the spec runner works' do
 
     it 'makes the first user friends with the second' do
       contact = @user1.contact_for @user2.person
-      @user1.friends.should include contact
-      @aspect1.people.should include contact
+      @user1.friends.include?(contact).should be_true
+      @aspect1.people.include?(contact).should be_true
       contact.aspects.include?( @aspect1 ).should be true
     end
 
     it 'makes the second user friends with the first' do
       contact = @user2.contact_for @user1.person
-      @user2.friends.should include contact
-      @aspect2.people.should include contact
+      @user2.friends.include?(contact).should be_true
+      @aspect2.people.include?(contact).should be_true
       contact.aspects.include?( @aspect2 ).should be true
     end
   end
diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb
index e99856df24..7d6cde6c99 100644
--- a/spec/models/aspect_spec.rb
+++ b/spec/models/aspect_spec.rb
@@ -153,10 +153,10 @@ describe Aspect do
 
     describe "#add_person_to_aspect" do
       it 'adds the user to the aspect' do
-        aspect1.people.should_not include contact
+        aspect1.people.include?(contact).should be_false 
         user.add_person_to_aspect(user2.person.id, aspect1.id)
         aspect1.reload
-        aspect1.people.should include contact
+        aspect1.people.include?(contact).should be_true
       end
 
       it 'raises if its an aspect that the user does not own'do
@@ -228,8 +228,8 @@ describe Aspect do
           aspect.reload
           aspect1.reload
 
-          aspect.people.include?(contact).should be false
-          aspect1.people.include?(contact).should be true
+          aspect.people.include?(contact).should be_false
+          aspect1.people.include?(contact).should be_true
         end
 
         it "should not move a person who is not a friend" do
diff --git a/spec/models/user/invite_spec.rb b/spec/models/user/invite_spec.rb
index 67b5f35bfd..21f4670a7c 100644
--- a/spec/models/user/invite_spec.rb
+++ b/spec/models/user/invite_spec.rb
@@ -68,7 +68,7 @@ describe User do
     it 'throws if you try to add someone you"re friends with' do
       friend_users(inviter, aspect, another_user, wrong_aspect)
       inviter.reload
-      proc{inviter.invite_user(:email => another_user.email, :aspect_id => aspect.id)}.should raise_error /You are already friends with this person/
+      proc{inviter.invite_user(:email => another_user.email, :aspect_id => aspect.id)}.should raise_error /You are already friends with that person/
     end
 
     it 'sends a friend request to a user with that email into the aspect' do
@@ -126,7 +126,7 @@ describe User do
       u.reload
       inviter
       inviter.receive_salmon(u.salmon(u.accept_friend_request(request.id, aspect2.id)).xml_for(inviter.person))
-      inviter.friends.include?(u.person).should be true
+      inviter.contact_for(u.person).should_not be_nil
     end
   end
 end
-- 
GitLab