Merge branch 'new_public_receive'

Gemfile.lock

@@ -241,6 +241,8 @@ GEM
       spruz (~> 0.2.8)
     jwt (0.1.3)
       json (>= 1.2.4)
+    kaminari (0.12.4)
+      rails (>= 3.0.0)
     linecache (0.43)
     linecache19 (0.5.12)
       ruby_core_source (>= 0.1.4)
@@ -504,6 +506,7 @@ DEPENDENCIES
   jasmine (= 1.1.0.rc3)
   json (= 1.4.6)
   jwt (= 0.1.3)
+  kaminari
   linecache (= 0.43)
   mini_magick (= 3.2)
   mobile-fu

lib/diaspora/relayable.rb

@@ -18,10 +18,12 @@ module Diaspora
       end
     end
 
+    # @return [Boolean] true
     def relayable?
       true
     end
 
+    # @return [String]
     def parent_guid
       self.parent.guid
     end
@@ -30,15 +32,19 @@ module Diaspora
       self.parent = parent_class.where(:guid => new_parent_guid).first
     end
 
+    # @return [Array<Person>]
     def subscribers(user)
       if user.owns?(self.parent)
         self.parent.subscribers(user)
       elsif user.owns?(self)
         [self.parent.author]
+      else
+        []
       end
     end
 
     def receive(user, person)
+
       self.class.transaction do
         comment_or_like = self.class.where(:guid => self.guid).first || self
 
@@ -51,23 +57,24 @@ module Diaspora
         #as the owner of the post being liked or commented on, you need to add your own signature in order to pass it to the people who received your original post
         if user.owns? comment_or_like.parent
           comment_or_like.parent_author_signature = comment_or_like.sign_with_key(user.encryption_key)
-
           comment_or_like.save!
         end
 
         #dispatch object DOWNSTREAM, received it via UPSTREAM
         unless user.owns?(comment_or_like)
-          comment_or_like.save 
+          comment_or_like.save!
           Postzord::Dispatcher.new(user, comment_or_like).post
         end
 
         comment_or_like.socket_to_user(user) if comment_or_like.respond_to? :socket_to_user
+
         if comment_or_like.after_receive(user, person)
           comment_or_like 
         end
       end
     end
 
+    # @return [Object]
     def after_receive(user, person)
       self
     end

lib/diaspora/webhooks.rb

@@ -1,4 +1,4 @@
-#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   Copyright (c) 2011, Diaspora Inc.  This file is
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
@@ -7,9 +7,11 @@ module Diaspora
     require 'builder/xchar'
 
     def to_diaspora_xml
-      xml = "<XML>"
+      <<XML
-      xml += "<post>#{to_xml.to_s}</post>"
+      <XML>
-      xml += "</XML>"
+      <post>#{to_xml.to_s}</post>
+      </XML>
+XML
     end
 
     def x(input)
@@ -17,16 +19,19 @@ module Diaspora
     end
 
     # @abstract
+    # @note this must return [Array<Person>]
+    # @return [Array<Person>]
     def subscribers(user)
-      raise 'you must override subscribers in order to enable federation on this model'
+      raise 'You must override subscribers in order to enable federation on this model'
     end
 
     # @abstract
     def receive(user, person)
-      raise 'you must override receive in order to enable federation on this model'
+      raise 'You must override receive in order to enable federation on this model'
     end
 
     # @param [User] sender
+    # @note this is a hook
     def after_dispatch sender
     end
   end

lib/postzord/dispatcher.rb

@@ -21,6 +21,5 @@ class Postzord::Dispatcher
      @zord =  Postzord::Dispatcher::Private.new(user, object, opts)
     #end
   end
-
 end
 

lib/postzord/receiver/private.rb

@@ -30,9 +30,13 @@ module Postzord
 
       def parse_and_receive(xml)
         @object ||= Diaspora::Parser.from_xml(xml)
+
         Rails.logger.info("event=receive status=start recipient=#{@user_person.diaspora_handle} payload_type=#{@object.class} sender=#{@sender.diaspora_handle}")
+
         if self.validate_object
           receive_object
+        else
+          raise 'not a valid object'
         end
       end
 

spec/controllers/comments_controller_spec.rb

@@ -6,9 +6,6 @@ require 'spec_helper'
 
 describe CommentsController do
   before do
-    @aspect1 = alice.aspects.where(:name => "generic").first
-    @aspect2 = bob.aspects.where(:name => "generic").first
-
     @controller.stub(:current_user).and_return(alice)
     sign_in :user, alice
   end
@@ -21,7 +18,8 @@ describe CommentsController do
 
     context "on my own post" do
       before do
-        @post = alice.post :status_message, :text => 'GIANTS', :to => @aspect1.id
+        aspect_to_post = alice.aspects.where(:name => "generic").first
+        @post = alice.post :status_message, :text => 'GIANTS', :to => aspect_to_post
       end
 
       it 'responds to format js' do
@@ -37,7 +35,8 @@ describe CommentsController do
 
     context "on a post from a contact" do
       before do
-        @post = bob.post :status_message, :text => 'GIANTS', :to => @aspect2.id
+        aspect_to_post = bob.aspects.where(:name => "generic").first
+        @post = bob.post :status_message, :text => 'GIANTS', :to => aspect_to_post
       end
 
       it 'comments' do
@@ -60,61 +59,64 @@ describe CommentsController do
       end
     end
 
-    context 'on a post from a stranger' do
+    it 'posts no comment on a post from a stranger' do
-      before do
+      aspect_to_post = eve.aspects.where(:name => "generic").first
-        @post = eve.post :status_message, :text => 'GIANTS', :to => eve.aspects.first.id
+      @post = eve.post :status_message, :text => 'GIANTS', :to => aspect_to_post
-      end
 
-      it 'posts no comment' do
       alice.should_not_receive(:comment)
       post :create, comment_hash
       response.code.should == '422'
     end
   end
-  end
 
   describe '#destroy' do
+    before do
+      aspect_to_post = bob.aspects.where(:name => "generic").first
+      @message = bob.post(:status_message, :text => "hey", :to => aspect_to_post)
+    end
+
     context 'your post' do
       before do
-        @message = alice.post(:status_message, :text => "hey", :to => @aspect1.id)
+        @controller.stub(:current_user).and_return(bob)
-        @comment = alice.comment("hey", :post => @message)
+        sign_in :user, bob
-        @comment2 = bob.comment("hey", :post => @message)
-        @comment3 = eve.comment("hey", :post => @message)
       end
 
       it 'lets the user delete his comment' do
-        alice.should_receive(:retract).with(@comment)
+        comment = bob.comment("hey", :post => @message)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment.id
+
+        bob.should_receive(:retract).with(comment)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment.id
         response.status.should == 204
       end
 
       it "lets the user destroy other people's comments" do
-        alice.should_receive(:retract).with(@comment2)
+        comment = alice.comment("hey", :post => @message)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment2.id
+
+        bob.should_receive(:retract).with(comment)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment.id
         response.status.should == 204
       end
     end
 
     context "another user's post" do
-      before do
-        @message = bob.post(:status_message, :text => "hey", :to => bob.aspects.first.id)
-        @comment = alice.comment("hey", :post => @message)
-        @comment2 = bob.comment("hey", :post => @message)
-        @comment3 = eve.comment("hey", :post => @message)
-      end
-
       it 'let the user delete his comment' do
-        alice.should_receive(:retract).with(@comment)
+        comment = alice.comment("hey", :post => @message)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment.id
+
+        alice.should_receive(:retract).with(comment)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment.id
         response.status.should == 204
       end
 
       it 'does not let the user destroy comments he does not own' do
-        alice.should_not_receive(:retract).with(@comment2)
+        comment1 = bob.comment("hey", :post => @message)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment3.id
+        comment2 = eve.comment("hey", :post => @message)
+
+        alice.should_not_receive(:retract).with(comment1)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment2.id
         response.status.should == 403
       end
     end
+
     it 'renders nothing and 404 on a nonexistent comment' do
       delete :destroy, :post_id => 1, :id => 343415
       response.status.should == 404
@@ -126,7 +128,6 @@ describe CommentsController do
     before do
       aspect_to_post = bob.aspects.where(:name => "generic").first
       @message = bob.post(:status_message, :text => "hey", :to => aspect_to_post.id)
-      @comments = [alice, bob, eve].map{ |u| u.comment("hey", :post => @message) }
     end
 
     it 'generates a jasmine fixture', :fixture => true do
@@ -142,9 +143,12 @@ describe CommentsController do
     end
 
     it 'returns all the comments for a post' do
+      comments = [alice, bob, eve].map{ |u| u.comment("hey", :post => @message) }
+
       get :index, :post_id => @message.id, :format => 'js'
-      assigns[:comments].should == @comments
+      assigns[:comments].should == comments
     end
+
     it 'returns a 404 on a nonexistent post' do
       get :index, :post_id => 235236, :format => 'js'
       response.status.should == 404

spec/integration/attack_vectors_spec.rb

@@ -6,221 +6,215 @@ require 'spec_helper'
 
 describe "attack vectors" do
 
-  let(:user) { Factory.create(:user_with_aspect) }
+  let(:eves_aspect) { eve.aspects.find_by_name("generic") }
-  let(:aspect) { user.aspects.first }
+  let(:alices_aspect) { alice.aspects.find_by_name("generic") }
-
-  let(:bad_user) { Factory.create(:user)}
-
-  let(:user2) { eve }
-  let(:aspect2) { user2.aspects.first }
-
-  let(:user3) { Factory.create(:user) }
-  let(:aspect3) { user3.aspects.create(:name => 'heroes') }
 
   context 'non-contact valid user' do
-
     it 'does not save a post from a non-contact' do
+      bad_user = Factory(:user)
+
       post_from_non_contact = bad_user.build_post( :status_message, :text => 'hi')
-      salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(user.person)
+      salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(bob.person)
 
       post_from_non_contact.delete
       bad_user.delete
       post_count = Post.count
 
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
         zord.perform
+      }.should raise_error /not a valid object/
 
-      user.visible_posts.include?(post_from_non_contact).should be_false
+      bob.visible_posts.include?(post_from_non_contact).should be_false
       Post.count.should == post_count
     end
-
   end
 
   it 'does not let a user attach to posts previously in the db unless its received from the author' do
-    connect_users(user, aspect, user3, aspect3)
+    original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
+    original_message.diaspora_handle = bob.diaspora_handle
 
-    original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+    alice.contacts.create(:person => eve.person, :aspects => [alice.aspects.first])
 
-    original_message.diaspora_handle = user.diaspora_handle
+    salmon_xml = bob.salmon(original_message).xml_for(alice.person)
-
+    zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
-    user3.contacts.create(:person => user2.person, :aspects => [user3.aspects.first])
+    expect {
-
-    salmon_xml = user.salmon(original_message).xml_for(user3.person)
-    zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
       zord.perform
+    }.should raise_error /not a valid object/
 
-    user3.reload.visible_posts.should_not include(StatusMessage.find(original_message.id))
+    alice.reload.visible_posts.should_not include(StatusMessage.find(original_message.id))
   end
 
   context 'malicious contact attack vector' do
-    before do
-      connect_users(user, aspect, user2, aspect2)
-      connect_users(user, aspect, user3, aspect3)
-    end
-
     describe 'mass assignment on id' do
       it "does not save a message over an old message with a different author" do
-        original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+        original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-        salmon_xml = user2.salmon(original_message).xml_for(user.person)
+        salmon_xml = eve.salmon(original_message).xml_for(bob.person)
 
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
-        malicious_message = Factory.build(:status_message, :id => original_message.id, :text => 'BAD!!!', :author => user3.person)
+        malicious_message = Factory.build(:status_message, :id => original_message.id, :text => 'BAD!!!', :author => alice.person)
-        salmon_xml = user3.salmon(malicious_message).xml_for(user.person)
+        salmon_xml = alice.salmon(malicious_message).xml_for(bob.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
         original_message.reload.text.should == "store this!"
       end
 
       it 'does not save a message over an old message with the same author' do
-        original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+        original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-        salmon_xml =  user2.salmon(original_message).xml_for(user.person)
+        salmon_xml =  eve.salmon(original_message).xml_for(bob.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
         lambda {
-          malicious_message = Factory.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => user2.person)
+          malicious_message = Factory.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => eve.person)
 
-          salmon_xml2 = user3.salmon(malicious_message).xml_for(user.person)
+          salmon_xml2 = alice.salmon(malicious_message).xml_for(bob.person)
-          zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+          zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
           zord.perform
 
-        }.should_not change{user.reload.visible_posts.count}
+        }.should_not change{
+          bob.reload.visible_posts.count
+        }
 
         original_message.reload.text.should == "store this!"
-        user.visible_posts.first.text.should == "store this!"
+        bob.visible_posts.first.text.should == "store this!"
       end
     end
+
     it 'should not overwrite another persons profile profile' do
-      profile = user2.profile.clone
+      profile = eve.profile.clone
       profile.first_name = "Not BOB"
 
-      user2.reload
+      eve.reload
 
-      first_name = user2.profile.first_name
+      first_name = eve.profile.first_name
-      salmon_xml = user3.salmon(profile).xml_for(user.person)
+      salmon_xml = alice.salmon(profile).xml_for(bob.person)
 
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
         zord.perform
+      }.should raise_error /not a valid object/
 
-      user2.reload
+      eve.reload.profile.first_name.should == first_name
-      user2.profile.first_name.should == first_name
     end
 
     it "ignores retractions on a post not owned by the retraction's sender" do
       StatusMessage.delete_all
-      original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+      original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-      salmon_xml = user2.salmon(original_message).xml_for(user.person)
+      salmon_xml = eve.salmon(original_message).xml_for(bob.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
-      user.visible_posts.count.should == 1
+      bob.visible_posts.count.should == 1
       StatusMessage.count.should == 1
 
       ret = Retraction.new
       ret.post_guid = original_message.guid
-      ret.diaspora_handle = user3.person.diaspora_handle
+      ret.diaspora_handle = alice.person.diaspora_handle
       ret.type = original_message.class.to_s
 
-      salmon_xml = user3.salmon(ret).xml_for(user.person)
+      salmon_xml = alice.salmon(ret).xml_for(bob.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
       StatusMessage.count.should == 1
-      user.visible_posts.count.should == 1
+      bob.visible_posts.count.should == 1
     end
 
     it "disregards retractions for non-existent posts that are from someone other than the post's author" do
       StatusMessage.delete_all
-      original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+      original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
       id = original_message.reload.id
 
       ret = Retraction.new
       ret.post_guid = original_message.guid
-      ret.diaspora_handle = user3.person.diaspora_handle
+      ret.diaspora_handle = alice.person.diaspora_handle
       ret.type = original_message.class.to_s
 
       original_message.delete
 
       StatusMessage.count.should == 0
       proc {
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
+        salmon_xml = alice.salmon(ret).xml_for(bob.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
       }.should_not raise_error
     end
 
     it 'should not receive retractions where the retractor and the salmon author do not match' do
-      original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+      original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-      salmon_xml = user2.salmon(original_message).xml_for(user.person)
+      salmon_xml = eve.salmon(original_message).xml_for(bob.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
-
+      bob.visible_posts.count.should == 1
-      user.visible_posts.count.should == 1
 
       ret = Retraction.new
       ret.post_guid = original_message.guid
-      ret.diaspora_handle = user2.person.diaspora_handle
+      ret.diaspora_handle = eve.person.diaspora_handle
       ret.type = original_message.class.to_s
 
-      lambda {
+      salmon_xml = alice.salmon(ret).xml_for(bob.person)
-
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
+      expect {
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
         zord.perform
+      }.should raise_error /not a valid object/
 
-      }.should_not change(StatusMessage, :count)
+      bob.reload.visible_posts.count.should == 1
-      user.reload.visible_posts.count.should == 1
     end
 
     it 'it should not allow you to send retractions for other people' do
       ret = Retraction.new
-      ret.post_guid = user2.person.guid
+      ret.post_guid = eve.person.guid
-      ret.diaspora_handle = user3.person.diaspora_handle
+      ret.diaspora_handle = alice.person.diaspora_handle
-      ret.type = user2.person.class.to_s
+      ret.type = eve.person.class.to_s
 
       proc{
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
+        salmon_xml = alice.salmon(ret).xml_for(bob.person)
 
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
-      }.should_not change{user.reload.contacts.count}
+      }.should_not change{bob.reload.contacts.count}
     end
 
     it 'it should not allow you to send retractions with xml and salmon handle mismatch' do
       ret = Retraction.new
-      ret.post_guid = user2.person.guid
+      ret.post_guid = eve.person.guid
-      ret.diaspora_handle = user2.person.diaspora_handle
+      ret.diaspora_handle = eve.person.diaspora_handle
-      ret.type = user2.person.class.to_s
+      ret.type = eve.person.class.to_s
 
-      proc{
+      bob.contacts.count.should == 2
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
+
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      salmon_xml = alice.salmon(ret).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
         zord.perform
-      }.should_not change{user.reload.contacts.count}
+      }.should raise_error /not a valid object/
+
+      bob.reload.contacts.count.should == 2
     end
 
     it 'does not let me update other persons post' do
-      original_message = user2.post(:photo, :user_file => uploaded_photo, :text => "store this!", :to => aspect2.id)
+      original_message = eve.post(:photo, :user_file => uploaded_photo, :text => "store this!", :to => eves_aspect.id)
 
-      salmon_xml = user2.salmon(original_message).xml_for(user.person)
+      salmon_xml = eve.salmon(original_message).xml_for(bob.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
-      original_message.diaspora_handle = user3.diaspora_handle
+      original_message.diaspora_handle = alice.diaspora_handle
       original_message.text= "bad bad bad"
 
-      salmon_xml = user3.salmon(original_message).xml_for(user.person)
+      salmon_xml = alice.salmon(original_message).xml_for(bob.person)
 
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
       original_message.reload.text.should == "store this!"

spec/mailers/notifier_spec.rb

@@ -3,24 +3,20 @@ require 'spec_helper'
 describe Notifier do
   include ActionView::Helpers::TextHelper
 
-  let!(:user) {alice}
+  let(:person) { Factory(:person) }
-  let!(:user2) {eve}
-
-  let!(:aspect) {user.aspects.create(:name => "win")}
-  let!(:aspect2) {user2.aspects.create(:name => "win")}
-  let!(:person) {Factory.create :person}
 
   before do
     Notifier.deliveries = []
   end
+
   describe '.administrative' do
     it 'mails a user' do
-      mails = Notifier.admin("Welcome to bureaucracy!", [user])
+      mails = Notifier.admin("Welcome to bureaucracy!", [bob])
       mails.length.should == 1
       mail = mails.first
-      mail.to.should == [user.email]
+      mail.to.should == [bob.email]
       mail.body.encoded.should match /Welcome to bureaucracy!/
-      mail.body.encoded.should match /#{user.username}/
+      mail.body.encoded.should match /#{bob.username}/
     end
     it 'mails a bunch of users' do
       users = []
@@ -39,23 +35,23 @@ describe Notifier do
 
   describe '.single_admin' do
     it 'mails a user' do
-      mail = Notifier.single_admin("Welcome to bureaucracy!", user)
+      mail = Notifier.single_admin("Welcome to bureaucracy!", bob)
-      mail.to.should == [user.email]
+      mail.to.should == [bob.email]
       mail.body.encoded.should match /Welcome to bureaucracy!/
-      mail.body.encoded.should match /#{user.username}/
+      mail.body.encoded.should match /#{bob.username}/
     end
 
     it 'has the layout' do
-
+      mail = Notifier.single_admin("Welcome to bureaucracy!", bob)
-      mail = Notifier.single_admin("Welcome to bureaucracy!", user)
       mail.body.encoded.should match /change your notification settings/
     end
   end
 
   describe ".started_sharing" do
-    let!(:request_mail) {Notifier.started_sharing(user.id, person.id)}
+    let!(:request_mail) {Notifier.started_sharing(bob.id, person.id)}
+
     it 'goes to the right person' do
-      request_mail.to.should == [user.email]
+      request_mail.to.should == [bob.email]
     end
 
     it 'has the name of person sending the request' do
@@ -168,11 +164,11 @@ describe Notifier do
 
       @cnv = Conversation.create(@create_hash)
 
-      @mail = Notifier.private_message(user.id, @cnv.author.id, @cnv.messages.first.id)
+      @mail = Notifier.private_message(bob.id, @cnv.author.id, @cnv.messages.first.id)
     end
 
     it 'TO: goes to the right person' do
-      @mail.to.should == [user.email]
+      @mail.to.should == [bob.email]
     end
 
     it "FROM: contains the sender's name" do
@@ -186,7 +182,7 @@ describe Notifier do
 
     it 'SUBJECT: has "Re:" if not the first message in a conversation' do
       @cnv.messages << Message.new(:text => 'yo', :author => eve.person)
-      @mail = Notifier.private_message(user.id, @cnv.author.id, @cnv.messages.last.id)
+      @mail = Notifier.private_message(bob.id, @cnv.author.id, @cnv.messages.last.id)
 
       @mail.subject.should == "Re: #{@cnv.subject}"
     end
@@ -201,15 +197,14 @@ describe Notifier do
   end
 
   context "comments" do
-    let(:connect) { connect_users(user, aspect, user2, aspect2)}
+    let(:commented_post) {bob.post(:status_message, :text => "It's really sunny outside today, and this is a super long status message!  #notreally", :to => :all)}
-    let(:commented_post) {user.post(:status_message, :text => "It's really sunny outside today, and this is a super long status message!  #notreally", :to => :all)}
+    let(:comment) { eve.comment("Totally is", :post => commented_post)}
-    let(:comment) { user2.comment("Totally is", :post => commented_post)}
 
     describe ".comment_on_post" do
-      let(:comment_mail) {Notifier.comment_on_post(user.id, person.id, comment.id).deliver}
+      let(:comment_mail) {Notifier.comment_on_post(bob.id, person.id, comment.id).deliver}
 
       it 'TO: goes to the right person' do
-        comment_mail.to.should == [user.email]
+        comment_mail.to.should == [bob.email]
       end
 
       it "FROM: contains the sender's name" do
@@ -237,7 +232,7 @@ describe Notifier do
 
       [:reshare, :activity_streams_photo].each do |post_type|
         context post_type.to_s do
-          let(:commented_post) { Factory(post_type, :author => user.person) }
+          let(:commented_post) { Factory(post_type, :author => bob.person) }
           it 'succeeds' do
             proc {
               comment_mail
@@ -248,10 +243,10 @@ describe Notifier do
     end
 
     describe ".also_commented" do
-      let(:comment_mail) {Notifier.also_commented(user.id, person.id, comment.id)}
+      let(:comment_mail) {Notifier.also_commented(bob.id, person.id, comment.id)}
 
       it 'TO: goes to the right person' do
-        comment_mail.to.should == [user.email]
+        comment_mail.to.should == [bob.email]
       end
 
       it 'FROM: has the name of person commenting as the sender' do
@@ -278,7 +273,7 @@ describe Notifier do
       end
       [:reshare, :activity_streams_photo].each do |post_type|
         context post_type.to_s do
-          let(:commented_post) { Factory(post_type, :author => user.person) }
+          let(:commented_post) { Factory(post_type, :author => bob.person) }
           it 'succeeds' do
             proc {
               comment_mail
@@ -290,29 +285,28 @@ describe Notifier do
 
     describe ".confirm_email" do
       before do
-        user.update_attribute(:unconfirmed_email, "my@newemail.com")
+        bob.update_attribute(:unconfirmed_email, "my@newemail.com")
+        @confirm_email = Notifier.confirm_email(bob.id)
       end
 
-      let!(:confirm_email) { Notifier.confirm_email(user.id) }
-
       it 'goes to the right person' do
-        confirm_email.to.should == [user.unconfirmed_email]
+        @confirm_email.to.should == [bob.unconfirmed_email]
       end
 
       it 'has the unconfirmed emil in the subject' do
-        confirm_email.subject.should include(user.unconfirmed_email)
+        @confirm_email.subject.should include(bob.unconfirmed_email)
       end
 
       it 'has the unconfirmed emil in the body' do
-        confirm_email.body.encoded.should include(user.unconfirmed_email)
+        @confirm_email.body.encoded.should include(bob.unconfirmed_email)
       end
 
       it 'has the receivers name in the body' do
-        confirm_email.body.encoded.should include(user.person.profile.first_name)
+        @confirm_email.body.encoded.should include(bob.person.profile.first_name)
       end
 
       it 'has the activation link in the body' do
-        confirm_email.body.encoded.should include(confirm_email_url(:token => user.confirm_email_token))
+        @confirm_email.body.encoded.should include(confirm_email_url(:token => bob.confirm_email_token))
       end
     end
   end

spec/models/comment_spec.rb

@@ -8,36 +8,32 @@ require File.join(Rails.root, "spec", "shared_behaviors", "relayable")
 describe Comment do
   before do
     @alices_aspect = alice.aspects.first
-    @bobs_aspect = bob.aspects.first
+    @status = bob.post(:status_message, :text => "hello", :to => bob.aspects.first.id)
-
-    @bob = bob
-    @eve = eve
-    @status = alice.post(:status_message, :text => "hello", :to => @alices_aspect.id)
   end
 
   describe 'comment#notification_type' do
     it "returns 'comment_on_post' if the comment is on a post you own" do
-      comment = bob.comment("why so formal?", :post => @status)
+      comment = alice.comment("why so formal?", :post => @status)
-      comment.notification_type(alice, bob.person).should == Notifications::CommentOnPost
+      comment.notification_type(bob, alice.person).should == Notifications::CommentOnPost
     end
 
     it 'returns false if the comment is not on a post you own and no one "also_commented"' do
       comment = alice.comment("I simply felt like issuing a greeting.  Do step off.", :post => @status)
-      comment.notification_type(@bob, alice.person).should == false
+      comment.notification_type(eve, alice.person).should be_false
     end
 
     context "also commented" do
       before do
-        @bob.comment("a-commenta commenta", :post => @status)
+        alice.comment("a-commenta commenta", :post => @status)
-        @comment = @eve.comment("I also commented on the first user's post", :post => @status)
+        @comment = eve.comment("I also commented on the first user's post", :post => @status)
       end
 
       it 'does not return also commented if the user commented' do
-        @comment.notification_type(@eve, alice.person).should == false
+        @comment.notification_type(eve, alice.person).should == false
       end
 
       it "returns 'also_commented' if another person commented on a post you commented on" do
-        @comment.notification_type(@bob, alice.person).should == Notifications::AlsoCommented
+        @comment.notification_type(alice, alice.person).should == Notifications::AlsoCommented
       end
     end
   end
@@ -103,6 +99,7 @@ describe Comment do
     end
   end
 
+  # NOTE(move this to the youtube module spec)
   describe 'youtube' do
     before do
       @message = alice.post :status_message, :text => "hi", :to => @alices_aspect.id