Index Reading List Reactions (#6953)

* Index reactions when they are created and updated, remove when destroyed

* update reading list reactions when related article fields are updated

* create indexable method and use for indexing to elasticsearch

app/models/article.rb

@@ -8,6 +8,7 @@ class Article < ApplicationRecord
 
   SEARCH_SERIALIZER = Search::ArticleSerializer
   SEARCH_CLASS = Search::FeedContent
+  REACTION_INDEXED_FIELDS = %w[body_markdown published tag_list title].freeze
 
   acts_as_taggable_on :tags
   resourcify
@@ -74,6 +75,9 @@ class Article < ApplicationRecord
   after_save :notify_slack_channel_about_publication, if: -> { published && published_at > 30.seconds.ago }
 
   after_update_commit :update_notifications, if: proc { |article| article.notifications.any? && !article.saved_changes.empty? }
+  after_update_commit :update_reading_list_reactions, if: proc { |article|
+    REACTION_INDEXED_FIELDS.any? { |field| article.saved_changes[field] } && reactions.readinglist.any?
+  }
   after_commit :async_score_calc, :update_main_image_background_hex, :touch_collection
   after_commit :index_to_elasticsearch, on: %i[create update]
   after_commit :remove_from_elasticsearch, on: [:destroy]
@@ -277,6 +281,14 @@ class Article < ApplicationRecord
     "article_#{id}"
   end
 
+  def update_reading_list_reactions
+    if published
+      reactions.readinglist.find_each(&:index_to_elasticsearch)
+    elsif saved_changes["published"]
+      reactions.readinglist.find_each(&:remove_from_elasticsearch)
+    end
+  end
+
   def processed_description
     text_portion = body_text.present? ? body_text[0..100].tr("\n", " ").strip.to_s : ""
     text_portion = text_portion.strip + "..." if body_text.size > 100

app/models/reaction.rb

@@ -19,6 +19,7 @@ class Reaction < ApplicationRecord
   counter_culture :user
 
   scope :positive, -> { where("points > ?", 0) }
+  scope :readinglist, -> { where(category: "readinglist") }
 
   validates :category, inclusion: { in: CATEGORIES }
   validates :reactable_type, inclusion: { in: REACTABLE_TYPES }
@@ -30,6 +31,8 @@ class Reaction < ApplicationRecord
   before_save :assign_points
   after_create_commit :record_field_test_event
   after_commit :async_bust, :bust_reactable_cache, :update_reactable
+  after_commit :index_to_elasticsearch, if: :indexable?, on: %i[create update]
+  after_commit :remove_from_elasticsearch, if: :indexable?, on: [:destroy]
   after_save :index_to_algolia
   after_save :touch_user
   before_destroy :update_reactable_without_delay, unless: :destroyed_by_association
@@ -105,6 +108,10 @@ class Reaction < ApplicationRecord
 
   private
 
+  def indexable?
+    category == "readinglist" && reactable && reactable.published
+  end
+
   def touch_user
     user.touch
   end

spec/models/article_spec.rb

@@ -46,6 +46,43 @@ RSpec.describe Article, type: :model do
       end
     end
 
+    describe "#after_update_commit" do
+      it "if article is unpublished removes reading list reactions from index" do
+        reaction = create(:reaction, reactable: article, category: "readinglist")
+        sidekiq_perform_enqueued_jobs
+        expect(reaction.elasticsearch_doc).not_to be_nil
+
+        unpublished_body = "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: false\ntags: hiring\n---\n\nHello"
+        article.update(body_markdown: unpublished_body)
+        sidekiq_perform_enqueued_jobs
+        expect { reaction.elasticsearch_doc }.to raise_error(Search::Errors::Transport::NotFound)
+      end
+
+      it "if article is published indexes reading list reactions" do
+        reaction = create(:reaction, reactable: article, category: "readinglist")
+        sidekiq_perform_enqueued_jobs
+        unpublished_body = "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: false\ntags: hiring\n---\n\nHello"
+        article.update(body_markdown: unpublished_body)
+        sidekiq_perform_enqueued_jobs
+        expect { reaction.elasticsearch_doc }.to raise_error(Search::Errors::Transport::NotFound)
+
+        published_body = "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: true\ntags: hiring\n---\n\nHello"
+        article.update(body_markdown: published_body)
+        sidekiq_perform_enqueued_jobs
+        expect(reaction.elasticsearch_doc).not_to be_nil
+      end
+
+      it "indexes reaction if a REACTION_INDEXED_FIELDS is changed" do
+        reaction = create(:reaction, reactable: article, category: "readinglist")
+        allow(article).to receive(:index_to_elasticsearch)
+        allow(article.user).to receive(:index_to_elasticsearch)
+
+        sidekiq_assert_enqueued_with(job: Search::IndexToElasticsearchWorker, args: ["Reaction", reaction.search_id]) do
+          article.update(body_markdown: "---\ntitle: NEW TITLE#{rand(1000)}\n")
+        end
+      end
+    end
+
     context "when published" do
       before do
         # rubocop:disable RSpec/NamedSubject

spec/models/reaction_spec.rb

@@ -68,6 +68,61 @@ RSpec.describe Reaction, type: :model do
     end
   end
 
+  describe "#after_commit" do
+    context "when category is readingList and reactable is published" do
+      it "on update enqueues job to index reaction to elasticsearch" do
+        reaction.save
+        sidekiq_assert_enqueued_with(job: Search::IndexToElasticsearchWorker, args: [described_class.to_s, reaction.id]) do
+          reaction.update(category: "readinglist")
+        end
+      end
+
+      it "on create enqueues job to index reaction to elasticsearch" do
+        reaction.category = "readinglist"
+        sidekiq_assert_enqueued_with(job: Search::IndexToElasticsearchWorker) do
+          reaction.save
+        end
+      end
+
+      it "on destroy enqueues job to delete reaction from elasticsearch" do
+        reaction.category = "readinglist"
+        reaction.save
+        sidekiq_assert_enqueued_with(job: Search::RemoveFromElasticsearchIndexWorker, args: [described_class::SEARCH_CLASS.to_s, reaction.id]) do
+          reaction.destroy
+        end
+      end
+    end
+
+    context "when category is not readinglist" do
+      before do
+        reaction.category = "like"
+        allow(reaction.user).to receive(:index_to_elasticsearch)
+        allow(reaction.reactable).to receive(:index_to_elasticsearch)
+        sidekiq_perform_enqueued_jobs
+      end
+
+      it "on update does not enqueue job to index reaction to elasticsearch" do
+        reaction.save
+        sidekiq_assert_no_enqueued_jobs(only: Search::IndexToElasticsearchWorker) do
+          reaction.update(category: "unicorn")
+        end
+      end
+
+      it "on create does not enqueue job to index reaction to elasticsearch" do
+        sidekiq_assert_no_enqueued_jobs(only: Search::IndexToElasticsearchWorker) do
+          reaction.save
+        end
+      end
+
+      it "on destroy does not enqueue job to delete reaction from elasticsearch" do
+        reaction.save
+        sidekiq_assert_no_enqueued_jobs(only: Search::RemoveFromElasticsearchIndexWorker) do
+          reaction.destroy
+        end
+      end
+    end
+  end
+
   describe "#skip_notification_for?" do
     let_it_be(:receiver) { build(:user) }
     let_it_be(:reaction) { build(:reaction, reactable: build(:article), user: nil) }