Improve performance for additional content boxes and tags index (#1750)

* Improve performance for additional content boxes and tags index * Modify followed_articles endpoint with new classic article api
forem · Feb 6, 2019 · 2d97a5d · 2d97a5d
1 parent dd63720
commit 2d97a5d
Show file tree

Hide file tree

Showing 18 changed files with 153 additions and 118 deletions.
diff --git a/app/assets/javascripts/initializers/initializeAdditionalContentBoxes.js b/app/assets/javascripts/initializers/initializeAdditionalContentBoxes.js
@@ -1,22 +1,40 @@
 function initializeAdditionalContentBoxes() {
-  var el = document.getElementById("additional-content-area");
+  var el = document.getElementById('additional-content-area');
   if (el) {
     var d = new Date();
-    var signature = d.getTime().toString().substring(0, 5);
-    window.fetch('/additional_content_boxes?article_id='+el.dataset.articleId+'&signature='+signature, {
+    var signature = d
+      .getTime()
+      .toString()
+      .substring(0, 5);
+    var user = userData();
+    var stateParam = 'include_sponsors';
+    if (!user.display_sponsors) {
+      stateParam = 'do_not_include_sponsors';
+    }
+    window
+      .fetch(
+        '/additional_content_boxes?article_id=' +
+          el.dataset.articleId +
+          '&signature=' +
+          signature +
+          '&state=' +
+          stateParam,
+        {
           method: 'GET',
-          credentials: 'same-origin'
-        }).then(function (response) {
-      if (response.status === 200) {
-        response.text().then(function(html){
-          el.innerHTML = html;
-          initializeReadingListIcons();
-          initializeAllFollowButts();
-          initializeSponsorshipVisibility();
-        })
-      } else {
-        // there's currently no errorCb.
-      }
-    });
+          credentials: 'same-origin',
+        },
+      )
+      .then(function(response) {
+        if (response.status === 200) {
+          response.text().then(function(html) {
+            el.innerHTML = html;
+            initializeReadingListIcons();
+            initializeAllFollowButts();
+            initializeSponsorshipVisibility();
+          });
+        } else {
+          // there's currently no errorCb.
+        }
+      });
   }
-}
+}
diff --git a/app/controllers/additional_content_boxes_controller.rb b/app/controllers/additional_content_boxes_controller.rb
@@ -1,26 +1,22 @@
 class AdditionalContentBoxesController < ApplicationController
   # No authorization required for entirely public controller
 
-  before_action :set_cache_control_headers, only: [:index], unless: -> { current_user }
+  before_action :set_cache_control_headers, only: [:index]
 
   def index
     article_ids = params[:article_id].split(",")
     @article = Article.find(article_ids[0])
-    @for_user_article = Suggester::Articles::Classic.
-      new(current_user || @article, not_ids: article_ids).get
-    if (!user_signed_in? || current_user&.display_sponsors) &&
+    @suggested_articles = Suggester::Articles::Classic.
+      new(@article, not_ids: article_ids).get(2)
+    if (!user_signed_in? || params[:state] == "include_sponsors") &&
         @article.user.permit_adjacent_sponsors &&
         randomize
       @boosted_article = Suggester::Articles::Boosted.new(
-        current_user,
-        @article,
-        not_ids: (article_ids + [@for_user_article&.id]), area: "additional_articles",
+        (@article.decorate.cached_tag_list_array + @article.boosted_additional_tags.split).sample,
+        not_ids: (article_ids + @suggested_articles.pluck(:id)), area: "additional_articles",
       ).suggest
-    else
-      @alt_classic = Suggester::Articles::Classic.
-        new(@article, not_ids: (article_ids + [@for_user_article&.id])).get
     end
-    set_surrogate_key_header "additional_content_boxes_" + params.to_s unless current_user
+    set_surrogate_key_header "additional_content_boxes_" + params.to_s
     render "boxes", layout: false
   end
 

diff --git a/app/controllers/followed_articles_controller.rb b/app/controllers/followed_articles_controller.rb
@@ -23,7 +23,7 @@ def index
                 else
                   @articles = []
                 end
-    classic_article = Suggester::Articles::Classic.new(current_user).get
+    classic_article = Suggester::Articles::Classic.new(current_user).get.first
     response.headers["Cache-Control"] = "public, max-age=150"
     render json: {
       articles: @articles,

diff --git a/app/labor/article_suggester.rb b/app/labor/article_suggester.rb
@@ -32,6 +32,6 @@ def suggestions_by_tag
   end
 
   def offsets
-    Rails.env.production? ? [10, 50] : [0, 0]
+    Rails.env.production? ? [10, 120] : [0, 0]
   end
 end
diff --git a/app/models/tag.rb b/app/models/tag.rb
@@ -28,6 +28,7 @@ class Tag < ActsAsTaggableOn::Tag
   before_validation :pound_it
   before_save :calculate_hotness_score
   after_save :bust_cache
+  before_save :mark_as_updated
 
   algoliasearch per_environment: true do
     attribute :name, :bg_color_hex, :text_color_hex, :hotness_score, :supported, :short_summary
@@ -85,4 +86,8 @@ def pound_it
     text_color_hex&.prepend("#") unless text_color_hex&.starts_with?("#") || text_color_hex.blank?
     bg_color_hex&.prepend("#") unless bg_color_hex&.starts_with?("#") || bg_color_hex.blank?
   end
+
+  def mark_as_updated
+    self.updated_at = Time.current # Acts-as-taggable didn't come with this by default
+  end
 end
diff --git a/app/services/suggester/articles/boosted.rb b/app/services/suggester/articles/boosted.rb
@@ -1,12 +1,10 @@
 module Suggester
   module Articles
     class Boosted
-      attr_accessor :user, :article, :tags, :not_ids, :area
+      attr_accessor :tag, :not_ids, :area
 
-      def initialize(user, article, options)
-        @user = user
-        @article = article
-        @tags = (user&.cached_followed_tag_names.to_a + article.decorate.cached_tag_list_array)
+      def initialize(tag, options)
+        @tag = tag
         @not_ids = options[:not_ids]
         @area = options[:area]
       end
@@ -15,7 +13,7 @@ def suggest
         base_articles = Article.includes(:user).
           includes(:organization).
           where.not(id: not_ids, organization_id: nil).
-          tagged_with(tags + article.boosted_additional_tags.split, any: true)
+          cached_tagged_with(tag)
 
         if area == "additional_articles"
           base_articles.boosted_via_additional_articles.sample

diff --git a/app/services/suggester/articles/classic.rb b/app/services/suggester/articles/classic.rb
@@ -9,13 +9,14 @@ def initialize(input = nil, options = {})
         @not_ids = options[:not_ids]
       end
 
-      def get(tag_names = random_supported_tag_names)
-        if rand(8) == 1
-          random_high_quality_article
-        else
-          qualifying_articles(tag_names).where.not(id: not_ids).compact.sample ||
-            random_high_quality_article
-        end
+      def get(num = 1)
+        articles = if rand(8) == 1
+                     random_high_quality_articles(num)
+                   else
+                     qualifying_articles(random_supported_tag_names).where.not(id: not_ids).compact.sample(num)
+                   end
+        articles = random_high_quality_articles(num) if articles.empty?
+        articles
       end
 
       def qualifying_articles(tag_names)
@@ -32,8 +33,8 @@ def qualifying_articles(tag_names)
         end
       end
 
-      def random_high_quality_article
-        HighQuality.new(not_ids: not_ids).suggest
+      def random_high_quality_articles(num)
+        HighQuality.new(not_ids: not_ids).suggest(num)
       end
 
       def random_supported_tag_names

diff --git a/app/services/suggester/articles/high_quality.rb b/app/services/suggester/articles/high_quality.rb
@@ -1,20 +1,20 @@
 module Suggester
   module Articles
     class HighQuality
-      MIN_HQ_REACTION_COUNT = Rails.env.production? ? 75 : -1
+      MIN_HQ_REACTION_COUNT = Rails.env.production? ? 75 : 1
 
       def initialize(options = {})
         @not_ids = options[:not_ids]
       end
 
-      def suggest
+      def suggest(num)
         Article.where(published: true, featured: true).
           includes(:user).
           where("positive_reactions_count > ?", MIN_HQ_REACTION_COUNT).
           order("RANDOM()").
           limited_column_select.
           where.not(id: @not_ids).
-          first
+          limit(num)
       end
     end
   end

diff --git a/app/views/additional_content_boxes/boxes.html.erb b/app/views/additional_content_boxes/boxes.html.erb
@@ -4,17 +4,13 @@
     classification: "boosted",
     classification_text: "From one of our Community Sponsors",
     follow_cue: @boosted_article.organization&.tag_line || @boosted_article.organization&.tag_line %>
-<% elsif @alt_classic %>
-  <%= render "additional_content_boxes/article_box",
-    article: @alt_classic,
-    classification: "alt_classic",
-    classification_text: "Classic DEV Post from #{@alt_classic.readable_publish_date}",
-    follow_cue: @alt_classic.organization&.tag_line || "Follow <a href='#{@alt_classic.user.path}'>@#{@alt_classic.user.username}</a> to see more of their posts in your feed." %>
 <% end %>
-<% if @for_user_article %>
-  <%= render "additional_content_boxes/article_box",
-    article: @for_user_article,
-    classification: "for_user_article",
-    classification_text: "Another Post You Might Like",
-    follow_cue: @for_user_article.organization&.tag_line || "Follow <a href='#{@for_user_article.user.path}'>@#{@for_user_article.user.username}</a> to see more of their posts in your feed." %>
+<% if @suggested_articles.any? %>
+  <% @suggested_articles.each do |article| %>
+    <%= render "additional_content_boxes/article_box",
+      article: article,
+      classification: "for_user_article",
+      classification_text: "Another Post You Might Like",
+      follow_cue: article.organization&.tag_line || "Follow <a href='#{article.user.path}'>@#{article.user.username}</a> to see more of their posts in your feed." %>
+    <% end %>
 <% end %>
diff --git a/app/views/articles/show.html.erb b/app/views/articles/show.html.erb
@@ -32,22 +32,22 @@
 <% if !internal_navigation? %>
   <%= content_for :page_meta do %>
     <% if @article.canonical_url.present? %>
-      <link rel="canonical" href="<%= @article.canonical_url %>"/>
+      <link rel="canonical" href="<%= @article.canonical_url %>" />
     <% else %>
-      <link rel="canonical" href="https://dev.to<%=@article.path%>"/>
+      <link rel="canonical" href="https://dev.to<%= @article.path %>" />
     <% end %>
-    <meta name="description" content="<%=@article.description || "An article from the community" %>">
+    <meta name="description" content="<%= @article.description || "An article from the community" %>">
     <meta name="keywords" content="software development, inclusive, community,engineering,<%= @article.cached_tag_list %>">
 
     <meta property="og:type" content="article" />
-    <meta property="og:url" content="https://dev.to<%=@article.path%>" />
-    <meta property="og:title" content="<%=@article.title %>" />
-    <meta property="og:description" content="<%=@article.description || "An article from the community" %>" />
+    <meta property="og:url" content="https://dev.to<%= @article.path%>" />
+    <meta property="og:title" content="<%= @article.title %>" />
+    <meta property="og:description" content="<%= @article.description || "An article from the community" %>" />
     <meta property="og:site_name" content="The Practical Dev" />
     <meta name="twitter:site" content="@ThePracticalDev">
-    <meta name="twitter:creator" content="@<%=@user.twitter_username %>">
-    <meta name="twitter:title" content="<%=@article.title %>">
-    <meta name="twitter:description" content="<%=@article.description || "An article from the community" %>">
+    <meta name="twitter:creator" content="@<%= @user.twitter_username %>">
+    <meta name="twitter:title" content="<%= @article.title %>">
+    <meta name="twitter:description" content="<%= @article.description || "An article from the community" %>">
     <meta name="twitter:card" content="summary_large_image">
     <meta name="twitter:widgets:new-embed-design" content="on">
     <% if @article.published %>
@@ -112,9 +112,9 @@
         </h1>
         <h3>
           <span itemprop="author" itemscope itemtype="http://schema.org/Person">
-            <meta itemprop="url" content="https://dev.to/<%=@user.username%>">
+            <meta itemprop="url" content="https://dev.to/<%=@user.username %>">
             <a href="/<%= @user.username %>" class="author">
-              <img class="profile-pic" src="<%= ProfileImage.new(@user).get(50)%>" alt="<%= @user.username %> profile image"/>
+              <img class="profile-pic" src="<%= ProfileImage.new(@user).get(50) %>" alt="<%= @user.username %> profile image"/>
               <span itemprop="name"><%= @user.name %></span>
             </a>
           </span>
@@ -208,7 +208,7 @@
     <% end %>
   </div>
   <% cache("article-bottom-content-#{@article.cached_tag_list_array.sample}", :expires_in => 18.hours) do %>
-    <% @classic_article = Suggester::Articles::Classic.new(@article).get %>
+    <% @classic_article = Suggester::Articles::Classic.new(@article).get.first %>
     <% if @classic_article %>
       <%= render "additional_content_boxes/article_box",
         article: @classic_article,

diff --git a/app/views/articles/tags/_sidebar.html.erb b/app/views/articles/tags/_sidebar.html.erb
@@ -1,43 +1,45 @@
-<div id="sidebar-wrapper-left" class="sidebar-wrapper sidebar-wrapper-left">
-  <div class="sidebar-bg" id="sidebar-bg-left"></div>
-  <div class="side-bar">
-    <% if @tag_model && @tag_model.short_summary.present? %>
-      <div class="widget">
-        <header>
-          <h4>#<%= @tag %> 👋</h4>
-        </header>
-        <div class="widget-body">
-          <%= @tag_model.short_summary.html_safe %>
+<% cache "tag-sidebar-#{@tag}-#{@tag_model&.updated_at}-#{@tag_model&.taggings_count}", expires_in: 5.hour do %>
+  <div id="sidebar-wrapper-left" class="sidebar-wrapper sidebar-wrapper-left">
+    <div class="sidebar-bg" id="sidebar-bg-left"></div>
+    <div class="side-bar">
+      <% if @tag_model && @tag_model.short_summary.present? %>
+        <div class="widget">
+          <header>
+            <h4>#<%= @tag %> 👋</h4>
+          </header>
+          <div class="widget-body">
+            <%= @tag_model.short_summary.html_safe %>
+          </div>
         </div>
-      </div>
-    <% end %>
-    <% if @tag_model && @tag_model.rules_html.present? %>
-      <div class="widget">
-        <header>
-          <h4>submission guidelines</h4>
-        </header>
-        <div class="widget-body">
-          <%= @tag_model.rules_html.html_safe %>
-          <a class="cta cta-button" href="/new/<%= @tag %>" >
-            WRITE A POST
-          </a>
+      <% end %>
+      <% if @tag_model && @tag_model.rules_html.present? %>
+        <div class="widget">
+          <header>
+            <h4>submission guidelines</h4>
+          </header>
+          <div class="widget-body">
+            <%= @tag_model.rules_html.html_safe %>
+            <a class="cta cta-button" href="/new/<%= @tag %>" >
+              WRITE A POST
+            </a>
+          </div>
         </div>
-      </div>
-    <% end %>
-    <% if @tag_model && @tag_model.wiki_body_html.present? %>
-      <div class="widget">
-        <header>
-          <h4>about #<%= @tag %></h4>
-        </header>
-        <div class="widget-body">
-          <%= @tag_model.wiki_body_html.html_safe %>
+      <% end %>
+      <% if @tag_model && @tag_model.wiki_body_html.present? %>
+        <div class="widget">
+          <header>
+            <h4>about #<%= @tag %></h4>
+          </header>
+          <div class="widget-body">
+            <%= @tag_model.wiki_body_html.html_safe %>
+          </div>
+        </div>
+      <% end %>
+      <div class="sidebar-data">
+        <div>
+          <%= pluralize Article.cached_tagged_with(@tag).where(published: true).size, "Post" %> Published
         </div>
-      </div>
-    <% end %>
-    <div class="sidebar-data">
-      <div>
-        <%= pluralize Article.cached_tagged_with(@tag).where(published: true).size, "Post" %> Published
       </div>
     </div>
   </div>
-</div>
+<% end %>
diff --git a/db/migrate/20190206164319_add_timestamps_to_tags.rb b/db/migrate/20190206164319_add_timestamps_to_tags.rb
@@ -0,0 +1,6 @@
+class AddTimestampsToTags < ActiveRecord::Migration[5.1]
+  def change
+    add_column :tags, :created_at, :datetime
+    add_column :tags, :updated_at, :datetime
+  end
+end