diff --git a/app/models/article.rb b/app/models/article.rb index c03fc4e2b..4a78fc818 100644 --- a/app/models/article.rb +++ b/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 diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 903aaa4df..06cefe8a7 100644 --- a/app/models/concerns/searchable.rb +++ b/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 diff --git a/app/services/data_sync/elasticsearch/article.rb b/app/services/data_sync/elasticsearch/article.rb new file mode 100644 index 000000000..ee4cdffb8 --- /dev/null +++ b/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 diff --git a/app/services/data_sync/elasticsearch/user.rb b/app/services/data_sync/elasticsearch/user.rb index b97da726c..08ea9d73e 100644 --- a/app/services/data_sync/elasticsearch/user.rb +++ b/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| diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 4abf178d5..76357e39f 100644 --- a/spec/models/article_spec.rb +++ b/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 diff --git a/spec/services/data_sync/elasticsearch/article_spec.rb b/spec/services/data_sync/elasticsearch/article_spec.rb new file mode 100644 index 000000000..d463cc5a4 --- /dev/null +++ b/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 diff --git a/spec/services/data_sync/elasticsearch/user_spec.rb b/spec/services/data_sync/elasticsearch/user_spec.rb index 5b95d7388..8228a79cb 100644 --- a/spec/services/data_sync/elasticsearch/user_spec.rb +++ b/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