diff --git a/app/services/articles/feeds/weighted_query_strategy.rb b/app/services/articles/feeds/weighted_query_strategy.rb index ccd8e3714..39af4e00c 100644 --- a/app/services/articles/feeds/weighted_query_strategy.rb +++ b/app/services/articles/feeds/weighted_query_strategy.rb @@ -273,6 +273,7 @@ module Articles user: @user, days_since_published: @days_since_published, ) + @must_have_main_image = true end # The goal of this query is to generate a list of articles that @@ -363,6 +364,14 @@ module Articles # # @note I really dislike this method name as it is opaque on # it's purpose. + # @note We're specifically In the LargeForemExperimental implementation, the + # default home feed omits the featured story. In this + # case, I don't want to do that. Instead, I want to see + # how this behaves. + def default_home_feed(**) + call + end + alias more_comments_minimal_weight_randomized call # The featured story should be the article that: @@ -374,8 +383,6 @@ module Articles # the `featured = true` attribute. In my envisioned # implementation, the pagination would omit the featured story. # - # @param must_have_main_image [Boolean] do we mandate that the - # featured story/stories require an image? # @return [Array] a featured story # Article and an array of Article objects. # @@ -400,33 +407,24 @@ module Articles # in the featured story. For non-signed in users, we may # want to use a completely different set of scoring # methods. - def featured_story_and_default_home_feed(must_have_main_image: true, **) + def featured_story_and_default_home_feed(**) # We could parameterize this, but callers would need to # consider the impact of that decision, and it would break the # current contract. number_of_featured_stories = 1 featured_story = call( only_featured: true, - must_have_main_image: must_have_main_image, + must_have_main_image: @must_have_main_image, limit: number_of_featured_stories, offset: 0, ).first articles = call( - must_have_main_image: must_have_main_image, # Make sure that we don't include the featured_story omit_article_ids: [featured_story&.id], ) [featured_story, articles] end - # @note In the LargeForemExperimental implementation, the - # default home feed omits the featured story. In this - # case, I don't want to do that. Instead, I want to see - # how this behaves. - def default_home_feed(must_have_main_image: true, **) - call(must_have_main_image: must_have_main_image) - end - private # Concatenate the required group by clauses. diff --git a/spec/services/articles/feeds/weighted_query_strategy_spec.rb b/spec/services/articles/feeds/weighted_query_strategy_spec.rb index 58bcaca30..3af7160e4 100644 --- a/spec/services/articles/feeds/weighted_query_strategy_spec.rb +++ b/spec/services/articles/feeds/weighted_query_strategy_spec.rb @@ -3,6 +3,17 @@ require "rails_helper" RSpec.describe Articles::Feeds::WeightedQueryStrategy, type: :service do subject(:feed_strategy) { described_class.new(user: user) } + let(:user) { nil } + + describe "#default_home_feed" do + # This test helps test the common interface between the + # WeightedQueryStrategy and the LargeForemExperimental + it "receives `user_signed_in: false` and behaves" do + response = feed_strategy.default_home_feed(user_signed_in: false) + expect(response).to be_a(ActiveRecord::Relation) + end + end + describe "with a nil user" do let(:user) { nil } @@ -16,7 +27,7 @@ RSpec.describe Articles::Feeds::WeightedQueryStrategy, type: :service do it "#call is successful with parameterization" do # NOTE: I'm not testing the SQL logic, merely that the SQL is # valid. - response = feed_strategy.call(only_featured: true, must_have_main_image: true) + response = feed_strategy.call(only_featured: true) expect(response).to be_a(ActiveRecord::Relation) expect(response).to match_array([]) end @@ -62,7 +73,7 @@ RSpec.describe Articles::Feeds::WeightedQueryStrategy, type: :service do it "#call is successful with parameterization" do # NOTE: I'm not testing the SQL logic, merely that the SQL is # valid. - response = feed_strategy.call(only_featured: true, must_have_main_image: true) + response = feed_strategy.call(only_featured: true) expect(response).to be_a(ActiveRecord::Relation) expect(response).to match_array([]) end