From 221c363af0fb5a26da89f91c6dc8acf38e3c96c1 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Sat, 13 Oct 2018 19:38:38 -0400 Subject: [PATCH] Reduce queries in typical stories#show response + small fixes (#903) * Reduce queries in typical stories#show response + small fixes * Change const to git in non-es6 code --- .../initializeCommentsPage.js.erb | 3 +- app/controllers/async_info_controller.rb | 1 - app/controllers/stories_controller.rb | 41 +------------- app/decorators/user_decorator.rb | 2 +- app/javascript/chat/__tests__/util.test.js | 8 --- app/labor/follow_checker.rb | 2 +- app/labor/sticky_article_collection.rb | 56 +++++++++++++++++++ app/models/user.rb | 4 +- app/views/articles/_sticky_nav.html.erb | 13 ++--- app/views/articles/show.html.erb | 4 +- 10 files changed, 71 insertions(+), 63 deletions(-) create mode 100644 app/labor/sticky_article_collection.rb diff --git a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb index a2e8d081d..07610643c 100644 --- a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb +++ b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb @@ -1,4 +1,4 @@ -const ENTER_KEY_CODE = 13; + function initializeCommentsPage() { if (document.getElementById('comments-container')) { @@ -274,6 +274,7 @@ function handleKeyUp(event) { function handleKeyDown(event) { // If Ctrl+Enter OR Command+Enter is pressed + var ENTER_KEY_CODE = 13; if((event.ctrlKey || event.metaKey) && event.keyCode === ENTER_KEY_CODE) { // Get user details and extract code of conduct & comment count var user = userData(); diff --git a/app/controllers/async_info_controller.rb b/app/controllers/async_info_controller.rb index a2a4f51df..e7b464f8b 100644 --- a/app/controllers/async_info_controller.rb +++ b/app/controllers/async_info_controller.rb @@ -40,7 +40,6 @@ class AsyncInfoController < ApplicationController followed_user_ids: @user.cached_following_users_ids, reading_list_ids: ReadingList.new(@user).cached_ids_of_articles, saw_onboarding: @user.saw_onboarding, - onboarding_checklist: UserStates.new(@user).cached_onboarding_checklist, checked_code_of_conduct: @user.checked_code_of_conduct, number_of_comments: @user.comments.count, display_sponsors: @user.display_sponsors, diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 6717a4615..5a1eaa1e5 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -20,8 +20,7 @@ class StoriesController < ApplicationController def show @story_show = true @podcast = Podcast.find_by_slug(params[:username]) - @episode = PodcastEpisode.find_by_slug(params[:slug]) - if @podcast && @episode + if @podcast && @episode = PodcastEpisode.find_by_slug(params[:slug]) handle_podcast_show else handle_article_show @@ -210,7 +209,6 @@ class StoriesController < ApplicationController @article_show = true @comment = Comment.new assign_article_and_user_and_organization - assign_sticky_nav handle_possible_redirect return if performed? not_found unless @article @@ -305,41 +303,4 @@ class StoriesController < ApplicationController per(num_articles). filter_excluded_tags(params[:tag]) end - - def assign_sticky_nav - return unless @article - reaction_count_num = Rails.env.production? ? 15 : -1 - comment_count_num = Rails.env.production? ? 7 : -2 - more_articles = [] - article_tags = @article.cached_tag_list_array - article_tags.delete("discuss") - tag_articles = Article.tagged_with(article_tags, any: true). - includes(:user). - where("positive_reactions_count > ? OR comments_count > ?", reaction_count_num, comment_count_num). - where(published: true). - where.not(id: @article.id, user_id: @article.user_id). - limited_column_select. - where("featured_number > ?", 5.days.ago.to_i). - order("RANDOM()"). - limit(8) - if tag_articles.size < 6 - more_articles = Article.tagged_with(["career", "productivity", "discuss", "explainlikeimfive"], any: true). - includes(:user). - where("comments_count > ?", comment_count_num). - limited_column_select. - where(published: true). - where.not(id: @article.id, user_id: @article.user_id). - where("featured_number > ?", 5.days.ago.to_i). - order("RANDOM()"). - limit(10 - tag_articles.size) - end - - @user_stickies = (@organization || @user).articles. - where(published: true). - limited_column_select. - tagged_with(article_tags, any: true). - where.not(id: @article.id).order("published_at DESC"). - limit(2) - @sticky_articles = (tag_articles + more_articles).sample(8) - end end diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index a2b7bd61f..fdbcad935 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -2,7 +2,7 @@ class UserDecorator < ApplicationDecorator delegate_all def cached_followed_tags - Rails.cache.fetch("user-#{id}-#{updated_at}/followed_tags", expires_in: 100.hours) do + Rails.cache.fetch("user-#{id}-#{updated_at}/followed_tags", expires_in: 20.hours) do Tag.where(id: Follow.where(follower_id: id, followable_type: "ActsAsTaggableOn::Tag").pluck(:followable_id)).order("hotness_score DESC") end end diff --git a/app/javascript/chat/__tests__/util.test.js b/app/javascript/chat/__tests__/util.test.js index 34aeca376..13260e66a 100644 --- a/app/javascript/chat/__tests__/util.test.js +++ b/app/javascript/chat/__tests__/util.test.js @@ -84,14 +84,6 @@ describe('Chat utilities', () => { ], reading_list_ids: [48, 49, 34, 51, 64, 56], saw_onboarding: true, - onboarding_checklist: { - write_your_first_article: false, - follow_your_first_tag: false, - fill_out_your_profile: false, - leave_your_first_reaction: true, - follow_your_first_dev: true, - leave_your_first_comment: false, - }, checked_code_of_conduct: false, number_of_comments: 0, display_sponsors: true, diff --git a/app/labor/follow_checker.rb b/app/labor/follow_checker.rb index 4970ba32c..262762eff 100644 --- a/app/labor/follow_checker.rb +++ b/app/labor/follow_checker.rb @@ -8,7 +8,7 @@ class FollowChecker end def cached_follow_check - Rails.cache.fetch("user-#{follower.id}-#{follower.updated_at}/is_following_#{followable_type}_#{followable_id}", expires_in: 100.hours) do + Rails.cache.fetch("user-#{follower.id}-#{follower.updated_at}/is_following_#{followable_type}_#{followable_id}", expires_in: 20.hours) do followable = if followable_type == "Tag" Tag.find(followable_id) elsif followable_type == "Organization" diff --git a/app/labor/sticky_article_collection.rb b/app/labor/sticky_article_collection.rb new file mode 100644 index 000000000..1f091b709 --- /dev/null +++ b/app/labor/sticky_article_collection.rb @@ -0,0 +1,56 @@ +class StickyArticleCollection + attr_accessor :article, :author, :tag_articles, :more_articles, :reaction_count_num, :comment_count_num + def initialize(article, author) + @article = article + @author = author + @article_tags = article_tags + @reaction_count_num = Rails.env.production? ? 15 : -1 + @comment_count_num = Rails.env.production? ? 7 : -2 + @tag_articles = tag_articles + @more_articles = more_articles + end + + def user_stickies + author.articles. + where(published: true). + limited_column_select. + tagged_with(article_tags, any: true). + where.not(id: article.id).order("published_at DESC"). + limit(2) + end + + def suggested_stickies + (tag_articles + more_articles).sample(8) + end + + def tag_articles + Article.tagged_with(article_tags, any: true). + includes(:user). + where("positive_reactions_count > ? OR comments_count > ?", reaction_count_num, comment_count_num). + where(published: true). + where.not(id: article.id, user_id: article.user_id). + limited_column_select. + where("featured_number > ?", 5.days.ago.to_i). + order("RANDOM()"). + limit(8) + end + + def more_articles + return [] if tag_articles.size < 6 + Article.tagged_with(["career", "productivity", "discuss", "explainlikeimfive"], any: true). + includes(:user). + where("comments_count > ?", comment_count_num). + limited_column_select. + where(published: true). + where.not(id: article.id, user_id: article.user_id). + where("featured_number > ?", 5.days.ago.to_i). + order("RANDOM()"). + limit(10 - tag_articles.size) + end + + def article_tags + tags = article.cached_tag_list_array + tags.delete("discuss") + tags + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 0032d577d..c834e36be 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -228,7 +228,7 @@ class User < ApplicationRecord end def cached_preferred_langs - Rails.cache.fetch("user-#{id}-#{updated_at}/cached_preferred_langs", expires_in: 80.hours) do + Rails.cache.fetch("user-#{id}-#{updated_at}/cached_preferred_langs", expires_in: 24.hours) do langs = [] langs << "en" if prefer_language_en langs << "ja" if prefer_language_ja @@ -251,7 +251,7 @@ class User < ApplicationRecord def cached_followed_tag_names cache_name = "user-#{id}-#{updated_at}/followed_tag_names" - Rails.cache.fetch(cache_name, expires_in: 100.hours) do + Rails.cache.fetch(cache_name, expires_in: 24.hours) do Tag.where( id: Follow.where( follower_id: id, diff --git a/app/views/articles/_sticky_nav.html.erb b/app/views/articles/_sticky_nav.html.erb index f557eb356..92254b129 100644 --- a/app/views/articles/_sticky_nav.html.erb +++ b/app/views/articles/_sticky_nav.html.erb @@ -1,3 +1,6 @@ +<% @sticky_collection = StickyArticleCollection.new(@article, @organization || @user) %> +<% @sticky_articles = @sticky_collection.suggested_stickies %> +<% @user_stickies = @sticky_collection.user_stickies %> <% @actor = @article.organization || @article.user %>