maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Extract tag stories to tag articles controller (#13457)

Browse source
This commit is contained in:
Chien-Wei Huang (Michael) 2021-05-25 01:44:43 +08:00 • committed by GitHub
parent 343a3a8a8a
commit a4c06d3207
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 303 additions and 279 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

72
app/controllers/stories/tagged_articles_controller.rb Normal file
View file

@ -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

52
app/controllers/stories_controller.rb
View file

@ -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

0
app/views/articles/tags/_main_feed.html.erb → app/views/stories/tagged_articles/_main_feed.html.erb
View file

0
app/views/articles/tags/_meta.html.erb → app/views/stories/tagged_articles/_meta.html.erb
View file

0
app/views/articles/tags/_sidebar.html.erb → app/views/stories/tagged_articles/_sidebar.html.erb
View file

0
app/views/articles/tags/_sidebar_additional.html.erb → app/views/stories/tagged_articles/_sidebar_additional.html.erb
View file

10
app/views/articles/tag_index.html.erb → app/views/stories/tagged_articles/index.html.erb
View file

@ -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" %>
<main class="articles-list" id="main-content">
<header class="flex items-center p-2 m:p-0 m:pb-2" id="on-page-nav-controls">
<button type="button" class="crayons-btn crayons-btn--ghost crayons-btn--icon mr-2 inline-block m:hidden" id="on-page-nav-butt-left" aria-label="nav-button-left">
@ -99,17 +99,17 @@
</header>
<div class="substories" id="substories">
<%# 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 %>
</div>
<div class="loading-articles" id="loading-articles">
loading...
</div>
</main>
<%= render "articles/tags/sidebar_additional" %>
<%= render "stories/tagged_articles/sidebar_additional" %>
</div>
<%= render "stories/stories_list_script" %>

13
config/routes.rb
View file

@ -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

4
spec/initializers/rack/attack_spec.rb
View file

@ -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)

217
spec/requests/stories/tagged_articles_spec.rb Normal file
View file

@ -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: "<p>Oh Yeah!!!</p>",
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("<meta name=\"keywords\" content=\"software engineering, ruby, #{tag.name}\">")
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(
"<meta name=\"keywords\" content=\"software engineering, ruby, #{tag.name}\">",
)
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('<span class="olderposts-pagenumber">')
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('<span class="olderposts-pagenumber">')
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('<span class="olderposts-pagenumber">1')
expect(response.body).not_to include("<a href=\"/t/#{tag.name}/page/1")
expect(response.body).not_to include("<a href=\"/t/#{tag.name}/page/3")
end
def does_not_set_remember_token
expect(response.cookies["remember_user_token"]).to be nil
end
def renders_pagination
expect(response.body).to include('<span class="olderposts-pagenumber">')
end
it "renders tag index without pagination when not needed" do
get "/t/#{tag.name}"
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
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('<div id="sidebar-wrapper-right"')
end
it "renders proper page 1", :aggregate_failures do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/1"
renders_title(tag)
renders_canonical_url(tag)
end
def renders_title(tag)
expect(response.body).to include("<title>#{tag.name.capitalize} - ")
end
def renders_canonical_url(tag)
expect(response.body).to include("<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}\" />")
end
it "renders proper page 2", :aggregate_failures do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/2"
renders_page_2_title(tag)
renders_page_2_canonical_url(tag)
end
def renders_page_2_title(tag)
expect(response.body).to include("<title>#{tag.name.capitalize} Page 2 - ")
end
def renders_page_2_canonical_url(tag)
expected_tag = "<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}/page/2\" />"
expect(response.body).to include(expected_tag)
end
end
end
end

214
spec/requests/stories_index_spec.rb
View file

@ -280,220 +280,6 @@ RSpec.describe "StoriesIndex", type: :request do
end
end
describe "GET tag index" 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: "<p>Oh Yeah!!!</p>",
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("<meta name=\"keywords\" content=\"software engineering, ruby, #{tag.name}\">")
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(
"<meta name=\"keywords\" content=\"software engineering, ruby, #{tag.name}\">",
)
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('<span class="olderposts-pagenumber">')
end
def sets_remember_token
expect(response.cookies["remember_user_token"]).not_to be nil
end
it "renders properly even if the Forem instance 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('<span class="olderposts-pagenumber">')
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("StoriesController::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('<span class="olderposts-pagenumber">1')
expect(response.body).not_to include("<a href=\"/t/#{tag.name}/page/1")
expect(response.body).not_to include("<a href=\"/t/#{tag.name}/page/3")
end
def does_not_set_remember_token
expect(response.cookies["remember_user_token"]).to be nil
end
def renders_pagination
expect(response.body).to include('<span class="olderposts-pagenumber">')
end
it "renders tag index without pagination when not needed" do
get "/t/#{tag.name}"
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
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('<div id="sidebar-wrapper-right"')
end
it "renders proper page 1", :aggregate_failures do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/1"
renders_title(tag)
renders_canonical_url(tag)
end
def renders_title(tag)
expect(response.body).to include("<title>#{tag.name.capitalize} - ")
end
def renders_canonical_url(tag)
expect(response.body).to include("<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}\" />")
end
it "renders proper page 2", :aggregate_failures do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/2"
renders_page_2_title(tag)
renders_page_2_canonical_url(tag)
end
def renders_page_2_title(tag)
expect(response.body).to include("<title>#{tag.name.capitalize} Page 2 - ")
end
def renders_page_2_canonical_url(tag)
expected_tag = "<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}/page/2\" />"
expect(response.body).to include(expected_tag)
end
end
end
describe "GET user_path" do
include_examples "redirects to the lowercase route" do
let(:path) { "/#{build(:user).username.upcase}" }