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

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

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