From 2772cb6e0c4280b7cf7f230773fac0a0fc7aee28 Mon Sep 17 00:00:00 2001
From: Steffen van Bergerem <svbergerem@omgsrsly.net>
Date: Sat, 18 Feb 2017 00:36:22 +0100
Subject: [PATCH] Refactor likes controller using like service
---
app/controllers/likes_controller.rb | 49 ++---
spec/controllers/likes_controller_spec.rb | 233 ++++++++++------------
2 files changed, 119 insertions(+), 163 deletions(-)
diff --git a/app/controllers/likes_controller.rb b/app/controllers/likes_controller.rb
index 8f9197c3c5..6767464f7d 100644
--- a/app/controllers/likes_controller.rb
+++ b/app/controllers/likes_controller.rb
@@ -11,42 +11,27 @@ class LikesController < ApplicationController
:json
def create
- begin
- @like = if target
- current_user.like!(target)
- end
- rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordInvalid => e
- # do nothing
- end
-
- if @like
- respond_to do |format|
- format.html { render :nothing => true, :status => 201 }
- format.mobile { redirect_to post_path(@like.post_id) }
- format.json { render :json => @like.as_api_response(:backbone), :status => 201 }
- end
- else
- render text: I18n.t("likes.create.error"), status: 422
+ like = like_service.create(params[:post_id])
+ rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordInvalid
+ render text: I18n.t("likes.create.error"), status: 422
+ else
+ respond_to do |format|
+ format.html { render nothing: true, status: 201 }
+ format.mobile { redirect_to post_path(like.post_id) }
+ format.json { render json: like.as_api_response(:backbone), status: 201 }
end
end
def destroy
- begin
- @like = Like.find_by_id_and_author_id!(params[:id], current_user.person.id)
- rescue ActiveRecord::RecordNotFound
+ if like_service.destroy(params[:id])
+ render nothing: true, status: 204
+ else
render text: I18n.t("likes.destroy.error"), status: 404
- return
- end
-
- current_user.retract(@like)
- respond_to do |format|
- format.json { render :nothing => true, :status => 204 }
end
end
- #I can go when the old stream goes.
def index
- @likes = target.likes.includes(:author => :profile)
+ @likes = like_service.find_for_post(params[:post_id]).includes(author: :profile)
@people = @likes.map(&:author)
respond_to do |format|
@@ -57,13 +42,7 @@ class LikesController < ApplicationController
private
- def target
- @target ||= if params[:post_id]
- current_user.find_visible_shareable_by_id(Post, params[:post_id]) || raise(ActiveRecord::RecordNotFound.new)
- else
- Comment.find(params[:comment_id]).tap do |comment|
- raise(ActiveRecord::RecordNotFound.new) unless current_user.find_visible_shareable_by_id(Post, comment.commentable_id)
- end
- end
+ def like_service
+ @like_service ||= LikeService.new(current_user)
end
end
diff --git a/spec/controllers/likes_controller_spec.rb b/spec/controllers/likes_controller_spec.rb
index 928b1f64ca..f26306776e 100644
--- a/spec/controllers/likes_controller_spec.rb
+++ b/spec/controllers/likes_controller_spec.rb
@@ -1,8 +1,8 @@
- # Copyright (c) 2010-2011, Diaspora Inc. This file is
+# Copyright (c) 2010-2011, Diaspora Inc. This file is
# licensed under the Affero General Public License version 3 or later. See
# the COPYRIGHT file.
-describe LikesController, :type => :controller do
+describe LikesController, type: :controller do
before do
@alices_aspect = alice.aspects.where(:name => "generic").first
@bobs_aspect = bob.aspects.where(:name => "generic").first
@@ -10,137 +10,114 @@ describe LikesController, :type => :controller do
sign_in(alice, scope: :user)
end
- [Comment, Post].each do |class_const|
- context class_const.to_s do
- let(:id_field){
- "#{class_const.to_s.underscore}_id"
- }
-
- describe '#create' do
- let(:like_hash) {
- {:positive => 1,
- id_field => "#{@target.id}"}
- }
- let(:dislike_hash) {
- {:positive => 0,
- id_field => "#{@target.id}"}
- }
-
- context "on my own post" do
- it 'succeeds' do
- @target = alice.post :status_message, :text => "AWESOME", :to => @alices_aspect.id
- @target = alice.comment!(@target, "hey") if class_const == Comment
- post :create, like_hash.merge(:format => :json)
- expect(response.code).to eq('201')
- end
- end
-
- context "on a post from a contact" do
- before do
- @target = bob.post(:status_message, :text => "AWESOME", :to => @bobs_aspect.id)
- @target = bob.comment!(@target, "hey") if class_const == Comment
- end
-
- it 'likes' do
- post :create, like_hash
- expect(response.code).to eq('201')
- end
-
- it 'dislikes' do
- post :create, dislike_hash
- expect(response.code).to eq('201')
- end
-
- it "doesn't post multiple times" do
- alice.like!(@target)
- post :create, dislike_hash
- expect(response.code).to eq('422')
- end
- end
-
- context "on a post from a stranger" do
- before do
- @target = eve.post :status_message, :text => "AWESOME", :to => eve.aspects.first.id
- @target = eve.comment!(@target, "hey") if class_const == Comment
- end
-
- it "doesn't post" do
- expect(alice).not_to receive(:like!)
- post :create, like_hash
- expect(response.code).to eq('422')
- end
- end
-
- context "when an the exception is raised" do
- before do
- @target = alice.post :status_message, :text => "AWESOME", :to => @alices_aspect.id
- @target = alice.comment!(@target, "hey") if class_const == Comment
- end
-
- it "should be catched when it means that the target is not found" do
- params = like_hash.merge(format: :json, id_field => -1)
- post :create, params
- expect(response.code).to eq('422')
- end
-
- it "should not be catched when it is unexpected" do
- @target = alice.post :status_message, :text => "AWESOME", :to => @alices_aspect.id
- @target = alice.comment!(@target, "hey") if class_const == Comment
- allow(alice).to receive(:like!).and_raise("something")
- allow(@controller).to receive(:current_user).and_return(alice)
- expect { post :create, like_hash.merge(:format => :json) }.to raise_error("something")
- end
- end
+ describe "#create" do
+ let(:like_hash) {
+ {post_id: @target.id}
+ }
+
+ context "on my own post" do
+ it "succeeds" do
+ @target = alice.post :status_message, text: "AWESOME", to: @alices_aspect.id
+ post :create, like_hash.merge(format: :json)
+ expect(response.code).to eq("201")
end
+ end
+
+ context "on a post from a contact" do
+ before do
+ @target = bob.post(:status_message, text: "AWESOME", to: @bobs_aspect.id)
+ end
+
+ it "likes" do
+ post :create, like_hash
+ expect(response.code).to eq("201")
+ end
+
+ it "doesn't post multiple times" do
+ alice.like!(@target)
+ post :create, like_hash
+ expect(response.code).to eq("422")
+ end
+ end
- describe '#index' do
- before do
- @message = alice.post(:status_message, :text => "hey", :to => @alices_aspect.id)
- @message = alice.comment!(@message, "hey") if class_const == Comment
- end
-
- it 'returns a 404 for a post not visible to the user' do
- sign_in eve
- expect{get :index, id_field => @message.id}.to raise_error(ActiveRecord::RecordNotFound)
- end
-
- it 'returns an array of likes for a post' do
- like = bob.like!(@message)
- get :index, id_field => @message.id
- expect(assigns[:likes].map(&:id)).to eq(@message.likes.map(&:id))
- end
-
- it 'returns an empty array for a post with no likes' do
- get :index, id_field => @message.id
- expect(assigns[:likes]).to eq([])
- end
+ context "on a post from a stranger" do
+ before do
+ @target = eve.post :status_message, text: "AWESOME", to: eve.aspects.first.id
end
- describe '#destroy' do
- before do
- @message = bob.post(:status_message, :text => "hey", :to => @alices_aspect.id)
- @message = bob.comment!(@message, "hey") if class_const == Comment
- @like = alice.like!(@message)
- end
-
- it 'lets a user destroy their like' do
- current_user = controller.send(:current_user)
- expect(current_user).to receive(:retract).with(@like)
-
- delete :destroy, :format => :json, id_field => @like.target_id, :id => @like.id
- expect(response.status).to eq(204)
- end
-
- it 'does not let a user destroy other likes' do
- like2 = eve.like!(@message)
- like_count = Like.count
-
- delete :destroy, :format => :json, id_field => like2.target_id, :id => like2.id
- expect(response.status).to eq(404)
- expect(response.body).to eq(I18n.t("likes.destroy.error"))
- expect(Like.count).to eq(like_count)
- end
+ it "doesn't post" do
+ expect(alice).not_to receive(:like!)
+ post :create, like_hash
+ expect(response.code).to eq("422")
end
end
+
+ context "when an the exception is raised" do
+ before do
+ @target = alice.post :status_message, text: "AWESOME", to: @alices_aspect.id
+ end
+
+ it "should be catched when it means that the target is not found" do
+ params = like_hash.merge(format: :json, post_id: -1)
+ post :create, params
+ expect(response.code).to eq("422")
+ end
+
+ it "should not be catched when it is unexpected" do
+ @target = alice.post :status_message, text: "AWESOME", to: @alices_aspect.id
+ allow(alice).to receive(:like!).and_raise("something")
+ allow(@controller).to receive(:current_user).and_return(alice)
+ expect { post :create, like_hash.merge(format: :json) }.to raise_error("something")
+ end
+ end
+ end
+
+ describe "#index" do
+ before do
+ @message = alice.post(:status_message, text: "hey", to: @alices_aspect.id)
+ end
+
+ it "returns a 404 for a post not visible to the user" do
+ sign_in eve
+ expect {
+ get :index, post_id: @message.id
+ }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+
+ it "returns an array of likes for a post" do
+ bob.like!(@message)
+ get :index, post_id: @message.id
+ expect(assigns[:likes].map(&:id)).to eq(@message.likes.map(&:id))
+ end
+
+ it "returns an empty array for a post with no likes" do
+ get :index, post_id: @message.id
+ expect(assigns[:likes]).to eq([])
+ end
+ end
+
+ describe "#destroy" do
+ before do
+ @message = bob.post(:status_message, text: "hey", to: @alices_aspect.id)
+ @like = alice.like!(@message)
+ end
+
+ it "lets a user destroy their like" do
+ current_user = controller.send(:current_user)
+ expect(current_user).to receive(:retract).with(@like)
+
+ delete :destroy, format: :json, post_id: @message.id, id: @like.id
+ expect(response.status).to eq(204)
+ end
+
+ it "does not let a user destroy other likes" do
+ like2 = eve.like!(@message)
+ like_count = Like.count
+
+ delete :destroy, format: :json, post_id: @message.id, id: like2.id
+ expect(response.status).to eq(404)
+ expect(response.body).to eq(I18n.t("likes.destroy.error"))
+ expect(Like.count).to eq(like_count)
+ end
end
end
--
GitLab