Break StickyArticleCollection into services (#12147)

* Break StickyArticleCollection into services

* Add tests
This commit is contained in:
Michael Kohl 2021-01-08 09:14:47 +07:00 committed by GitHub
parent 100990f7c6
commit 45cecc8592
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 87 additions and 56 deletions

View file

@ -1,48 +0,0 @@
class StickyArticleCollection
attr_accessor :article, :author, :reaction_count_num, :comment_count_num
def initialize(article, author)
@article = article
@author = author
@reaction_count_num = Rails.env.production? ? 15 : -1
@comment_count_num = Rails.env.production? ? 7 : -2
end
def user_stickies
author.articles.published
.limited_column_select
.tagged_with(article_tags, any: true)
.where.not(id: article.id).order(published_at: :desc)
.limit(3)
end
def suggested_stickies
(tag_articles.load + more_articles).sample(3)
end
def tag_articles
@tag_articles ||= Article.published.tagged_with(article_tags, any: true)
.limited_column_select
.where("public_reactions_count > ? OR comments_count > ?", reaction_count_num, comment_count_num)
.where.not(id: article.id).where.not(user_id: article.user_id)
.where("featured_number > ?", 5.days.ago.to_i)
.order(Arel.sql("RANDOM()"))
.limit(3)
end
def more_articles
return [] if tag_articles.size > 6
Article.published.tagged_with(%w[career productivity discuss explainlikeimfive], any: true)
.limited_column_select
.where("comments_count > ?", comment_count_num)
.where.not(id: article.id).where.not(user_id: article.user_id)
.where("featured_number > ?", 5.days.ago.to_i)
.order(Arel.sql("RANDOM()"))
.limit(10 - tag_articles.size)
end
def article_tags
@article_tags ||= article.cached_tag_list_array - ["discuss"]
end
end

View file

@ -0,0 +1,16 @@
module Articles
class GetUserStickies
def self.call(article, author)
article_tags = article.cached_tag_list_array - ["discuss"]
author
.articles
.published
.limited_column_select
.tagged_with(article_tags, any: true)
.where.not(id: article.id)
.order(published_at: :desc)
.limit(3)
end
end
end

View file

@ -0,0 +1,53 @@
module Articles
class SuggestStickies
SUGGESTION_TAGS = %w[career productivity discuss explainlikeimfive].freeze
def self.call(article)
new(article).call
end
def initialize(article)
@article = article
@reaction_count_num = Rails.env.production? ? 15 : -1
@comment_count_num = Rails.env.production? ? 7 : -2
end
def call
(tag_articles.load + more_articles).sample(3)
end
private
attr_accessor :article, :reaction_count_num, :comment_count_num
def tag_articles
article_tags = article.cached_tag_list_array - ["discuss"]
Article
.published
.tagged_with(article_tags, any: true)
.limited_column_select
.where("public_reactions_count > ? OR comments_count > ?", reaction_count_num, comment_count_num)
.where.not(id: article.id)
.where.not(user_id: article.user_id)
.where("featured_number > ?", 5.days.ago.to_i)
.order(Arel.sql("RANDOM()"))
.limit(3)
end
def more_articles
return [] if tag_articles.size > 6
Article
.published
.tagged_with(SUGGESTION_TAGS, any: true)
.limited_column_select
.where("comments_count > ?", comment_count_num)
.where.not(id: article.id)
.where.not(user_id: article.user_id)
.where("featured_number > ?", 5.days.ago.to_i)
.order(Arel.sql("RANDOM()"))
.limit(10 - tag_articles.size)
end
end
end

View file

@ -36,11 +36,7 @@
</div>
<% cache("article-sticky-nav-articles-0-#{@article.id}-#{@actor.last_article_at}", expires_in: 48.hours) do %>
<% @sticky_collection = StickyArticleCollection.new(@article, @organization || @user) %>
<% @sticky_articles = @sticky_collection.suggested_stickies %>
<% @user_stickies = @sticky_collection.user_stickies %>
<% if @user_stickies.present? %>
<% if (user_stickies = Articles::GetUserStickies.call(@article, @actor)).present? %>
<div class="crayons-card crayons-card--secondary">
<header class="crayons-card__header">
<h3 class="crayons-subtitle-2">
@ -48,7 +44,7 @@
</h3>
</header>
<div>
<% @user_stickies.each_with_index do |article, index| %>
<% user_stickies.each_with_index do |article, index| %>
<a class="crayons-link crayons-link--contentful" href="<%= article.path %>">
<%= article.title %>
<div class="crayons-link__secondary -ml-1">
@ -60,7 +56,7 @@
<% end %>
</div>
</div>
<% elsif @sticky_articles.present? %>
<% elsif (sticky_articles = Articles::SuggestStickies.call(@article)).present? %>
<div class="crayons-card crayons-card--secondary">
<header class="crayons-card__header">
<h3 class="crayons-subtitle-2">
@ -69,7 +65,7 @@
</h3>
</header>
<div>
<% @sticky_articles.each_with_index do |article, index| %>
<% sticky_articles.each_with_index do |article, index| %>
<a class="crayons-link crayons-link--contentful flex" href="<%= article.path %>">
<span class="crayons-avatar mr-2 shrink-0">
<img src="<%= Images::Profile.call(article.cached_user.profile_image_url, length: 90) %>" class="crayons-avatar__image" loading="lazy" alt="<%= article.cached_user.name %> profile image">

View file

@ -26,6 +26,20 @@ RSpec.describe "Views an article", type: :system do
expect { visit("/#{user.username}/#{article.slug}/mod") }.to raise_error(Pundit::NotAuthorizedError)
end
describe "sticky nav sidebar" do
it "suggests articles by other users if the author has no other articles" do
create(:article, user: create(:user))
visit article.path
expect(page).to have_text("Trending on #{SiteConfig.community_name}")
end
it "suggests more articles by the author if there are any" do
create(:article, user: user)
visit article.path
expect(page).to have_text("More from #{user.name}")
end
end
describe "when showing the date" do
# TODO: @sre ideally this spec should have js:true enabled since we use
# js helpers to ensure the datetime is locale. However, testing locale