From 1374ab98e639db186bce9b378b81589c6abc5af2 Mon Sep 17 00:00:00 2001 From: Joshua Wehner Date: Fri, 17 Feb 2023 09:29:59 +0100 Subject: [PATCH] Ensure spam doesn't hit logged-out /latest (#19110) * Ensure spam doesn't hit logged-out /latest * Restore load-bearing constant * Narrow selector avoids the new 'latest' banner * Test scenario for signed-in low-score content * Remove false-negative test * Actual test for the logged-in scenario * Signed-in users get poor quality content * Feeds (RSS) use an entirely different query * Pass @stories * Update tests with new minimum score --- app/controllers/stories_controller.rb | 2 +- app/helpers/articles_helper.rb | 7 +++++++ app/services/articles/feeds/latest.rb | 7 +++++-- app/views/stories/_main_stories_feed.html.erb | 9 ++++++++ config/locales/views/articles/en.yml | 1 + config/locales/views/articles/fr.yml | 1 + spec/requests/articles/articles_spec.rb | 7 +++++++ spec/requests/stories/feeds_spec.rb | 20 ++++++++++++++++++ spec/requests/stories_index_spec.rb | 18 ++++++++++++++++ .../user_visits_articles_by_timeframe_spec.rb | 21 ++++++++++++------- 10 files changed, 83 insertions(+), 10 deletions(-) diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 6e2f45e43..7e543741c 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -231,7 +231,7 @@ class StoriesController < ApplicationController if params[:timeframe].in?(Timeframe::FILTER_TIMEFRAMES) @stories = Articles::Feeds::Timeframe.call(params[:timeframe]) elsif params[:timeframe] == Timeframe::LATEST_TIMEFRAME - @stories = Articles::Feeds::Latest.call + @stories = Articles::Feeds::Latest.call(minimum_score: Settings::UserExperience.home_feed_minimum_score) else @default_home_feed = true feed = Articles::Feeds::LargeForemExperimental.new(page: @page, tag: params[:tag]) diff --git a/app/helpers/articles_helper.rb b/app/helpers/articles_helper.rb index dbbdb5732..bd4349a8f 100644 --- a/app/helpers/articles_helper.rb +++ b/app/helpers/articles_helper.rb @@ -1,4 +1,11 @@ module ArticlesHelper + def should_show_latest_spam_suppression?(stories) + return false if user_signed_in? + return false unless stories.size > 1 + + params[:timeframe] == Timeframe::LATEST_TIMEFRAME + end + def sort_options [ [I18n.t("helpers.articles_helper.recently_created"), "creation-desc"], diff --git a/app/services/articles/feeds/latest.rb b/app/services/articles/feeds/latest.rb index 3e0182127..04117b545 100644 --- a/app/services/articles/feeds/latest.rb +++ b/app/services/articles/feeds/latest.rb @@ -3,10 +3,13 @@ module Articles module Latest MINIMUM_SCORE = -20 - def self.call(tag: nil, number_of_articles: Article::DEFAULT_FEED_PAGINATION_WINDOW_SIZE, page: 1) + def self.call(tag: nil, number_of_articles: nil, page: 1, minimum_score: nil) + number_of_articles ||= Article::DEFAULT_FEED_PAGINATION_WINDOW_SIZE + minimum_score ||= MINIMUM_SCORE + Articles::Feeds::Tag.call(tag) .order(published_at: :desc) - .where("score > ?", MINIMUM_SCORE) + .where("score > ?", minimum_score) .page(page) .per(number_of_articles) end diff --git a/app/views/stories/_main_stories_feed.html.erb b/app/views/stories/_main_stories_feed.html.erb index 6ca4a68e8..b2595266a 100644 --- a/app/views/stories/_main_stories_feed.html.erb +++ b/app/views/stories/_main_stories_feed.html.erb @@ -1,3 +1,12 @@ +<% if should_show_latest_spam_suppression?(@stories) %> +
+

+ <%= t "views.articles.sign_in_for_full_latest_html", + link: sign_up_path(state: "new-user") %> +

+
+<% end %> + <% if @pinned_article %> <%= render partial: "articles/single_story", locals: { story: @pinned_article, pinned: true, featured: @pinned_article.id == @featured_story.id } %> <% end %> diff --git a/config/locales/views/articles/en.yml b/config/locales/views/articles/en.yml index 6b50f67d1..b10e01340 100644 --- a/config/locales/views/articles/en.yml +++ b/config/locales/views/articles/en.yml @@ -105,6 +105,7 @@ en: missing: Article No Longer Available published_html: Posted on %{date} scheduled_html: Scheduled for %{date} + sign_in_for_full_latest_html: Some latest posts are only visible for members. Sign in to see all latest. read_next: Read next reading_time: one: "1 min read" diff --git a/config/locales/views/articles/fr.yml b/config/locales/views/articles/fr.yml index 74dbc6863..af9b0b349 100644 --- a/config/locales/views/articles/fr.yml +++ b/config/locales/views/articles/fr.yml @@ -105,6 +105,7 @@ fr: missing: Article No Longer Available published_html: Posted on %{date} scheduled_html: Scheduled for %{date} + sign_in_for_full_latest_html: Some latest posts are only visible for members. Sign in to see all latest. read_next: Lire ensuite reading_time: one: "1 min read" diff --git a/spec/requests/articles/articles_spec.rb b/spec/requests/articles/articles_spec.rb index 2b4133b5b..4b99cdbdc 100644 --- a/spec/requests/articles/articles_spec.rb +++ b/spec/requests/articles/articles_spec.rb @@ -191,12 +191,19 @@ RSpec.describe "Articles" do let!(:article_with_low_score) do create(:article, score: Articles::Feeds::Latest::MINIMUM_SCORE) end + let!(:article_with_mid_score) do + minimum = Articles::Feeds::Latest::MINIMUM_SCORE + maximum = Settings::UserExperience.home_feed_minimum_score + score = (minimum..maximum).to_a.sample + create(:article, score: score) + end before { get "/feed/latest" } it "contains latest articles" do expect(response.body).to include(last_article.title) expect(response.body).to include(not_featured_article.title) + expect(response.body).to include(article_with_mid_score.title) expect(response.body).not_to include(article_with_low_score.title) end end diff --git a/spec/requests/stories/feeds_spec.rb b/spec/requests/stories/feeds_spec.rb index d6cd35ff8..e456505ec 100644 --- a/spec/requests/stories/feeds_spec.rb +++ b/spec/requests/stories/feeds_spec.rb @@ -199,5 +199,25 @@ RSpec.describe "Stories::Feeds" do expect(response_article["top_comments"].first["username"]).not_to be_nil end end + + context "when there are low-scoring articles" do + let!(:article) { create(:article, featured: false) } + let!(:article_with_mid_score) do + minimum = Articles::Feeds::Latest::MINIMUM_SCORE + maximum = Settings::UserExperience.home_feed_minimum_score + score = (minimum..maximum).to_a.sample + create(:article, score: score) + end + let!(:article_with_low_score) do + create(:article, score: Articles::Feeds::Latest::MINIMUM_SCORE) + end + + it "excludes low-score article but not mid-score" do + get timeframe_stories_feed_path(:latest) + expect(response.body).to include(article.title) + expect(response.body).to include(article_with_mid_score.title) + expect(response.body).not_to include(article_with_low_score.title) + end + end end end diff --git a/spec/requests/stories_index_spec.rb b/spec/requests/stories_index_spec.rb index 160d1a291..f7bdcb4ed 100644 --- a/spec/requests/stories_index_spec.rb +++ b/spec/requests/stories_index_spec.rb @@ -289,6 +289,13 @@ RSpec.describe "StoriesIndex" do describe "GET stories index with timeframe" do describe "/latest" do + let(:user) { create(:user) } + let!(:low_score) { create(:article, score: -10) } + + before do + create_list(:article, 3, score: Settings::UserExperience.home_feed_minimum_score + 1) + end + it "includes a link to Relevant", :aggregate_failures do get "/latest" @@ -296,6 +303,17 @@ RSpec.describe "StoriesIndex" do expected_tag = "