hook up data syncing for articles and switch to using call (#7115)

app/models/article.rb

@@ -8,7 +8,7 @@ class Article < ApplicationRecord
 
   SEARCH_SERIALIZER = Search::ArticleSerializer
   SEARCH_CLASS = Search::FeedContent
-  REACTION_INDEXED_FIELDS = %w[body_markdown published tag_list title].freeze
+  DATA_SYNC_CLASS = DataSync::Elasticsearch::Article
 
   acts_as_taggable_on :tags
   resourcify
@@ -75,11 +75,9 @@ class Article < ApplicationRecord
   after_save :notify_slack_channel_about_publication
 
   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 :sync_related_elasticsearch_docs, on: %i[create update]
   after_commit :remove_from_elasticsearch, on: [:destroy]
 
   before_destroy :before_destroy_actions, prepend: true
@@ -288,14 +286,6 @@ 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/concerns/searchable.rb

@@ -24,6 +24,6 @@ module Searchable
   end
 
   def sync_related_elasticsearch_docs
-    self.class::DATA_SYNC_CLASS.new(self, saved_changes).sync_documents
+    self.class::DATA_SYNC_CLASS.new(self, saved_changes).call
   end
 end

app/services/data_sync/elasticsearch/article.rb

@@ -0,0 +1,47 @@
+module DataSync
+  module Elasticsearch
+    class Article
+      RELATED_DOCS = %i[
+        reactions
+      ].freeze
+
+      SHARED_ARTICLE_FIELDS = %i[
+        body_markdown
+        path
+        published
+        reading_time
+        tag_list
+        title
+      ].freeze
+
+      attr_accessor :article, :updated_fields
+
+      def initialize(article, updated_fields)
+        @article = article
+        @updated_fields = updated_fields.deep_symbolize_keys
+      end
+
+      def call
+        return unless sync_needed?
+
+        RELATED_DOCS.each do |relation_name|
+          if article.published
+            send(relation_name).find_each(&:index_to_elasticsearch)
+          elsif updated_fields.key?(:published)
+            send(relation_name).find_each(&:remove_from_elasticsearch)
+          end
+        end
+      end
+
+      private
+
+      def sync_needed?
+        updated_fields.slice(*SHARED_ARTICLE_FIELDS).any? && reactions.any?
+      end
+
+      def reactions
+        article.reactions.readinglist
+      end
+    end
+  end
+end

app/services/data_sync/elasticsearch/user.rb

@@ -25,7 +25,7 @@ module DataSync
         @updated_fields = updated_fields.deep_symbolize_keys
       end
 
-      def sync_documents
+      def call
         return unless sync_needed?
 
         RELATED_DOCS.each do |relation_name|

spec/models/article_spec.rb

@@ -44,6 +44,12 @@ RSpec.describe Article, type: :model do
           article.destroy
         end
       end
+
+      it "on update syncs elasticsearch data" do
+        allow(article).to receive(:sync_related_elasticsearch_docs)
+        article.save
+        expect(article).to have_received(:sync_related_elasticsearch_docs)
+      end
     end
 
     describe "#after_update_commit" do

spec/services/data_sync/elasticsearch/article_spec.rb

@@ -0,0 +1,39 @@
+require "rails_helper"
+
+RSpec.describe DataSync::Elasticsearch::Article, type: :service do
+  let(:article) { create(:article) }
+
+  describe "#call" do
+    it "reindexes RELATED_DOCS when sync is needed " do
+      syncer = described_class.new(article, title: %w[old_title new_title])
+      described_class::RELATED_DOCS.each do |method_name|
+        allow(syncer).to receive(method_name).and_call_original
+      end
+      syncer.call
+      described_class::RELATED_DOCS.each do |method_name|
+        expect(syncer).to have_received(method_name)
+      end
+    end
+
+    it "does not reindex when sync is not needed" do
+      syncer = described_class.new(article, page_views_count: [nil, 1])
+      described_class::RELATED_DOCS.each { |method_name| allow(syncer).to receive(method_name) }
+      syncer.call
+      described_class::RELATED_DOCS.each do |method_name|
+        expect(syncer).not_to have_received(method_name)
+      end
+    end
+
+    it "removes docs from elasticsearch if article is unpublished" do
+      reaction = create(:reaction, reactable: article, category: "readinglist")
+      sidekiq_perform_enqueued_jobs
+      expect(reaction.elasticsearch_doc).not_to be_nil
+
+      article.update_column(:published, false)
+
+      described_class.new(article, published: [true, false]).call
+      sidekiq_perform_enqueued_jobs
+      expect { reaction.elasticsearch_doc }.to raise_error(Search::Errors::Transport::NotFound)
+    end
+  end
+end

spec/services/data_sync/elasticsearch/user_spec.rb

@@ -3,13 +3,13 @@ require "rails_helper"
 RSpec.describe DataSync::Elasticsearch::User, type: :service do
   let!(:user) { create(:user) }
 
-  describe "#sync_documents" do
+  describe "#call" do
     it "reindexes RELATED_DOCS when sync is needed " do
       syncer = described_class.new(user, username: %w[name1 name2])
       described_class::RELATED_DOCS.each do |method_name|
         allow(syncer).to receive(method_name).and_call_original
       end
-      syncer.sync_documents
+      syncer.call
       described_class::RELATED_DOCS.each do |method_name|
         expect(syncer).to have_received(method_name)
       end
@@ -18,7 +18,7 @@ RSpec.describe DataSync::Elasticsearch::User, type: :service do
     it "does not reindex when sync is not needed" do
       syncer = described_class.new(user, stackoverflow_url: [nil, "url"])
       described_class::RELATED_DOCS.each { |method_name| allow(syncer).to receive(method_name) }
-      syncer.sync_documents
+      syncer.call
       described_class::RELATED_DOCS.each do |method_name|
         expect(syncer).not_to have_received(method_name)
       end