refactoring StatusMessageCreationService

* move parameter normalization back to controller, because this is
  frontend-specific.
* if the StatusMessage is public, save also public photos

app/controllers/status_messages_controller.rb

@@ -47,12 +47,17 @@ class StatusMessagesController < ApplicationController
   end
 
   def create
-    @status_message = StatusMessageCreationService.new(params, current_user).status_message
-    handle_mention_feedback
+    normalized_params = params.merge(
+      services:   normalize_services,
+      aspect_ids: normalize_aspect_ids,
+      public:     normalize_public_flag
+    )
+    status_message = StatusMessageCreationService.new(current_user).create(normalized_params)
+    handle_mention_feedback(status_message)
     respond_to do |format|
       format.html { redirect_to :back }
       format.mobile { redirect_to stream_path }
-      format.json { render json: PostPresenter.new(@status_message, current_user), status: 201 }
+      format.json { render json: PostPresenter.new(status_message, current_user), status: 201 }
     end
   rescue StandardError => error
     handle_create_error(error)
@@ -73,9 +78,9 @@ class StatusMessagesController < ApplicationController
     end
   end
 
-  def handle_mention_feedback
+  def handle_mention_feedback(status_message)
     return unless comes_from_others_profile_page?
-    flash[:notice] = successful_mention_message
+    flash[:notice] = t("status_messages.create.success", names: status_message.mentioned_people_names)
   end
 
   def comes_from_others_profile_page?
@@ -87,11 +92,24 @@ class StatusMessagesController < ApplicationController
   end
 
   def own_profile_page?
-    request.env["HTTP_REFERER"].include?("/people/" + params[:status_message][:author][:guid].to_s)
+    request.env["HTTP_REFERER"].include?("/people/" + current_user.guid)
   end
 
-  def successful_mention_message
-    t("status_messages.create.success", names: @status_message.mentioned_people_names)
+  def normalize_services
+    [*params[:services]].compact
+  end
+
+  def normalize_aspect_ids
+    aspect_ids = [*params[:aspect_ids]]
+    if aspect_ids.first == "all_aspects"
+      current_user.aspect_ids
+    else
+      aspect_ids
+    end
+  end
+
+  def normalize_public_flag
+    [*params[:aspect_ids]].first == "public"
   end
 
   def remove_getting_started

app/models/post.rb

@@ -133,12 +133,10 @@ class Post < ActiveRecord::Base
   #############
 
   def self.diaspora_initialize(params)
-    new_post = self.new params.to_hash.stringify_keys.slice(*self.column_names)
-    new_post.author = params[:author]
-    new_post.public = params[:public] if params[:public]
-    new_post.pending = params[:pending] if params[:pending]
-    new_post.diaspora_handle = new_post.author.diaspora_handle
-    new_post
+    new(params.to_hash.stringify_keys.slice(*column_names)).tap do |new_post|
+      new_post.author = params[:author]
+      new_post.diaspora_handle = new_post.author.diaspora_handle
+    end
   end
 
   # @return Returns true if this Post will accept updates (i.e. updates to the caption of a photo).

app/services/status_message_creation_service.rb

@@ -1,89 +1,66 @@
 class StatusMessageCreationService
   include Rails.application.routes.url_helpers
 
-  attr_reader :status_message
+  def initialize(user)
+    @user = user
+  end
 
-  def initialize(params, user)
-    normalize_params(params, user)
-    status_message_initial = user.build_post(:status_message, params[:status_message])
-    @status_message = add_attachments(params, status_message_initial)
-    @status_message.save
-    process_status_message(user)
+  def create(params)
+    build_status_message(params).tap do |status_message|
+      add_attachments(status_message, params)
+      status_message.save
+      process(status_message, params[:aspect_ids], params[:services])
+    end
   end
 
   private
 
-  attr_reader :services, :destination_aspect_ids
+  attr_reader :user
 
-  def normalize_params(params, user)
-    normalize_aspect_ids(params)
-    normalize_public_flag!(params)
-    @services = [*params[:services]].compact
-    @destination_aspect_ids = destination_aspect_ids(params, user)
+  def build_status_message(params)
+    public = params[:public] || false
+    user.build_post(:status_message, params[:status_message].merge(public: public))
   end
 
-  def normalize_aspect_ids(params)
-    params[:status_message][:aspect_ids] = [*params[:aspect_ids]]
+  def add_attachments(status_message, params)
+    add_location(status_message, params[:location_address], params[:location_coords])
+    add_poll(status_message, params)
+    add_photos(status_message, params[:photos])
   end
 
-  def normalize_public_flag!(params)
-    sm = params[:status_message]
-    public_flag_string = (sm[:aspect_ids] && sm[:aspect_ids].first == "public") || sm[:public]
-    public_flag = public_flag_string.to_s.match(/(true)|(on)/) ? true : false
-    params[:status_message][:public] = public_flag
-  end
-
-  def destination_aspect_ids(params, user)
-    if params[:status_message][:aspect_ids].first == "all_aspects"
-      user.aspect_ids
-    elsif !params[:status_message][:public]
-      params[:aspect_ids]
-    end
-  end
-
-  def add_attachments(params, status_message_initial)
-    status_message_with_location = add_location(params, status_message_initial)
-    status_message_with_poll = add_poll(params, status_message_with_location)
-    add_photos(params, status_message_with_poll)
-  end
-
-  def add_location(params, status_message)
-    address = params[:location_address]
-    coordinates = params[:location_coords]
+  def add_location(status_message, address, coordinates)
     status_message.build_location(address: address, coordinates: coordinates) if address.present?
-    status_message
   end
 
-  def add_poll(params, status_message)
+  def add_poll(status_message, params)
     if params[:poll_question].present?
       status_message.build_poll(question: params[:poll_question])
       [*params[:poll_answers]].each do |poll_answer|
         status_message.poll.poll_answers.build(answer: poll_answer)
       end
     end
-    status_message
   end
 
-  def add_photos(params, status_message)
-    status_message.attach_photos_by_ids(params[:photos])
-    status_message
+  def add_photos(status_message, photos)
+    status_message.attach_photos_by_ids(photos)
+    status_message.photos.each {|photo| photo.public = status_message.public }
   end
 
-  def process_status_message(user)
-    add_status_message_to_streams(user)
-    dispatch_status_message(user)
-    user.participate!(@status_message)
+  def process(status_message, aspect_ids, services)
+    add_to_streams(status_message, aspect_ids) unless status_message.public
+    dispatch(status_message, services)
+    user.participate!(status_message)
   end
 
-  def add_status_message_to_streams(user)
-    aspects = user.aspects_from_ids(@destination_aspect_ids)
-    user.add_to_streams(@status_message, aspects)
+  def add_to_streams(status_message, aspect_ids)
+    aspects = user.aspects_from_ids(aspect_ids)
+    user.add_to_streams(status_message, aspects)
   end
 
-  def dispatch_status_message(user)
-    receiving_services = Service.titles(@services)
-    user.dispatch_post(@status_message,
-                       url:           short_post_url(@status_message.guid, host: AppConfig.environment.url),
+  def dispatch(status_message, services)
+    receiving_services = services ? Service.titles(services) : []
+    user.dispatch_post(status_message,
+                       url:           short_post_url(status_message.guid, host: AppConfig.environment.url),
                        service_types: receiving_services)
   end
 end

spec/controllers/status_messages_controller_spec.rb

@@ -185,7 +185,8 @@ describe StatusMessagesController, :type => :controller do
 
       it "attaches all referenced photos" do
         post :create, @hash
-        expect(assigns[:status_message].photos.map(&:id)).to match_array([@photo1, @photo2].map(&:id))
+        status_message = StatusMessage.find_by_text(status_message_hash[:status_message][:text])
+        expect(status_message.photos.map(&:id)).to match_array([@photo1, @photo2].map(&:id))
       end
 
       it "sets the pending bit of referenced photos" do