From b31b2de6f581ac193d6db8360772486a1a7918cb Mon Sep 17 00:00:00 2001
From: ilya <ilya@laptop.(none)>
Date: Tue, 19 Oct 2010 18:25:59 -0700
Subject: [PATCH 1/9] MS IZ aspect add and delete for a person

---
 app/models/user.rb                     |  18 +++-
 lib/diaspora/user/querying.rb          |   5 +
 spec/models/aspect_spec.rb             | 134 +++++++++++++++++++------
 spec/models/user/visible_posts_spec.rb |  15 ++-
 4 files changed, 138 insertions(+), 34 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 1593ee29c..f5b9b7ffa 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -54,7 +54,7 @@ class User
   many :visible_people, :in => :visible_person_ids, :class_name => 'Person' # One of these needs to go
   many :pending_requests, :in => :pending_request_ids, :class_name => 'Request'
   many :raw_visible_posts, :in => :visible_post_ids, :class_name => 'Post'
-  many :aspects, :class_name => 'Aspect', :dependent => :destroy
+  many :aspects, :class_name => 'Aspect'
 
   after_create :seed_aspects
 
@@ -118,6 +118,22 @@ class User
     false
   end
 
+  def add_person_to_aspect(person_id, aspect_id)
+    raise "Can not add person to an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id) 
+    raise "Can not add person you are not friends with" unless person = self.find_friend_by_id(person_id)
+    raise 'Can not add person who is already in the aspect' if aspect.person_ids.include?(person_id)
+    aspect.people << person 
+    aspect.save
+  end
+
+  def delete_person_from_aspect(person_id, aspect_id)
+    raise "Can not delete a person from an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id)
+    aspect.person_ids.delete(person_id)
+    id_array = aspect.posts.find_all_by_person_id(person_id).collect{|x| x.id}
+    aspect.post_ids = aspect.post_ids - id_array
+    aspect.save
+  end
+
   ######## Posting ########
   def post(class_name, options = {})
     if class_name == :photo
diff --git a/lib/diaspora/user/querying.rb b/lib/diaspora/user/querying.rb
index 7e6413749..eae34862b 100644
--- a/lib/diaspora/user/querying.rb
+++ b/lib/diaspora/user/querying.rb
@@ -34,6 +34,11 @@ module Diaspora
         aspects.detect{|x| x.id == id }
       end
 
+      def find_friend_by_id(id)
+        id = id.to_id
+        friends.detect{|x| x.id == id }
+      end
+
       def aspects_with_post( id )
         self.aspects.find_all_by_post_ids( id.to_id )
       end
diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb
index e91d4a258..abb995ebd 100644
--- a/spec/models/aspect_spec.rb
+++ b/spec/models/aspect_spec.rb
@@ -129,59 +129,129 @@ describe Aspect do
     end
   end
 
-  describe "aspect editing" do
+  context "aspect editing" do
+    let(:aspect) {@user.aspect(:name => 'losers')}
+    let(:aspect2) {@user2.aspect(:name => 'failures')}
+    let(:aspect1) {@user.aspect(:name => 'cats')}
+    let(:not_friend) { Factory(:person, :diaspora_handle => "not@person.com")}
+    let(:user3) {Factory(:user)}
+    let(:aspect3) {user3.aspect(:name => "lala")}
+
     before do
-      @aspect = @user.aspect(:name => 'losers')
-      @aspect2 = @user2.aspect(:name => 'failures')
-      friend_users(@user, @aspect, @user2, @aspect2)
-      @aspect.reload
-      @aspect3 = @user.aspect(:name => 'cats')
+      friend_users(@user, aspect, @user2, aspect2)
+      aspect.reload
       @user.reload
     end
 
     it 'should be able to move a friend from one of users existing aspects to another' do
-      @user.move_friend(:friend_id => @user2.person.id, :from => @aspect.id, :to => @aspect3.id)
-      @aspect.reload
-      @aspect3.reload
+      @user.move_friend(:friend_id => @user2.person.id, :from => aspect.id, :to => aspect1.id)
+      aspect.reload
+      aspect1.reload
 
-      @aspect.person_ids.include?(@user2.person.id).should be false
-      @aspect3.people.include?(@user2.person).should be true
+      aspect.person_ids.include?(@user2.person.id).should be false
+      aspect1.people.include?(@user2.person).should be true
     end
 
     it "should not move a person who is not a friend" do
-      @user.move_friend(:friend_id => @friend.id, :from => @aspect.id, :to => @aspect3.id)
-      @aspect.reload
-      @aspect3.reload
-      @aspect.people.include?(@friend).should be false
-      @aspect3.people.include?(@friend).should be false
+      @user.move_friend(:friend_id => @friend.id, :from => aspect.id, :to => aspect1.id)
+      aspect.reload
+      aspect1.reload
+      aspect.people.include?(@friend).should be false
+      aspect1.people.include?(@friend).should be false
     end
 
     it "should not move a person to a aspect that's not his" do
-      @user.move_friend(:friend_id => @user2.person.id, :from => @aspect.id, :to => @aspect2.id)
-      @aspect.reload
-      @aspect2.reload
-      @aspect.people.include?(@user2.person).should be true
-      @aspect2.people.include?(@user2.person).should be false
+      @user.move_friend(:friend_id => @user2.person.id, :from => aspect.id, :to => aspect2.id)
+      aspect.reload
+      aspect2.reload
+      aspect.people.include?(@user2.person).should be true
+      aspect2.people.include?(@user2.person).should be false
     end
 
-    it 'should move all the by that user to the new aspect' do
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => @aspect2.id)
+    it 'should move all posts by that user to the new aspect' do
+      message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
 
       @user.receive message.to_diaspora_xml, @user2.person
-      @aspect.reload
+      aspect.reload
 
-      @aspect.posts.count.should == 1
-      @aspect3.posts.count.should == 0
+      aspect.posts.count.should == 1
+      aspect1.posts.count.should == 0
 
       @user.reload
-      @user.move_friend(:friend_id => @user2.person.id, :from => @aspect.id, :to => @aspect3.id)
-      @aspect.reload
-      @aspect3.reload
-
-      @aspect3.posts.count.should == 1
-      @aspect.posts.count.should == 0
+      @user.move_friend(:friend_id => @user2.person.id, :from => aspect.id, :to => aspect1.id)
+      aspect.reload
+      aspect1.reload
 
+      aspect1.posts.count.should == 1
+      aspect.posts.count.should == 0
     end
 
+    describe "#add_person_to_aspect" do
+      it 'adds the user to the aspect' do
+        aspect1.people.should_not include @user2.person
+        @user.add_person_to_aspect(@user2.person.id, aspect1.id)
+        aspect1.reload
+        aspect1.people.should include @user2.person
+      end
+
+      it 'raises if its an aspect that the user does not own'do
+        proc{@user.add_person_to_aspect(@user2.person.id, aspect2.id) }.should raise_error /Can not add person to an aspect you do not own/
+      end
+
+      it 'does not allow to have duplicate people in an aspect' do
+        proc{@user.add_person_to_aspect(not_friend.id, aspect1.id) }.should raise_error /Can not add person you are not friends with/
+      end
+
+      it 'does not allow you to add a person if they are already in the aspect' do
+        proc{@user.add_person_to_aspect(@user2.person.id, aspect.id) }.should raise_error /Can not add person who is already in the aspect/
+      end
+    end
+
+    describe '#delete_person_from_aspect' do
+      it 'deletes a user from the aspect' do
+         @user.add_person_to_aspect(@user2.person.id, aspect1.id)
+         @user.reload
+         @user.aspects.find_by_id(aspect1.id).people.include?(@user2.person).should be true
+         @user.delete_person_from_aspect(@user2.person.id, aspect1.id)
+         @user.reload
+         @user.aspects.find_by_id(aspect1.id).people.include?(@user2.person).should be false
+      end
+
+      it 'should check to make sure you have the aspect ' do
+        proc{@user.delete_person_from_aspect(@user2.person.id, aspect2.id) }.should raise_error /Can not delete a person from an aspect you do not own/
+      end
+
+        context 'removing posts' do
+          before do
+            friend_users(@user, aspect, user3, aspect3)
+
+            message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
+            @user.receive message.to_diaspora_xml, @user2.person
+            aspect.reload
+            aspect.posts.count.should == 1
+          end
+
+          it 'should remove the users posts from that aspect' do
+            @user.reload
+            @user.delete_person_from_aspect(@user2.person.id, aspect.id)
+            aspect.reload
+            aspect.posts.count.should == 0
+          end
+
+          it 'should not delete other peoples posts' do
+            message2 = user3.post(:status_message, :message => "other post", :to => aspect3.id)
+
+            @user.receive message2.to_diaspora_xml, user3.person
+
+            aspect.reload
+            aspect.posts.count.should == 2
+
+            @user.reload
+            @user.delete_person_from_aspect(@user2.person.id, aspect.id)
+            aspect.reload
+            aspect.posts.should == [message2]
+          end
+        end
+      end
   end
 end
diff --git a/spec/models/user/visible_posts_spec.rb b/spec/models/user/visible_posts_spec.rb
index 6f4345251..4c06643c1 100644
--- a/spec/models/user/visible_posts_spec.rb
+++ b/spec/models/user/visible_posts_spec.rb
@@ -23,6 +23,7 @@ describe User do
 
   before do
     friend_users(user, first_aspect, user2, user2.aspects.first)
+    friend_users(user, second_aspect, user3, user3.aspects.first)
   end
 
   describe "#visible_posts" do
@@ -48,7 +49,6 @@ describe User do
     end
 
     it "queries by aspect" do
-      friend_users(user, second_aspect, user3, user3.aspects.first)
       friend_users(user, second_aspect, user4, user4.aspects.first)
 
       user.receive status_message4.to_diaspora_xml, user2.person
@@ -67,6 +67,19 @@ describe User do
         user.find_visible_post_by_id(status_message1.id).should == nil
       end
     end
+
+    describe '#find_friend_by_id' do
+      it 'should find both friends' do
+        user.reload
+        user.find_friend_by_id(user2.person.id).should == user2.person
+        user.find_friend_by_id(user3.person.id).should == user3.person
+      end
+
+      it 'should not find a non-friend' do
+        user3.find_friend_by_id(user4.person.id).should be nil
+      end
+
+    end
   end
 
   context 'albums' do

From 38e8af23001d3ded1c0ac43419fd965db5cf3ade Mon Sep 17 00:00:00 2001
From: ilya <ilya@laptop.(none)>
Date: Tue, 19 Oct 2010 19:16:44 -0700
Subject: [PATCH 2/9] better querying

---
 Gemfile.lock       | 21 +++++++--------------
 app/models/user.rb |  2 +-
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 91fca3349..14e3f10ef 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -97,7 +97,6 @@ GEM
       activesupport (= 3.0.1)
     activesupport (3.0.1)
     addressable (2.2.2)
-    archive-tar-minitar (0.5.2)
     arel (1.0.1)
       activesupport (~> 3.0.0)
     aws (2.3.21)
@@ -164,8 +163,7 @@ GEM
     i18n (0.4.1)
     json (1.4.6)
     json_pure (1.4.6)
-    linecache19 (0.5.11)
-      ruby_core_source (>= 0.1.4)
+    linecache (0.43)
     mail (2.2.7)
       activesupport (>= 2.3.6)
       mime-types
@@ -227,16 +225,11 @@ GEM
       rspec-expectations (= 2.0.0)
     rspec-rails (2.0.0)
       rspec (= 2.0.0)
-    ruby-debug-base19 (0.11.24)
-      columnize (>= 0.3.1)
-      linecache19 (>= 0.5.11)
-      ruby_core_source (>= 0.1.4)
-    ruby-debug19 (0.11.6)
-      columnize (>= 0.3.1)
-      linecache19 (>= 0.5.11)
-      ruby-debug-base19 (>= 0.11.19)
-    ruby_core_source (0.1.4)
-      archive-tar-minitar (>= 0.5.2)
+    ruby-debug (0.10.3)
+      columnize (>= 0.1)
+      ruby-debug-base (~> 0.10.3.0)
+    ruby-debug-base (0.10.3)
+      linecache (>= 0.3)
     rubyzip (0.9.4)
     selenium-webdriver (0.0.29)
       childprocess (>= 0.0.7)
@@ -294,7 +287,7 @@ DEPENDENCIES
   roxml!
   rspec (>= 2.0.0)
   rspec-rails (>= 2.0.0)
-  ruby-debug19
+  ruby-debug
   sprinkle!
   thin
   webmock
diff --git a/app/models/user.rb b/app/models/user.rb
index e79a51de3..9c1305430 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -129,7 +129,7 @@ class User
   def delete_person_from_aspect(person_id, aspect_id)
     raise "Can not delete a person from an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id)
     aspect.person_ids.delete(person_id)
-    id_array = aspect.posts.find_all_by_person_id(person_id).collect{|x| x.id}
+    id_array = aspect.posts.all(:person_id => person_id, :select => "_id").collect{|x| x.id}
     aspect.post_ids = aspect.post_ids - id_array
     aspect.save
   end

From 4b588ccefb0803e84d0e6f3d09063c1bd57ffbe4 Mon Sep 17 00:00:00 2001
From: Joseph Method <tristil@gmail.com>
Date: Tue, 19 Oct 2010 23:03:07 -0400
Subject: [PATCH 3/9] Addresses [#380] and [#239] by handling the errors from
 bad identities

---
 app/controllers/requests_controller.rb       | 11 +++++--
 app/models/person.rb                         |  6 +++-
 config/locales/diaspora/en.yml               |  2 ++
 spec/controllers/requests_controller_spec.rb | 30 ++++++++++++++++++++
 spec/models/person_spec.rb                   | 20 +++++++++++++
 5 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb
index 347d8bfce..4a10821aa 100644
--- a/app/controllers/requests_controller.rb
+++ b/app/controllers/requests_controller.rb
@@ -35,8 +35,15 @@ class RequestsController < ApplicationController
     begin
       rel_hash = relationship_flow(params[:request][:destination_url].strip)
     rescue Exception => e
-      raise e unless e.message.include? "not found"
-      flash[:error] = I18n.t 'requests.create.error'
+      if e.message.include? "not found"
+        flash[:error] = I18n.t 'requests.create.error'
+      elsif e.message.include?  "Connection timed out"
+        flash[:error] = I18n.t 'requests.create.error_server'
+      elsif e.message == "Identifier is invalid"
+        flash[:error] = I18n.t 'requests.create.invalid_identity'
+      else
+        raise e
+      end
       respond_with :location => aspect
       return
     end
diff --git a/app/models/person.rb b/app/models/person.rb
index 8a9011fa4..136af95b9 100644
--- a/app/models/person.rb
+++ b/app/models/person.rb
@@ -85,7 +85,11 @@ class Person
   end
 
   def self.by_webfinger(identifier, opts = {})
-    #need to check if this is a valid email structure, maybe should do in JS
+    # Raise an error if identifier has a port number 
+    raise "Identifier is invalid" if(identifier.strip.match(/\:\d+$/))
+    # Raise an error if identifier is not a valid email (generous regexp)
+    raise "Identifier is invalid" if !(identifier =~ /\A.*\@.*\..*\Z/)
+
     query = /#{Regexp.escape(identifier.gsub('acct:', '').to_s)}/i
     local_person = Person.first(:diaspora_handle => query)
 
diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml
index 45d80437a..5f09b603c 100644
--- a/config/locales/diaspora/en.yml
+++ b/config/locales/diaspora/en.yml
@@ -229,6 +229,8 @@ en:
           ignore: "Ignored friend request."
       create:
           error: "No diaspora seed found with this email!"
+          invalid_identity: "This identity is not properly formatted"
+          error_server: "Problem with other server. Maybe it doesn't exist?"
           yourself: "You cannot befriend yourself!"
           already_friends: "You are already friends with %{destination_url}!"
           success: "A friend request was sent to %{destination_url}."
diff --git a/spec/controllers/requests_controller_spec.rb b/spec/controllers/requests_controller_spec.rb
index eb3720cd2..0beb8d373 100644
--- a/spec/controllers/requests_controller_spec.rb
+++ b/spec/controllers/requests_controller_spec.rb
@@ -22,4 +22,34 @@ describe RequestsController do
     response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
   end
 
+  it "should not error out when requesting an invalid identity" do
+    put("create", "request" => {
+      "destination_url" => "not_a_@valid_email",
+      "aspect_id" => @user.aspects[0].id 
+      } 
+    )
+    response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
+  end
+
+  it "should not error out when requesting an invalid identity with a port number" do
+    put("create", "request" => {
+      "destination_url" => "johndoe@email.com:3000",
+      "aspect_id" => @user.aspects[0].id 
+      } 
+    )
+    response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
+  end
+
+  it "should not error out when requesting an identity from an invalid server" do
+    stub_request(:get, /notadiasporaserver\.com/).to_raise(Errno::ETIMEDOUT)
+    put("create", "request" => {
+      "destination_url" => "johndoe@notadiasporaserver.com",
+      "aspect_id" => @user.aspects[0].id 
+      } 
+    )
+    response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
+  end
+
+
+
 end
diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb
index fd5ea0162..7506d0433 100644
--- a/spec/models/person_spec.rb
+++ b/spec/models/person_spec.rb
@@ -197,6 +197,26 @@ describe Person do
       end
     end
 
+    it 'identifier should be a valid email' do
+      stub_success("joe.valid+email@my-address.com")
+      Proc.new { 
+        Person.by_webfinger("joe.valid+email@my-address.com")
+      }.should_not raise_error(RuntimeError, "Identifier is invalid")
+
+      stub_success("not_a_@valid_email")
+      Proc.new { 
+        Person.by_webfinger("not_a_@valid_email")
+      }.should raise_error(RuntimeError, "Identifier is invalid")
+
+    end
+
+    it 'should not accept a port number' do
+      stub_success("eviljoe@diaspora.local:3000")
+      Proc.new { 
+        Person.by_webfinger('eviljoe@diaspora.local:3000')
+      }.should raise_error(RuntimeError, "Identifier is invalid")
+    end
+
     it 'creates a stub for a remote user' do
       stub_success("tom@tom.joindiaspora.com")
       tom = Person.by_webfinger('tom@tom.joindiaspora.com')

From 0758f9245f6ff9dbf2b2069a2834a09e38ef64ca Mon Sep 17 00:00:00 2001
From: Raphael <raphael@joindiaspora.com>
Date: Wed, 20 Oct 2010 10:28:08 -0700
Subject: [PATCH 4/9] Clean up specs a little, fix unfriending

---
 lib/diaspora/user/friending.rb          |  5 ++++-
 spec/lib/diaspora_parser_spec.rb        |  2 +-
 spec/models/user/user_friending_spec.rb | 20 ++++++++------------
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/diaspora/user/friending.rb b/lib/diaspora/user/friending.rb
index db14d329b..6946910f2 100644
--- a/lib/diaspora/user/friending.rb
+++ b/lib/diaspora/user/friending.rb
@@ -89,7 +89,10 @@ module Diaspora
       def remove_friend(bad_friend)
         raise "Friend not deleted" unless self.friend_ids.delete( bad_friend.id )
         aspects.each{|aspect|
-          aspect.person_ids.delete( bad_friend.id )}
+          if aspect.person_ids.delete( bad_friend.id )
+            aspect.posts.delete_if { |post| 
+              post.person_id == bad_friend.id}
+          end}
         self.save
 
         self.raw_visible_posts.find_all_by_person_id( bad_friend.id ).each{|post|
diff --git a/spec/lib/diaspora_parser_spec.rb b/spec/lib/diaspora_parser_spec.rb
index 6c3c697df..1585d5d75 100644
--- a/spec/lib/diaspora_parser_spec.rb
+++ b/spec/lib/diaspora_parser_spec.rb
@@ -153,7 +153,7 @@ describe Diaspora::Parser do
       person.save
 
       #Cache profile for checking against marshaled profile
-      old_profile = person.profile
+      old_profile = person.profile.dup
       old_profile.first_name.should == 'bob'
 
       #Build xml for profile, clear profile
diff --git a/spec/models/user/user_friending_spec.rb b/spec/models/user/user_friending_spec.rb
index 2756c0841..221887885 100644
--- a/spec/models/user/user_friending_spec.rb
+++ b/spec/models/user/user_friending_spec.rb
@@ -160,22 +160,18 @@ describe User do
     describe 'unfriending' do
       before do
         friend_users(user,aspect, user2, aspect2)
-        user.reload
-        user2.reload
       end
 
       it 'should unfriend the other user on the same seed' do
-        user.friends.count.should == 1
-        user2.friends.count.should == 1
+        lambda {user2.unfriend user.person}.should change{
+          user2.friends.count}.by(-1)
+        aspect2.people.count.should == 0
+      end
 
-        user2.unfriend user.person
-        user2.reload
-
-        user2.friends.count.should == 0
-        user.unfriended_by user2.person
-
-        aspect.reload.people.count.should == 0
-        aspect2.reload.people.count.should == 0
+      it 'is unfriended by another user' do
+        lambda {user.unfriended_by user2.person}.should change{
+          user.friends.count}.by(-1)
+        aspect.people.count.should == 0
       end
 
       context 'with a post' do

From 4986de2c3323d74aecb08920b68efe45166f29da Mon Sep 17 00:00:00 2001
From: Raphael <raphael@joindiaspora.com>
Date: Wed, 20 Oct 2010 10:35:16 -0700
Subject: [PATCH 5/9] Put reloads back in

---
 spec/models/user/user_friending_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/models/user/user_friending_spec.rb b/spec/models/user/user_friending_spec.rb
index 221887885..bae837b09 100644
--- a/spec/models/user/user_friending_spec.rb
+++ b/spec/models/user/user_friending_spec.rb
@@ -165,13 +165,13 @@ describe User do
       it 'should unfriend the other user on the same seed' do
         lambda {user2.unfriend user.person}.should change{
           user2.friends.count}.by(-1)
-        aspect2.people.count.should == 0
+        aspect2.reload.people.count.should == 0
       end
 
       it 'is unfriended by another user' do
         lambda {user.unfriended_by user2.person}.should change{
           user.friends.count}.by(-1)
-        aspect.people.count.should == 0
+        aspect.reload.people.count.should == 0
       end
 
       context 'with a post' do

From 961510a8ed06590109a8090686355ffdcde71180 Mon Sep 17 00:00:00 2001
From: Raphael <raphael@joindiaspora.com>
Date: Wed, 20 Oct 2010 11:03:47 -0700
Subject: [PATCH 6/9] Rename instantiate! to build, no more raising in build,
 no saving in build, no seeding of aspects in build.

---
 app/controllers/registrations_controller.rb   |  9 +---
 app/models/user.rb                            |  4 +-
 db/seeds/backer.rb                            |  4 +-
 db/seeds/dev.rb                               | 10 ++--
 db/seeds/tom.rb                               |  9 ++--
 .../registrations_controller_spec.rb          |  4 +-
 spec/models/user_spec.rb                      | 47 +++++++++++--------
 7 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 541563d48..75ee3b7f7 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -4,15 +4,10 @@
 
 class RegistrationsController < Devise::RegistrationsController
   def create
-    begin
-      @user = User.instantiate!(params[:user])
-    rescue MongoMapper::DocumentNotValid => e
-      flash[:error] = e.message
-      redirect_to new_user_registration_path
-      return
-    end
+    @user = User.build(params[:user])
     if @user.save
       flash[:notice] = I18n.t 'registrations.create.success'
+      @user.seed_aspects
       sign_in_and_redirect(:user, @user)
     else
       flash[:error] = @user.errors.full_messages.join(', ')
diff --git a/app/models/user.rb b/app/models/user.rb
index cc9531c8c..fd05fcb1c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -379,7 +379,7 @@ class User
   end
 
   ###Helpers############
-  def self.instantiate!(opts = {})
+  def self.build(opts = {})
     opts[:person][:diaspora_handle] = "#{opts[:username]}@#{APP_CONFIG[:terse_pod_url]}"
     opts[:person][:url] = APP_CONFIG[:pod_url]
 
@@ -387,8 +387,6 @@ class User
     opts[:person][:serialized_public_key] = opts[:serialized_private_key].public_key
 
     u = User.new(opts)
-    u.seed_aspects
-    u.save!
     u
   end
 
diff --git a/db/seeds/backer.rb b/db/seeds/backer.rb
index 2305dea93..7f27c5626 100644
--- a/db/seeds/backer.rb
+++ b/db/seeds/backer.rb
@@ -25,14 +25,14 @@ def create
   require File.join(File.dirname(__FILE__), "..", "..", "config", "initializers", "_load_app_config.rb")
 
   # Create seed user
-  user = User.instantiate!(:email => "#{username}@#{username}.joindiaspora.com",
+  user = User.build(:email => "#{username}@#{username}.joindiaspora.com",
                      :username => username,
                      :password => "#{username+backer_info[backer_number]['pin'].to_s}",
                      :password_confirmation => "#{username+backer_info[backer_number]['pin'].to_s}",
                      :person => Person.new(
                        :profile => Profile.new( :first_name => backer_info[backer_number]['given_name'], :last_name => backer_info[backer_number]['family_name'],
                                              :image_url => "http://#{username}.joindiaspora.com/images/user/#{username}.jpg")
-                    ))
+                    )).save
   user.person.save!
 
   user.aspect(:name => "Presidents")
diff --git a/db/seeds/dev.rb b/db/seeds/dev.rb
index 519636922..7eed484c0 100644
--- a/db/seeds/dev.rb
+++ b/db/seeds/dev.rb
@@ -18,23 +18,25 @@ username = "tom"
 set_app_config username
 
 # Create seed user
-user = User.instantiate!( :email => "tom@tom.joindiaspora.com",
+user = User.build( :email => "tom@tom.joindiaspora.com",
                      :username => "tom",
                     :password => "evankorth",
                     :password_confirmation => "evankorth",
                     :person => Person.new(
                       :profile => Profile.new( :first_name => "Alexander", :last_name => "Hamiltom" ))
-                  )
+                  ).save
 user.person.save!
+user.seed_aspects
 
-user2 = User.instantiate!( :email => "korth@tom.joindiaspora.com",
+user2 = User.build( :email => "korth@tom.joindiaspora.com",
                      :username => "korth",
                     :password => "evankorth",
                     :password_confirmation => "evankorth",
                     :person => Person.new(
-                      :profile => Profile.new( :first_name => "Evan", :last_name => "Korth")))
+                      :profile => Profile.new( :first_name => "Evan", :last_name => "Korth"))).save
 
 user2.person.save!
+user2.seed_aspects
 
 # friending users
 aspect = user.aspect(:name => "other dudes")
diff --git a/db/seeds/tom.rb b/db/seeds/tom.rb
index 074b37caf..2bd21e169 100644
--- a/db/seeds/tom.rb
+++ b/db/seeds/tom.rb
@@ -18,23 +18,24 @@ set_app_config "tom"
 require 'config/initializers/_load_app_config.rb'
 
 # Create seed user
-user = User.instantiate!( :email => "tom@tom.joindiaspora.com",
+user = User.build( :email => "tom@tom.joindiaspora.com",
                      :username => "tom",
                     :password => "evankorth",
                     :password_confirmation => "evankorth",
                     :person => {
                       :profile => { :first_name => "Alexander", :last_name => "Hamiltom",
                       :image_url => "http://tom.joindiaspora.com/images/user/tom.jpg"}}
-                  )
+                  ).save!
+user.seed_aspects
 user.person.save!
 
-user2 = User.instantiate!( :email => "korth@tom.joindiaspora.com",
+user2 = User.build( :email => "korth@tom.joindiaspora.com",
                     :password => "evankorth",
                     :password_confirmation => "evankorth",
                      :username => "korth",
                     :person => {:profile => { :first_name => "Evan", :last_name => "Korth",
                       :image_url => "http://tom.joindiaspora.com/images/user/korth.jpg"}})
-
+user2.seed_aspects
 user2.person.save!
 
 # friending users
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index e89795532..3c9bbde07 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -48,7 +48,6 @@ describe RegistrationsController do
         lambda { get :create, @invalid_params }.should_not change(User, :count)
       end
       it "assigns @user" do
-        pending "GAAAH stupid mongo mapper. Figure out why it thinks it's persisted when validations fail"
         get :create, @valid_params
         assigns(:user).should_not be_nil
       end
@@ -57,9 +56,8 @@ describe RegistrationsController do
         flash[:error].should_not be_blank
       end
       it "goes back to the form" do
-        pending "GAAAH stupid mongo mapper. Figure out why it thinks it's persisted when validations fail"
         get :create, @invalid_params
-        response.should be_success
+        response.should be_redirect
       end
     end
   end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 536e7b68c..427b6b74f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -86,17 +86,29 @@ describe User do
     end
   end
 
-  describe ".instantiate!" do
-    it "creates the user if params are valid" do
-      User.find_by_username("ohai").should be_nil
-      user = User.instantiate!({
-                                 :username => "ohai",
-                                 :email => "ohai@example.com",
-                                 :password => "password",
-                                 :password_confirmation => "password",
-                                 :person => {:profile => {:first_name => "O", :last_name => "Hai"}}})
-      user.should be_valid
-      User.find_by_username("ohai").should == user
+  describe ".build" do
+    context 'with valid params' do
+      before do
+        params = {:username => "ohai",
+                  :email => "ohai@example.com",
+                  :password => "password",
+                  :password_confirmation => "password",
+                  :person => 
+                    {:profile => 
+                      {:first_name => "O", 
+                       :last_name => "Hai"}
+                    }
+        }
+        @user = User.build(params)
+      end
+      it "makes a valid user" do
+        @user.should be_valid
+        User.find_by_username("ohai").should be_nil
+      end
+      it 'saves successfully' do
+        @user.save.should be_true
+        User.find_by_username("ohai").should == @user
+      end
     end
     describe "with invalid params" do
       before do
@@ -107,16 +119,11 @@ describe User do
           :password_confirmation => "password",
           :person => {:profile => {:first_name => "", :last_name => ""}}}
       end
-      it "raises an error" do
-        lambda { User.instantiate!(@invalid_params) }.should raise_error
+      it "raises no error" do
+        lambda { User.build(@invalid_params) }.should_not raise_error
       end
-      it "does not create the user" do
-        User.find_by_username("ohai").should be_nil
-        begin
-          User.instantiate!(@invalid_params)
-        rescue
-        end
-        User.find_by_username("ohai").should be_nil
+      it "does not save" do
+        User.build(@invalid_params).save.should be_false
       end
     end
   end

From ea1ab59c3a2e7878b93f96e152f07397a5d75ebe Mon Sep 17 00:00:00 2001
From: ilya <ilya@laptop.(none)>
Date: Wed, 20 Oct 2010 11:28:56 -0700
Subject: [PATCH 7/9] MS, IZ finished adding and removing people from aspects
 methods, refactored the move friend method

---
 app/models/user.rb         |  28 ++--
 spec/models/aspect_spec.rb | 268 +++++++++++++++++++------------------
 2 files changed, 148 insertions(+), 148 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 83ce569f6..971311471 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -100,37 +100,33 @@ class User
 
   def move_friend(opts = {})
     return true if opts[:to] == opts[:from]
-    friend = Person.first(:_id => opts[:friend_id])
-    if self.friend_ids.include?(friend.id)
-      from_aspect = self.aspect_by_id(opts[:from])
-      to_aspect = self.aspect_by_id(opts[:to])
-      if from_aspect && to_aspect
-        posts_to_move = from_aspect.posts.find_all_by_person_id(friend.id)
-        to_aspect.people << friend
-        to_aspect.posts << posts_to_move
-        from_aspect.person_ids.delete(friend.id.to_id)
-        posts_to_move.each { |x| from_aspect.post_ids.delete(x.id) }
-        from_aspect.save
-        to_aspect.save
+    if opts[:friend_id] && opts[:to] && opts[:from] 
+      from_aspect = self.aspects.first(:_id => opts[:from])
+      posts_to_move = from_aspect.posts.find_all_by_person_id(opts[:friend_id])
+      if add_person_to_aspect(opts[:friend_id], opts[:to], :posts => posts_to_move)
+        delete_person_from_aspect(opts[:friend_id], opts[:from], :posts => posts_to_move) 
         return true
       end
     end
     false
   end
 
-  def add_person_to_aspect(person_id, aspect_id)
+  def add_person_to_aspect(person_id, aspect_id, opts = {})
     raise "Can not add person to an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id) 
     raise "Can not add person you are not friends with" unless person = self.find_friend_by_id(person_id)
     raise 'Can not add person who is already in the aspect' if aspect.person_ids.include?(person_id)
     aspect.people << person 
+    opts[:posts] ||= self.raw_visible_posts.all(:person_id => person_id)
+    
+    aspect.posts += opts[:posts]
     aspect.save
   end
 
-  def delete_person_from_aspect(person_id, aspect_id)
+  def delete_person_from_aspect(person_id, aspect_id, opts = {})
     raise "Can not delete a person from an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id)
     aspect.person_ids.delete(person_id)
-    id_array = aspect.posts.all(:person_id => person_id, :select => "_id").collect{|x| x.id}
-    aspect.post_ids = aspect.post_ids - id_array
+    opts[:posts] ||= aspect.posts.all(:person_id => person_id)
+    aspect.posts -= opts[:posts]
     aspect.save
   end
 
diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb
index abb995ebd..1c10f9bf1 100644
--- a/spec/models/aspect_spec.rb
+++ b/spec/models/aspect_spec.rb
@@ -5,123 +5,128 @@
 require 'spec_helper'
 
 describe Aspect do
-  before do
-    @user = Factory.create(:user)
-    @friend = Factory.create(:person)
-    @user2 = Factory.create(:user)
-    @friend_2 = Factory.create(:person)
-  end
+  let(:user ) { Factory.create(:user) }
+  let(:friend) { Factory.create(:person) }
+  let(:user2) { Factory.create(:user) }
+  let(:friend_2) { Factory.create(:person) }
+
+  let(:aspect) {user.aspect(:name => 'losers')}
+  let(:aspect2) {user2.aspect(:name => 'failures')}
+  let(:aspect1) {user.aspect(:name => 'cats')}
+  let(:not_friend) { Factory(:person, :diaspora_handle => "not@person.com")}
+  let(:user3) {Factory(:user)}
+  let(:aspect3) {user3.aspect(:name => "lala")}
 
   describe 'creation' do
     it 'should have a name' do
-      aspect = @user.aspect(:name => 'losers')
+      aspect = user.aspect(:name => 'losers')
       aspect.name.should == "losers"
     end
 
     it 'should be creatable with people' do
-      aspect = @user.aspect(:name => 'losers', :people => [@friend, @friend_2])
+      aspect = user.aspect(:name => 'losers', :people => [friend, friend_2])
       aspect.people.size.should == 2
     end
 
     it 'should be able to have other users' do
-      aspect = @user.aspect(:name => 'losers', :people => [@user2.person])
-      aspect.people.include?(@user.person).should be false
-      aspect.people.include?(@user2.person).should be true
+      aspect = user.aspect(:name => 'losers', :people => [user2.person])
+      aspect.people.include?(user.person).should be false
+      aspect.people.include?(user2.person).should be true
       aspect.people.size.should == 1
     end
 
     it 'should be able to have users and people' do
-      aspect = @user.aspect(:name => 'losers', :people => [@user2.person, @friend_2])
-      aspect.people.include?(@user.person).should be false
-      aspect.people.include?(@user2.person).should be true
-      aspect.people.include?(@friend_2).should be true
+      aspect = user.aspect(:name => 'losers', :people => [user2.person, friend_2])
+      aspect.people.include?(user.person).should be false
+      aspect.people.include?(user2.person).should be true
+      aspect.people.include?(friend_2).should be true
       aspect.people.size.should == 2
     end
   end
 
   describe 'validation' do
     before do
-      @aspect = @user.aspect(:name => 'losers')
+      @aspect = user.aspect(:name => 'losers')
     end
     it 'has a unique name for one user' do
-      aspect2 = @user.aspect(:name => @aspect.name)
+      aspect2 = user.aspect(:name => @aspect.name)
       aspect2.valid?.should be_false
     end
 
     it 'has no uniqueness between users' do
-      aspect2 = @user2.aspect(:name => @aspect.name)
+      aspect2 = user2.aspect(:name => @aspect.name)
       aspect2.valid?.should be_true
     end
   end
 
   describe 'querying' do
     before do
-      @aspect = @user.aspect(:name => 'losers')
-      @user.activate_friend(@friend, @aspect)
-      @aspect2 = @user2.aspect(:name => 'failures')
-      friend_users(@user, @aspect, @user2, @aspect2)
+      @aspect = user.aspect(:name => 'losers')
+      user.activate_friend(friend, @aspect)
+      @aspect2 = user2.aspect(:name => 'failures')
+      friend_users(user, @aspect, user2, @aspect2)
       @aspect.reload
     end
 
     it 'belong to a user' do
-      @aspect.user.id.should == @user.id
-      @user.aspects.size.should == 3
+      @aspect.user.id.should == user.id
+      user.aspects.size.should == 3
     end
 
     it 'should have people' do
-      @aspect.people.all.include?(@friend).should be true
+      @aspect.people.all.include?(friend).should be true
       @aspect.people.size.should == 2
     end
 
     it 'should be accessible through the user' do
-      aspects = @user.aspects_with_person(@friend)
+      aspects = user.aspects_with_person(friend)
       aspects.size.should == 1
       aspects.first.id.should == @aspect.id
       aspects.first.people.size.should == 2
-      aspects.first.people.include?(@friend).should be true
-      aspects.first.people.include?(@user2.person).should be true
+      aspects.first.people.include?(friend).should be true
+      aspects.first.people.include?(user2.person).should be true
     end
   end
 
   describe 'posting' do
 
     it 'should add post to aspect via post method' do
-      aspect = @user.aspect(:name => 'losers', :people => [@friend])
+      aspect = user.aspect(:name => 'losers', :people => [friend])
 
-      status_message = @user.post( :status_message, :message => "hey", :to => aspect.id )
+      status_message = user.post( :status_message, :message => "hey", :to => aspect.id )
 
       aspect.reload
       aspect.posts.include?(status_message).should be true
     end
 
     it 'should add post to aspect via receive method' do
-      aspect  = @user.aspect(:name => 'losers')
-      aspect2 = @user2.aspect(:name => 'winners')
-      friend_users(@user, aspect, @user2, aspect2)
+      aspect  = user.aspect(:name => 'losers')
+      aspect2 = user2.aspect(:name => 'winners')
+      friend_users(user, aspect, user2, aspect2)
 
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
+      message = user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
 
-      @user.receive message.to_diaspora_xml, @user2.person
+      user.receive message.to_diaspora_xml, user2.person
 
       aspect.reload
       aspect.posts.include?(message).should be true
-      @user.visible_posts(:by_members_of => aspect).include?(message).should be true
+      user.visible_posts(:by_members_of => aspect).include?(message).should be true
     end
 
     it 'should retract the post from the aspects as well' do
-      aspect  = @user.aspect(:name => 'losers')
-      aspect2 = @user2.aspect(:name => 'winners')
-      friend_users(@user, aspect, @user2, aspect2)
+      aspect  = user.aspect(:name => 'losers')
+      aspect2 = user2.aspect(:name => 'winners')
+      friend_users(user, aspect, user2, aspect2)
 
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
+      message = user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
 
-      @user.receive message.to_diaspora_xml, @user2.person
+      user.receive message.to_diaspora_xml, user2.person
       aspect.reload
 
       aspect.post_ids.include?(message.id).should be true
 
-      retraction = @user2.retract(message)
-      @user.receive retraction.to_diaspora_xml, @user2.person
+      retraction = user2.retract(message)
+      user.receive retraction.to_diaspora_xml, user2.person
 
 
       aspect.reload
@@ -129,129 +134,128 @@ describe Aspect do
     end
   end
 
-  context "aspect editing" do
-    let(:aspect) {@user.aspect(:name => 'losers')}
-    let(:aspect2) {@user2.aspect(:name => 'failures')}
-    let(:aspect1) {@user.aspect(:name => 'cats')}
-    let(:not_friend) { Factory(:person, :diaspora_handle => "not@person.com")}
-    let(:user3) {Factory(:user)}
-    let(:aspect3) {user3.aspect(:name => "lala")}
+  context "aspect management" do
+
 
     before do
-      friend_users(@user, aspect, @user2, aspect2)
+      friend_users(user, aspect, user2, aspect2)
       aspect.reload
-      @user.reload
-    end
-
-    it 'should be able to move a friend from one of users existing aspects to another' do
-      @user.move_friend(:friend_id => @user2.person.id, :from => aspect.id, :to => aspect1.id)
-      aspect.reload
-      aspect1.reload
-
-      aspect.person_ids.include?(@user2.person.id).should be false
-      aspect1.people.include?(@user2.person).should be true
-    end
-
-    it "should not move a person who is not a friend" do
-      @user.move_friend(:friend_id => @friend.id, :from => aspect.id, :to => aspect1.id)
-      aspect.reload
-      aspect1.reload
-      aspect.people.include?(@friend).should be false
-      aspect1.people.include?(@friend).should be false
-    end
-
-    it "should not move a person to a aspect that's not his" do
-      @user.move_friend(:friend_id => @user2.person.id, :from => aspect.id, :to => aspect2.id)
-      aspect.reload
-      aspect2.reload
-      aspect.people.include?(@user2.person).should be true
-      aspect2.people.include?(@user2.person).should be false
-    end
-
-    it 'should move all posts by that user to the new aspect' do
-      message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
-
-      @user.receive message.to_diaspora_xml, @user2.person
-      aspect.reload
-
-      aspect.posts.count.should == 1
-      aspect1.posts.count.should == 0
-
-      @user.reload
-      @user.move_friend(:friend_id => @user2.person.id, :from => aspect.id, :to => aspect1.id)
-      aspect.reload
-      aspect1.reload
-
-      aspect1.posts.count.should == 1
-      aspect.posts.count.should == 0
+      user.reload
     end
+    
 
     describe "#add_person_to_aspect" do
       it 'adds the user to the aspect' do
-        aspect1.people.should_not include @user2.person
-        @user.add_person_to_aspect(@user2.person.id, aspect1.id)
+        aspect1.people.should_not include user2.person
+        user.add_person_to_aspect(user2.person.id, aspect1.id)
         aspect1.reload
-        aspect1.people.should include @user2.person
+        aspect1.people.should include user2.person
       end
 
       it 'raises if its an aspect that the user does not own'do
-        proc{@user.add_person_to_aspect(@user2.person.id, aspect2.id) }.should raise_error /Can not add person to an aspect you do not own/
+        proc{user.add_person_to_aspect(user2.person.id, aspect2.id) }.should raise_error /Can not add person to an aspect you do not own/
       end
 
       it 'does not allow to have duplicate people in an aspect' do
-        proc{@user.add_person_to_aspect(not_friend.id, aspect1.id) }.should raise_error /Can not add person you are not friends with/
+        proc{user.add_person_to_aspect(not_friend.id, aspect1.id) }.should raise_error /Can not add person you are not friends with/
       end
 
       it 'does not allow you to add a person if they are already in the aspect' do
-        proc{@user.add_person_to_aspect(@user2.person.id, aspect.id) }.should raise_error /Can not add person who is already in the aspect/
+        proc{user.add_person_to_aspect(user2.person.id, aspect.id) }.should raise_error /Can not add person who is already in the aspect/
       end
     end
 
     describe '#delete_person_from_aspect' do
       it 'deletes a user from the aspect' do
-         @user.add_person_to_aspect(@user2.person.id, aspect1.id)
-         @user.reload
-         @user.aspects.find_by_id(aspect1.id).people.include?(@user2.person).should be true
-         @user.delete_person_from_aspect(@user2.person.id, aspect1.id)
-         @user.reload
-         @user.aspects.find_by_id(aspect1.id).people.include?(@user2.person).should be false
+         user.add_person_to_aspect(user2.person.id, aspect1.id)
+         user.reload
+         user.aspects.find_by_id(aspect1.id).people.include?(user2.person).should be true
+         user.delete_person_from_aspect(user2.person.id, aspect1.id)
+         user.reload
+         user.aspects.find_by_id(aspect1.id).people.include?(user2.person).should be false
       end
 
       it 'should check to make sure you have the aspect ' do
-        proc{@user.delete_person_from_aspect(@user2.person.id, aspect2.id) }.should raise_error /Can not delete a person from an aspect you do not own/
+        proc{user.delete_person_from_aspect(user2.person.id, aspect2.id) }.should raise_error /Can not delete a person from an aspect you do not own/
+      end
+    end
+
+    context 'moving and removing posts' do
+
+      let(:message) { user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)}
+      let(:message2){user3.post(:status_message, :message => "other post", :to => aspect3.id)}
+
+      before do
+        friend_users(user, aspect, user3, aspect3)
+        user.receive message.to_diaspora_xml, user2.person
+        user.receive message2.to_diaspora_xml, user3.person
+        aspect.reload
+        @post_count  = aspect.posts.count
+        @post_count1 = aspect1.posts.count
+
+        user.reload
+      end
+      
+      it 'moves the persons posts into the new aspect' do
+        user.add_person_to_aspect(user2.person.id, aspect1.id, :posts => [message] )
+        aspect1.reload
+        aspect1.posts.should == [message]
       end
 
-        context 'removing posts' do
-          before do
-            friend_users(@user, aspect, user3, aspect3)
+      
+      it 'should remove the users posts from that aspect' do
+        user.delete_person_from_aspect(user2.person.id, aspect.id)
+        aspect.reload
+        aspect.posts.count.should == @post_count - 1
+      end
 
-            message = @user2.post(:status_message, :message => "Hey Dude", :to => aspect2.id)
-            @user.receive message.to_diaspora_xml, @user2.person
-            aspect.reload
-            aspect.posts.count.should == 1
-          end
+      it 'should not delete other peoples posts' do
+        user.delete_person_from_aspect(user2.person.id, aspect.id)
+        aspect.reload
+        aspect.posts.should == [message2]
+      end
 
-          it 'should remove the users posts from that aspect' do
-            @user.reload
-            @user.delete_person_from_aspect(@user2.person.id, aspect.id)
-            aspect.reload
-            aspect.posts.count.should == 0
-          end
+      describe '#move_friend' do
+        it 'should be able to move a friend from one of users existing aspects to another' do
+          user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect1.id)
+          aspect.reload
+          aspect1.reload
 
-          it 'should not delete other peoples posts' do
-            message2 = user3.post(:status_message, :message => "other post", :to => aspect3.id)
+          aspect.person_ids.include?(user2.person.id).should be false
+          aspect1.people.include?(user2.person).should be true
+        end
 
-            @user.receive message2.to_diaspora_xml, user3.person
+        it "should not move a person who is not a friend" do
+          proc{ user.move_friend(:friend_id => friend.id, :from => aspect.id, :to => aspect1.id) }.should raise_error /Can not add person you are not friends with/
+          aspect.reload
+          aspect1.reload
+          aspect.people.include?(friend).should be false
+          aspect1.people.include?(friend).should be false
+        end
 
-            aspect.reload
-            aspect.posts.count.should == 2
+        it "should not move a person to a aspect that's not his" do
+          proc {user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect2.id )}.should raise_error /Can not add person to an aspect you do not own/
+          aspect.reload
+          aspect2.reload
+          aspect.people.include?(user2.person).should be true
+          aspect2.people.include?(user2.person).should be false
+        end
 
-            @user.reload
-            @user.delete_person_from_aspect(@user2.person.id, aspect.id)
-            aspect.reload
-            aspect.posts.should == [message2]
-          end
+        it 'should move all posts by that user to the new aspect' do
+          user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect1.id)
+          aspect.reload
+          aspect1.reload
+
+          aspect1.posts.count.should == @post_count1 + 1
+          aspect.posts.count.should == @post_count - 1
+        end
+
+        it 'does not try to delete if add person did not go through' do
+          user.should_receive(:add_person_to_aspect).and_return(false)
+          user.should_not_receive(:delete_person_from_aspect)
+          user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect1.id)
         end
       end
+    end
   end
 end

From e6de6179e9c925166b8413b4ad2ee94378232295 Mon Sep 17 00:00:00 2001
From: ilya <ilya@laptop.(none)>
Date: Wed, 20 Oct 2010 11:47:09 -0700
Subject: [PATCH 8/9] MS IZ dependant destroy is back for aspects

---
 app/models/user.rb         |  2 +-
 spec/models/aspect_spec.rb | 32 ++++++++++++++------------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index dff616ef5..bf301bc10 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -60,7 +60,7 @@ class User
   many :visible_people, :in => :visible_person_ids, :class_name => 'Person' # One of these needs to go
   many :pending_requests, :in => :pending_request_ids, :class_name => 'Request'
   many :raw_visible_posts, :in => :visible_post_ids, :class_name => 'Post'
-  many :aspects, :class_name => 'Aspect'
+  many :aspects, :class_name => 'Aspect', :dependent => :destroy
 
   #after_create :seed_aspects
 
diff --git a/spec/models/aspect_spec.rb b/spec/models/aspect_spec.rb
index ef217042c..9e6d01200 100644
--- a/spec/models/aspect_spec.rb
+++ b/spec/models/aspect_spec.rb
@@ -46,47 +46,43 @@ describe Aspect do
 
   describe 'validation' do
     before do
-      @aspect = user.aspect(:name => 'losers')
+      aspect
     end
     it 'has a unique name for one user' do
-      aspect2 = user.aspect(:name => @aspect.name)
+      aspect2 = user.aspect(:name => aspect.name)
       aspect2.valid?.should be_false
     end
 
     it 'has no uniqueness between users' do
-      aspect2 = user2.aspect(:name => @aspect.name)
+      aspect2 = user2.aspect(:name => aspect.name)
       aspect2.valid?.should be_true
     end
   end
 
   describe 'querying' do
     before do
-      @aspect = user.aspect(:name => 'losers')
-      user.activate_friend(friend, @aspect)
-      @aspect2 = user2.aspect(:name => 'failures')
-      friend_users(user, @aspect, user2, @aspect2)
-      @aspect.reload
+      aspect
+      user.activate_friend(friend, aspect)
+      aspect2
+      friend_users(user, aspect, user2, aspect2)
+      aspect.reload
+      user.reload
     end
 
     it 'belong to a user' do
-<<<<<<< HEAD
-      @aspect.user.id.should == user.id
-      user.aspects.size.should == 3
-=======
-      @aspect.user.id.should == @user.id
-      @user.aspects.size.should == 1 
->>>>>>> 961510a8ed06590109a8090686355ffdcde71180
+      aspect.user.id.should == user.id
+      user.aspects.should == [aspect]
     end
 
     it 'should have people' do
-      @aspect.people.all.include?(friend).should be true
-      @aspect.people.size.should == 2
+      aspect.people.all.include?(friend).should be true
+      aspect.people.size.should == 2
     end
 
     it 'should be accessible through the user' do
       aspects = user.aspects_with_person(friend)
       aspects.size.should == 1
-      aspects.first.id.should == @aspect.id
+      aspects.first.id.should == aspect.id
       aspects.first.people.size.should == 2
       aspects.first.people.include?(friend).should be true
       aspects.first.people.include?(user2.person).should be true

From 9c8e514642751eda55acc2963b82a7a7a5c52788 Mon Sep 17 00:00:00 2001
From: Raphael <raphael@joindiaspora.com>
Date: Wed, 20 Oct 2010 12:15:13 -0700
Subject: [PATCH 9/9] Adding a spec for a mass-assignment attack through
 profile update

---
 spec/controllers/users_controller_spec.rb | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 502353f6a..f00a519db 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -24,17 +24,26 @@ describe UsersController do
       before do
         @user.person.profile.image_url = "http://tom.joindiaspora.com/images/user/tom.jpg"
         @user.person.profile.save
+        
+        @params = {"profile"=>
+          {"image_url"   => "",
+            "last_name"  => @user.person.profile.last_name,
+            "first_name" => @user.person.profile.first_name}}
       end
 
       it "doesn't overwrite the profile photo when an empty string is passed in" do
         image_url = @user.person.profile.image_url
-        put("update", :id => @user.id, "user"=> {"profile"=>
-          {"image_url"   => "",
-            "last_name"  => @user.person.profile.last_name,
-            "first_name" => @user.person.profile.first_name}})
+        put("update", :id => @user.id, "user" => @params)
 
         @user.person.profile.image_url.should == image_url
       end
+      it "doesn't overwrite random attributes" do
+        new_user = Factory.create(:user)
+        @params[:owner_id] = new_user.id
+        person = @user.person
+        put('update', :id => @user.id, "user" => @params)
+        Person.find(person.id).owner_id.should == @user.id
+      end
     end
 
     context 'should allow the user to update their password' do