Optimize tagged articles under feature flag (#14451)
This commit is contained in:
Jamie Gaskins
GitHub
parent
5003bec99d
commit
ad9f54dad4
2 changed files with 203 additions and 185 deletions
|
|
@ -28,7 +28,17 @@ module Articles
|
|||
end
|
||||
|
||||
def published_articles_by_tag
|
||||
articles = @tag.present? ? Tag.find_by(name: @tag).articles : Article
|
||||
articles =
|
||||
if @tag.present?
|
||||
if FeatureFlag.enabled?(:optimize_article_tag_query)
|
||||
Article.cached_tagged_with_any(@tag)
|
||||
else
|
||||
Tag.find_by(name: @tag).articles
|
||||
end
|
||||
else
|
||||
Article.all
|
||||
end
|
||||
|
||||
articles.published.limited_column_select
|
||||
.includes(top_comments: :user)
|
||||
.page(@page).per(@number_of_articles)
|
||||
|
|
|
|||
|
|
@ -1,216 +1,224 @@
|
|||
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
|
||||
%i[enable disable].each do |method|
|
||||
context "when :optimize_article_tag_query is #{method}d" do
|
||||
before do
|
||||
sign_in user
|
||||
FeatureFlag.public_send method, :optimize_article_tag_query
|
||||
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
|
||||
describe "GET /tag/:tag" do
|
||||
let(:user) { create(:user) }
|
||||
let(:tag) { create(:tag) }
|
||||
let(:org) { create(:organization) }
|
||||
|
||||
def has_mod_action_button
|
||||
expect(response.body).to include('class="crayons-btn crayons-btn--outlined mod-action-button fs-s"')
|
||||
end
|
||||
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
|
||||
|
||||
def does_not_paginate
|
||||
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
|
||||
end
|
||||
context "with caching headers" do
|
||||
it "renders page and sets proper headers", :aggregate_failures do
|
||||
get "/t/#{tag.name}"
|
||||
|
||||
def sets_remember_token
|
||||
expect(response.cookies["remember_user_token"]).not_to be nil
|
||||
end
|
||||
renders_page
|
||||
sets_fastly_headers
|
||||
sets_nginx_headers
|
||||
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
|
||||
def renders_page
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.body).to include(tag.name)
|
||||
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
|
||||
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)
|
||||
|
||||
context "without user signed in" do
|
||||
let(:tag) { create(:tag) }
|
||||
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)
|
||||
|
||||
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}"
|
||||
expected_surrogate_key_headers = %W[articles-#{tag}]
|
||||
expect(response.headers["Surrogate-Key"].split(", ")).to match_array(expected_surrogate_key_headers)
|
||||
end
|
||||
|
||||
shows_sign_in_notice
|
||||
does_not_include_current_page_link(tag)
|
||||
does_not_set_remember_token
|
||||
renders_pagination
|
||||
end
|
||||
def sets_nginx_headers
|
||||
expect(response.headers["X-Accel-Expires"]).to eq("600")
|
||||
end
|
||||
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
|
||||
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
|
||||
|
||||
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
|
||||
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
|
||||
|
||||
def does_not_set_remember_token
|
||||
expect(response.cookies["remember_user_token"]).to be nil
|
||||
end
|
||||
it "does not render sponsor if not live" do
|
||||
sponsorship = create(
|
||||
:sponsorship, level: :tag, tagline: "Oh Yeah!!!", status: "pending", organization: org, sponsorable: tag
|
||||
)
|
||||
|
||||
def renders_pagination
|
||||
expect(response.body).to include('<span class="olderposts-pagenumber">')
|
||||
end
|
||||
get "/t/#{tag.name}"
|
||||
expect(response.body).not_to include("is sponsored by")
|
||||
expect(response.body).not_to include(sponsorship.tagline)
|
||||
end
|
||||
|
||||
it "renders tag index without pagination when not needed" do
|
||||
get "/t/#{tag.name}"
|
||||
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
|
||||
|
||||
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
|
||||
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 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 "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
|
||||
|
||||
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"
|
||||
context "with user signed in" do
|
||||
before do
|
||||
sign_in user
|
||||
end
|
||||
|
||||
renders_title(tag)
|
||||
renders_canonical_url(tag)
|
||||
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 renders_title(tag)
|
||||
expect(response.body).to include("<title>#{tag.name.capitalize} - ")
|
||||
end
|
||||
def has_mod_action_button
|
||||
expect(response.body).to include('class="crayons-btn crayons-btn--outlined mod-action-button fs-s"')
|
||||
end
|
||||
|
||||
def renders_canonical_url(tag)
|
||||
expect(response.body).to include("<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}\" />")
|
||||
end
|
||||
def does_not_paginate
|
||||
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
|
||||
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"
|
||||
def sets_remember_token
|
||||
expect(response.cookies["remember_user_token"]).not_to be nil
|
||||
end
|
||||
|
||||
renders_page_2_title(tag)
|
||||
renders_page_2_canonical_url(tag)
|
||||
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
|
||||
|
||||
def renders_page_2_title(tag)
|
||||
expect(response.body).to include("<title>#{tag.name.capitalize} Page 2 - ")
|
||||
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
|
||||
|
||||
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)
|
||||
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
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue