From fa20539238af63200ac634c5c0e2493330eecbd7 Mon Sep 17 00:00:00 2001 From: rhymes Date: Tue, 6 Apr 2021 17:05:35 +0200 Subject: [PATCH] Adjust default Forem's RSS feed algorithm to allow non featured articles (#13247) Since we've removed Buffer related algorithm, posts aren't automatically featured anymore, leaving the RSS feed pretty empty. The default view should also include those posts, above a minimum score, that are not featured. --- app/controllers/articles_controller.rb | 7 ++- spec/requests/articles/articles_spec.rb | 71 +++++++++++++++++-------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index 118f7b0e5..ab1bf682b 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -27,10 +27,13 @@ class ArticlesController < ApplicationController elsif params[:tag] handle_tag_feed elsif request.path == latest_feed_path - @articles.where("score > ?", Articles::Feeds::LargeForemExperimental::MINIMUM_SCORE_LATEST_FEED) + @articles + .where("score > ?", Articles::Feeds::LargeForemExperimental::MINIMUM_SCORE_LATEST_FEED) .includes(:user) else - @articles.where(featured: true).includes(:user) + @articles + .where(featured: true).or(@articles.where(score: SiteConfig.home_feed_minimum_score..)) + .includes(:user) end not_found unless @articles&.any? diff --git a/spec/requests/articles/articles_spec.rb b/spec/requests/articles/articles_spec.rb index 9cb400c8c..6b091eec8 100644 --- a/spec/requests/articles/articles_spec.rb +++ b/spec/requests/articles/articles_spec.rb @@ -4,11 +4,13 @@ RSpec.describe "Articles", type: :request do let(:user) { create(:user) } let(:tag) { build_stubbed(:tag) } - describe "GET /feed" do + describe "GET /feed(/:username|/:tag_name)" do it "returns rss+xml content" do create(:article, featured: true) - get "/feed" - expect(response.status).to eq(200) + + get feed_path + + expect(response).to have_http_status(:ok) expect(response.media_type).to eq("application/rss+xml") end @@ -20,59 +22,54 @@ RSpec.describe "Articles", type: :request do expect(response.body).to include("#{URL.url}") end - it "returns not found if no articles" do - expect { get "/feed" }.to raise_error(ActiveRecord::RecordNotFound) - expect { get "/feed/#{user.username}" }.to raise_error(ActiveRecord::RecordNotFound) - expect { get "/feed/#{tag.name}" }.to raise_error(ActiveRecord::RecordNotFound) + it "returns not found if no articles", :aggregate_failures do + expect { get feed_path }.to raise_error(ActiveRecord::RecordNotFound) + expect { get user_feed_path(user.username) }.to raise_error(ActiveRecord::RecordNotFound) + expect { get tag_feed_path(tag.name) }.to raise_error(ActiveRecord::RecordNotFound) end it "does not contain image tag" do create(:article, featured: true) get feed_path + expect(response.body).not_to include("") end context "with caching headers" do before do create(:article, featured: true) - get "/feed" + + get feed_path end it "sets Fastly Cache-Control headers" do - expect(response.status).to eq(200) - expected_cache_control_headers = %w[public no-cache] expect(response.headers["Cache-Control"].split(", ")).to match_array(expected_cache_control_headers) end it "sets Fastly Surrogate-Control headers" do - expect(response.status).to eq(200) - expected_surrogate_control_headers = %w[max-age=600 stale-while-revalidate=30 stale-if-error=86400] expect(response.headers["Surrogate-Control"].split(", ")).to match_array(expected_surrogate_control_headers) end it "sets Fastly Surrogate-Key headers" do - expect(response.status).to eq(200) - expected_surrogate_key_headers = %w[feed] expect(response.headers["Surrogate-Key"].split(", ")).to match_array(expected_surrogate_key_headers) end it "sets Nginx X-Accel-Expires headers" do - expect(response.status).to eq(200) expect(response.headers["X-Accel-Expires"]).to eq("600") end end context "when :username param is not given" do let!(:featured_article) { create(:article, featured: true) } - let!(:not_featured_article) { create(:article, featured: false) } + let!(:not_featured_article) { create(:article, featured: false, score: SiteConfig.home_feed_minimum_score - 1) } - before { get "/feed" } + before { get feed_path } - it "returns only featured articles" do + it "returns only featured articles", :aggregate_failures do expect(response.body).to include(featured_article.title) expect(response.body).not_to include(not_featured_article.title) end @@ -91,7 +88,7 @@ RSpec.describe "Articles", type: :request do before { get user_feed_path(user.username) } - it "returns only articles for that user" do + it "returns only articles for that user", :aggregate_failures do expect(response.body).to include(user_article.title) expect(response.body).not_to include(organization_article.title) end @@ -100,7 +97,7 @@ RSpec.describe "Articles", type: :request do expect(response.body).to include("#{URL.user(user)}") end - it "contains a user composite profile image tag" do + it "contains a user composite profile image tag", :aggregate_failures do expect(response.body).to include("") expect(response.body).to include("#{user.profile_image_90}") expect(response.body).to include("#{user.name} profile image") @@ -117,7 +114,7 @@ RSpec.describe "Articles", type: :request do before { get user_feed_path(organization.slug) } - it "returns only articles for that organization" do + it "returns only articles for that organization", :aggregate_failures do expect(response.body).not_to include(user_article.title) expect(response.body).to include(organization_article.title) end @@ -126,7 +123,7 @@ RSpec.describe "Articles", type: :request do expect(response.body).to include("#{URL.organization(organization)}") end - it "contains an organization composite profile image tag" do + it "contains an organization composite profile image tag", :aggregate_failures do expect(response.body).to include("") expect(response.body).to include("#{organization.profile_image_90}") expect(response.body).to include("#{organization.name} profile image") @@ -138,7 +135,7 @@ RSpec.describe "Articles", type: :request do context "when :username param is given but it belongs to neither user nor organization" do it "renders empty body" do expect do - get feed_path("unknown") + get user_feed_path("unknown") end.to raise_error(ActiveRecord::RecordNotFound) end end @@ -157,6 +154,34 @@ RSpec.describe "Articles", type: :request do rss_feed = Feedjira.parse(response.body) expect(rss_feed.entries.first.categories).to match_array(article.tag_list) end + + context "with scored articles" do + before do + allow(SiteConfig).to receive(:home_feed_minimum_score).and_return(10) + end + + it "does not contain non featured articles with a score below SiteConfig.home_feed_minimum_score" do + create(:article, featured: false, score: SiteConfig.home_feed_minimum_score - 1) + + expect { get feed_path }.to raise_error(ActiveRecord::RecordNotFound) + end + + it "contains non featured articles with a score equal to SiteConfig.home_feed_minimum_score" do + create(:article, featured: false, score: SiteConfig.home_feed_minimum_score) + + get feed_path + + expect(response).to have_http_status(:ok) + end + + it "contains non featured articles with a score above SiteConfig.home_feed_minimum_score" do + create(:article, featured: false, score: SiteConfig.home_feed_minimum_score + 1) + + get feed_path + + expect(response).to have_http_status(:ok) + end + end end describe "GET /feed/latest" do