Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Reduce the number of sql queries for articles comments #1606 (#1766) 
* Reduce the number of sql queries for articles comments

* Refactor comments tree display

* Refactor retrieving the comments tree

* Remove comments tree view object

* Update score in comments spec for consistent order
  • Loading branch information
@lightalloy @benhalpern
lightalloy authored and benhalpern committed Feb 16, 2019
1 parent bce0b4f commit 1cc0fcc
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 102 deletions.
4 changes: 2 additions & 2 deletions app/helpers/comments_helper.rb
View file
Expand Up @@ -11,8 +11,8 @@ def comment_user_id_unless_deleted(comment)
comment.deleted ? 0 : comment.user_id
end

def tree_for(comment, commentable)
nested_comments(tree: comment.subtree.includes(:user).arrange, commentable: commentable, is_view_root: true)
def tree_for(comment, sub_comments, commentable)
nested_comments(tree: { comment => sub_comments }, commentable: commentable, is_view_root: true)
end

private
Expand Down
4 changes: 4 additions & 0 deletions app/models/comment.rb
View file
Expand Up @@ -128,6 +128,10 @@ def self.rooted_on(commentable_id, commentable_type)
commentable_type: commentable_type)
end

def self.tree_for(commentable, limit = 0)
commentable.comments.includes(:user).arrange(order: "score DESC").to_a[0..limit - 1].to_h
end

def path
"/#{user.username}/comment/#{id_code_generated}"
rescue StandardError
Expand Down
12 changes: 6 additions & 6 deletions app/views/articles/_full_comment_area.html.erb
View file
@@ -1,6 +1,6 @@
<% cache("whole-comment-area-#{@article.id}-#{@article.last_comment_at}-#{@article.show_comments}", :expires_in => 2.hours) do %>
<% cache("whole-comment-area-#{@article.id}-#{@article.last_comment_at}-#{@article.show_comments}", expires_in: 2.hours) do %>
<div id="comments" style="margin-top: -56px;padding-bottom:56px;position:relative;z-index:-100" data-updated-at="<%= Time.current %>"></div>
<% if @article.show_comments%>
<% if @article.show_comments %>
<div class="comments-container-container">
<div
class="comments-container"
Expand All @@ -11,17 +11,17 @@
commentable: @article,
commentable_type: "Article" %>
<div class="comment-trees" id="comment-trees-container">
<% if @article.comments_count > 0 %>
<% Comment.rooted_on(@article.id, "Article").order("score DESC").limit(@comments_to_show_count).each_with_index do |comment, i| %>
<% if @article.comments_count > 0 %>
<% Comment.tree_for(@article, @comments_to_show_count).each do |comment, sub_comments| %>
<% cache ["comment_root_cached_tree", comment] do %>
<%= tree_for(comment, @article) %>
<%= tree_for(comment, sub_comments, @article) %>
<% end %>
<% end %>
<% end %>
</div>
</div>
<div class="show-comments-footer">
<%= render 'articles/comments_actions' %>
<%= render "articles/comments_actions" %>
</div>
</div>
<% end %>
Expand Down
101 changes: 50 additions & 51 deletions app/views/comments/index.html.erb
View file
@@ -1,98 +1,97 @@
<% if @root_comment.present? %>
<% title truncate(strip_tags(@root_comment.processed_html), length: 100) + " — DEV" %>
<% else %>
<% title "Discussion of "+ @commentable.title + " — DEV" %>
<% title "Discussion of " + @commentable.title + " — DEV" %>
<% end %>
<%= content_for :page_meta do %>
<meta name="description" content="<%=@commentable.description || "An article from the community" %>">
<meta name="description" content="<%= @commentable.description || "An article from the community" %>">
<meta name="keywords" content="software development,engineering,rails,javascript,ruby">

<meta property="og:type" content="article" />
<meta property="og:title" content="Discussion of <%=@commentable.title %>" />
<meta property="og:description" content="<%=@commentable.description || "A DEV Comment" %>" />
<meta property="og:title" content="Discussion of <%= @commentable.title %>" />
<meta property="og:description" content="<%= @commentable.description || "A DEV Comment" %>" />
<meta property="og:site_name" content="The DEV Community" />
<meta name="twitter:site" content="@ThePracticalDev">
<meta name="twitter:creator" content="@<%=@user.twitter_username %>">
<meta name="twitter:title" content="<%=@commentable.title %>">
<meta name="twitter:description" content="<%=@commentable.description || "A DEV Comment" %>">
<meta name="twitter:creator" content="@<%= @user.twitter_username %>">
<meta name="twitter:title" content="<%= @commentable.title %>">
<meta name="twitter:description" content="<%= @commentable.description || "A DEV Comment" %>">
<% if @root_comment.present? %>
<link rel="canonical" href="https://dev.to<%=@root_comment.path%>"/>
<meta property="og:url" content="https://dev.to<%=@root_comment.path%>" />
<meta property="og:title" content="<%=truncate(strip_tags(@root_comment.processed_html), length: 50)%> — DEV" />
<meta name="twitter:title" content="<%=truncate(strip_tags(@root_comment.processed_html), length: 50)%> — DEV">
<link rel="canonical" href="https://dev.to<%= @root_comment.path %>" />
<meta property="og:url" content="https://dev.to<%= @root_comment.path %>" />
<meta property="og:title" content="<%= truncate(strip_tags(@root_comment.processed_html), length: 50) %> — DEV" />
<meta name="twitter:title" content="<%= truncate(strip_tags(@root_comment.processed_html), length: 50) %> — DEV">
<% else %>
<link rel="canonical" href="https://dev.to<%=@commentable.path%>/comments"/>
<meta property="og:url" content="https://dev.to<%=@commentable.path%>/comments" />
<meta property="og:title" content="[Discussion] <%=@commentable.title %> — DEV" />
<meta name="twitter:title" content="[Discussion] <%=@commentable.title %> — DEV">
<link rel="canonical" href="https://dev.to<%= @commentable.path %>/comments" />
<meta property="og:url" content="https://dev.to<%= @commentable.path %>/comments" />
<meta property="og:title" content="[Discussion] <%= @commentable.title %> — DEV" />
<meta name="twitter:title" content="[Discussion] <%= @commentable.title %> — DEV">
<% end %>
<% if @commentable.class.name == "Article" %>
<meta property="og:image" content="<%= cloud_social_image(@commentable) %>" />
<meta name="twitter:image:src" content="<%= cloud_social_image(@commentable) %>">
<% end %>
<% end %>
<% if @root_comment %>
<div class="single-comment-header"></div>
<% else %>
<div class="article-header">
<a href="<%=@commentable.path%>" class="header-link">
<% if @commentable.main_image.present? %>
<div class="picture" style="background-image:url(<%=cloud_cover_url(@commentable.main_image)%>)"></div>
<% else %>
<div class="blank-comment-space"></div>
<% end %>
<h3 id="comments-header"><%= @commentable.title %></h3>
<h4>
<%= @commentable.user.name %>
<span class="published-at"><%= "on "+@commentable.published_at.strftime("%B %d, %Y") if @commentable.published_at %></span>
</h4>
</a>
<% if @commentable.processed_html.present? %>
<% if @commentable.processed_html.size < 350 %>
<div class="body">
<%= sanitize_rendered_markdown(@commentable.processed_html.html_safe) %>
</div>
<% else %>
<div class="body">
<%= truncate(strip_tags(@commentable.processed_html), length:150).html_safe %> <a href="<%=@commentable.path %>"><span class="read-more">[Read Full]</span></a>
</div>
<% end %>
<% if @root_comment %>
<div class="single-comment-header"></div>
<% else %>
<div class="article-header">
<a href="<%= @commentable.path %>" class="header-link">
<% if @commentable.main_image.present? %>
<div class="picture" style="background-image:url(<%= cloud_cover_url(@commentable.main_image) %>)"></div>
<% else %>
<div class="blank-comment-space"></div>
<% end %>
</div>
<% end %>
<h3 id="comments-header"><%= @commentable.title %></h3>
<h4>
<%= @commentable.user.name %>
<span class="published-at"><%= "on " + @commentable.published_at.strftime("%B %d, %Y") if @commentable.published_at %></span>
</h4>
</a>
<% if @commentable.processed_html.present? %>
<% if @commentable.processed_html.size < 350 %>
<div class="body">
<%= sanitize_rendered_markdown(@commentable.processed_html.html_safe) %>
</div>
<% else %>
<div class="body">
<%= truncate(strip_tags(@commentable.processed_html), length: 150).html_safe %> <a href="<%= @commentable.path %>"><span class="read-more">[Read Full]</span></a>
</div>
<% end %>
<% end %>
</div>
<% end %>
<div class="comments-container comments-dedicated-page-container"
id="comments-container"
data-commentable-id="<%= @commentable.id %>"
data-commentable-type="<%= @commentable.class.name %>">
<% if @root_comment %>
<div class="top-level-actions">
<h3>re: <%= @commentable.title %> <a href="<%=@commentable.path%>">VIEW POST</a></h3>
<h3>re: <%= @commentable.title %> <a href="<%= @commentable.path %>">VIEW POST</a></h3>
<span class="comment-action-buttons">
<% unless @root_comment.is_root? %>
<a href="<%= @root_comment.parent.path %>">VIEW PARENT COMMENT</a>
<% end %>
<a href="<%=@commentable.path%>/comments">VIEW FULL DISCUSSION</a>
<a href="<%= @commentable.path %>/comments">VIEW FULL DISCUSSION</a>
</span>
</div>
<% else %>
<%= render "form",
commentable: @commentable,
commentable_type: "Article"
%>
commentable_type: "Article" %>
<% end %>
<div class="comment-trees" id="comment-trees-container">
<% if @root_comment.present? %>
<div class="root-comment">
<% cache ["comment_root-view-root_#{user_signed_in?}", @root_comment] do %>
<%= tree_for(@root_comment, @commentable) %>
<%= tree_for(@root_comment, @root_comment.subtree.arrange[@root_comment], @commentable) %>
<% end %>
</div>
<% else %>
<% Comment.rooted_on(@commentable.id, @commentable_type).order("score DESC").each do |comment| %>
<% Comment.tree_for(@commentable).each do |comment, sub_comments| %>
<% cache ["comment_root_#{user_signed_in?}", comment] do %>
<%= tree_for(comment, @commentable) %>
<%= tree_for(comment, sub_comments, @commentable) %>
<% end %>
<% end %>
<% end %>
Expand All @@ -102,7 +101,7 @@
</center>
</div>
<% if has_vid?(@commentable) %>
<%= render 'articles/fitvids' %>
<%= render "articles/fitvids" %>
<% end %>

<script async>
Expand Down
82 changes: 39 additions & 43 deletions app/views/podcast_episodes/show.html.erb
View file
@@ -1,20 +1,20 @@
<% title @episode.title %>
<%= content_for :page_meta do %>
<link rel="canonical" href="https://dev.to/<%=@podcast.slug %>/<%=@episode.slug %>"/>
<meta name="description" content="<%=truncate(strip_tags(@episode.body), length: 140) %>">
<link rel="canonical" href="https://dev.to/<%= @podcast.slug %>/<%= @episode.slug %>" />
<meta name="description" content="<%= truncate(strip_tags(@episode.body), length: 140) %>">
<meta name="keywords" content="software development,engineering,rails,javascript,ruby">

<meta property="og:type" content="article" />
<meta property="og:url" content="https://dev.to/<%=@podcast.slug %>/<%=@episode.slug %>" />
<meta property="og:title" content="<%=@episode.title %>" />
<meta property="og:description" content="<%=truncate(strip_tags(@episode.body), length: 140) %>" />
<meta property="og:url" content="https://dev.to/<%= @podcast.slug %>/<%= @episode.slug %>" />
<meta property="og:title" content="<%= @episode.title %>" />
<meta property="og:description" content="<%= truncate(strip_tags(@episode.body), length: 140) %>" />
<meta property="og:site_name" content="The Practical Dev" />

<meta name="twitter:site" content="@ThePracticalDev">
<meta name="twitter:creator" content="@software_daily">
<meta name="twitter:title" content="<%=@episode.title %>">
<meta name="twitter:description" content="<%=truncate(strip_tags(@episode.body), length: 140) %>" />
<meta name="twitter:title" content="<%= @episode.title %>">
<meta name="twitter:description" content="<%= truncate(strip_tags(@episode.body), length: 140) %>" />

<% if @episode.social_image.present? %>
<meta name="twitter:card" content="summary_large_image">
Expand All @@ -28,21 +28,19 @@
<% end %>
<%= render "podcast_episodes/podcast_bar" %>


<div class="podcast-episode-container">
<div class="hero">
<div class="title" style="background:#<%= @podcast.main_color_hex %> url(<%= cl_image_path(@podcast.pattern_image_url || "https://i.imgur.com/fKYKgo4.png",
:type=>"fetch",
:quality => "auto",
:sign_url => true,
:flags => "progressive",
:fetch_format => "jpg") %>)">
type: "fetch",
quality: "auto",
sign_url: true,
flags: "progressive",
fetch_format: "jpg") %>)">
<h2>
<a href="/<%= @podcast.slug %>">
<img alt="<%= @podcast.title %>" src="<%=cloudinary(@podcast.image_url,width=60,quality=80) %>" /><%= @podcast.title %>
<img alt="<%= @podcast.title %>" src="<%= cloudinary(@podcast.image_url, 60, 80) %>" /><%= @podcast.title %>
</a>
</h2>
<% if @episode.title.size > 60 %>
Expand All @@ -51,37 +49,36 @@
<h1><%= @episode.title %></h1>
<% end %>
</div>
<div id="record-<%= @episode.slug%>" class="record-wrapper" data-podcast="<%= @podcast.slug %>" data-episode="<%= @episode.slug %>">
<div id="record-<%= @episode.slug %>" class="record-wrapper" data-podcast="<%= @podcast.slug %>" data-episode="<%= @episode.slug %>">
<div class="record" id="record">
<%=cl_image_tag(@podcast.image_url,
:type=>"fetch",
:crop => "fill",
:width => 420,
:height => 420,
:quality => "auto",
:sign_url => true,
:flags => "progressive",
:fetch_format => "auto",
:class => "main-image")
%>
<img alt="Play Button" class="butt play-butt" src="<%= image_path('playbutt.png') %>"/>
<img alt="Pause Button" class="butt pause-butt" src="<%= image_path('pausebutt.png') %>"/>
<%= cl_image_tag(@podcast.image_url,
type: "fetch",
crop: "fill",
width: 420,
height: 420,
quality: "auto",
sign_url: true,
flags: "progressive",
fetch_format: "auto",
class: "main-image") %>
<img alt="Play Button" class="butt play-butt" src="<%= image_path("playbutt.png") %>" />
<img alt="Pause Button" class="butt pause-butt" src="<%= image_path("pausebutt.png") %>" />
<div class="status-message" id="status-message-<%= @episode.slug %>">
play
</div>
</div>
</div>
</div>
<% if !@episode.podcast.status_notice.empty? %>
<center>
<h1 style="color: #e05252;">
<%= @episode.podcast.status_notice %>
</h1>
<p>
<a href="<%= @episode.media_url %>">Click here to download/listen</a>
</p>
</center>
<% end %>
<% if !@episode.podcast.status_notice.empty? %>
<center>
<h1 style="color: #e05252;">
<%= @episode.podcast.status_notice %>
</h1>
<p>
<a href="<%= @episode.media_url %>">Click here to download/listen</a>
</p>
</center>
<% end %>
<%= render "podcast_episodes/subscribe_buttons" %>
<div class="container">
<div class="body">
Expand All @@ -90,8 +87,7 @@
<% else %>
<%= sanitize (@episode.processed_html || "").html_safe,
tags: %w(strong em a table tbody thead tfoot th tr td col colgroup del p h1 h2 h3 h4 h5 h6 blockquote time div span i em u b ul ol li dd dl dt q code pre img sup cite center small),
attributes: %w(href strong em class ref rel src title alt colspan height width size rowspan span value start data-conversation data-lang id)
%>
attributes: %w(href strong em class ref rel src title alt colspan height width size rowspan span value start data-conversation data-lang id) %>
<% end %>
<p style="font-size:16px;">
<a href="<%= @episode.website_url %>">Episode source</a>
Expand All @@ -107,9 +103,9 @@
commentable: @episode,
commentable_type: "PodcastEpisode" %>
<div class="comment-trees" id="comment-trees-container">
<% Comment.rooted_on(@episode.id,"PodcastEpisode").order("score DESC").limit(12).each do |comment| %>
<% Comment.tree_for(@episode, 12).each do |comment, sub_comments| %>
<% cache ["comment_root_#{user_signed_in?}", comment] do %>
<%= tree_for(comment, @episode) %>
<%= tree_for(comment, sub_comments, @episode) %>
<% end %>
<% end %>
</div>
Expand Down
48 changes: 48 additions & 0 deletions spec/features/comments/user_views_article_comments_spec.rb
View file
@@ -0,0 +1,48 @@
require "rails_helper"

RSpec.describe "Visiting article comments", type: :feature, js: true do
let(:user) { create(:user) }
let(:article) { create(:article, user_id: user.id, show_comments: true) }
let!(:comment) { create(:comment, commentable: article, user: user) }
let!(:child_comment) { create(:comment, commentable: article, parent: comment) }
let!(:grandchild_comment) { create(:comment, commentable: article, parent: child_comment) }

before do
create(:comment, commentable: article, parent: comment)
comments = create_list(:comment, 3, commentable: article)
create(:comment, commentable: article, parent: comments.sample)
sign_in user
end

context "when all comments" do
before { visit "#{article.path}/comments" }

it "displays comments" do
expect(page).to have_selector(".single-comment-node", visible: true, count: 8)
end

it "displays child comments" do
expect(page).to have_selector(".comment-deep-1", visible: true, count: 3)
end

it "displays grandchild comments" do
expect(page).to have_selector("#comment-node-#{grandchild_comment.id}.comment-deep-2", visible: true, count: 1)
end
end

context "when root is specified" do
before { visit "#{article.path}/comments/#{comment.id.to_s(26)}" }

it "displays related comments" do
expect(page).to have_selector(".single-comment-node", visible: true, count: 4)
end

it "displays child comments" do
expect(page).to have_selector(".comment-deep-1", visible: true, count: 2)
end

it "displays grandchild comments" do
expect(page).to have_selector("#comment-node-#{grandchild_comment.id}.comment-deep-2", visible: true, count: 1)
end
end
end

0 comments on commit 1cc0fcc

Please sign in to comment.