* 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
This commit is contained in:
Anna Buianova
Ben Halpern
parent
bce0b4fff6
commit
1cc0fccbf7
7 changed files with 170 additions and 102 deletions
|
|
@ -11,8 +11,8 @@ module CommentsHelper
|
|||
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
|
||||
|
|
|
|||
|
|
@ -128,6 +128,10 @@ class Comment < ApplicationRecord
|
|||
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
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -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 %>
|
||||
|
|
|
|||
|
|
@ -1,31 +1,31 @@
|
|||
<% 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) %>" />
|
||||
|
|
@ -33,66 +33,65 @@
|
|||
<% 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 %>
|
||||
|
|
@ -102,7 +101,7 @@
|
|||
</center>
|
||||
</div>
|
||||
<% if has_vid?(@commentable) %>
|
||||
<%= render 'articles/fitvids' %>
|
||||
<%= render "articles/fitvids" %>
|
||||
<% end %>
|
||||
|
||||
<script async>
|
||||
|
|
|
|||
|
|
@ -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">
|
||||
|
|
@ -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 %>
|
||||
|
|
@ -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">
|
||||
|
|
@ -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>
|
||||
|
|
@ -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>
|
||||
|
|
|
|||
48
spec/features/comments/user_views_article_comments_spec.rb
Normal file
48
spec/features/comments/user_views_article_comments_spec.rb
Normal 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
|
||||
|
|
@ -50,6 +50,7 @@ RSpec.describe Comment, type: :model do
|
|||
end
|
||||
end
|
||||
|
||||
# rubocop:disable RSpec/ExampleLength
|
||||
it "adds timestamp url if commentable has video and timestamp" do
|
||||
video_comment.body_markdown = "I like the part at 4:30"
|
||||
video_comment.save
|
||||
|
|
@ -68,6 +69,7 @@ RSpec.describe Comment, type: :model do
|
|||
expect(video_comment.processed_html.include?(">1:52:30</a>")).to eq(true)
|
||||
expect(video_comment.processed_html.include?(">1:20</a>")).to eq(true)
|
||||
end
|
||||
# rubocop:enable RSpec/ExampleLength
|
||||
|
||||
it "does not add timestamp if commentable does not have video" do
|
||||
comment.body_markdown = "I like the part at 1:52:30 and 1:20"
|
||||
|
|
@ -192,4 +194,23 @@ RSpec.describe Comment, type: :model do
|
|||
expect(comment).not_to be_valid
|
||||
end
|
||||
end
|
||||
|
||||
describe "tree" do
|
||||
let(:article2) { create(:article) }
|
||||
let!(:tree_comment) { create(:comment, commentable: article2) }
|
||||
let!(:child) { create(:comment, commentable: article2, parent: tree_comment) }
|
||||
let!(:tree_comment2) { create(:comment, commentable: article2) }
|
||||
|
||||
before { tree_comment.update_column(:score, 1) }
|
||||
|
||||
it "returns a full tree" do
|
||||
comments = described_class.tree_for(article2)
|
||||
expect(comments).to eq(tree_comment => { child => {} }, tree_comment2 => {})
|
||||
end
|
||||
|
||||
it "returns part of the tree" do
|
||||
comments = described_class.tree_for(article2, 1)
|
||||
expect(comments).to eq(tree_comment => { child => {} })
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue