Refactor posts controller

app/assets/javascripts/app/models/post.js

@@ -38,13 +38,6 @@ app.models.Post = Backbone.Model.extend(_.extend({}, app.models.formatDateMixin,
              .done(function(){ app.events.trigger('person:block:'+personId); });
   },
 
-  toggleFavorite : function(options){
-    this.set({favorite : !this.get("favorite")});
-
-    /* guard against attempting to save a model that a user doesn't own */
-    if(options.save){ this.save() }
-  },
-
   headline : function() {
     var headline = this.get("text").trim()
       , newlineIdx = headline.indexOf("\n");

app/controllers/posts_controller.rb

@@ -5,94 +5,60 @@
 class PostsController < ApplicationController
   include PostsHelper
 
-  before_action :authenticate_user!, :except => [:show, :iframe, :oembed, :interactions]
+  before_action :authenticate_user!, only: :destroy
-  before_action :set_format_if_malformed_from_status_net, :only => :show
+  before_action :set_format_if_malformed_from_status_net, only: :show
-  before_action :find_post, :only => [:show, :interactions]
 
-  respond_to :html,
+  respond_to :html, :mobile, :json, :xml
-             :mobile,
-             :json,
-             :xml
 
-  rescue_from Diaspora::NonPublic do |exception|
+  rescue_from Diaspora::NonPublic do |_exception|
     respond_to do |format|
-      format.all { render :template=>'errors/not_public', :status=>404, :layout => "application"}
+      format.all { render template: "errors/not_public", status: 404, layout: "application" }
     end
   end
 
   def show
-    mark_corresponding_notifications_read if user_signed_in?
+    post_service = PostService.new(id: params[:id], user: current_user)
-
+    post_service.assign_post_and_mark_notifications
+    @post = post_service.post
     respond_to do |format|
-      format.html {
+      format.html { gon.post = post_service.present_json }
-        gon.post = PostPresenter.new(@post, current_user)
-        render "posts/show"
-      }
       format.xml { render xml: @post.to_diaspora_xml }
-      format.mobile { render "posts/show" }
+      format.json { render json: post_service.present_json }
-      format.json { render json: PostPresenter.new(@post, current_user) }
     end
   end
 
   def iframe
-    render :text => post_iframe_url(params[:id]), :layout => false
+    render text: post_iframe_url(params[:id]), layout: false
   end
 
   def oembed
     post_id = OEmbedPresenter.id_from_url(params.delete(:url))
-    post = Post.find_by_guid_or_id_with_user(post_id, current_user)
+    post_service = PostService.new(id: post_id, user: current_user,
-    if post.present?
+                                    oembed: params.slice(:format, :maxheight, :minheight))
-      oembed = OEmbedPresenter.new(post, params.slice(:format, :maxheight, :minheight))
+    post_service.assign_post
-      render :json => oembed
+    render json: post_service.present_oembed
-    else
-      render :nothing => true, :status => 404
-    end
   end
 
   def interactions
-    respond_with(PostInteractionPresenter.new(@post, current_user))
+    post_service = PostService.new(id: params[:id], user: current_user)
+    post_service.assign_post
+    respond_with(post_service.present_interactions_json)
   end
 
   def destroy
-    find_current_user_post(params[:id])
+    post_service = PostService.new(id: params[:id], user: current_user)
-    current_user.retract(@post)
+    post_service.retract_post
-
+    @post = post_service.post
     respond_to do |format|
-      format.js { render 'destroy',:layout => false, :format => :js }
+      format.js { render "destroy", layout: false, format: :js }
-      format.json { render :nothing => true, :status => 204 }
+      format.json { render nothing: true, status: 204 }
       format.any { redirect_to stream_path }
     end
   end
 
-  def update
+  private
-    find_current_user_post(params[:id])
-    @post.favorite = !@post.favorite
-    @post.save
-    render :nothing => true, :status => 202
-  end
-
-  protected
-
-  def find_post #checks whether current user can see it
-    @post = Post.find_by_guid_or_id_with_user(params[:id], current_user)
-  end
-
-  def find_current_user_post(id) #makes sure current_user can modify
-    @post = current_user.posts.find(id)
-  end
 
   def set_format_if_malformed_from_status_net
-   request.format = :html if request.format == 'application/html+xml'
+    request.format = :html if request.format == "application/html+xml"
-  end
-
-  def mark_corresponding_notifications_read
-    # For comments, reshares, likes
-    Notification.where(recipient_id: current_user.id, target_type: "Post", target_id: @post.id, unread: true).each do |n|
-      n.set_read_state( true )
-    end
-
-    # For mentions
-    mention = @post.mentions.where(person_id: current_user.person_id).first
-    Notification.where(recipient_id: current_user.id, target_type: "Mention", target_id: mention.id, unread: true).first.try(:set_read_state, true) if mention
   end
 end

app/models/post.rb

@@ -148,17 +148,18 @@ class Post < ActiveRecord::Base
     self.author.profile.nsfw?
   end
 
-  def self.find_by_guid_or_id_with_user(id, user=nil)
+  def self.find_public(id)
-    key = id.to_s.length <= 8 ? :id : :guid
+    post = Post.where(Post.key_sym(id) => id).includes(:author, comments: :author).first
-    post = if user
+    post.try(:public?) || raise(Diaspora::NonPublic)
-             user.find_visible_shareable_by_id(Post, id, :key => key)
-           else
-             Post.where(key => id).includes(:author, :comments => :author).first
-           end
-
-    # is that a private post?
-    raise(Diaspora::NonPublic) unless user || post.try(:public?)
-
     post || raise(ActiveRecord::RecordNotFound.new("could not find a post with id #{id}"))
   end
+
+  def self.find_non_public_by_guid_or_id_with_user(id, user)
+    post = user.find_visible_shareable_by_id(Post, id, key: Post.key_sym(id))
+    post || raise(ActiveRecord::RecordNotFound.new("could not find a post with id #{id}"))
+  end
+
+  def self.key_sym(id)
+    id.to_s.length <= 8 ? :id : :guid
+  end
 end

app/presenters/post_interaction_presenter.rb

@@ -16,6 +16,8 @@ class PostInteractionPresenter
     }
   end
 
+  private
+
   def participations
     return @post.participations.none unless @current_user
     @post.participations.where(author: @current_user.person)

app/services/post_service.rb

@@ -0,0 +1,65 @@
+class PostService
+  attr_reader :post
+
+  def initialize(params)
+    @id = params[:id]
+    @user = params[:user]
+    @oembed = params[:oembed]
+  end
+
+  def present_json
+    PostPresenter.new(post, user)
+  end
+
+  def present_interactions_json
+    PostInteractionPresenter.new(post, user)
+  end
+
+  def present_oembed
+    OEmbedPresenter.new(post, oembed)
+  end
+
+  def assign_post_and_mark_notifications
+    assign_post
+    mark_corresponding_notifications_read if user
+  end
+
+  def assign_post
+    if user
+      @post = Post.find_non_public_by_guid_or_id_with_user(id, user)
+    else
+      @post = Post.find_public(id)
+    end
+  end
+
+  def retract_post
+    find_user_post
+    user.retract(@post)
+  end
+
+  private
+
+  attr_reader :user, :id, :oembed
+
+  def find_user_post
+    @post = user.posts.find(id)
+  end
+
+  def mark_corresponding_notifications_read
+    mark_comment_reshare_like_notifications_read
+    mark_mention_notifications_read
+  end
+
+  def mark_comment_reshare_like_notifications_read
+    notification = Notification.where(recipient_id: user.id, target_type: "Post", target_id: post.id, unread: true)
+    notification.each do |notification|
+      notification.set_read_state(true)
+    end
+  end
+
+  def mark_mention_notifications_read
+    mention = post.mentions.where(person_id: user.person_id).first
+    Notification.where(recipient_id: user.id, target_type: "Mention", target_id: mention.id, unread: true)
+      .first.try(:set_read_state, true) if mention
+  end
+end

spec/controllers/posts_controller_spec.rb

@@ -25,11 +25,6 @@ describe PostsController, :type => :controller do
         expect(response).to be_success
       end
 
-      it 'succeeds on mobile' do
-        get :show, "id" => @message.id
-        expect(response).to be_success
-      end
-
       it 'succeeds after removing a mention when closing the mentioned user\'s account' do
         user = FactoryGirl.create(:user, :username => "user")
         alice.share_with(user.person, alice.aspects.first)