RS, IZ; A variety of fixes and some refactoring on unfriending and activate friend

app/controllers/people_controller.rb

@@ -15,7 +15,7 @@ class PeopleController < ApplicationController
   end
   
   def destroy
-    current_user.unfriend(params[:id])
+    current_user.unfriend(current_user.friends.first(params[:id]))
     flash[:notice] = "unfriended person."
     redirect_to people_url
   end

app/models/person.rb

@@ -27,7 +27,6 @@ class Person
   timestamps!
 
   before_validation :clean_url
-  before_create :check_for_owner
   validates_presence_of :email, :url, :profile, :serialized_key 
   validates_format_of :url, :with =>
      /^(https?):\/\/[a-z0-9]+([\-\.]{1}[a-z0-9]+)*(\.[a-z]{2,5})?(:[0-9]{1,5})?(\/.*)?$/ix
@@ -124,9 +123,6 @@ class Person
 
   private
 
-  def check_for_owner
-    self.user_refs += 1 unless self.owner_id.nil?
-  end
   def remove_all_traces
     self.posts.delete_all
   end

app/models/user.rb

@@ -60,12 +60,7 @@ class User
     request = Request.where(:id => friend_request_id).first
     n = pending_requests.delete(request)
     
-    friends << request.person
-    save
-
-    group = self.groups.first(:id => group_id)
-    group.people << request.person
-    group.save
+    activate_friend(request.person, groups.first(:id => group_id))
 
     request.reverse self
 
@@ -102,26 +97,22 @@ class User
     end
   end
 
-  def unfriend(friend_id)
-    Rails.logger.info("#{self.real_name} is unfriending #{bad_friend.inspect}"
-    bad_friend = Person.first(:_id => friend_id)
+  def unfriend(bad_friend)
+    Rails.logger.info("#{self.real_name} is unfriending #{bad_friend.inspect}")
     Retraction.for(self).push_to_url(bad_friend.receive_url) 
-    remove_friend friend_id
+    remove_friend(bad_friend)
   end
   
-  def remove_friend friend_id
-    bad_friend = Person.first(:_id => friend_id)
-
-    self.friend_ids.delete( friend_id )
+  def remove_friend(bad_friend)
+    raise "Friend not deleted" unless self.friend_ids.delete( bad_friend.id )
     self.save
-
     bad_friend.user_refs -= 1
     (bad_friend.user_refs > 0 || bad_friend.owner.nil? == false) ?  bad_friend.save : bad_friend.destroy
   end
 
-  def unfriended_by friend_id
-    Rails.logger.info("#{self.real_name} is being unfriended by #{bad_friend.inspect}"
-    remove_friend friend_id
+  def unfriended_by(bad_friend)
+    Rails.logger.info("#{self.real_name} is being unfriended by #{bad_friend.inspect}")
+    remove_friend bad_friend
   end
 
   def send_request(rel_hash, group)
@@ -133,8 +124,10 @@ class User
   end
   
   def activate_friend(person, group)
+    person.user_refs += 1
     group.people << person
     friends << person
+    person.save
     group.save
     save
   end
@@ -149,7 +142,7 @@ class User
     Rails.logger.debug("Receiving object:\n#{object.inspect}")
 
     if object.is_a? Retraction
-      (object.type == 'Person' )? (unfriended_by object.post_id) : (object.perform self.id)
+      (object.type == 'Person' )? (unfriended_by friends.first(object.post_id)) : (object.perform self.id)
     elsif object.is_a? Request
       person = Diaspora::Parser.get_or_create_person_object_from_xml( xml )
       person.serialized_key ||= object.exported_key

spec/models/person_spec.rb

@@ -2,7 +2,11 @@ require File.dirname(__FILE__) + '/../spec_helper'
 
 describe Person do
   before do
+    @user = Factory.create(:user)
+    @user2 = Factory.create(:user)
     @person = Factory.create(:person)
+    @group = @user.group(:name => "Dudes")
+    @group2 = @user2.group(:name => "Abscence of Babes")
   end
 
 
@@ -33,8 +37,6 @@ describe Person do
   end
 
   it 'should delete all of user except comments upon user deletion' do
-    Factory.create(:user)
-
     f = Factory.create(:person)
 
     Factory.create(:status_message, :person => f)
@@ -57,45 +59,39 @@ describe Person do
 
   describe "unfriending" do
     it 'should delete an orphaned friend' do
-      user = Factory.create(:user)
-      user.save
       
+      request = @user.send_friend_request_to @person.receive_url, @group.id
 
-      user.friends << @person
-      @person.user_refs += 1
-      @person.save
-
+      @user.activate_friend(@person, @group) 
+      @user.reload
+      
+      Person.all.count.should == 3
+      @user.friends.count.should == 1
+      @user.unfriend(@person)
+      @user.reload
+      @user.friends.count.should == 0
       Person.all.count.should == 2
-      user.friends.count.should == 1
-      user.unfriend(@person.id)
-      user.friends.count.should == 0
-      Person.all.count.should == 1
     end
 
     it 'should not delete an un-orphaned friend' do
-      user_one = Factory.create(:user)
-      user_two = Factory.create(:user)
-
-      user_one.save
-      user_two.save
+      request = @user.send_friend_request_to @person.receive_url, @group.id
+      request2 = @user2.send_friend_request_to @person.receive_url, @group2.id
 
+      @user.activate_friend(@person, @group) 
+      @user2.activate_friend(@person, @group2)
 
-      user_one.friends << @person
-      user_two.friends << @person
-      user_one.save
-      user_two.save
-
-      @person.user_refs += 2
-      @person.save
-
+      @user.reload
+      @user2.reload
+      
       Person.all.count.should == 3
-      user_one.friends.count.should == 1
-      user_two.friends.count.should == 1
-
-      user_one.unfriend(@person.id)
-
-      user_one.friends.count.should == 0
-      user_two.friends.count.should == 1
+      @user.friends.count.should == 1
+      @user2.friends.count.should == 1
+
+      @user.unfriend(@person)
+      @user.reload
+      @user2.reload
+      @user.friends.count.should == 0
+      @user2.friends.count.should == 1
 
       Person.all.count.should == 3
     end

spec/models/user_spec.rb

@@ -235,43 +235,36 @@ describe User do
 
   describe 'unfriending' do
     before do
-      @user = Factory.create :user
       @user2 = Factory.create :user
-
-      @user.friends << @user2.person
-      @user.person.user_refs += 1
-      @user2.friends << @user.person
-
-      @user2.person.user_refs += 1
-
-      @user2.person.save
-      @user.person.save
+      @group2 = @user2.group(:name => "Gross people")
+      
+      request = @user.send_friend_request_to( @user2.receive_url, @group.id)
+      request.reverse @user2 
+      @user2.activate_friend(@user.person, @group2)
+      @user.receive request.to_diaspora_xml
     end
 
     it 'should unfriend the other user on the same seed' do
-      person = @user2.person 
-      person.url = @user.person.url
-      person.save
-      
+      @user.reload
+      @user2.reload
+
       @user.friends.count.should == 1
       @user2.friends.count.should == 1
       
-      @user.person.user_refs.should == 2
+      @user.person.user_refs.should == 1
 
-      @user2.person.user_refs.should == 2
+      @user2.person.user_refs.should == 1
 
-      @user2.unfriend @user.person.id
+      @user2.unfriend @user.person
       @user2.friends.count.should be 0
 
       @user.person.reload
-      @user.person.user_refs.should == 1 
+      @user.person.user_refs.should == 0 
 
-      @user.unfriended_by @user2.person.id
+      @user.unfriended_by @user2.person
 
       @user2.person.reload
-      @user2.person.user_refs.should == 1
+      @user2.person.user_refs.should == 0
     end
-    
-
   end
 end