Refactor StatusMessagesController#create, move the photo dispatching into an after_dispatch hook

app/controllers/status_messages_controller.rb

@@ -36,37 +36,25 @@ class StatusMessagesController < ApplicationController
   def create
     params[:status_message][:aspect_ids] = params[:aspect_ids]
 
-    photos = Photo.where(:id => [*params[:photos]], :diaspora_handle => current_user.person.diaspora_handle)
+    normalize_public_flag!
-
-    public_flag = params[:status_message][:public]
-    public_flag.to_s.match(/(true)|(on)/) ? public_flag = true : public_flag = false
-    params[:status_message][:public] = public_flag
 
     @status_message = current_user.build_post(:status_message, params[:status_message])
-    aspects = current_user.aspects_from_ids(params[:aspect_ids])
 
-    if !photos.empty?
-      @status_message.photos << photos
-    end
     if @status_message.save
       Rails.logger.info("event=create type=status_message chars=#{params[:status_message][:text].length}")
 
-      current_user.add_to_streams(@status_message, aspects)
+      photos = Photo.where(:id => [*params[:photos]], :diaspora_handle => current_user.person.diaspora_handle)
-      receiving_services = params[:services].map{|s| current_user.services.where(
-                                  :type => "Services::"+s.titleize).first} if params[:services]
-      current_user.dispatch_post(@status_message, :url => post_url(@status_message), :services => receiving_services)
       if !photos.empty?
-        for photo in photos
+        @status_message.photos << photos
-          was_pending = photo.pending
-          if was_pending
-            current_user.add_to_streams(photo, aspects)
-            current_user.dispatch_post(photo)
-          end
-        end
-        photos.update_all(:pending => false, :public => public_flag)
       end
 
-      if request.env['HTTP_REFERER'].include?("people")
+      aspects = current_user.aspects_from_ids(params[:aspect_ids])
+      current_user.add_to_streams(@status_message, aspects)
+      receiving_services = current_user.services.where( :type => params[:services].map{|s| "Services::"+s.titleize}) if params[:services]
+      current_user.dispatch_post(@status_message, :url => post_url(@status_message), :services => receiving_services)
+
+
+      if request.env['HTTP_REFERER'].include?("people") # if this is a post coming from a profile page
         flash[:notice] = t('status_messages.create.success', :names => @status_message.mentions.includes(:person => :profile).map{ |mention| mention.person.name }.join(', '))
       end
 
@@ -76,9 +64,7 @@ class StatusMessagesController < ApplicationController
         format.mobile{ redirect_to root_url}
       end
     else
-      if !photos.empty?
+
-        photos.update_all(:status_message_guid => nil)
-      end
       respond_to do |format|
         format.js {
           errors = @status_message.errors.full_messages.collect { |msg| msg.gsub(/^Text/, "") }
@@ -89,22 +75,11 @@ class StatusMessagesController < ApplicationController
     end
   end
 
-  def show
+  def normalize_public_flag!
-    @status_message = current_user.find_visible_post_by_id params[:id]
+    public_flag = params[:status_message][:public]
-    if @status_message
+    public_flag.to_s.match(/(true)|(on)/) ? public_flag = true : public_flag = false
-
+    params[:status_message][:public] = public_flag
-      # mark corresponding notification as read
+    public_flag
-      if notification = Notification.where(:recipient_id => current_user.id, :target_id => @status_message.id).first
-        notification.unread = false
-        notification.save
-      end
-
-      respond_with @status_message
-    else
-      Rails.logger.info(:event => :link_to_nonexistent_post, :ref => request.env['HTTP_REFERER'], :user_id => current_user.id, :post_id => params[:id])
-      flash[:error] = I18n.t('status_messages.show.not_found')
-      redirect_to :back
-    end
   end
 
   helper_method :comments_expanded

app/models/status_message.rb

@@ -120,6 +120,18 @@ class StatusMessage < Post
     super(user_or_id, opts)
   end
 
+  def after_dispatch sender
+    unless self.photos.empty?
+      self.photos.update_all(:pending => false, :public => self.public)
+      for photo in self.photos
+        if photo.pending
+          sender.add_to_streams(photo, self.aspects)
+          sender.dispatch_post(photo)
+        end
+      end
+    end
+  end
+
   protected
 
   def message_or_photos_present?

lib/diaspora/webhooks.rb

@@ -16,12 +16,18 @@ module Diaspora
       input.to_s.to_xs
     end
 
+    # @abstract
     def subscribers(user)
       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'
     end
+
+    # @param [User] sender
+    def after_dispatch sender
+    end
   end
 end

lib/postzord/dispatch.rb

@@ -35,6 +35,7 @@ class Postzord::Dispatch
       self.deliver_to_remote(remote_people)
     end
     self.deliver_to_services(opts[:url], opts[:services] || [])
+    @object.after_dispatch(@sender)
   end
 
   protected

spec/controllers/status_messages_controller_spec.rb

@@ -106,11 +106,8 @@ describe StatusMessagesController do
 
     context 'with photos' do
       before do
-        fixture_filename  = 'button.png'
+        @photo1 = alice.build_post(:photo, :pending => true, :user_file=> File.open(photo_fixture_name), :to => @aspect1.id)
-        fixture_name      = File.join(File.dirname(__FILE__), '..', 'fixtures', fixture_filename)
+        @photo2 = alice.build_post(:photo, :pending => true, :user_file=> File.open(photo_fixture_name), :to => @aspect1.id)
-
-        @photo1 = alice.build_post(:photo, :pending => true, :user_file=> File.open(fixture_name), :to => @aspect1.id)
-        @photo2 = alice.build_post(:photo, :pending => true, :user_file=> File.open(fixture_name), :to => @aspect1.id)
 
         @photo1.save!
         @photo2.save!
@@ -123,9 +120,9 @@ describe StatusMessagesController do
         post :create, @hash
         response.should be_redirect
       end
-      it "dispatches all referenced photos" do
+      it "attaches all referenced photos" do
-        alice.should_receive(:dispatch_post).exactly(3).times
         post :create, @hash
+        assigns[:status_message].photos.map(&:id).should =~ [@photo1, @photo2].map(&:id)
       end
       it "sets the pending bit of referenced photos" do
         post :create, @hash

spec/models/status_message_spec.rb

@@ -244,4 +244,26 @@ STR
       Post.find(post.id).youtube_titles.should == {video_id => CGI::escape(expected_title)}
     end
   end
+  describe '#after_dispatch' do
+    before do
+      @photos = [alice.build_post(:photo, :pending => true, :user_file=> File.open(photo_fixture_name)),
+                 alice.build_post(:photo, :pending => true, :user_file=> File.open(photo_fixture_name))]
+
+      @photos.each(&:save!)
+
+      @status_message = alice.build_post(:status_message, :text => "the best pebble.")
+        @status_message.photos << @photos
+
+      @status_message.save!
+      alice.add_to_streams(@status_message, alice.aspects)
+    end
+    it 'sets pending to false on any attached photos' do
+      @status_message.after_dispatch(alice)
+      @photos.all?{|p| p.reload.pending}.should be_false
+    end
+    it 'dispatches any attached photos' do
+      alice.should_receive(:dispatch_post).twice
+      @status_message.after_dispatch(alice)
+    end
+  end
 end

spec/support/user_methods.rb

@@ -13,6 +13,7 @@ class User
       p = build_post(class_name, opts)
       if p.save!
         self.aspects.reload
+
         aspects = self.aspects_from_ids(opts[:to])
         add_to_streams(p, aspects)
         dispatch_post(p, :to => opts[:to])