From 6d7cbfff1fa5227fe248a2296094a170faae77ed Mon Sep 17 00:00:00 2001 From: Josh Puetz Date: Wed, 4 Mar 2020 08:01:24 -0600 Subject: [PATCH] Restore featured story on timeframe pages for logged out users (#6427) [deploy] * Restore featured story on timeframe pages for logged out users * Handle arrays and empty collections when finding featured article --- app/controllers/stories_controller.rb | 6 +- app/services/articles/feed.rb | 13 + spec/services/articles/feed_spec.rb | 37 +++ .../user_visits_articles_by_timeframe_spec.rb | 227 ++++++++++++++++++ 4 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 spec/system/articles/user_visits_articles_by_timeframe_spec.rb diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 3636a702f..c74cf5f99 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -151,7 +151,7 @@ class StoriesController < ApplicationController assign_classified_listings get_latest_campaign_articles if SiteConfig.campaign_sidebar_enabled? @article_index = true - @featured_story = (@featured_story || Article.new)&.decorate + @featured_story = (featured_story || Article.new)&.decorate @stories = ArticleDecorator.decorate_collection(@stories) set_surrogate_key_header "main_app_home_page" response.headers["Surrogate-Control"] = "max-age=600, stale-while-revalidate=30, stale-if-error=86400" @@ -159,6 +159,10 @@ class StoriesController < ApplicationController render template: "articles/index" end + def featured_story + @featured_story ||= Articles::Feed.find_featured_story(@stories) + end + def handle_podcast_index @podcast_index = true @list_of = "podcast-episodes" diff --git a/app/services/articles/feed.rb b/app/services/articles/feed.rb index 3e1c456e1..5e426f3ef 100644 --- a/app/services/articles/feed.rb +++ b/app/services/articles/feed.rb @@ -9,6 +9,19 @@ module Articles @tag_weight = 1 # default weight tags play in rankings end + def self.find_featured_story(stories) + featured_story = if stories.is_a?(ActiveRecord::Relation) + stories.where.not(main_image: nil).first + else + stories.detect { |story| !story.main_image.nil? } + end + featured_story || Article.new + end + + def find_featured_story(stories) + self.class.find_featured_story(stories) + end + def published_articles_by_tag articles = Article.published.limited_column_select.page(@page).per(@number_of_articles) articles = articles.cached_tagged_with(@tag) if @tag.present? # More efficient than tagged_with diff --git a/spec/services/articles/feed_spec.rb b/spec/services/articles/feed_spec.rb index 61f73e7f9..e008d0a9b 100644 --- a/spec/services/articles/feed_spec.rb +++ b/spec/services/articles/feed_spec.rb @@ -364,4 +364,41 @@ RSpec.describe Articles::Feed, type: :service do expect(feed.rank_and_sort_articles(articles)).to eq [article3, article2, article1] end end + + describe ".find_featured_story" do + let(:featured_story) { described_class.find_featured_story(stories) } + + context "when passed an ActiveRecord collection" do + let(:stories) { Article.all } + + it "returns first article with a main image" do + expect(featured_story.main_image).not_to be_nil + end + end + + context "when passed an array" do + let(:stories) { Article.all.to_a } + + it "returns first article with a main image" do + expect(featured_story.main_image).not_to be_nil + end + end + + context "when passed collection without any articles" do + let(:stories) { [] } + + it "returns an new, empty Article object" do + expect(featured_story.main_image).to be_nil + expect(featured_story.id).to be_nil + end + end + end + + describe "#find_featured_story" do + it "calls the class method" do + allow(described_class).to receive(:find_featured_story) + feed.find_featured_story([]) + expect(described_class).to have_received(:find_featured_story) + end + end end diff --git a/spec/system/articles/user_visits_articles_by_timeframe_spec.rb b/spec/system/articles/user_visits_articles_by_timeframe_spec.rb new file mode 100644 index 000000000..1029d5d9b --- /dev/null +++ b/spec/system/articles/user_visits_articles_by_timeframe_spec.rb @@ -0,0 +1,227 @@ +require "rails_helper" + +RSpec.describe "User visits articles by timeframe", type: :system do + let(:author) { create(:user) } + let!(:article) { create(:article, user: author, published_at: Time.current) } + let!(:days_old_article) { create(:article, user: author, published_at: 2.days.ago) } + let!(:weeks_old_article) { create(:article, user: author, published_at: 2.weeks.ago) } + let!(:months_old_article) { create(:article, user: author, published_at: 2.months.ago) } + let!(:years_old_article) { create(:article, user: author, published_at: 2.years.ago) } + + context "when user hasn't logged in" do + context "when viewing articles for week" do + before { visit "/top/week" } + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 1) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + end + end + end + + context "when viewing articles for month" do + before { visit "/top/month" } + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 2) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + end + end + end + + context "when viewing articles for year" do + before { visit "/top/year" } + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 3) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + expect(page).to have_text(months_old_article.title) + end + end + end + + context "when viewing articles for infinity" do + before { visit "/top/infinity" } + + it "shows correct articles and cta count" do + expect(page).to have_selector(".single-article-small-pic", count: 5) + expect(page).to have_selector(".feed-cta", count: 1) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + expect(page).to have_text(months_old_article.title) + expect(page).to have_text(years_old_article.title) + end + end + end + + context "when viewing articles for latest" do + before { visit "/latest" } + + it "shows correct articles and cta count" do + expect(page).to have_selector(".single-article-small-pic", count: 5) + expect(page).to have_selector(".feed-cta", count: 1) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + expect(page).to have_text(months_old_article.title) + expect(page).to have_text(years_old_article.title) + end + end + end + end + + context "when user has logged in", js: true do + let(:user) { create(:user) } + + before do + sign_in user + visit "/top/week" + end + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 1) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + end + end + + context "when viewing articles for month" do + before { visit "/top/month" } + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 2) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + end + end + end + + context "when viewing articles for year" do + before { visit "/top/year" } + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 3) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + expect(page).to have_text(months_old_article.title) + end + end + end + + context "when viewing articles for infinity" do + before { visit "/top/infinity" } + + it "shows correct articles count" do + expect(page).to have_selector(".single-article-small-pic", count: 4) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + expect(page).to have_text(months_old_article.title) + expect(page).to have_text(years_old_article.title) + end + end + end + + context "when viewing articles for latest" do + before { visit "/latest" } + + it "shows correct articles" do + expect(page).to have_selector(".single-article-small-pic", count: 4) + end + + it "shows the main article" do + expect(page).to have_selector(".big-article", visible: true, count: 1) + end + + it "shows the correct articles" do + within("#articles-list") do + expect(page).to have_text(article.title) + expect(page).to have_text(days_old_article.title) + expect(page).to have_text(weeks_old_article.title) + expect(page).to have_text(months_old_article.title) + expect(page).to have_text(years_old_article.title) + end + end + end + end +end