initiating a request now just creates a pending contact instead of persisting a request

app/controllers/application_controller.rb

@@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base
   end
 
   def count_requests
-    @request_count = current_user.requests_for_me.count if current_user
+    @request_count = current_user.pending_requests.count if current_user
   end
 
   def set_invites

app/controllers/people_controller.rb

@@ -22,6 +22,7 @@ class PeopleController < ApplicationController
       end
     end
   end
+
   def hashes_for_people people, aspects
     ids = people.map{|p| p.id}
     requests = {}

app/controllers/requests_controller.rb

@@ -36,15 +36,18 @@ class RequestsController < ApplicationController
      current_user.accept_and_respond(existing_request.id, aspect.id)
      redirect_to :back
    else
-     @request = Request.instantiate(:to => person,
+
-                                    :from => current_user.person,
+     @contact = Contact.new(:user => current_user,
-                                    :into => aspect)
+                            :person => person,
-     if @request.save
+                            :aspect_ids => [aspect.id],
-       current_user.dispatch_request(@request)
+                            :pending => true)
+
+     if @contact.save
+       @contact.dispatch_request
        flash.now[:notice] = I18n.t('requests.create.sent')
        redirect_to :back
      else
-       flash.now[:error] = @request.errors.full_messages.join(', ')
+       flash.now[:error] = @contact.errors.full_messages.join(', ')
        redirect_to :back
      end
    end

app/mailers/notifier.rb

@@ -33,10 +33,9 @@ class Notifier < ActionMailer::Base
          :subject => I18n.t('notifier.new_request.subject', :from => @sender.name), :host => APP_CONFIG[:pod_uri].host)
   end
 
-  def request_accepted(recipient_id, sender_id, aspect_id)
+  def request_accepted(recipient_id, sender_id)
     @receiver = User.find_by_id(recipient_id)
     @sender = Person.find_by_id(sender_id)
-    @aspect = Aspect.find_by_id(aspect_id)
 
     log_mail(recipient_id, sender_id, 'request_accepted')
 

app/models/aspect.rb

@@ -6,12 +6,10 @@ class Aspect
   include MongoMapper::Document
 
   key :name,        String
-  key :request_ids, Array
   key :post_ids,    Array
 
   many :contacts, :foreign_key => 'aspect_ids', :class_name => 'Contact'
-  many :requests, :in => :request_ids, :class_name => 'Request'
+  many :posts,    :in => :post_ids, :class_name => 'Post'
-  many :posts,    :in => :post_ids,    :class_name => 'Post'
 
   belongs_to :user, :class_name => 'User'
 

app/models/contact.rb

@@ -5,13 +5,36 @@
 class Contact
   include MongoMapper::Document
 
+  key :pending, Boolean
+
+  key :user_id, ObjectId
   belongs_to :user
   validates_presence_of :user
 
+  key :person_id, ObjectId
   belongs_to :person
   validates_presence_of :person
+  validates_uniqueness_of :person_id, :scope => :user_id
+
+  validate :not_contact_for_self
 
   key :aspect_ids, Array, :typecast => 'ObjectId'  
   many :aspects, :in => :aspect_ids, :class_name => 'Aspect'
- 
+
+  def dispatch_request
+    request = self.generate_request
+    self.user.push_to_people(request, [self.person])
+    request
+  end
+
+  def generate_request
+    Request.new(:from => self.user, :to => self.person)
+  end
+
+  private
+  def not_contact_for_self
+    if person.owner_id == user.id
+      errors[:base] << 'Cannot create self-contact'
+    end
+  end
 end

app/models/jobs/mail_request_acceptance.rb

@@ -1,8 +1,8 @@
 module Jobs
   class MailRequestAcceptance
     @queue = :mail
-    def self.perform(recipient_id, sender_id, aspect_id)
+    def self.perform(recipient_id, sender_id)
-      Notifier.request_accepted(recipient_id, sender_id, aspect_id).deliver
+      Notifier.request_accepted(recipient_id, sender_id).deliver
     end
   end
 end

app/models/request.rb

@@ -2,9 +2,9 @@
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
-class Request  
+class Request
   require File.join(Rails.root, 'lib/diaspora/webhooks')
-  
+
   include MongoMapper::Document
   include Diaspora::Webhooks
   include ROXML
@@ -22,12 +22,12 @@ class Request
   validate :not_already_connected_if_not_sent
   validate :no_pending_request, :if => :sent
   validate :not_friending_yourself
-  
+
   scope :from, lambda { |person| 
     target = (person.is_a?(User) ? person.person : person)
     where(:from_id => target.id)
   }
-  
+
   scope :to, lambda { |person| 
     target = (person.is_a?(User) ? person.person : person)
     where(:to_id => target.id)
@@ -59,7 +59,7 @@ class Request
   def recipient_handle
     to.diaspora_handle
   end
-  
+
   def recipient_handle= recipient_handle
     self.to = Person.first(:diaspora_handle => recipient_handle)
   end

app/views/notifier/request_accepted.html.haml

@@ -3,8 +3,6 @@
 %p
   = "#{@sender.name} (#{@sender.diaspora_handle})"
   = t('.accepted')
-  = link_to @aspect.name, aspect_url(@aspect)
-  = t('.aspect')
 
   %br
   = t('notifier.love') 

app/views/notifier/request_accepted.text.haml

@@ -1,8 +1,6 @@
 = t('notifier.hello', :name => @receiver.profile.first_name)
 = "#{@sender.name} (#{@sender.diaspora_handle})"
 = t('notifier.request_accepted.accepted')
-= "#{@aspect.name} #{t('notifier.request_accepted.aspect')}\n" 
-= "#{aspect_url(@aspect)}"
 
 
 = "#{t('notifier.love')} \n"

config/locales/diaspora/en.yml

@@ -53,6 +53,10 @@ en:
                   attributes:
                       diaspora_handle:
                           taken: "is already taken"
+              contact:
+                  attributes:
+                      person_id:
+                          taken: "must be unique among this user's contacts"
   application:
       helper:
           unknown_person: "unknown person"
@@ -402,8 +406,7 @@ en:
           sign_in: "sign in here"
       request_accepted:
           subject: "%{name} has accepted your contact request on Diaspora*"
-          accepted: "has accepted your contact request. They are now in your"
+          accepted: "has accepted your contact request!"
-          aspect: "aspect."
   home:
       show:
           share_what_you_want: "Share what you want, with whom you want."

features/support/env.rb

@@ -66,17 +66,6 @@ class FakeRedis
     true
   end
 end
-class User
-  def send_contact_request_to(desired_contact, aspect)
-    request = Request.instantiate(:to   => desired_contact,
-                                  :from => self.person,
-                                  :into => aspect)
-    if request.save!
-      dispatch_request request
-    end
-    request
-  end
-end
 
 Before do
   UserFixer.regenerate_user_fixtures

lib/diaspora/user/connecting.rb

@@ -6,21 +6,17 @@ module Diaspora
   module UserModules
     module Connecting
       def send_contact_request_to(desired_contact, aspect)
-        request = Request.instantiate(:to => desired_contact,
+        contact = Contact.new(:person => desired_contact,
-                                      :from => self.person,
+                              :user => self,
-                                      :into => aspect)
+                              :pending => true)
-        if request.save!
+        contact.aspects << aspect
-          dispatch_request request
-        end
-        request
-      end
-      def dispatch_request(request)
-        self.pending_requests << request
-        self.save
 
-        request.into.requests << request
+        if contact.save!
-        request.into.save
+          request = contact.dispatch_request
-        push_to_people request, [request.to]
+          request
+        else
+          nil
+        end
       end
 
       def accept_contact_request(request, aspect)
@@ -56,11 +52,11 @@ module Diaspora
       def receive_contact_request(contact_request)
 
         #response from a contact request you sent
-        if original_request = original_request(contact_request)
+        if original_contact = self.contact_for(contact_request.from)
-          receive_request_acceptance(contact_request, original_request)
+          receive_request_acceptance(contact_request, original_contact)
 
         #this is a new contact request
-        elsif !request_from_me?(contact_request)
+        elsif contact_request.from != self.person
           if contact_request.save!
             self.pending_requests << contact_request
             self.save!
@@ -74,16 +70,14 @@ module Diaspora
         contact_request
       end
 
-      def receive_request_acceptance(received_request, sent_request)
+      def receive_request_acceptance(received_request, contact)
-        destination_aspect = self.aspect_by_id(sent_request.into_id)
+        contact.pending = false
-        activate_contact(received_request.from, destination_aspect)
+        contact.save
         Rails.logger.info("event=contact_request status=received_acceptance from=#{received_request.from.diaspora_handle} to=#{self.diaspora_handle}")
 
         received_request.destroy
-        pending_requests.delete(sent_request)
-        sent_request.destroy
         self.save
-        self.mail(Jobs::MailRequestAcceptance, self.id, received_request.from.id, destination_aspect.id)
+        self.mail(Jobs::MailRequestAcceptance, self.id, received_request.from.id)
       end
 
       def disconnect(bad_contact)
@@ -129,14 +123,6 @@ module Diaspora
         aspect.save!
       end
 
-      def request_from_me?(request)
-        request.from == self.person
-      end
-
-      def original_request(response)
-        pending_requests.first(:from_id => self.person.id, :to_id => response.from.id)
-      end
-
       def requests_for_me
         pending_requests.select { |req| req.to == self.person }
       end

spec/controllers/invitations_controller_spec.rb

@@ -11,22 +11,9 @@ describe InvitationsController do
 
   let!(:user) {make_user}
   let!(:aspect){user.aspects.create(:name => "WIN!!")}
- 
+
   before do
     request.env["devise.mapping"] = Devise.mappings[:user]
-    module Resque
-      def enqueue(mod, *args)
-        mod.send(:perform, *args)
-      end
-    end
-  end
-
-  after do
-    module Resque
-      def enqueue(mod, *args)
-        true
-      end
-    end
   end
 
   describe "#create" do

spec/controllers/people_controller_spec.rb

@@ -44,14 +44,14 @@ describe PeopleController do
       hash = @hashes[4]
       hash[:person].should == @people[4]
       hash[:contact].should be_true
-      hash[:request].should be_false
+      hash[:contact].should_not be_pending
       hash[:aspects].should == user.aspects
     end
     it 'has the correct result for a requested person' do
       hash = @hashes[2]
       hash[:person].should == @people[2]
-      hash[:contact].should be_false
+      hash[:contact].should be_true
-      hash[:request].should be_true
+      hash[:contact].should be_pending
       hash[:aspects].should == user.aspects
     end
   end

spec/controllers/publics_controller_spec.rb

@@ -13,10 +13,6 @@ describe PublicsController do
   describe '#receive' do
     let(:xml) { "<walruses></walruses>" }
 
-    before do
-      Jobs::ReceiveSalmon.stub!(:perform)
-    end
-
     it 'succeeds' do
       post :receive, "id" => user.person.id.to_s, "xml" => xml
       response.should be_success

spec/controllers/requests_controller_spec.rb

@@ -56,7 +56,7 @@ describe RequestsController do
         lambda {
           post :create, @params
         }.should change(Contact,:count).by(1)
-        new_contact = @user.reload.contact_for(@other_user)
+        new_contact = @user.reload.contact_for(@other_user.person)
         new_contact.should_not be_nil
         new_contact.should be_pending
       end

spec/helper_methods.rb

@@ -8,20 +8,6 @@ module HelperMethods
     Mocha::Mockery.instance.stubba.unstub_all
   end
 
-  def get_models
-    models = []
-    Dir.glob( File.dirname(__FILE__) + '/../app/models/*' ).each do |f|
-      models << File.basename( f ).gsub( /^(.+).rb/, '\1')
-    end
-    models
-  end
-
-  def stub_user_message_handle_methods(user)
-    user.stub!(:push_to_people)
-    user.stub!(:push_to_hub)
-    user.stub!(:push_to_person)
-  end
-
   def fantasy_resque
     former_value = $process_queue
     $process_queue = true

spec/mailers/notifier_spec.rb

@@ -4,7 +4,6 @@ require 'spec_helper'
 describe Notifier do
 
   let!(:user) {make_user}
-  let!(:aspect) {user.aspects.create(:name => "science")}
   let!(:person) {Factory.create :person}
 
   before do
@@ -64,8 +63,8 @@ describe Notifier do
     end
   end
 
-  describe "#request_accpeted" do
+  describe "#request_accepted" do
-    let!(:request_accepted_mail) {Notifier.request_accepted(user.id, person.id, aspect.id)}
+    let!(:request_accepted_mail) {Notifier.request_accepted(user.id, person.id)}
     it 'goes to the right person' do
       request_accepted_mail.to.should == [user.email]
     end
@@ -74,13 +73,8 @@ describe Notifier do
       request_accepted_mail.body.encoded.include?(user.person.profile.first_name).should be true
     end
 
-
     it 'has the name of person sending the request' do
       request_accepted_mail.body.encoded.include?(person.name).should be true
     end
-
-    it 'has the name of the aspect in the body' do
-      request_accepted_mail.body.encoded.include?(aspect.name).should be true
-    end
   end
 end

spec/models/contact_spec.rb

@@ -7,19 +7,77 @@ require 'spec_helper'
 describe Contact do
   describe 'validations' do
     let(:contact){Contact.new}
-    
+
     it 'requires a user' do
       contact.valid?
       contact.errors.full_messages.should include "User can't be blank"
     end
-    
+
     it 'requires a person' do
       contact.valid?
       contact.errors.full_messages.should include "Person can't be blank"
     end
 
+    it 'ensures user is not making a contact for himself' do
+      user = make_user
+
+      contact.person = user.person
+      contact.user = user
+
+      contact.valid?
+      contact.errors.full_messages.should include "Cannot create self-contact"
+    end
+
     it 'has many aspects' do
       contact.associations[:aspects].type.should == :many
     end
+
+    it 'validates uniqueness' do
+      user = make_user
+      person = Factory(:person)
+
+      contact2 = Contact.create(:user => user,
+                                :person => person)
+
+      contact2.should be_valid
+
+      contact.user = user
+      contact.person = person
+      contact.should_not be_valid
+    end
+  end
+
+
+  context 'requesting' do
+    before do
+      @contact = Contact.new
+      @user = make_user
+      @person = Factory(:person)
+
+      @contact.user = @user
+      @contact.person = @person
+    end
+
+    describe '#generate_request' do
+      it 'makes a request' do
+        @contact.stub(:user).and_return(@user)
+        request = @contact.generate_request
+
+        request.from.should == @user
+        request.to.should == @person
+      end
+    end
+
+    describe '#dispatch_request' do
+      it 'pushes to people' do
+        @contact.stub(:user).and_return(@user)
+        @user.should_receive(:push_to_people).with(anything, [@contact.person])
+        @contact.dispatch_request
+      end
+      it 'persists no request' do
+        @contact.dispatch_request
+        Request.from(@user).to(@person).first.should be_nil
+      end
+    end
   end
 end

spec/models/invitation_spec.rb

@@ -250,7 +250,13 @@ describe Invitation do
     it 'creates a request, and sends it to the new user' do
       lambda {
         @invitation.to_request!
-      }.should change(Request, :count).by(2)
+      }.should change(Request, :count).by(1)
+    end
+    it 'creates a pending contact for the inviter' do
+      lambda {
+        @invitation.to_request!
+      }.should change(Contact, :count).by(1)
+      @invitation.from.contact_for(@new_user.person).should be_pending
     end
     describe 'return values' do
       before do

spec/models/request_spec.rb

@@ -79,16 +79,6 @@ describe Request do
     end
   end
 
-  describe '#request_from_me' do
-    it 'recognizes requests from me' do
-      user.request_from_me?(request).should be_true
-    end
-
-    it 'recognized when a request is not from me' do
-      user2.request_from_me?(request).should be_false
-    end
-  end
-
   context 'quering request through user' do
     it 'finds requests for that user' do
       request
@@ -97,17 +87,6 @@ describe Request do
     end
   end
 
-  describe '#original_request' do
-    it 'returns nil on a request from me' do
-      request
-      user.original_request(request).should be_nil
-    end
-    it 'returns the original request on a response to a request from me' do
-      new_request = request.reverse_for(user2)
-      user.original_request(new_request).should == request
-    end
-  end
-
   describe '.hashes_for_person' do
     before do
       @user = make_user

spec/models/user/connecting_spec.rb

@@ -18,13 +18,30 @@ describe Diaspora::UserModules::Connecting do
   let(:aspect2) { user2.aspects.create(:name => "aspect two") }
 
   describe '#send_contact_request_to' do
-    it "should assign a request to a aspect for the user that sent it out" do
+    it 'should not be able to contact request an existing contact' do
-      aspect.requests.size.should == 0
+      user.activate_contact(user2.person, aspect1)
 
-      user.send_contact_request_to(person, aspect)
+      proc {
+        user.send_contact_request_to(user2.person, aspect1)
+      }.should raise_error(MongoMapper::DocumentNotValid)
+    end
 
-      aspect.reload
+    it 'should not be able to contact request no-one' do
-      aspect.requests.size.should == 1
+      proc {
+        user.send_contact_request_to(nil, aspect) 
+      }.should raise_error(MongoMapper::DocumentNotValid)
+    end
+    it 'creates a pending contact' do
+      proc {
+        user.send_contact_request_to(user2.person, aspect1)
+      }.should change(Contact, :count).by(1)
+      user.contact_for(user2.person).pending.should == true
+      user.contact_for(user2.person).should be_pending
+    end
+    it 'persists no request for requester' do
+      proc {
+        user.send_contact_request_to(user2.person, aspect1)
+      }.should_not change{user.reload.pending_requests.count}
     end
   end
 
@@ -65,7 +82,7 @@ describe Diaspora::UserModules::Connecting do
         Request.find(@acceptance.id).should be_nil
       end
       it 'enqueues a mail job' do
-        Resque.should_receive(:enqueue).with(Jobs::MailRequestAcceptance, user.id, user2.person.id, aspect.id).once
+        Resque.should_receive(:enqueue).with(Jobs::MailRequestAcceptance, user.id, user2.person.id).once
         user.receive_request(@acceptance, user2.person)
       end
     end
@@ -107,17 +124,6 @@ describe Diaspora::UserModules::Connecting do
       end
     end
 
-    it 'should not be able to contact request an existing contact' do
-      connect_users(user, aspect, user2, aspect2)
-      proc { user.send_contact_request_to(user2.person, aspect1) 
-      }.should raise_error(MongoMapper::DocumentNotValid, /already connected/)
-    end
-    
-    it 'should not be able to contact request no-one' do
-      proc { user.send_contact_request_to(nil, aspect) 
-      }.should raise_error(MongoMapper::DocumentNotValid)
-    end
-
     describe 'multiple users accepting/rejecting the same person' do
 
       before do

spec/models/user/invite_spec.rb

@@ -34,7 +34,7 @@ describe User do
     it 'throws if you try to add someone you"re connected to' do
       connect_users(inviter, aspect, another_user, wrong_aspect)
       inviter.reload
-      proc{inviter.invite_user(another_user.email, aspect.id)}.should raise_error /already connected/
+      proc{inviter.invite_user(another_user.email, aspect.id)}.should raise_error MongoMapper::DocumentNotValid
     end
 
   end
@@ -75,7 +75,6 @@ describe User do
 
       it 'resolves incoming invitations into contact requests' do
         invited_user.reload.pending_requests.count.should == 1
-        inviter.reload.pending_requests.count.should == 1
       end
 
       context 'after request acceptance' do

spec/spec_helper.rb

@@ -68,13 +68,16 @@ ImageUploader.enable_processing = false
 class User
   def send_contact_request_to(desired_contact, aspect)
     fantasy_resque do
-      request = Request.instantiate(:to => desired_contact,
+      contact = Contact.new(:person => desired_contact,
-                                    :from => self.person,
+                            :user => self,
-                                    :into => aspect)
+                            :pending => true)
-      if request.save!
+      contact.aspects << aspect
-        dispatch_request request
+
+      if contact.save!
+        contact.dispatch_request
+      else
+        nil
       end
-      request
     end
   end