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

Add pagination to tag pages for better SEO crawling (#6771) [deploy]

Browse source 
* Add pagination to tag pages for better SEO crawling

* Modify system specs

* Add some tests

* Finalize tests

* Fix things up

* Add url helpers
This commit is contained in:
Ben Halpern 2020-03-27 10:34:42 -04:00 • committed by GitHub
parent d03c751c65
commit e35d55c422
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 178 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
app/assets/stylesheets/sidebar-data.scss
View file

@ -16,4 +16,16 @@
margin-top: 5px;
border-radius: 3px;
}
hr {
opacity: 0.3;
margin: 25px 0px 15px;
width: 200px;
}
.olderposts-header {
margin-bottom: 5px;
}
.olderposts-pagenumber {
padding: 8px;
margin-right: -8px;
}
}

16
app/controllers/stories_controller.rb
View file

@ -11,6 +11,8 @@ class StoriesController < ApplicationController
]
}.freeze
SIGNED_OUT_RECORD_COUNT = (Rails.env.production? ? 60 : 10).freeze
before_action :authenticate_user!, except: %i[index search show]
before_action :set_cache_control_headers, only: %i[index search show]
@ -112,6 +114,7 @@ class StoriesController < ApplicationController
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?
@ -122,10 +125,11 @@ class StoriesController < ApplicationController
@num_published_articles = if @tag_model.requires_approval?
Article.published.cached_tagged_by_approval_with(@tag).size
else
Article.published.cached_tagged_with(@tag).size
Article.published.cached_tagged_with(@tag).where("score > 2").size
end
number_of_articles = user_signed_in? ? 5 : 45
@stories = Articles::Feed.new(number_of_articles: number_of_articles, tag: @tag).published_articles_by_tag
@number_of_articles = user_signed_in? ? 5 : SIGNED_OUT_RECORD_COUNT
@stories = Articles::Feed.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
@ -293,7 +297,7 @@ class StoriesController < ApplicationController
@stories = ArticleDecorator.decorate_collection(@user.articles.published.
limited_column_select.
where.not(id: @pinned_stories.pluck(:id)).
order("published_at DESC").page(@page).per(user_signed_in? ? 2 : 20))
order("published_at DESC").page(@page).per(user_signed_in? ? 2 : SIGNED_OUT_RECORD_COUNT))
end
def stories_by_timeframe
@ -301,9 +305,9 @@ class StoriesController < ApplicationController
@stories.where("published_at > ?", Timeframer.new(params[:timeframe]).datetime).
order("positive_reactions_count DESC")
elsif params[:timeframe] == "latest"
@stories.where("score > ?", -40).order("published_at DESC")
@stories.where("score > ?", -20).order("published_at DESC")
else
@stories.order("hotness_score DESC")
@stories.order("hotness_score DESC").where("score > 2")
end
end

4
app/helpers/application_helper.rb
View file

@ -169,6 +169,10 @@ module ApplicationHelper
URL.url(uri)
end
def tag_url(tag, page)
URL.tag(tag, page)
end
def article_url(article)
URL.article(article)
end

7
app/lib/url.rb
View file

@ -30,6 +30,13 @@ module URL
url(article.path)
end
# Creates an article URL
#
# @param tag [Tag] the tag to create the URL for
def self.tag(tag, page = 1)
url(["/t/#{tag.name}", ("/page/#{page}" if page > 1)].join)
end
# Creates a user URL
#
# @param user [User] the user to create the URL for

2
app/services/articles/feed.rb
View file

@ -40,7 +40,7 @@ module Articles
def latest_feed
published_articles_by_tag.order("published_at DESC").
where("featured_number > ? AND score > ?", 1_449_999_999, -40).
where("featured_number > ? AND score > ?", 1_449_999_999, -20).
page(@page).per(@number_of_articles)
end

14
app/views/articles/tags/_meta.html.erb
View file

@ -3,20 +3,20 @@
timeframe: params[:timeframe],
content_for: true,
) %>
<link rel="canonical" href="https://dev.to/t/<%= @tag %>" />
<meta name="description" content="<%= @tag %> content on dev.to">
<meta name="keywords" content="software development,engineering,rails,javascript,ruby">
<link rel="canonical" href="<%= tag_url(@tag_model, @page) %>" />
<meta name="description" content="<%= @tag %> content on <%= ApplicationConfig["COMMUNITY_NAME"] %>">
<meta name="keywords" content="software development,engineering,<%= @tag %>">
<meta property="og:type" content="website" />
<meta property="og:url" content="https://dev.to/t/<%= @tag %>" />
<meta property="og:url" content="<%= tag_url(@tag_model, @page) %>" />
<meta property="og:title" content="<%= title_with_timeframe(page_title: @tag.capitalize, timeframe: params[:timeframe]) %>" />
<meta property="og:description" content="<%= @tag.capitalize %> content on dev.to" />
<meta property="og:site_name" content="The DEV Community" />
<meta property="og:description" content="<%= @tag.capitalize %> content on <%= ApplicationConfig["COMMUNITY_NAME"] %>" />
<meta property="og:site_name" content="<%= ApplicationConfig["COMMUNITY_NAME"] %> Community" />
<meta name="twitter:site" content="@<%= SiteConfig.social_networks_handle %>">
<meta name="twitter:creator" content="@<%= @tag.capitalize %>">
<meta name="twitter:title" content="<%= title_with_timeframe(page_title: @tag.capitalize, timeframe: params[:timeframe]) %>">
<meta name="twitter:description" content="<%= @tag.capitalize %> content on dev.to">
<meta name="twitter:description" content="<%= @tag.capitalize %> content on <%= ApplicationConfig["COMMUNITY_NAME"] %>">
<% if @tag_model %>
<meta name="twitter:card" content="summary_large_image">

16
app/views/articles/tags/_sidebar.html.erb
View file

@ -76,6 +76,22 @@
MODERATE
</a>
</span>
<% else %>
<% number_of_pages = @num_published_articles / @number_of_articles %>
<% if number_of_pages > 1 %>
<hr />
<div class="olderposts-header">Older #<%= @tag %> posts</div>
<div class="olderposts-links">
<% (1..number_of_pages).each do |n| %>
<% next if (@page - n).abs > 5 && number_of_pages > 10 %>
<% if @page == n %>
<span class="olderposts-pagenumber"><%= n %></span>
<% else %>
<a href="/t/<%= @tag_model.name %><%= "/page/#{n}" if n > 1 %>" class="olderposts-pagenumber"><%= n %></a>
<% end %>
<% end %>
</div>
<% end %>
<% end %>
</div>
</div>

78
app/views/articles/tags/_sidebar_additional.html.erb
View file

@ -1,43 +1,45 @@
<div id="sidebar-wrapper-right" class="sidebar-wrapper sidebar-wrapper-right">
<div class="sidebar-bg" id="sidebar-bg-right"></div>
<div class="side-bar sidebar-additional showing" id="sidebar-additional">
<% active_threads = Article.active_threads([@tag, "discuss"], Timeframer.new(params[:timeframe]).datetime) %>
<% if active_threads.present? %>
<div class="widget">
<header>
<h4>#discuss</h4>
</header>
<div class="widget-body">
<div class="widget-link-list">
<% active_threads.each do |plucked_article| %>
<%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: true %>
<% end %>
</div>
</div>
</div>
<% end %>
<% if user_signed_in? %>
<%= javascript_packs_with_chunks_tag "sidebarWidget", defer: true %>
<div id="sidebarWidget__pack" data-tag-info="<%= @tag_model.attributes.slice("id", "text_color_hex", "bg_color_hex", "name").to_json %>">
</div>
<% else %>
<% cache("seo-boostable-posts-for-tag-#{@tag}-#{params[:timeframe]}-xoxo", expires_in: 18.hours) do %>
<% boostable_posts = Article.seo_boostable(@tag, Timeframer.new(params[:timeframe]).datetime) %>
<% if boostable_posts.present? %>
<div class="widget">
<header>
<h4>trending guides/resources</h4>
</header>
<div class="widget-body">
<div class="widget-link-list">
<% boostable_posts.each do |plucked_article| %>
<%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: false %>
<% end %>
</div>
<% if @page == 1 %>
<div id="sidebar-wrapper-right" class="sidebar-wrapper sidebar-wrapper-right">
<div class="sidebar-bg" id="sidebar-bg-right"></div>
<div class="side-bar sidebar-additional showing" id="sidebar-additional">
<% active_threads = Article.active_threads([@tag, "discuss"], Timeframer.new(params[:timeframe]).datetime) %>
<% if active_threads.present? %>
<div class="widget">
<header>
<h4>#discuss</h4>
</header>
<div class="widget-body">
<div class="widget-link-list">
<% active_threads.each do |plucked_article| %>
<%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: true %>
<% end %>
</div>
</div>
</div>
<% end %>
<% if user_signed_in? %>
<%= javascript_packs_with_chunks_tag "sidebarWidget", defer: true %>
<div id="sidebarWidget__pack" data-tag-info="<%= @tag_model.attributes.slice("id", "text_color_hex", "bg_color_hex", "name").to_json %>">
</div>
<% else %>
<% cache("seo-boostable-posts-for-tag-#{@tag}-#{params[:timeframe]}-xoxo", expires_in: 18.hours) do %>
<% boostable_posts = Article.seo_boostable(@tag, Timeframer.new(params[:timeframe]).datetime) %>
<% if boostable_posts.present? %>
<div class="widget">
<header>
<h4>trending guides/resources</h4>
</header>
<div class="widget-body">
<div class="widget-link-list">
<% boostable_posts.each do |plucked_article| %>
<%= render "articles/widget_list_item", plucked_article: plucked_article, show_comment_count: false %>
<% end %>
</div>
</div>
</div>
<% end %>
<% end %>
<% end %>
<% end %>
</div>
</div>
</div>
<% end %>

1
config/routes.rb
View file

@ -393,6 +393,7 @@ Rails.application.routes.draw do
get "/t/:tag/admin", to: "tags#admin"
patch "/tag/:id", to: "tags#update"
get "/t/:tag/top/:timeframe" => "stories#index"
get "/t/:tag/page/:page" => "stories#index"
get "/t/:tag/:timeframe" => "stories#index",
:constraints => { timeframe: /latest/ }

12
spec/lib/url_spec.rb
View file

@ -52,4 +52,16 @@ RSpec.describe URL, type: :lib do
expect(described_class.user(user)).to eq("https://dev.to/#{user.username}")
end
end
describe ".tag" do
let(:tag) { build(:tag) }
it "returns the correct URL for a tag with no page" do
expect(described_class.tag(tag)).to eq("https://dev.to/t/#{tag.name}")
end
it "returns the correct URL for a tag" do
expect(described_class.tag(tag, 2)).to eq("https://dev.to/t/#{tag.name}/page/2")
end
end
end

65
spec/requests/stories_index_spec.rb
View file

@ -1,6 +1,7 @@
require "rails_helper"
RSpec.describe "StoriesIndex", type: :request do
let!(:user) { create(:user) }
let!(:article) { create(:article, featured: true) }
describe "GET stories index" do
@ -215,5 +216,69 @@ RSpec.describe "StoriesIndex", type: :request do
expect(response.body).to include("is sponsored by")
expect(response.body).to include(sponsorship.blurb_html)
end
context "with user signed in" do
before do
sign_in user
end
it "has mod-action-button" do
get "/t/#{tag.name}"
expect(response.body).to include('<a class="cta mod-action-button"')
end
it "does not render pagination" do
get "/t/#{tag.name}"
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
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 "does not render pagination" do
get "/t/#{tag.name}"
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
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).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 "does not include current page link" do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/2"
expect(response.body).to include('<span class="olderposts-pagenumber">2')
expect(response.body).not_to include("<a href=\"/t/#{tag.name}/page/2")
get "/t/#{tag.name}"
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 proper canonical url for page 1" do
get "/t/#{tag.name}"
expect(response.body).to include("<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}\" />")
end
it "renders proper canonical url for page 2" do
get "/t/#{tag.name}/page/2"
expect(response.body).to include("<link rel=\"canonical\" href=\"http://localhost:3000/t/#{tag.name}/page/2\" />")
end
end
end
end

6
spec/system/articles/user_visits_articles_by_tag_spec.rb
View file

@ -6,9 +6,9 @@ RSpec.describe "User visits articles by tag", type: :system do
let!(:func_tag) { create(:tag, name: "functional") }
let(:author) { create(:user) }
let!(:article) { create(:article, tags: "javascript, IoT", user: author, published_at: 2.days.ago) }
let!(:article2) { create(:article, tags: "functional", user: author, published_at: Time.current) }
let!(:article3) { create(:article, tags: "functional, javascript", user: author, published_at: 2.weeks.ago) }
let!(:article) { create(:article, tags: "javascript, IoT", user: author, published_at: 2.days.ago, score: 5) }
let!(:article2) { create(:article, tags: "functional", user: author, published_at: Time.current, score: 5) }
let!(:article3) { create(:article, tags: "functional, javascript", user: author, published_at: 2.weeks.ago, score: 5) }
context "when user hasn't logged in" do
context "when 2 articles" do