Parameterizing featured story requiring main image (#15333)

* Parameterizing featured story requiring main image

This begins to ease the resolution of #15292.

If we merge #15240, I could see setting an `Article` constant for this
value and allowing the administrator to choose the particular behavior.

Once we have insight from the product team, we can move forward with a
more comprehensive solution.

* Adding default parameter

While this is an `@api private` method, I am aligning the defaults with
it's `@api public` caller.  Yes, only specs call the method, but I'd
rather not fiddle with the specs at this moment in time.

* Reworking logic to be more scannable

app/services/articles/feeds/find_featured_story.rb

@@ -1,12 +1,23 @@
 module Articles
   module Feeds
     module FindFeaturedStory
-      def self.call(stories)
+      # @param stories [ActiveRecord::Relation, #detect] pick a
+      #        featured story from this enumerable
+      # @param must_have_main_image [Boolean] if true, the featured
+      #        story must have a main image
+      # @return [Article]
+      #
+      # @note the must_have_main_image parameter name matches PR #15240
+      def self.call(stories, must_have_main_image: true)
         featured_story =
-          if stories.is_a?(ActiveRecord::Relation)
-            stories.where.not(main_image: nil).first
+          if must_have_main_image
+            if stories.is_a?(ActiveRecord::Relation)
+              stories.where.not(main_image: nil).first
+            else
+              stories.detect { |story| story.main_image.present? }
+            end
           else
-            stories.detect { |story| story.main_image.present? }
+            stories.first
           end
 
         featured_story || Article.new

app/services/articles/feeds/large_forem_experimental.rb

@@ -14,8 +14,15 @@ module Articles
         stories
       end
 
-      def featured_story_and_default_home_feed(user_signed_in: false, ranking: true)
-        featured_story, hot_stories = globally_hot_articles(user_signed_in)
+      # @param user_signed_in [Boolean] are we treating this as an
+      #        anonymous user?
+      # @param ranking [Boolean] if true, apply a ranking algorithm
+      # @param must_have_main_image [Boolean] if true, the featured
+      #        story must have a main image
+      #
+      # @note the must_have_main_image parameter name matches PR #15240
+      def featured_story_and_default_home_feed(user_signed_in: false, ranking: true, must_have_main_image: true)
+        featured_story, hot_stories = globally_hot_articles(user_signed_in, must_have_main_image: must_have_main_image)
         hot_stories = rank_and_sort_articles(hot_stories) if @user && ranking
         [featured_story, hot_stories]
       end
@@ -63,11 +70,13 @@ module Articles
                to: :@article_score_applicator)
 
       # @api private
-      def globally_hot_articles(user_signed_in, article_score_threshold: -15, min_rand_limit: 15, max_rand_limit: 80)
+      # rubocop:disable Layout/LineLength
+      def globally_hot_articles(user_signed_in, must_have_main_image: true, article_score_threshold: -15, min_rand_limit: 15, max_rand_limit: 80)
+        # rubocop:enable Layout/LineLength
         if user_signed_in
           hot_stories = experimental_hot_story_grab
           hot_stories = hot_stories.where.not(user_id: UserBlock.cached_blocked_ids_for_blocker(@user.id))
-          featured_story = hot_stories.where.not(main_image: nil).first
+          featured_story = featured_story_from(stories: hot_stories, must_have_main_image: must_have_main_image)
           new_stories = Article.published
             .where("score > ?", article_score_threshold)
             .limited_column_select.includes(top_comments: :user).order(published_at: :desc)
@@ -78,13 +87,19 @@ module Articles
             .page(@page).per(@number_of_articles)
             .where("score >= ? OR featured = ?", Settings::UserExperience.home_feed_minimum_score, true)
             .order(hotness_score: :desc)
-          featured_story = hot_stories.where.not(main_image: nil).first
+          featured_story = featured_story_from(stories: hot_stories, must_have_main_image: must_have_main_image)
         end
         [featured_story, hot_stories.to_a]
       end
 
       private
 
+      def featured_story_from(stories:, must_have_main_image:)
+        return stories.first unless must_have_main_image
+
+        stories.where.not(main_image: nil).first
+      end
+
       def experimental_hot_story_grab
         start_time = [(@user.page_views.second_to_last&.created_at || 7.days.ago) - 18.hours, 7.days.ago].max
         Article.published.limited_column_select.includes(top_comments: :user)

spec/services/articles/feeds/find_featured_story_spec.rb

@@ -8,6 +8,13 @@ RSpec.describe Articles::Feeds::FindFeaturedStory, type: :service do
       featured_story = described_class.call(Article.all)
       expect(featured_story.main_image).not_to be_nil
     end
+
+    context "when must_have_main_image is false" do
+      it "returns the first article" do
+        featured_story = described_class.call(Article.all)
+        expect(featured_story).to be_persisted
+      end
+    end
   end
 
   context "when passed an array" do
@@ -15,6 +22,13 @@ RSpec.describe Articles::Feeds::FindFeaturedStory, type: :service do
       featured_story = described_class.call(Article.all.to_a)
       expect(featured_story.main_image).to be_present
     end
+
+    context "when must_have_main_image is false" do
+      it "returns the first article" do
+        featured_story = described_class.call(Article.all.to_a)
+        expect(featured_story).to be_persisted
+      end
+    end
   end
 
   context "when passed collection without any articles" do