Refactor Reaction (#1320)

* Refactor Reaction's spec

* Create singleton class in Reaction

* Refactor Reaction

app/models/reaction.rb

@@ -1,50 +1,48 @@
 class Reaction < ApplicationRecord
+  CATEGORIES = %w(like readinglist unicorn thinking hands thumbsdown vomit).freeze
+
   belongs_to :reactable, polymorphic: true
+  belongs_to :user
+
   counter_culture :reactable,
     column_name: proc { |model|
       model.points.positive? ? "positive_reactions_count" : "reactions_count"
     }
   counter_culture :user
-  belongs_to :user
 
-  validates :category, inclusion: { in: %w(like thinking hands unicorn thumbsdown vomit readinglist) }
+  validates :category, inclusion: { in: CATEGORIES }
   validates :reactable_type, inclusion: { in: %w(Comment Article) }
   validates :status, inclusion: { in: %w(valid invalid confirmed) }
   validates :user_id, uniqueness: { scope: %i[reactable_id reactable_type category] }
   validate  :permissions
 
   before_save :assign_points
-  after_save :update_reactable
-  after_save :touch_user
-  after_save :async_bust
-  before_destroy :update_reactable_without_delay
-  before_destroy :clean_up_before_destroy
+  after_save :update_reactable, :touch_user, :async_bust
+  before_destroy :update_reactable_without_delay, :clean_up_before_destroy
 
-  scope :for_article, ->(id) { where(reactable_id: id, reactable_type: "Article") }
-
-  def self.count_for_article(id)
-    Rails.cache.fetch("count_for_reactable-Article-#{id}", expires_in: 1.hour) do
-      reactions = Reaction.for_article(id)
-
-      ["like", "readinglist", "unicorn"].map do |type|
-        { category: type, count: reactions.where(category: type).size }
+  class << self
+    def count_for_article(id)
+      Rails.cache.fetch("count_for_reactable-Article-#{id}", expires_in: 1.hour) do
+        reactions = Reaction.where(reactable_id: id, reactable_type: "Article")
+        %w(like readinglist unicorn).map do |type|
+          { category: type, count: reactions.where(category: type).size }
+        end
       end
     end
-  end
 
-  def self.for_display(user)
-    includes(:reactable).
-      where(reactable_type: "Article", user_id: user.id).
-      where("created_at > ?", 5.days.ago).
-      select("distinct on (reactable_id) *").
-      take(15)
-  end
+    def for_display(user)
+      includes(:reactable).
+        where(reactable_type: "Article", user: user).
+        where("created_at > ?", 5.days.ago).
+        select("distinct on (reactable_id) *").
+        take(15)
+    end
 
-  def self.cached_any_reactions_for?(reactable, user, category)
-    Rails.cache.fetch("any_reactions_for-#{reactable.class.name}-#{reactable.id}-#{user.updated_at}-#{category}", expires_in: 24.hours) do
-      Reaction.
-        where(reactable_id: reactable.id, reactable_type: reactable.class.name, user_id: user.id, category: category).
-        any?
+    def cached_any_reactions_for?(reactable, user, category)
+      cache_name = "any_reactions_for-#{reactable.class.name}-#{reactable.id}-#{user.updated_at}-#{category}"
+      Rails.cache.fetch(cache_name, expires_in: 24.hours) do
+        Reaction.where(reactable: reactable, user: user, category: category).any?
+      end
     end
   end
 

spec/models/reaction_spec.rb

@@ -2,15 +2,10 @@
 require "rails_helper"
 
 RSpec.describe Reaction, type: :model do
-  let(:user)        { create(:user) }
-  let(:author)      { create(:user) }
-  let(:article)     { create(:article, user_id: author.id, featured: true) }
-  let(:comment) do
-    create(:comment, user_id: user.id, commentable_id: article.id, commentable_type: "Article")
-  end
-  let(:reaction) do
-    build(:reaction, reactable: comment, reactable_type: "Comment")
-  end
+  let(:user) { create(:user) }
+  let(:article) { create(:article, featured: true) }
+  let(:comment) { create(:comment, user: user, commentable: article) }
+  let(:reaction) { build(:reaction, reactable: comment) }
 
   describe "actual validation" do
     subject { Reaction.new(reactable: article, reactable_type: "Article", user: user) }
@@ -18,7 +13,7 @@ RSpec.describe Reaction, type: :model do
     before { user.add_role(:trusted) }
 
     it { is_expected.to belong_to(:user) }
-    it { is_expected.to validate_inclusion_of(:category).in_array(%w(like thinking hands unicorn thumbsdown vomit readinglist)) }
+    it { is_expected.to validate_inclusion_of(:category).in_array(Reaction::CATEGORIES) }
     it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_id reactable_type category]) }
 
     # Thumbsdown and Vomits test needed
@@ -47,14 +42,10 @@ RSpec.describe Reaction, type: :model do
     end
 
     it "does not allow reaction on unpublished article" do
-      reaction = build(
-        :reaction, user_id: user.id, reactable_id: article.id, reactable_type: "Article"
-      )
+      reaction = build(:reaction, user: user, reactable: article)
       expect(reaction).to be_valid
       article.update_column(:published, false)
-      reaction = build(
-        :reaction, user_id: user.id, reactable_id: article.id, reactable_type: "Article"
-      )
+      reaction = build(:reaction, user: user, reactable: article)
       expect(reaction).not_to be_valid
     end
 
@@ -88,8 +79,8 @@ RSpec.describe Reaction, type: :model do
     it "runs async jobs effectively" do
       u2 = create(:user)
       c2 = create(:comment, commentable_id: article.id)
-      create(:reaction, user_id: u2.id, reactable_id: c2.id, reactable_type: "Comment")
-      create(:reaction, user_id: u2.id, reactable_id: article.id, reactable_type: "Article")
+      create(:reaction, user: u2, reactable: c2)
+      create(:reaction, user: u2, reactable: article)
       expect(reaction).to be_valid
     end
   end