From e7f9735354006e6f314744e3beb8c5f6d22002d8 Mon Sep 17 00:00:00 2001 From: rhymes Date: Fri, 8 Jan 2021 17:45:15 +0100 Subject: [PATCH] Remove RssReader (#12169) * Add script to remove feature flag * Remove :feeds_import feature flag and RssReader and related classes * Change Feeds::ImportArticlesWorker signature to support Sidekiq Cron serialization * Replace RssReader::Assembler with Feeds::AssembleArticleMarkdown * Removing Assembler --- app/controllers/users_controller.rb | 6 +- app/models/user.rb | 6 +- app/services/feature_flag.rb | 2 +- .../assemble_article_markdown.rb} | 8 +- app/services/feeds/import.rb | 8 +- app/services/rss_reader.rb | 100 --------- app/workers/articles/rss_reader_worker.rb | 21 -- app/workers/feeds/import_articles_worker.rb | 15 +- app/workers/rss_reader_fetch_user_worker.rb | 11 - config/initializers/honeybadger.rb | 2 +- config/schedule.yml | 4 +- ...151449_remove_feeds_import_feature_flag.rb | 7 + lib/tasks/fetch.rake | 15 -- .../remove_feeds_import_feature_flag_spec.rb | 20 ++ spec/models/user_spec.rb | 26 +-- spec/requests/user/user_settings_spec.rb | 15 +- spec/services/rss_reader_spec.rb | 191 ------------------ .../articles/rss_reader_worker_spec.rb | 51 ----- .../feeds/import_articles_worker_spec.rb | 39 ++-- .../rss_reader_fetch_user_worker_spec.rb | 40 ---- 20 files changed, 75 insertions(+), 512 deletions(-) rename app/services/{rss_reader/assembler.rb => feeds/assemble_article_markdown.rb} (98%) delete mode 100644 app/services/rss_reader.rb delete mode 100644 app/workers/articles/rss_reader_worker.rb delete mode 100644 app/workers/rss_reader_fetch_user_worker.rb create mode 100644 lib/data_update_scripts/20210107151449_remove_feeds_import_feature_flag.rb create mode 100644 spec/lib/data_update_scripts/remove_feeds_import_feature_flag_spec.rb delete mode 100644 spec/services/rss_reader_spec.rb delete mode 100644 spec/workers/articles/rss_reader_worker_spec.rb delete mode 100644 spec/workers/rss_reader_fetch_user_worker_spec.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4c1f22a3d..e12e85453 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -336,11 +336,7 @@ class UsersController < ApplicationController def import_articles_from_feed(user) return if user.feed_url.blank? - if FeatureFlag.enabled?(:feeds_import) - Feeds::ImportArticlesWorker.perform_async(nil, user.id) - else - RssReaderFetchUserWorker.perform_async(user.id) - end + Feeds::ImportArticlesWorker.perform_async(nil, user.id) end def profile_params diff --git a/app/models/user.rb b/app/models/user.rb index 7c296a31d..69a5e7774 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -622,11 +622,7 @@ class User < ApplicationRecord def validate_feed_url return if feed_url.blank? - valid = if FeatureFlag.enabled?(:feeds_import) - Feeds::ValidateUrl.call(feed_url) - else - RssReader.new.valid_feed_url?(feed_url) - end + valid = Feeds::ValidateUrl.call(feed_url) errors.add(:feed_url, "is not a valid RSS/Atom feed") unless valid rescue StandardError => e diff --git a/app/services/feature_flag.rb b/app/services/feature_flag.rb index 0e3ed0add..2a1681916 100644 --- a/app/services/feature_flag.rb +++ b/app/services/feature_flag.rb @@ -1,6 +1,6 @@ module FeatureFlag class << self - delegate :disable, :enable, :enabled?, :exist?, to: Flipper + delegate :disable, :enable, :enabled?, :exist?, :remove, to: Flipper def accessible?(feature_flag_name, *args) feature_flag_name.blank? || !exist?(feature_flag_name) || enabled?(feature_flag_name, *args) diff --git a/app/services/rss_reader/assembler.rb b/app/services/feeds/assemble_article_markdown.rb similarity index 98% rename from app/services/rss_reader/assembler.rb rename to app/services/feeds/assemble_article_markdown.rb index a09905dfc..51bab88aa 100644 --- a/app/services/rss_reader/assembler.rb +++ b/app/services/feeds/assemble_article_markdown.rb @@ -1,7 +1,7 @@ -class RssReader - class Assembler +module Feeds + class AssembleArticleMarkdown def self.call(item, user, feed, feed_source_url) - new(item, user, feed, feed_source_url).assemble + new(item, user, feed, feed_source_url).call end def initialize(item, user, feed, feed_source_url) @@ -13,7 +13,7 @@ class RssReader @feed_source_url = feed_source_url end - def assemble + def call body = <<~HEREDOC --- title: #{@title} diff --git a/app/services/feeds/import.rb b/app/services/feeds/import.rb index 0fccd9bda..bac4013b4 100644 --- a/app/services/feeds/import.rb +++ b/app/services/feeds/import.rb @@ -29,8 +29,8 @@ module Feeds # NOTE: doing this sequentially to avoid locking problems with the DB # and unnecessary conflicts articles = feedjira_objects.flat_map do |user_id, feed| - # TODO: replace `feed` with `feed.url` as `RssReader::Assembler` - # only actually needs feed.url + # TODO: replace `feed` with `feed.url` as `Feeds::AssembleArticleMarkdown` + # only actually uses feed.url user = batch_of_users.detect { |u| u.id == user_id } DatadogStatsClient.time("feeds::import::create_articles_from_user_feed", tags: ["user_id:#{user_id}"]) do @@ -118,7 +118,7 @@ module Feeds result.compact.to_h end - # TODO: currently this is exactly as in RSSReader, but we might find + # TODO: currently this is exactly as it was in the RssReader, but we might find # avenues for optimization, like: # 1. why are we sending N exists query to the DB, one per each item, can we fetch them all? # 2. should we queue a batch of workers to create articles, but then, following issues ensue: @@ -137,7 +137,7 @@ module Feeds user_id: user.id, published_from_feed: true, show_comments: true, - body_markdown: RssReader::Assembler.call(item, user, feed, feed_source_url), + body_markdown: Feeds::AssembleArticleMarkdown.call(item, user, feed, feed_source_url), organization_id: nil, ) diff --git a/app/services/rss_reader.rb b/app/services/rss_reader.rb deleted file mode 100644 index 0ac32f1c7..000000000 --- a/app/services/rss_reader.rb +++ /dev/null @@ -1,100 +0,0 @@ -class RssReader - def self.get_all_articles(force: true) - new.get_all_articles(force: force) - end - - def get_all_articles(force: true) - articles = [] - - User.where.not(feed_url: [nil, ""]).find_each do |user| - # unless forced, fetch sparingly - next if force == false && (rand(2) == 1 || user.feed_fetched_at > 15.minutes.ago) - - user_articles = create_articles_for_user(user) - articles.concat(user_articles) if user_articles - end - - articles - end - - def fetch_user(user) - create_articles_for_user(user) - end - - def valid_feed_url?(link) - true if fetch_rss(link) - rescue StandardError - false - end - - private - - def create_articles_for_user(user) - user.update_column(:feed_fetched_at, Time.current) - feed = fetch_rss(user.feed_url.strip) - - articles = [] - - feed.entries.reverse_each do |item| - article = make_from_rss_item(item, user, feed) - articles.append(article) - rescue StandardError => e - report_error( - e, - rss_reader_info: { - username: user.username, - feed_url: user.feed_url, - item_count: item_count_error(feed), - error: "RssReaderError: occurred while creating article #{item.url}" - }, - ) - end - - articles - rescue StandardError => e - report_error( - e, - rss_reader_info: { - username: user.username, - feed_url: user.feed_url, - item_count: item_count_error(feed), - error_message: "RssReaderError: occurred while fetching feed" - }, - ) - [] - end - - def fetch_rss(url) - xml = HTTParty.get(url).body - Feedjira.parse xml - end - - def make_from_rss_item(item, user, feed) - return if Feeds::CheckItemMediumReply.call(item) || Feeds::CheckItemPreviouslyImported.call(item, user) - - feed_source_url = item.url.strip.split("?source=")[0] - article = Article.create!( - feed_source_url: feed_source_url, - user_id: user.id, - published_from_feed: true, - show_comments: true, - body_markdown: RssReader::Assembler.call(item, user, feed, feed_source_url), - organization_id: nil, - ) - - Slack::Messengers::ArticleFetchedFeed.call(article: article) - - article - end - - def report_error(error, metadata) - Honeybadger.context(metadata) - Honeybadger.notify(error) - end - - def item_count_error(feed) - return "NIL FEED, INVALID URL" unless feed - - feed.entries ? feed.entries.length : "no count" - end -end diff --git a/app/workers/articles/rss_reader_worker.rb b/app/workers/articles/rss_reader_worker.rb deleted file mode 100644 index 1cfae57de..000000000 --- a/app/workers/articles/rss_reader_worker.rb +++ /dev/null @@ -1,21 +0,0 @@ -module Articles - class RssReaderWorker - include Sidekiq::Worker - - sidekiq_options queue: :medium_priority, retry: 10 - - def perform - # Temporary - # @sre:mstruve This is temporary until we have an efficient way to handle this job - # for our large DEV community. Smaller Forems should be able to handle it no problem - return if SiteConfig.dev_to? - - if FeatureFlag.enabled?(:feeds_import) - ::Feeds::ImportArticlesWorker.perform_async(4.hours.ago) - else - # don't force fetch. Fetch "random" subset instead of all of them. - ::RssReader.get_all_articles(force: false) - end - end - end -end diff --git a/app/workers/feeds/import_articles_worker.rb b/app/workers/feeds/import_articles_worker.rb index 77a1ee66d..795a1e298 100644 --- a/app/workers/feeds/import_articles_worker.rb +++ b/app/workers/feeds/import_articles_worker.rb @@ -4,8 +4,19 @@ module Feeds sidekiq_options queue: :medium_priority, retry: 10 - def perform(earlier_than, user_ids = []) - users = user_ids.present? ? User.where(id: user_ids) : nil + # NOTE: [@rhymes] we need to default earlier_than to `nil` because sidekiq-cron, + # by using YAML to define jobs arguments does not support datetimes evaluated + # at runtime + def perform(earlier_than = nil, user_ids = []) + if user_ids.present? + users = User.where(id: user_ids) + # we assume that forcing a single import should not take into account + # the last time a feed was fetched at + earlier_than = nil + else + users = nil + earlier_than ||= 4.hours.ago + end ::Feeds::Import.call(users: users, earlier_than: earlier_than) end diff --git a/app/workers/rss_reader_fetch_user_worker.rb b/app/workers/rss_reader_fetch_user_worker.rb deleted file mode 100644 index b014cc2a6..000000000 --- a/app/workers/rss_reader_fetch_user_worker.rb +++ /dev/null @@ -1,11 +0,0 @@ -class RssReaderFetchUserWorker - include Sidekiq::Worker - - sidekiq_options queue: :medium_priority, lock: :until_executed - - def perform(user_id) - user = User.find_by(id: user_id) - - RssReader.new.fetch_user(user) if user&.feed_url.present? - end -end diff --git a/config/initializers/honeybadger.rb b/config/initializers/honeybadger.rb index 807c2bb33..1d7b8745d 100644 --- a/config/initializers/honeybadger.rb +++ b/config/initializers/honeybadger.rb @@ -35,7 +35,7 @@ Honeybadger.configure do |config| config.breadcrumbs.enabled = true config.before_notify do |notice| - notice.fingerprint = if notice.error_message&.include?("SIGTERM") && notice.component&.include?("fetch_all_rss") + notice.fingerprint = if notice.error_message&.include?("SIGTERM") && notice.component&.include?("feeds_import") notice.error_message elsif (msg_key = MESSAGE_FINGERPRINTS.keys.detect do |k, _v| notice.error_message&.include?(k) diff --git a/config/schedule.yml b/config/schedule.yml index 205027c75..7661a3c99 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -1,6 +1,6 @@ -fetch_all_rss: +feeds_import: cron: "20 * * * *" # every hour, 20 min after the hour - class: "Articles::RssReaderWorker" + class: "Feeds::ImportArticlesWorker" log_worker_queue_stats: cron: "*/10 * * * *" # every 10 minutes class: "Metrics::RecordBackgroundQueueStatsWorker" diff --git a/lib/data_update_scripts/20210107151449_remove_feeds_import_feature_flag.rb b/lib/data_update_scripts/20210107151449_remove_feeds_import_feature_flag.rb new file mode 100644 index 000000000..8996a8405 --- /dev/null +++ b/lib/data_update_scripts/20210107151449_remove_feeds_import_feature_flag.rb @@ -0,0 +1,7 @@ +module DataUpdateScripts + class RemoveFeedsImportFeatureFlag + def run + FeatureFlag.remove(:feeds_import) + end + end +end diff --git a/lib/tasks/fetch.rake b/lib/tasks/fetch.rake index b45c53cc0..15241d31f 100644 --- a/lib/tasks/fetch.rake +++ b/lib/tasks/fetch.rake @@ -1,18 +1,3 @@ -desc "This task is called by the Heroku scheduler add-on" - -task fetch_all_rss: :environment do - if FeatureFlag.enabled?(:feeds_import) - Feeds::Import.call(earlier_than: 4.hours.ago) - else - # don't force fetch. Fetch "random" subset instead of all of them. - RssReader.get_all_articles(force: false) - end -end - -task fetch_feeds_import: :environment do - Feeds::Import.call(earlier_than: 4.hours.ago) -end - # Temporary # @sre:mstruve This is temporary until we have an efficient way to handle this task # in Sidekiq for our large DEV community. diff --git a/spec/lib/data_update_scripts/remove_feeds_import_feature_flag_spec.rb b/spec/lib/data_update_scripts/remove_feeds_import_feature_flag_spec.rb new file mode 100644 index 000000000..b47fcb596 --- /dev/null +++ b/spec/lib/data_update_scripts/remove_feeds_import_feature_flag_spec.rb @@ -0,0 +1,20 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20210107151449_remove_feeds_import_feature_flag.rb", +) + +describe DataUpdateScripts::RemoveFeedsImportFeatureFlag do + it "removes the :feeds_import flag" do + FeatureFlag.enable(:feeds_import) + + described_class.new.run + + expect(FeatureFlag.exist?(:feeds_import)).to be(false) + end + + it "works if the flag is not available" do + described_class.new.run + + expect(FeatureFlag.exist?(:feeds_import)).to be(false) + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4d9f711b4..f20317897 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -278,31 +278,7 @@ RSpec.describe User, type: :model do expect(limiter).to have_received(:track_limit_by_action).with(:user_update).twice end - context "when validating feed_url with RSSReader", vcr: true do - it "is valid with no feed_url" do - user.feed_url = nil - - expect(user).to be_valid - end - - it "is not valid with an invalid feed_url", vcr: { cassette_name: "feeds_validate_url_invalid" } do - user.feed_url = "http://example.com" - - expect(user).not_to be_valid - end - - it "is valid with a valid feed_url", vcr: { cassette_name: "feeds_import_medium_vaidehi" } do - user.feed_url = "https://medium.com/feed/@vaidehijoshi" - - expect(user).to be_valid - end - end - - context "with Feeds::ValidateUrl" do - before do - allow(FeatureFlag).to receive(:enabled?).with(:feeds_import).and_return(true) - end - + context "when validating feed_url", vcr: true do it "is valid with no feed_url" do user.feed_url = nil diff --git a/spec/requests/user/user_settings_spec.rb b/spec/requests/user/user_settings_spec.rb index 3059880b6..388df62fa 100644 --- a/spec/requests/user/user_settings_spec.rb +++ b/spec/requests/user/user_settings_spec.rb @@ -308,25 +308,12 @@ RSpec.describe "UserSettings", type: :request do let(:feed_url) { "https://medium.com/feed/@vaidehijoshi" } let(:user) { create(:user, feed_url: feed_url) } - it "invokes RssReaderFetchUserWorker" do + it "invokes Feeds::ImportArticlesWorker" do allow(Feeds::ImportArticlesWorker).to receive(:perform_async).with(nil, user.id) - allow(RssReaderFetchUserWorker).to receive(:perform_async).with(user.id) - - put user_path(user.id), params: { user: { feed_url: feed_url } } - - expect(Feeds::ImportArticlesWorker).not_to have_received(:perform_async) - expect(RssReaderFetchUserWorker).to have_received(:perform_async).with(user.id) - end - - it "invokes Feeds::ImportArticlesWorker if feeds_import feature flag is on" do - allow(Feeds::ImportArticlesWorker).to receive(:perform_async).with(nil, user.id) - allow(RssReaderFetchUserWorker).to receive(:perform_async).with(user.id) - allow(FeatureFlag).to receive(:enabled?).with(:feeds_import).and_return(true) put user_path(user.id), params: { user: { feed_url: feed_url } } expect(Feeds::ImportArticlesWorker).to have_received(:perform_async).with(nil, user.id) - expect(RssReaderFetchUserWorker).not_to have_received(:perform_async).with(user.id) end end end diff --git a/spec/services/rss_reader_spec.rb b/spec/services/rss_reader_spec.rb deleted file mode 100644 index 969378573..000000000 --- a/spec/services/rss_reader_spec.rb +++ /dev/null @@ -1,191 +0,0 @@ -require "rails_helper" -require "rss" - -RSpec.describe RssReader, type: :service, vcr: true, db_strategy: :truncation do - self.use_transactional_tests = false - - let(:link) { "https://medium.com/feed/@vaidehijoshi" } - let(:nonmedium_link) { "https://circleci.com/blog/feed.xml" } - let(:nonpermanent_link) { "https://medium.com/feed/@macsiri/" } - let!(:rss_reader) { described_class.new } - - describe "#get_all_articles" do - before do - [link, nonmedium_link, nonpermanent_link].each do |feed_url| - create(:user, feed_url: feed_url) - end - end - - it "fetch only articles from a feed_url", vcr: { cassette_name: "rss_reader_fetch_articles" } do - articles = rss_reader.get_all_articles - - # the result within the approval file depends on the feed - # not fetching comments is baked into this - verify(format: :txt) { articles.length } - end - - it "does not recreate articles if they already exist", vcr: { cassette_name: "rss_reader_fetch_articles_twice" } do - rss_reader.get_all_articles - - expect { rss_reader.get_all_articles }.not_to change(Article, :count) - end - - it "parses correctly", vcr: { cassette_name: "rss_reader_fetch_articles" } do - rss_reader.get_all_articles - - verify format: :txt do - User.find_by(feed_url: nonpermanent_link).articles.first.body_markdown - end - end - - it "sets feed_fetched_at to the current time", vcr: { cassette_name: "rss_reader_fetch_articles" } do - Timecop.freeze(Time.current) do - rss_reader.get_all_articles - - user = User.find_by(feed_url: nonpermanent_link) - feed_fetched_at = user.feed_fetched_at - expect(feed_fetched_at.to_i).to eq(Time.current.to_i) - end - end - - it "does refetch same user over and over by default", vcr: { cassette_name: "rss_reader_fetch_multiple_times" } do - user = User.find_by(feed_url: nonpermanent_link) - - Timecop.freeze(Time.current) do - user.update_columns(feed_fetched_at: Time.current) - - fetched_at_time = user.reload.feed_fetched_at - - # travel a few seconds in the future to simulate a new time - 3.times do |i| - Timecop.travel((i + 5).seconds.from_now) do - rss_reader.get_all_articles - end - end - - expect(user.reload.feed_fetched_at > fetched_at_time).to be(true) - end - end - - it "reports an article creation error" do - allow(rss_reader).to receive(:make_from_rss_item).and_raise(StandardError) - allow(Honeybadger).to receive(:notify) - - rss_reader.get_all_articles - - expect(Honeybadger).to have_received(:notify).at_least(:once) - end - - it "reports a fetching error" do - allow(rss_reader).to receive(:fetch_rss).and_raise(StandardError) - allow(Honeybadger).to receive(:notify) - - rss_reader.get_all_articles - - expect(Honeybadger).to have_received(:notify).at_least(:once) - end - - it "queues as many slack messages as there are articles", vcr: { cassette_name: "rss_reader_fetch_articles" } do - old_count = Slack::Messengers::Worker.jobs.count - articles = rss_reader.get_all_articles - expect(Slack::Messengers::Worker.jobs.count).to eq(old_count + articles.length) - end - end - - context "when feed_referential_link is false" do - it "does not self-reference links for user" do - # Article.find_by is used by find_and_replace_possible_links! - # checking its invocation is a shortcut to testing the functionality. - allow(Article).to receive(:find_by).and_call_original - - create(:user, feed_url: nonpermanent_link, feed_referential_link: false) - - rss_reader.get_all_articles - - expect(Article).not_to have_received(:find_by) - end - end - - describe "#fetch_user", vcr: { cassette_name: "rss_reader_fetch_medium_feed" } do - before do - [link, nonmedium_link, nonpermanent_link].each do |feed_url| - create(:user, feed_url: feed_url) - end - end - - it "gets articles for user" do - articles = rss_reader.fetch_user(User.find_by(feed_url: link)) - - # the result within the approval file depends on the feed - verify(format: :txt) { articles.length } - end - - it "does not set featured_number" do - user = User.find_by(feed_url: link) - rss_reader.fetch_user(user) - - expect(user.articles.select(&:featured_number)).to be_empty - end - - it "reports an article creation error on the standard logger" do - allow(rss_reader).to receive(:make_from_rss_item).and_raise(StandardError) - allow(Honeybadger).to receive(:notify) - - rss_reader.fetch_user(User.find_by(feed_url: link)) - - expect(Honeybadger).to have_received(:notify).at_least(:once) - end - - it "reports a fetching error on the standard logger" do - allow(rss_reader).to receive(:fetch_rss).and_raise(StandardError) - allow(Honeybadger).to receive(:notify) - - rss_reader.fetch_user(User.find_by(feed_url: link)) - - expect(Honeybadger).to have_received(:notify).at_least(:once) - end - - it "queues as many slack messages as there are user articles" do - old_count = Slack::Messengers::Worker.jobs.count - articles = rss_reader.fetch_user(User.find_by(feed_url: link)) - expect(Slack::Messengers::Worker.jobs.count).to eq(old_count + articles.length) - end - end - - describe "#valid_feed_url?" do - it "returns true on valid feed url" do - expect(rss_reader.valid_feed_url?(link)).to be(true) - end - - it "returns false on invalid feed url" do - bad_link = "www.google.com" - expect(rss_reader.valid_feed_url?(bad_link)).to be(false) - end - end - - describe "feeds parsing and regressions" do - it "parses https://medium.com/feed/@dvirsegal correctly", vcr: { cassette_name: "rss_reader_dvirsegal" } do - user = create(:user, feed_url: "https://medium.com/feed/@dvirsegal") - - expect do - rss_reader.fetch_user(user) - end.to change(user.articles, :count).by(10) - end - - it "converts/replaces tags to ", vcr: { cassette_name: "rss_reader_swimburger" } do - user = create(:user, feed_url: "https://swimburger.net/atom.xml") - - expect do - rss_reader.fetch_user(user) - end.to change(user.articles, :count).by(10) - - body_markdown = user.articles.last.body_markdown - - expect(body_markdown).not_to include("") - expected_image_markdown = - "![Screenshot of Azure left navigation pane](https://swimburger.net/media/lxypkhak/azure-create-a-resource.png)" - - expect(body_markdown).to include(expected_image_markdown) - end - end -end diff --git a/spec/workers/articles/rss_reader_worker_spec.rb b/spec/workers/articles/rss_reader_worker_spec.rb deleted file mode 100644 index e283f6a3b..000000000 --- a/spec/workers/articles/rss_reader_worker_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -require "rails_helper" - -RSpec.describe Articles::RssReaderWorker, type: :worker do - let(:worker) { subject } - - include_examples "#enqueues_on_correct_queue", "medium_priority" - - describe "#perform" do - it "instructs RssReader to fetch all articles" do - allow(RssReader).to receive(:get_all_articles) - - worker.perform - - expect(RssReader).to have_received(:get_all_articles).with(force: false) - end - - it "does not enqueue Feeds::ImportArticlesWorker if the :feeds_import flag is not enabled" do - allow(RssReader).to receive(:get_all_articles) - - sidekiq_assert_no_enqueued_jobs do - worker.perform - end - - expect(RssReader).to have_received(:get_all_articles).with(force: false) - end - - it "enqueues Feeds::ImportArticlesWorker if the :feeds_import flag is enabled" do - allow(Feeds::ImportArticlesWorker).to receive(:perform_async) - allow(RssReader).to receive(:get_all_articles) - allow(FeatureFlag).to receive(:enabled?).with(:feeds_import).and_return(true) - - Timecop.freeze(Time.current) do - worker.perform - - expect(RssReader).not_to have_received(:get_all_articles) - expect(Feeds::ImportArticlesWorker).to have_received(:perform_async).with(4.hours.ago) - end - end - - it "short circuits if it's running on DEV" do - allow(SiteConfig).to receive(:dev_to?).and_return(true) - allow(RssReader).to receive(:get_all_articles) - allow(FeatureFlag).to receive(:enabled?) - - worker.perform - - expect(RssReader).not_to have_received(:get_all_articles) - expect(FeatureFlag).not_to have_received(:enabled?) - end - end -end diff --git a/spec/workers/feeds/import_articles_worker_spec.rb b/spec/workers/feeds/import_articles_worker_spec.rb index 26c29a05a..b0858fbbf 100644 --- a/spec/workers/feeds/import_articles_worker_spec.rb +++ b/spec/workers/feeds/import_articles_worker_spec.rb @@ -6,35 +6,34 @@ RSpec.describe Feeds::ImportArticlesWorker, type: :worker do include_examples "#enqueues_on_correct_queue", "medium_priority" describe "#perform" do - it "calls the Feeds::Import to get all articles" do + it "calls the Feeds::Import defaulting to 4 hours ago" do allow(Feeds::Import).to receive(:call) - worker.perform(1.hour.ago) + Timecop.freeze(Time.current) do + worker.perform - expect(Feeds::Import).to have_received(:call) - end - - context "with user ids" do - it "calls Feeds::Import with the correct users if given user ids" do - user = create(:user) - allow(Feeds::Import).to receive(:call) - - worker.perform(nil, [user.id]) - - expect(Feeds::Import).to have_received(:call).with(users: User.where(id: [user.id]), earlier_than: nil) + expect(Feeds::Import).to have_received(:call).with(users: nil, earlier_than: 4.hours.ago) end end - context "with earlier_than time" do - it "calls Feeds::Import with the correct time if given" do - allow(Feeds::Import).to receive(:call) + it "calls the Feeds::Import with the given time" do + allow(Feeds::Import).to receive(:call) - Timecop.freeze(Time.current) do - worker.perform(4.hours.ago) + Timecop.freeze(Time.current) do + worker.perform(1.minute.ago) - expect(Feeds::Import).to have_received(:call).with(users: nil, earlier_than: 4.hours.ago) - end + expect(Feeds::Import).to have_received(:call).with(users: nil, earlier_than: 1.minute.ago) end end + + it "calls Feeds::Import with the users from the given user ids and no time" do + user = create(:user) + + allow(Feeds::Import).to receive(:call) + + worker.perform(nil, [user.id]) + + expect(Feeds::Import).to have_received(:call).with(users: User.where(id: [user.id]), earlier_than: nil) + end end end diff --git a/spec/workers/rss_reader_fetch_user_worker_spec.rb b/spec/workers/rss_reader_fetch_user_worker_spec.rb deleted file mode 100644 index 0e95d1913..000000000 --- a/spec/workers/rss_reader_fetch_user_worker_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require "rails_helper" - -RSpec.describe RssReaderFetchUserWorker, type: :worker do - let(:worker) { subject } - - # passing in a random user_id argument since the worker itself won't be executed - include_examples "#enqueues_on_correct_queue", "medium_priority", [456] - - describe "#perform_now" do - let(:rss_reader_service) { instance_double(RssReader) } - - before do - allow(RssReader).to receive(:new).and_return(rss_reader_service) - allow(rss_reader_service).to receive(:fetch_user) - end - - context "when user found and feed_url present" do - let(:user) { double } - - before do - allow(User).to receive(:find_by).and_return(user) - allow(user).to receive(:feed_url).and_return(:feed_url) - allow(user).to receive(:id) - end - - it "calls the service" do - worker.perform(user.id) - expect(rss_reader_service).to have_received(:fetch_user).with(user).once - end - end - - context "when no user found" do - it "does not call the service" do - allow(User).to receive(:find_by) - worker.perform(9999) - expect(rss_reader_service).not_to have_received(:fetch_user) - end - end - end -end