Refactor raises in requesting to validations in request, move to better controller structure

app/controllers/requests_controller.rb

@@ -32,39 +32,19 @@ class RequestsController < ApplicationController
   end
 
  def create
-    aspect = current_user.aspect_by_id(params[:aspect_id])
+    aspect = current_user.aspect_by_id(params[:request][:into])
-    account = params[:destination_handle].strip  
+    account = params[:request][:to].strip  
-    begin 
+    person = Person.by_account_identifier(account)
-      finger = EMWebfinger.new(account)
+    @request = Request.instantiate(:to => person,
-    
+                                   :from => current_user.person,
-      finger.on_person{ |person|
+                                   :into => aspect)
-      
+    if @request.save
-      if person.class == Person
+      current_user.dispatch_request(@request)
-        rel_hash = {:person => person}
+      flash.now[:notice] = I18n.t('requests.create.sent')
-
+      redirect_to :back
-        Rails.logger.debug("Sending request: #{rel_hash}")
-
-        begin
-          @request = current_user.send_contact_request_to(rel_hash[:person], aspect)
-        rescue Exception => e
-          Rails.logger.debug("error: #{e.message}")
-          flash[:error] = e.message
-        end
-      else
-        #socket to tell people this failed?
-      end
-      }
-
-    rescue Exception => e 
-      flash[:error] = e.message
-    end
-    
-    if params[:getting_started]
-      redirect_to getting_started_path(:step=>params[:getting_started])
     else
-      flash[:notice] = I18n.t('requests.create.tried', :account => account) unless flash[:error]
+      flash.now[:error] = @request.errors.full_messages.join(', ')
-      respond_with :location => aspects_manage_path 
+      redirect_to :back
-      return
+    end
-    end    
   end
 end

app/models/request.rb

@@ -16,14 +16,19 @@ class Request
   belongs_to :into, :class => Aspect
   belongs_to :from, :class => Person
   belongs_to :to,   :class => Person
+  key :sent,                  Boolean, :default => false
 
   validates_presence_of :from, :to
+  validate :not_already_connected, :if => :sent
+  validate :no_pending_request, :if => :sent
+  
   #before_validation :clean_link
 
   def self.instantiate(opts = {})
     self.new(:from => opts[:from],
              :to   => opts[:to],
-             :into => opts[:into])
+             :into => opts[:into],
+             :sent => true)
   end
 
   def reverse_for accepting_user
@@ -68,4 +73,16 @@ class Request
     self.from.diaspora_handle
   end
 
+  private
+  def no_pending_request
+    if Request.first(:from_id => from_id, :to_id => to_id)
+      errors[:base] << 'You have already sent a request to that person'
+    end
+  end
+
+  def not_already_connected
+    if Contact.first(:user_id => self.from.owner_id, :person_id => self.to_id)
+      errors[:base] << 'You have already connected to this person!'
+    end
+  end
 end

lib/diaspora/user/connecting.rb

@@ -6,30 +6,27 @@ module Diaspora
   module UserModules
     module Connecting
       def send_contact_request_to(desired_contact, aspect)
-        # should have different exception types for these?
+        request = Request.instantiate(:to => desired_contact, 
-        raise "You cannot connect yourself" if desired_contact.nil?
+                                      :from => self.person,
-        raise "You have already sent a contact request to that person!" if self.pending_requests.detect {
+                                      :into => aspect)
-          |x| x.to == desired_contact }
+        if request.save!
-        raise "You are already connected to that person!" if contact_for desired_contact
+          dispatch_request request
-        request = Request.instantiate(
-          :to   => desired_contact,
-          :from => self.person,
-          :into => aspect)
-        if request.save
-          self.pending_requests << request
-          self.save
-
-          aspect.requests << request
-          aspect.save
-          push_to_people request, [desired_contact]
         end
         request
       end
 
+      def dispatch_request(request)
+        self.pending_requests << request
+        self.save
+
+        request.into.requests << request
+        request.into.save
+        push_to_people request, [request.to]
+      end
+
       def accept_contact_request(request, aspect)
         pending_request_ids.delete(request.id.to_id)
         activate_contact(request.from, aspect)
-
         request.reverse_for(self)
       end
 

spec/models/request_spec.rb

@@ -13,7 +13,7 @@ describe Request do
 
   describe 'validations' do
     before do
-      @request = Request.new(:from => user.person, :to => user2.person, :into => aspect) 
+      @request = Request.instantiate(:from => user.person, :to => user2.person, :into => aspect) 
     end
     it 'is valid' do
       @request.should be_valid
@@ -33,6 +33,14 @@ describe Request do
       @request.into = nil
       @request.should be_valid
     end
+    it 'is not a duplicate of an existing pending request' do
+      request
+      @request.should_not be_valid
+    end
+    it 'is not to an existing friend' do
+      connect_users(user, aspect, user2, user2.aspects.create(:name => 'new aspect'))
+      @request.should_not be_valid
+    end
   end
 
   describe '#request_from_me' do