Commit f0ec4fa49a11b9f72cc80affcad105e04e3f8510
Exists in
master
and in
14 other branches
Merge branch 'comment_paragraph_performance' into 'master'
Improve comment paragraph performance Also fix a broken unit test See merge request !788
Showing
7 changed files
with
42 additions
and
5 deletions
Show diff stats
plugins/comment_paragraph/controllers/profile/comment_paragraph_plugin_profile_controller.rb
| 1 | -class CommentParagraphPluginProfileController < ProfileController | 1 | +class CommentParagraphPluginProfileController < CommentController |
| 2 | append_view_path File.join(File.dirname(__FILE__) + '/../../views') | 2 | append_view_path File.join(File.dirname(__FILE__) + '/../../views') |
| 3 | 3 | ||
| 4 | def view_comments | 4 | def view_comments |
| @@ -11,4 +11,14 @@ class CommentParagraphPluginProfileController < ProfileController | @@ -11,4 +11,14 @@ class CommentParagraphPluginProfileController < ProfileController | ||
| 11 | render :partial => 'comment/comment.html.erb', :collection => @comments | 11 | render :partial => 'comment/comment.html.erb', :collection => @comments |
| 12 | end | 12 | end |
| 13 | 13 | ||
| 14 | + def comment_form | ||
| 15 | + @page = profile.articles.find(params[:article_id]) | ||
| 16 | + render :partial => 'comment/comment_form', :locals => { | ||
| 17 | + :comment => Comment.new, | ||
| 18 | + :display_link => true, | ||
| 19 | + :cancel_triggers_hide => true, | ||
| 20 | + :paragraph_uuid => params[:paragraph_uuid] | ||
| 21 | + } | ||
| 22 | + end | ||
| 23 | + | ||
| 14 | end | 24 | end |
plugins/comment_paragraph/lib/comment_paragraph_plugin/macros/allow_comment.rb
| @@ -10,7 +10,8 @@ class CommentParagraphPlugin::AllowComment < Noosfero::Plugin::Macro | @@ -10,7 +10,8 @@ class CommentParagraphPlugin::AllowComment < Noosfero::Plugin::Macro | ||
| 10 | def parse(params, inner_html, source) | 10 | def parse(params, inner_html, source) |
| 11 | paragraph_uuid = params[:paragraph_uuid] | 11 | paragraph_uuid = params[:paragraph_uuid] |
| 12 | article = source | 12 | article = source |
| 13 | - count = article.paragraph_comments.without_spam.in_paragraph(paragraph_uuid).count | 13 | + @paragraph_comments_counts ||= article.paragraph_comments.without_spam.group(:paragraph_uuid).reorder(:paragraph_uuid).count |
| 14 | + count = @paragraph_comments_counts.fetch(paragraph_uuid, 0) | ||
| 14 | 15 | ||
| 15 | proc { | 16 | proc { |
| 16 | if controller.kind_of?(ContentViewerController) && article.comment_paragraph_plugin_activated? | 17 | if controller.kind_of?(ContentViewerController) && article.comment_paragraph_plugin_activated? |
plugins/comment_paragraph/lib/ext/article.rb
| @@ -36,7 +36,7 @@ class Article | @@ -36,7 +36,7 @@ class Article | ||
| 36 | if body && (body_changed? || setting_changed?(:comment_paragraph_plugin_activate)) | 36 | if body && (body_changed? || setting_changed?(:comment_paragraph_plugin_activate)) |
| 37 | updated = body_changed? ? body_change[1] : body | 37 | updated = body_changed? ? body_change[1] : body |
| 38 | doc = Nokogiri::HTML(updated) | 38 | doc = Nokogiri::HTML(updated) |
| 39 | - doc.css('li, body > div, body > span, body > p').each do |paragraph| | 39 | + (doc.css('li') + doc.css('body > div, body > span, body > p')).each do |paragraph| |
| 40 | next if paragraph.css('[data-macro="comment_paragraph_plugin/allow_comment"]').present? || paragraph.content.blank? | 40 | next if paragraph.css('[data-macro="comment_paragraph_plugin/allow_comment"]').present? || paragraph.content.blank? |
| 41 | 41 | ||
| 42 | commentable = Nokogiri::XML::Node.new("span", doc) | 42 | commentable = Nokogiri::XML::Node.new("span", doc) |
plugins/comment_paragraph/public/comment_paragraph_macro.js
| @@ -80,6 +80,12 @@ jQuery(document).ready(function($) { | @@ -80,6 +80,12 @@ jQuery(document).ready(function($) { | ||
| 80 | container.find('.display-comment-form').show(); | 80 | container.find('.display-comment-form').show(); |
| 81 | } | 81 | } |
| 82 | }); | 82 | }); |
| 83 | + var formDiv = container.find('.side-comment .post_comment_box'); | ||
| 84 | + if(formDiv.find('.page-comment-form').length==0) { | ||
| 85 | + $.ajax(formDiv.data('comment_paragraph_form_url')).done(function(data) { | ||
| 86 | + formDiv.append(data); | ||
| 87 | + }); | ||
| 88 | + } | ||
| 83 | }); | 89 | }); |
| 84 | 90 | ||
| 85 | 91 |
plugins/comment_paragraph/test/functional/comment_paragraph_plugin_profile_controller_test.rb
| @@ -10,6 +10,9 @@ class CommentParagraphPluginProfileControllerTest < ActionController::TestCase | @@ -10,6 +10,9 @@ class CommentParagraphPluginProfileControllerTest < ActionController::TestCase | ||
| 10 | @profile = create_user('testuser').person | 10 | @profile = create_user('testuser').person |
| 11 | @article = profile.articles.build(:name => 'test') | 11 | @article = profile.articles.build(:name => 'test') |
| 12 | @article.save! | 12 | @article.save! |
| 13 | + @environment = Environment.default | ||
| 14 | + @environment.enabled_plugins = ['CommentParagraphPlugin'] | ||
| 15 | + @environment.save! | ||
| 13 | end | 16 | end |
| 14 | attr_reader :article, :profile | 17 | attr_reader :article, :profile |
| 15 | 18 | ||
| @@ -39,4 +42,12 @@ class CommentParagraphPluginProfileControllerTest < ActionController::TestCase | @@ -39,4 +42,12 @@ class CommentParagraphPluginProfileControllerTest < ActionController::TestCase | ||
| 39 | assert_match /d comment/, @response.body | 42 | assert_match /d comment/, @response.body |
| 40 | end | 43 | end |
| 41 | 44 | ||
| 45 | + should 'load the comment form for a paragraph' do | ||
| 46 | + login_as('testuser') | ||
| 47 | + comment = fast_create(Comment, :source_id => article, :author_id => profile, :title => 'a comment', :body => 'lalala', :paragraph_uuid => 0) | ||
| 48 | + xhr :get, :comment_form, :profile => @profile.identifier, :article_id => article.id, :paragraph_uuid => 0 | ||
| 49 | + assert_select ".page-comment-form" | ||
| 50 | + assert_select "#comment_paragraph_uuid[value=?]", '0' | ||
| 51 | + end | ||
| 52 | + | ||
| 42 | end | 53 | end |
plugins/comment_paragraph/test/unit/allow_comment_test.rb
| @@ -46,4 +46,13 @@ class AllowCommentTest < ActiveSupport::TestCase | @@ -46,4 +46,13 @@ class AllowCommentTest < ActiveSupport::TestCase | ||
| 46 | assert_equal 'inner', instance_eval(&content) | 46 | assert_equal 'inner', instance_eval(&content) |
| 47 | end | 47 | end |
| 48 | 48 | ||
| 49 | + should 'preload comment counts when parsing content' do | ||
| 50 | + 3.times { fast_create(Comment, :paragraph_uuid => '2', :source_id => article.id) } | ||
| 51 | + content = macro.parse({:paragraph_uuid => comment.paragraph_uuid}, article.body, article) | ||
| 52 | + paragraph_comments_counts = macro.instance_variable_get(:@paragraph_comments_counts) | ||
| 53 | + assert_equivalent ['1', '2'], paragraph_comments_counts.keys | ||
| 54 | + assert_equal 1, paragraph_comments_counts['1'] | ||
| 55 | + assert_equal 3, paragraph_comments_counts['2'] | ||
| 56 | + end | ||
| 57 | + | ||
| 49 | end | 58 | end |
plugins/comment_paragraph/views/comment_paragraph_plugin_profile/_comment_paragraph.html.erb
| @@ -11,14 +11,14 @@ | @@ -11,14 +11,14 @@ | ||
| 11 | </div> | 11 | </div> |
| 12 | 12 | ||
| 13 | <% load_comments_url = url_for({:profile => profile_identifier, :controller => 'comment_paragraph_plugin_profile', :action => 'view_comments', :paragraph_uuid => paragraph_uuid, :article_id => article_id}) %> | 13 | <% load_comments_url = url_for({:profile => profile_identifier, :controller => 'comment_paragraph_plugin_profile', :action => 'view_comments', :paragraph_uuid => paragraph_uuid, :article_id => article_id}) %> |
| 14 | + <% load_comment_form_url = url_for({:profile => profile_identifier, :controller => 'comment_paragraph_plugin_profile', :action => 'comment_form', :paragraph_uuid => paragraph_uuid, :article_id => article_id}) %> | ||
| 14 | 15 | ||
| 15 | <div class="side-comment" data-comment_paragraph_url="<%= load_comments_url %>"> | 16 | <div class="side-comment" data-comment_paragraph_url="<%= load_comments_url %>"> |
| 16 | <div class="article-comments-list"> | 17 | <div class="article-comments-list"> |
| 17 | <div class="loading"></div> | 18 | <div class="loading"></div> |
| 18 | </div> | 19 | </div> |
| 19 | <div class ="article-comments-list-more"></div> | 20 | <div class ="article-comments-list-more"></div> |
| 20 | - <div class='post_comment_box closed'> | ||
| 21 | - <%= render :partial => 'comment/comment_form', :locals => {:comment => Comment.new, :display_link => true, :cancel_triggers_hide => true, :paragraph_uuid => paragraph_uuid}%> | 21 | + <div class='post_comment_box closed' data-comment_paragraph_form_url="<%= load_comment_form_url %>"> |
| 22 | </div> | 22 | </div> |
| 23 | </div> | 23 | </div> |
| 24 | </div> | 24 | </div> |