Guarantee articles in main article feed (#6490) [deploy]

* Adjust feed offset, remove time limits for main feed

* Improve offset construction readability

app/services/articles/feed.rb

@@ -1,5 +1,7 @@
 module Articles
   class Feed
+    RANDOM_OFFSET_VALUES = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 5, 6, 7, 8, 9, 10, 11].freeze
+
     def initialize(user: nil, number_of_articles: 35, page: 1, tag: nil)
       @user = user
       @number_of_articles = number_of_articles
@@ -137,10 +139,10 @@ module Articles
         order("hotness_score DESC")
       featured_story = hot_stories.where.not(main_image: nil).first
       if user_signed_in
-        offset = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 5, 6, 7, 8, 9, 10, 11].sample # random offset, weighted more towards zero
+        offset = RANDOM_OFFSET_VALUES.select { |i| i < hot_stories.count }.sample # random offset, weighted more towards zero
         hot_stories = hot_stories.offset(offset)
         new_stories = Article.published.
-          where("published_at > ? AND score > ?", rand(2..6).hours.ago, -15).
+          where("score > ?", -15).
           limited_column_select.order("published_at DESC").limit(rand(15..80))
         hot_stories = hot_stories.to_a + new_stories.to_a
       end

spec/services/articles/feed_spec.rb

@@ -77,12 +77,10 @@ RSpec.describe Articles::Feed, type: :service do
       let(:featured_story) { result.first }
       let(:stories) { result.second }
 
-      it "only includes stories from less than 6 hours ago" do
-        expect(stories).not_to include(old_story)
-        expect(stories).not_to include(article)
-
-        # Ideally we'd test for hot_story in the stories list, but the random offset selection makes that random
-        expect(featured_story).to eq(hot_story)
+      it "only includes stories" do
+        expect(stories).to include(old_story)
+        expect(stories).to include(article)
+        expect(stories).to include(hot_story)
       end
     end
 
@@ -132,8 +130,8 @@ RSpec.describe Articles::Feed, type: :service do
     context "when user logged in" do
       let(:stories) { feed.default_home_feed(user_signed_in: true) }
 
-      it "includes stories from between 2 and 6 hours ago" do
-        expect(stories).not_to include(old_story)
+      it "includes stories " do
+        expect(stories).to include(old_story)
         expect(stories).to include(new_story)
       end
     end
@@ -143,8 +141,8 @@ RSpec.describe Articles::Feed, type: :service do
     let!(:new_story) { create(:article, published_at: 10.minutes.ago, score: 10) }
     let(:stories) { feed.default_home_feed_with_more_randomness }
 
-    it "includes stories from between 2 and 6 hours ago" do
-      expect(stories).not_to include(old_story)
+    it "includes stories" do
+      expect(stories).to include(old_story)
       expect(stories).to include(new_story)
     end
   end
@@ -153,8 +151,8 @@ RSpec.describe Articles::Feed, type: :service do
     let!(:new_story) { create(:article, published_at: 10.minutes.ago, score: 10) }
     let(:stories) { feed.mix_default_and_more_random }
 
-    it "includes stories from between 2 and 6 hours ago" do
-      expect(stories).not_to include(old_story)
+    it "includes stories" do
+      expect(stories).to include(old_story)
       expect(stories).to include(new_story)
     end
   end
@@ -163,8 +161,8 @@ RSpec.describe Articles::Feed, type: :service do
     let!(:new_story) { create(:article, published_at: 10.minutes.ago, score: 10) }
     let(:stories) { feed.more_tag_weight }
 
-    it "includes stories from between 2 and 6 hours ago" do
-      expect(stories).not_to include(old_story)
+    it "includes stories" do
+      expect(stories).to include(old_story)
       expect(stories).to include(new_story)
     end
   end
@@ -173,8 +171,8 @@ RSpec.describe Articles::Feed, type: :service do
     let!(:new_story) { create(:article, published_at: 10.minutes.ago, score: 10) }
     let(:stories) { feed.more_tag_weight_more_random }
 
-    it "includes stories from between 2 and 6 hours ago" do
-      expect(stories).not_to include(old_story)
+    it "includes stories" do
+      expect(stories).to include(old_story)
       expect(stories).to include(new_story)
     end
   end
@@ -375,6 +373,52 @@ RSpec.describe Articles::Feed, type: :service do
     end
   end
 
+  describe ".globally_hot_articles" do
+    let!(:recently_published_article) { create(:article, published_at: 3.hours.ago) }
+    let(:globally_hot_articles) { feed.globally_hot_articles(true).second }
+
+    it "returns hot stories" do
+      expect(globally_hot_articles).not_to be_empty
+    end
+
+    it "returns recent stories" do
+      expect(globally_hot_articles).to include(recently_published_article)
+    end
+
+    context "when low number of hot stories and no recently published articles" do
+      before do
+        Article.delete_all
+        create(:article, hotness_score: 1000, score: 1000, published_at: 3.hours.ago)
+      end
+
+      # This test handles a situation in which there are a low number of hot or new stories, and the user is logged in.
+      # Previously the offest factor could result in zero stories being returned sometimes.
+
+      # We manually called `feed.globally_hot_articles` here because `let` caches it!
+      it "still returns articles" do
+        empty_feed = false
+        20.times do
+          if feed.globally_hot_articles(true).second.empty?
+            empty_feed = true
+            break
+          end
+        end
+        expect(empty_feed).to be false
+      end
+    end
+
+    context "when now hot stories and no recently published articles" do
+      before do
+        Article.delete_all
+        create(:article, hotness_score: 0, score: 0, published_at: 3.days.ago)
+      end
+
+      it "still returns articles" do
+        expect(globally_hot_articles).not_to be_empty
+      end
+    end
+  end
+
   describe ".find_featured_story" do
     let(:featured_story) { described_class.find_featured_story(stories) }