diff --git a/app/controllers/stories/tagged_articles_controller.rb b/app/controllers/stories/tagged_articles_controller.rb new file mode 100644 index 000000000..57831ba32 --- /dev/null +++ b/app/controllers/stories/tagged_articles_controller.rb @@ -0,0 +1,72 @@ +module Stories + class TaggedArticlesController < ApplicationController + before_action :set_cache_control_headers, only: :index + + SIGNED_OUT_RECORD_COUNT = 60 + + rescue_from ArgumentError, with: :bad_request + + def index + @page = (params[:page] || 1).to_i + @article_index = true + @tag = params[:tag].downcase + @tag_model = Tag.find_by(name: @tag) || not_found + @moderators = User.with_role(:tag_moderator, @tag_model).select(:username, :profile_image, :id) + + if @tag_model.alias_for.present? + redirect_permanently_to("/t/#{@tag_model.alias_for}") + return + end + + set_number_of_articles + set_stories + + set_surrogate_key_header "articles-#{@tag}" + set_cache_control_headers(600, + stale_while_revalidate: 30, + stale_if_error: 86_400) + end + + private + + def set_number_of_articles + @num_published_articles = if @tag_model.requires_approval? + @tag_model.articles.published.where(approved: true).count + elsif Settings::UserExperience.feed_strategy == "basic" + tagged_count + else + Rails.cache.fetch("article-cached-tagged-count-#{@tag}", expires_in: 2.hours) do + tagged_count + end + end + + @number_of_articles = user_signed_in? ? 5 : SIGNED_OUT_RECORD_COUNT + end + + def set_stories + @stories = Articles::Feeds::LargeForemExperimental + .new(number_of_articles: @number_of_articles, tag: @tag, page: @page) + .published_articles_by_tag + + @stories = @stories.where(approved: true) if @tag_model&.requires_approval + + @stories = stories_by_timeframe + @stories = @stories.decorate + end + + def tagged_count + @tag_model.articles.published.where("score >= ?", Settings::UserExperience.tag_feed_minimum_score).count + end + + def stories_by_timeframe + if %w[week month year infinity].include?(params[:timeframe]) + @stories.where("published_at > ?", Timeframe.datetime(params[:timeframe])) + .order(public_reactions_count: :desc) + elsif params[:timeframe] == "latest" + @stories.where("score > ?", -20).order(published_at: :desc) + else + @stories.order(hotness_score: :desc).where("score >= ?", Settings::UserExperience.home_feed_minimum_score) + end + end + end +end diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index df135255a..e3b836a0a 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -27,7 +27,6 @@ class StoriesController < ApplicationController @article_index = true return handle_user_or_organization_or_podcast_or_page_index if params[:username] - return handle_tag_index if params[:tag] handle_base_index end @@ -125,42 +124,6 @@ class StoriesController < ApplicationController end end - def handle_tag_index - @tag = params[:tag].downcase - @page = (params[:page] || 1).to_i - @tag_model = Tag.find_by(name: @tag) || not_found - @moderators = User.with_role(:tag_moderator, @tag_model).select(:username, :profile_image, :id) - if @tag_model.alias_for.present? - redirect_permanently_to("/t/#{@tag_model.alias_for}") - return - end - - @num_published_articles = if @tag_model.requires_approval? - @tag_model.articles.published.where(approved: true).count - elsif Settings::UserExperience.feed_strategy == "basic" - tagged_count - else - Rails.cache.fetch("article-cached-tagged-count-#{@tag}", expires_in: 2.hours) do - tagged_count - end - end - @number_of_articles = user_signed_in? ? 5 : SIGNED_OUT_RECORD_COUNT - @stories = Articles::Feeds::LargeForemExperimental - .new(number_of_articles: @number_of_articles, tag: @tag, page: @page) - .published_articles_by_tag - - @stories = @stories.where(approved: true) if @tag_model&.requires_approval - - @stories = stories_by_timeframe - @stories = @stories.decorate - - set_surrogate_key_header "articles-#{@tag}" - set_cache_control_headers(600, - stale_while_revalidate: 30, - stale_if_error: 86_400) - render template: "articles/tag_index" - end - def handle_page_display @story_show = true set_surrogate_key_header "show-page-#{params[:username]}" @@ -348,17 +311,6 @@ class StoriesController < ApplicationController @github_repositories = @user.github_repos.featured.order(stargazers_count: :desc, name: :asc) end - def stories_by_timeframe - if %w[week month year infinity].include?(params[:timeframe]) - @stories.where("published_at > ?", Timeframe.datetime(params[:timeframe])) - .order(public_reactions_count: :desc) - elsif params[:timeframe] == "latest" - @stories.where("score > ?", -20).order(published_at: :desc) - else - @stories.order(hotness_score: :desc).where("score >= ?", Settings::UserExperience.home_feed_minimum_score) - end - end - def assign_podcasts return unless user_signed_in? @@ -485,8 +437,4 @@ class StoriesController < ApplicationController params[:sort_direction] == "desc" ? :newest : :oldest end - - def tagged_count - @tag_model.articles.published.where("score >= ?", Settings::UserExperience.tag_feed_minimum_score).count - end end diff --git a/app/views/articles/tags/_main_feed.html.erb b/app/views/stories/tagged_articles/_main_feed.html.erb similarity index 100% rename from app/views/articles/tags/_main_feed.html.erb rename to app/views/stories/tagged_articles/_main_feed.html.erb diff --git a/app/views/articles/tags/_meta.html.erb b/app/views/stories/tagged_articles/_meta.html.erb similarity index 100% rename from app/views/articles/tags/_meta.html.erb rename to app/views/stories/tagged_articles/_meta.html.erb diff --git a/app/views/articles/tags/_sidebar.html.erb b/app/views/stories/tagged_articles/_sidebar.html.erb similarity index 100% rename from app/views/articles/tags/_sidebar.html.erb rename to app/views/stories/tagged_articles/_sidebar.html.erb diff --git a/app/views/articles/tags/_sidebar_additional.html.erb b/app/views/stories/tagged_articles/_sidebar_additional.html.erb similarity index 100% rename from app/views/articles/tags/_sidebar_additional.html.erb rename to app/views/stories/tagged_articles/_sidebar_additional.html.erb diff --git a/app/views/articles/tag_index.html.erb b/app/views/stories/tagged_articles/index.html.erb similarity index 95% rename from app/views/articles/tag_index.html.erb rename to app/views/stories/tagged_articles/index.html.erb index b7cba726e..2899c49f8 100644 --- a/app/views/articles/tag_index.html.erb +++ b/app/views/stories/tagged_articles/index.html.erb @@ -1,5 +1,5 @@ <%= content_for :page_meta do %> - <%= render "articles/tags/meta" %> + <%= render "stories/tagged_articles/meta" %> <% end %> <% expiry_minutes = params[:timeframe].blank? || params[:timeframe] == "latest" ? 4 : 20 %> <% flag = true %> @@ -42,7 +42,7 @@ data-feed="<%= params[:timeframe] || "base-feed" %>" data-requires-approval="<%= @tag_model.requires_approval %>" data-articles-since="<%= Timeframe.datetime_iso8601(params[:timeframe]) %>"> - <%= render "articles/tags/sidebar" %> + <%= render "stories/tagged_articles/sidebar" %>
- <%# articles/tags/main_feed will iterate on stories with .each_with_index, + <%# stories/tagged_articles/main_feed will iterate on stories with .each_with_index, thus by using .present? here we preload the items %> <% if @stories.present? %> - <%= render "articles/tags/main_feed" %> + <%= render "stories/tagged_articles/main_feed" %> <% end %>
loading...
- <%= render "articles/tags/sidebar_additional" %> + <%= render "stories/tagged_articles/sidebar_additional" %> <%= render "stories/stories_list_script" %> diff --git a/config/routes.rb b/config/routes.rb index 11e93c661..953c90097 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -390,15 +390,16 @@ Rails.application.routes.draw do get "/feed/:username", to: "articles#feed", as: "user_feed", defaults: { format: "rss" } get "/rss", to: "articles#feed", defaults: { format: "rss" } - get "/tag/:tag", to: "stories#index" - get "/t/:tag", to: "stories#index", as: :tag + get "/tag/:tag", to: "stories/tagged_articles#index" + get "/t/:tag", to: "stories/tagged_articles#index", as: :tag + get "/t/:tag/top/:timeframe", to: "stories/tagged_articles#index" + get "/t/:tag/page/:page", to: "stories/tagged_articles#index" + get "/t/:tag/:timeframe", to: "stories/tagged_articles#index", + constraints: { timeframe: /latest/ } + get "/t/:tag/edit", to: "tags#edit" get "/t/:tag/admin", to: "tags#admin" patch "/tag/:id", to: "tags#update" - get "/t/:tag/top/:timeframe", to: "stories#index" - get "/t/:tag/page/:page", to: "stories#index" - get "/t/:tag/:timeframe", to: "stories#index", - constraints: { timeframe: /latest/ } get "/badge/:slug", to: "badges#show", as: :badge diff --git a/spec/initializers/rack/attack_spec.rb b/spec/initializers/rack/attack_spec.rb index 7c56ef5d4..f7c974126 100644 --- a/spec/initializers/rack/attack_spec.rb +++ b/spec/initializers/rack/attack_spec.rb @@ -133,8 +133,8 @@ describe Rack::Attack, type: :request, throttle: true do # rubocop:disable RSpec/AnyInstance, RSpec/ExampleLength it "throttles viewing tags" do - allow_any_instance_of(StoriesController).to receive(:tagged_count).and_return(0) - allow_any_instance_of(StoriesController).to receive(:stories_by_timeframe).and_return(Article.none) + allow_any_instance_of(Stories::TaggedArticlesController).to receive(:tagged_count).and_return(0) + allow_any_instance_of(Stories::TaggedArticlesController).to receive(:stories_by_timeframe).and_return(Article.none) allow_any_instance_of(Articles::Feeds::LargeForemExperimental).to receive( :published_articles_by_tag, ).and_return(Article.none) diff --git a/spec/requests/stories/tagged_articles_spec.rb b/spec/requests/stories/tagged_articles_spec.rb new file mode 100644 index 000000000..9058fd074 --- /dev/null +++ b/spec/requests/stories/tagged_articles_spec.rb @@ -0,0 +1,217 @@ +require "rails_helper" + +RSpec.describe "Stories::TaggedArticlesIndex", type: :request do + describe "GET /tag/:tag" do + let(:user) { create(:user) } + let(:tag) { create(:tag) } + let(:org) { create(:organization) } + + def create_live_sponsor(org, tag) + create( + :sponsorship, + level: :tag, + blurb_html: "

Oh Yeah!!!

", + status: "live", + organization: org, + sponsorable: tag, + expires_at: 30.days.from_now, + ) + end + + context "with caching headers" do + it "renders page and sets proper headers", :aggregate_failures do + get "/t/#{tag.name}" + + renders_page + sets_fastly_headers + sets_nginx_headers + end + + def renders_page + expect(response.status).to eq(200) + expect(response.body).to include(tag.name) + end + + def sets_fastly_headers + expected_cache_control_headers = %w[public no-cache] + expect(response.headers["Cache-Control"].split(", ")).to match_array(expected_cache_control_headers) + + 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) + + expected_surrogate_key_headers = %W[articles-#{tag}] + expect(response.headers["Surrogate-Key"].split(", ")).to match_array(expected_surrogate_key_headers) + end + + def sets_nginx_headers + expect(response.headers["X-Accel-Expires"]).to eq("600") + end + end + + it "renders page with top/week etc." do + get "/t/#{tag.name}/top/week" + expect(response.body).to include(tag.name) + get "/t/#{tag.name}/top/month" + expect(response.body).to include(tag.name) + get "/t/#{tag.name}/top/year" + expect(response.body).to include(tag.name) + get "/t/#{tag.name}/top/infinity" + expect(response.body).to include(tag.name) + end + + it "renders tag after alias change" do + tag2 = create(:tag, alias_for: tag.name) + get "/t/#{tag2.name}" + expect(response.body).to redirect_to "/t/#{tag.name}" + expect(response).to have_http_status(:moved_permanently) + end + + it "does not render sponsor if not live" do + sponsorship = create( + :sponsorship, level: :tag, tagline: "Oh Yeah!!!", status: "pending", organization: org, sponsorable: tag + ) + + get "/t/#{tag.name}" + expect(response.body).not_to include("is sponsored by") + expect(response.body).not_to include(sponsorship.tagline) + end + + it "renders live sponsor" do + sponsorship = create_live_sponsor(org, tag) + get "/t/#{tag.name}" + expect(response.body).to include("is sponsored by") + expect(response.body).to include(sponsorship.blurb_html) + end + + it "shows meta keywords if set" do + allow(Settings::General).to receive(:meta_keywords).and_return({ tag: "software engineering, ruby" }) + get "/t/#{tag.name}" + expect(response.body).to include("") + end + + it "does not show meta keywords if not set" do + allow(Settings::General).to receive(:meta_keywords).and_return({ tag: "" }) + get "/t/#{tag.name}" + expect(response.body).not_to include( + "", + ) + end + + context "with user signed in" do + before do + sign_in user + end + + it "shows tags and renders properly", :aggregate_failures do + get "/t/#{tag.name}" + expect(response.body).to include("crayons-tabs__item crayons-tabs__item--current") + has_mod_action_button + does_not_paginate + sets_remember_token + end + + def has_mod_action_button + expect(response.body).to include('class="crayons-btn crayons-btn--outlined mod-action-button fs-s"') + end + + def does_not_paginate + expect(response.body).not_to include('') + end + + def sets_remember_token + expect(response.cookies["remember_user_token"]).not_to be nil + end + + it "renders properly even if site config is private" do + allow(Settings::UserExperience).to receive(:public).and_return(false) + get "/t/#{tag.name}" + expect(response.body).to include("crayons-tabs__item crayons-tabs__item--current") + end + + it "does not render pagination even with many posts" do + create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20) + get "/t/#{tag.name}" + expect(response.body).not_to include('') + end + end + + context "without user signed in" do + let(:tag) { create(:tag) } + + it "renders tag index properly with many posts", :aggregate_failures do + stub_const("Stories::TaggedArticlesController::SIGNED_OUT_RECORD_COUNT", 10) + create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20) + get "/t/#{tag.name}" + + shows_sign_in_notice + does_not_include_current_page_link(tag) + does_not_set_remember_token + renders_pagination + end + + def shows_sign_in_notice + expect(response.body).not_to include("crayons-tabs__item crayons-tabs__item--current") + expect(response.body).to include("for the ability sort posts by") + end + + def does_not_include_current_page_link(tag) + expect(response.body).to include('1') + expect(response.body).not_to include("') + end + + it "renders tag index without pagination when not needed" do + get "/t/#{tag.name}" + + expect(response.body).not_to include('') + end + + it "does not include sidebar for page tag" do + create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20) + get "/t/#{tag.name}/page/2" + expect(response.body).not_to include('