From 802e225a071a456cb076df490cc4749e472a7ef9 Mon Sep 17 00:00:00 2001 From: Josh Puetz Date: Mon, 13 Apr 2020 09:21:03 -0500 Subject: [PATCH] [deploy] Add top comments to articles feed (#6988) * Add 'top_comments' to article feed * Only include root comments * Cache and eagerly load top comments * Clear top comments cache when article score is updated * Add articles.top_comments to bullet whitelist * Update app/models/article.rb Co-Authored-By: rhymes * Remove limit in top_comments association, remove Rails caching of top comments * Remove unnecessary test Co-authored-by: rhymes --- app/models/article.rb | 8 ++++++- app/services/articles/feed.rb | 4 ++-- app/views/stories/feeds/show.json.jbuilder | 1 + config/environments/development.rb | 3 +++ config/environments/test.rb | 1 + spec/models/article_spec.rb | 28 ++++++++++++++++++++++ spec/requests/stories/feeds_spec.rb | 11 +++++++++ 7 files changed, 53 insertions(+), 3 deletions(-) diff --git a/app/models/article.rb b/app/models/article.rb index 4a78fc818..e53d79ef3 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -29,6 +29,11 @@ class Article < ApplicationRecord counter_culture :organization has_many :comments, as: :commentable, inverse_of: :commentable + has_many :top_comments, + -> { where("comments.score > ? AND ancestry IS NULL", 10).order("comments.score DESC") }, + as: :commentable, + inverse_of: :commentable, + class_name: "Comment" has_many :profile_pins, as: :pinnable, inverse_of: :pinnable has_many :buffer_updates, dependent: :destroy has_many :notifications, as: :notifiable, inverse_of: :notifiable, dependent: :delete_all @@ -120,7 +125,8 @@ class Article < ApplicationRecord :video, :user_id, :organization_id, :video_source_url, :video_code, :video_thumbnail_url, :video_closed_caption_track_url, :language, :experience_level_rating, :experience_level_rating_distribution, :cached_user, :cached_organization, - :published_at, :crossposted_at, :boost_states, :description, :reading_time, :video_duration_in_seconds) + :published_at, :crossposted_at, :boost_states, :description, :reading_time, :video_duration_in_seconds, + :last_comment_at) } scope :limited_columns_internal_select, lambda { diff --git a/app/services/articles/feed.rb b/app/services/articles/feed.rb index a66d573fe..4dbcb4d25 100644 --- a/app/services/articles/feed.rb +++ b/app/services/articles/feed.rb @@ -27,7 +27,7 @@ module Articles end def published_articles_by_tag - articles = Article.published.limited_column_select.page(@page).per(@number_of_articles) + articles = Article.published.limited_column_select.includes(:top_comments).page(@page).per(@number_of_articles) articles = articles.cached_tagged_with(@tag) if @tag.present? # More efficient than tagged_with articles end @@ -213,7 +213,7 @@ module Articles hot_stories = hot_stories.offset(offset) new_stories = Article.published. where("score > ?", -15). - limited_column_select.order("published_at DESC").limit(rand(15..80)) + limited_column_select.includes(:top_comments).order("published_at DESC").limit(rand(15..80)) hot_stories = hot_stories.to_a + new_stories.to_a end [featured_story, hot_stories.to_a] diff --git a/app/views/stories/feeds/show.json.jbuilder b/app/views/stories/feeds/show.json.jbuilder index 92d51dfa0..44e4cb7e8 100644 --- a/app/views/stories/feeds/show.json.jbuilder +++ b/app/views/stories/feeds/show.json.jbuilder @@ -24,5 +24,6 @@ json.array!(@stories) do |article| end json.tag_list article.cached_tag_list_array + json.top_comments article.top_comments json.extract! article, *article_methods_to_include end diff --git a/config/environments/development.rb b/config/environments/development.rb index a1aa57925..f9e031281 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -1,3 +1,4 @@ +# rubocop:disable Metrics/BlockLength # Silence all Ruby 2.7 deprecation warnings $VERBOSE = nil @@ -126,6 +127,7 @@ Rails.application.configure do Bullet.add_whitelist(type: :unused_eager_loading, class_name: "ApiSecret", association: :user) # acts-as-taggable-on has super weird eager loading problems: Bullet.add_whitelist(type: :n_plus_one_query, class_name: "ActsAsTaggableOn::Tagging", association: :tag) + Bullet.add_whitelist(type: :unused_eager_loading, class_name: "Article", association: :top_comments) # Check if there are any data update scripts to run during startup if %w[c console runner s server].include?(ENV["COMMAND"]) @@ -137,3 +139,4 @@ Rails.application.configure do end Rails.application.routes.default_url_options = { host: Rails.application.config.app_domain } +# rubocop:enable Metrics/BlockLength diff --git a/config/environments/test.rb b/config/environments/test.rb index bbd2338ad..bbf8c0e61 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -68,6 +68,7 @@ Rails.application.configure do Bullet.add_whitelist(type: :unused_eager_loading, class_name: "ApiSecret", association: :user) # acts-as-taggable-on has super weird eager loading problems: Bullet.add_whitelist(type: :n_plus_one_query, class_name: "ActsAsTaggableOn::Tagging", association: :tag) + Bullet.add_whitelist(type: :unused_eager_loading, class_name: "Article", association: :top_comments) end end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 76357e39f..d4fa0a1fe 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -845,6 +845,34 @@ RSpec.describe Article, type: :model do end end + describe "#top_comments" do + context "when article has comments" do + let(:root_comment) { create(:comment, commentable: article, score: 20) } + let(:child_comment) { create(:comment, commentable: article, score: 20, parent: root_comment) } + + before do + root_comment + child_comment + create_list(:comment, 2, commentable: article, score: 20) + article.reload + end + + it "returns comments with score greater than 10" do + expect(article.top_comments.first.score).to be > 10 + end + + it "only includes root comments" do + expect(article.top_comments).not_to include(child_comment) + end + end + + context "when article does not have any comments" do + it "retrns empty set if there aren't any top comments" do + expect(article.top_comments).to be_empty + end + end + end + describe "#touch_by_reaction" do it "reindexes elasticsearch doc" do sidekiq_assert_enqueued_with(job: Search::IndexToElasticsearchWorker, args: [described_class.to_s, article.search_id]) do diff --git a/spec/requests/stories/feeds_spec.rb b/spec/requests/stories/feeds_spec.rb index 0b8577ea0..7299891f3 100644 --- a/spec/requests/stories/feeds_spec.rb +++ b/spec/requests/stories/feeds_spec.rb @@ -114,6 +114,17 @@ RSpec.describe "Stories::Feeds", type: :request do end end + context "when there are highly rated comments" do + let(:comment) { create(:comment, score: 20) } + let(:article) { comment.commentable } + + it "renders top comments for the article" do + get "/stories/feed/infinity", headers: headers + + expect(response_article["top_comments"]).not_to be_nil + end + end + context "when user is signed in but there's no field_test" do before do sign_in user