[Git][noosfero/noosfero][master] 2 commits: Fix article comments ordering bug

[Rodrigo Souto]

Rodrigo Souto pushed to branch master at Noosfero / noosfero


Commits:
951fcb57 by Eduardo Vital at 2016-02-22T17:07:02-03:00
Fix article comments ordering bug

Signed-off-by: Eduardo Vital <vitaldu at gmail.com>
Signed-off-by: Pedro de Lyra <pedrodelyra at gmail.com>

- - - - -
ea20a3dc by Rodrigo Souto at 2016-02-25T17:34:52+00:00
Merge branch 'fix-article-comments-ordering' into 'master'

Fix article comments ordering

Fix article's comments ordering bug (had wrong logic before correction) and pagination missing bug (js was overwriting pagination after ajax request).

References:

https://softwarepublico.gov.br/gitlab/softwarepublico/softwarepublico/issues/630
https://softwarepublico.gov.br/gitlab/softwarepublico/softwarepublico/issues/632

See merge request !792
- - - - -


5 changed files:

- app/controllers/public/content_viewer_controller.rb
- + app/views/comment/_comments_with_pagination.html.erb
- app/views/content_viewer/view_page.html.erb
- public/javascripts/comment_order.js
- test/functional/content_viewer_controller_test.rb


Changes:

=====================================
app/controllers/public/content_viewer_controller.rb
=====================================
--- a/app/controllers/public/content_viewer_controller.rb
+++ b/app/controllers/public/content_viewer_controller.rb
@@ -64,11 +64,7 @@ class ContentViewerController < ApplicationController
     process_comments(params)
 
     if request.xhr? and params[:comment_order]
-      if @comment_order == 'newest'
-        @comments = @comments.reverse
-      end
-
-      return render :partial => 'comment/comment', :collection => @comments
+      return render :partial => 'comment/comments_with_pagination'
     end
 
     if params[:slideshow]
@@ -275,8 +271,12 @@ class ContentViewerController < ApplicationController
     @comments = @page.comments.without_spam
     @comments = @plugins.filter(:unavailable_comments, @comments)
     @comments_count = @comments.count
-    @comments = @comments.without_reply.paginate(:per_page => per_page, :page => params[:comment_page] )
     @comment_order = params[:comment_order].nil? ? 'oldest' : params[:comment_order]
+    @comments = @comments.without_reply
+    if @comment_order == 'newest'
+      @comments = @comments.reverse
+    end
+    @comments = @comments.paginate(:per_page => per_page, :page => params[:comment_page] )
   end
 
   private


=====================================
app/views/comment/_comments_with_pagination.html.erb
=====================================
--- /dev/null
+++ b/app/views/comment/_comments_with_pagination.html.erb
@@ -0,0 +1,4 @@
+<% if @comments.present? %>
+  <%= render :partial => 'comment/comment', :collection => @comments %>
+  <%= pagination_links @comments, :param_name => 'comment_page', :params => { :comment_order => @comment_order } %>
+<% end %>


=====================================
app/views/content_viewer/view_page.html.erb
=====================================
--- a/app/views/content_viewer/view_page.html.erb
+++ b/app/views/content_viewer/view_page.html.erb
@@ -81,7 +81,7 @@
     <ul class="article-comments-list">
       <% if @comments.present? %>
         <%= render :partial => 'comment/comment', :collection => @comments %>
-        <%= pagination_links @comments, :param_name => 'comment_page' %>
+        <%= pagination_links @comments, :param_name => 'comment_page', :params => { :comment_order => @comment_order } %>
       <% end %>
     </ul>
 


=====================================
public/javascripts/comment_order.js
=====================================
--- a/public/javascripts/comment_order.js
+++ b/public/javascripts/comment_order.js
@@ -10,10 +10,9 @@ function send_order(order, url) {
   });
 }
 
-
 jQuery(document).ready(function(){
   jQuery("#comment_order").change(function(){
-    var url = jQuery("#page_url").val();
+    var url = window.location.href;
     send_order(this.value, url);
   });
-});
\ No newline at end of file
+});


=====================================
test/functional/content_viewer_controller_test.rb
=====================================
--- a/test/functional/content_viewer_controller_test.rb
+++ b/test/functional/content_viewer_controller_test.rb
@@ -328,6 +328,27 @@ class ContentViewerControllerTest < ActionController::TestCase
     assert_tag :content => /list my comment/
   end
 
+  should 'order comments according to comments ordering option' do
+    article = fast_create(Article, :profile_id => profile.id)
+    for n in 1..24
+      article.comments.create!(:author => profile, :title => "some title #{n}", :body => "some body #{n}")
+    end
+
+    get 'view_page', :profile => profile.identifier, :page => article.path.split('/')
+
+    for i in 1..12
+      assert_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i}" }
+      assert_no_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i + 12}" }
+    end
+
+    xhr :get, :view_page, :profile => profile.identifier, :page => article.path.split('/'), :comment_page => 1, :comment_order => 'newest'
+
+    for i in 1..12
+      assert_no_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i}" }
+      assert_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i + 12}" }
+    end
+  end
+
   should 'redirect to new article path under an old path' do
     p = create_user('test_user').person
     a = p.articles.create(:name => 'old-name')
@@ -1351,7 +1372,7 @@ class ContentViewerControllerTest < ActionController::TestCase
     assert_equal 15, article.comments.count
 
     get 'view_page', :profile => profile.identifier, :page => article.path.split('/')
-    assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_page=2", :rel => 'next' }
+    assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_order=oldest&comment_page=2", :rel => 'next' }
   end
 
   should 'not escape acceptable HTML in list of blog posts' do



View it on GitLab: https://gitlab.com/noosfero/noosfero/compare/1c3dc68da731f78b61f2923c4c022137475eab19...ea20a3dc19a595f36b9c153b6a897485a792b53f
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listas.softwarelivre.org/pipermail/noosfero-dev/attachments/20160225/56b7a38d/attachment.html>