Commit 7d225e95e4809b21a977c05e6df0977d88d884cb

Authored by Eduardo Vital
Committed by Gabriel Silva
1 parent 64bb5d29

Fix article comments ordering bug

Signed-off-by: Eduardo Vital <vitaldu@gmail.com>
Signed-off-by: Pedro de Lyra <pedrodelyra@gmail.com>
(cherry picked from commit 951fcb57484a69911997572a2bc6d52b27ccbc6b)
app/controllers/public/content_viewer_controller.rb
@@ -64,11 +64,7 @@ class ContentViewerController &lt; ApplicationController @@ -64,11 +64,7 @@ class ContentViewerController &lt; ApplicationController
64 process_comments(params) 64 process_comments(params)
65 65
66 if request.xhr? and params[:comment_order] 66 if request.xhr? and params[:comment_order]
67 - if @comment_order == 'newest'  
68 - @comments = @comments.reverse  
69 - end  
70 -  
71 - return render :partial => 'comment/comment', :collection => @comments 67 + return render :partial => 'comment/comments_with_pagination'
72 end 68 end
73 69
74 if params[:slideshow] 70 if params[:slideshow]
@@ -272,8 +268,12 @@ class ContentViewerController &lt; ApplicationController @@ -272,8 +268,12 @@ class ContentViewerController &lt; ApplicationController
272 @comments = @page.comments.without_spam 268 @comments = @page.comments.without_spam
273 @comments = @plugins.filter(:unavailable_comments, @comments) 269 @comments = @plugins.filter(:unavailable_comments, @comments)
274 @comments_count = @comments.count 270 @comments_count = @comments.count
275 - @comments = @comments.without_reply.paginate(:per_page => per_page, :page => params[:comment_page] )  
276 @comment_order = params[:comment_order].nil? ? 'oldest' : params[:comment_order] 271 @comment_order = params[:comment_order].nil? ? 'oldest' : params[:comment_order]
  272 + @comments = @comments.without_reply
  273 + if @comment_order == 'newest'
  274 + @comments = @comments.reverse
  275 + end
  276 + @comments = @comments.paginate(:per_page => per_page, :page => params[:comment_page] )
277 end 277 end
278 278
279 private 279 private
app/views/comment/_comments_with_pagination.html.erb 0 → 100644
@@ -0,0 +1,4 @@ @@ -0,0 +1,4 @@
  1 +<% if @comments.present? %>
  2 + <%= render :partial => 'comment/comment', :collection => @comments %>
  3 + <%= pagination_links @comments, :param_name => 'comment_page', :params => { :comment_order => @comment_order } %>
  4 +<% end %>
app/views/content_viewer/view_page.html.erb
@@ -81,7 +81,7 @@ @@ -81,7 +81,7 @@
81 <ul class="article-comments-list"> 81 <ul class="article-comments-list">
82 <% if @comments.present? %> 82 <% if @comments.present? %>
83 <%= render :partial => 'comment/comment', :collection => @comments %> 83 <%= render :partial => 'comment/comment', :collection => @comments %>
84 - <%= pagination_links @comments, :param_name => 'comment_page' %> 84 + <%= pagination_links @comments, :param_name => 'comment_page', :params => { :comment_order => @comment_order } %>
85 <% end %> 85 <% end %>
86 </ul> 86 </ul>
87 87
public/javascripts/comment_order.js
@@ -10,10 +10,9 @@ function send_order(order, url) { @@ -10,10 +10,9 @@ function send_order(order, url) {
10 }); 10 });
11 } 11 }
12 12
13 -  
14 jQuery(document).ready(function(){ 13 jQuery(document).ready(function(){
15 jQuery("#comment_order").change(function(){ 14 jQuery("#comment_order").change(function(){
16 - var url = jQuery("#page_url").val(); 15 + var url = window.location.href;
17 send_order(this.value, url); 16 send_order(this.value, url);
18 }); 17 });
19 -});  
20 \ No newline at end of file 18 \ No newline at end of file
  19 +});
test/functional/content_viewer_controller_test.rb
@@ -332,6 +332,27 @@ class ContentViewerControllerTest &lt; ActionController::TestCase @@ -332,6 +332,27 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
332 assert_tag :content => /list my comment/ 332 assert_tag :content => /list my comment/
333 end 333 end
334 334
  335 + should 'order comments according to comments ordering option' do
  336 + article = fast_create(Article, :profile_id => profile.id)
  337 + for n in 1..24
  338 + article.comments.create!(:author => profile, :title => "some title #{n}", :body => "some body #{n}")
  339 + end
  340 +
  341 + get 'view_page', :profile => profile.identifier, :page => article.path.split('/')
  342 +
  343 + for i in 1..12
  344 + assert_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i}" }
  345 + assert_no_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i + 12}" }
  346 + end
  347 +
  348 + xhr :get, :view_page, :profile => profile.identifier, :page => article.path.split('/'), :comment_page => 1, :comment_order => 'newest'
  349 +
  350 + for i in 1..12
  351 + assert_no_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i}" }
  352 + assert_tag :tag => 'div', :attributes => { :class => 'comment-details' }, :descendant => { :tag => 'h4', :content => "some title #{i + 12}" }
  353 + end
  354 + end
  355 +
335 should 'redirect to new article path under an old path' do 356 should 'redirect to new article path under an old path' do
336 p = create_user('test_user').person 357 p = create_user('test_user').person
337 a = p.articles.create(:name => 'old-name') 358 a = p.articles.create(:name => 'old-name')
@@ -1368,7 +1389,7 @@ class ContentViewerControllerTest &lt; ActionController::TestCase @@ -1368,7 +1389,7 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
1368 assert_equal 15, article.comments.count 1389 assert_equal 15, article.comments.count
1369 1390
1370 get 'view_page', :profile => profile.identifier, :page => article.path.split('/') 1391 get 'view_page', :profile => profile.identifier, :page => article.path.split('/')
1371 - assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_page=2", :rel => 'next' } 1392 + assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_order=oldest&amp;comment_page=2", :rel => 'next' }
1372 end 1393 end
1373 1394
1374 should 'not escape acceptable HTML in list of blog posts' do 1395 should 'not escape acceptable HTML in list of blog posts' do