From 69b04d38aaebe954bfe95bbc0d2032adf8fe9b00 Mon Sep 17 00:00:00 2001 From: rhymes Date: Fri, 7 Feb 2020 18:39:24 +0100 Subject: [PATCH] Refactor API Reactions controller and add specs (#5936) * Add specs * Check the object, not the ID * Use permitted params and delete cache only after create * Inherit from ApiController, bail if the reaction user does not exist, fix naming * Remove superflous test --- .../api/v0/reactions_controller.rb | 38 ++++-- spec/requests/api/v0/reactions_spec.rb | 129 ++++++++++++++++-- 2 files changed, 139 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/v0/reactions_controller.rb b/app/controllers/api/v0/reactions_controller.rb index fdf4e8286..7b0fcb1ab 100644 --- a/app/controllers/api/v0/reactions_controller.rb +++ b/app/controllers/api/v0/reactions_controller.rb @@ -1,25 +1,24 @@ module Api module V0 - class ReactionsController < ApplicationController + class ReactionsController < ApiController skip_before_action :verify_authenticity_token def create - @user = valid_user - - unless @user - render json: { message: "invalid_user" }, status: :unprocessable_entity + reaction_user = load_reaction_user + unless reaction_user + render json: { error: "invalid user" }, status: :unprocessable_entity return end - Rails.cache.delete "count_for_reactable-#{params[:reactable_type]}-#{params[:reactable_id]}" - - @reaction = Reaction.create( - user_id: @user.id, - reactable_id: params[:reactable_id], - reactable_type: params[:reactable_type], - category: params[:category] || "like", + @reaction = Reaction.create!( + user: reaction_user, + reactable_id: reaction_params[:reactable_id], + reactable_type: reaction_params[:reactable_type], + category: reaction_params[:category] || "like", ) + delete_reactable_cache(reaction_params[:reactable_id], reaction_params[:reactable_type]) + Notification.send_reaction_notification(@reaction, @reaction.reactable.user) Notification.send_reaction_notification(@reaction, @reaction.reactable.organization) if org_article?(@reaction) @@ -36,14 +35,23 @@ module Api private - def valid_user - user = User.find_by(secret: params[:key]) + def load_reaction_user + user = User.find_by!(secret: params[:key]) user = nil unless user.has_role?(:super_admin) user end + def delete_reactable_cache(reactable_id, reactable_type) + key = "count_for_reactable-#{reactable_type}-#{reactable_id}" + Rails.cache.delete(key) + end + + def reaction_params + params.permit(:reactable_id, :reactable_type, :category) + end + def org_article?(reaction) - reaction.reactable_type == "Article" && reaction.reactable.organization_id + reaction.reactable.is_a?(Article) && reaction.reactable.organization end end end diff --git a/spec/requests/api/v0/reactions_spec.rb b/spec/requests/api/v0/reactions_spec.rb index 31b794133..155f2d0f1 100644 --- a/spec/requests/api/v0/reactions_spec.rb +++ b/spec/requests/api/v0/reactions_spec.rb @@ -4,27 +4,130 @@ RSpec.describe "Api::V0::Reactions", type: :request do let(:user) { create(:user, secret: "TEST_SECRET") } let(:article) { create(:article) } - describe "POST /api/reactions" do - it "creates a new reaction for super admin users" do - user.add_role(:super_admin) - sign_in user + def create_reaction(user, reactable, category: "like") + post api_reactions_path( + reactable_id: reactable.id, + reactable_type: reactable.class.name, + category: category, + key: user.secret, + ) + end - expect do + describe "POST /api/reactions" do + context "when authorized as a super admin" do + before do + user.add_role(:super_admin) + sign_in user + end + + it "returns 422 if the reaction is invalid" do + create_reaction(user, create(:tag)) + expect(response).to have_http_status(:unprocessable_entity) + end + + it "returns 404 if the key is not found" do post api_reactions_path( - reactable_id: article.id, reactable_type: "Article", - category: "like", key: user.secret + reactable_id: article.id, + reactable_type: article.class.name, + category: "like", + key: "foobar", ) - expect(response).to have_http_status(:ok) - end.to change(article.reactions, :count).by(1) + expect(response).to have_http_status(:not_found) + end + end + end + + describe "POST /api/reactions - articles" do + context "when authorized as a super admin" do + before do + user.add_role(:super_admin) + sign_in user + end + + it "creates a new like reaction" do + expect do + create_reaction(user, article) + expect(response).to have_http_status(:ok) + end.to change(article.reactions, :count).by(1) + + expect(article.reactions.last.category).to eq("like") + end + + it "creates a new unicorn reaction" do + expect do + create_reaction(user, article, category: "unicorn") + expect(response).to have_http_status(:ok) + end.to change(article.reactions, :count).by(1) + + expect(article.reactions.last.category).to eq("unicorn") + end + + it "sends a reaction notification to the article's user" do + expect do + sidekiq_perform_enqueued_jobs do + create_reaction(user, article) + end + + expect(response).to have_http_status(:ok) + end.to change(article.user.notifications, :count).by(1) + end + + it "sends a reaction notification to the article's organization" do + article.update(organization: create(:organization)) + + expect do + sidekiq_perform_enqueued_jobs do + create_reaction(user, article) + end + + expect(response).to have_http_status(:ok) + end.to change(article.organization.notifications, :count).by(1) + end end it "rejects non-authorized users" do sign_in user - post api_reactions_path( - reactable_id: article.id, reactable_type: "Article", - category: "like", key: user.secret - ) + create_reaction(user, article) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + describe "POST /api/reactions - comments" do + let(:comment) { create(:comment, commentable: article) } + + context "when authorized as a super admin" do + before do + user.add_role(:super_admin) + sign_in user + end + + it "creates a new like reaction" do + expect do + create_reaction(user, comment) + expect(response).to have_http_status(:ok) + end.to change(comment.reactions, :count).by(1) + + expect(comment.reactions.last.category).to eq("like") + end + + it "sends a reaction notification to the comment's user" do + expect do + sidekiq_perform_enqueued_jobs do + create_reaction(user, comment) + end + + expect(response).to have_http_status(:ok) + end.to change(comment.user.notifications, :count).by(1) + end + end + + it "rejects non-authorized users" do + sign_in user + + create_reaction(user, comment) + expect(response).to have_http_status(:unprocessable_entity) end end