Commit 9a88a3f30ae36b4fa2546e249e5d852f093bd5c0
1 parent
8748da0c
Exists in
master
and in
29 other branches
adding pagination for article's comments
Showing
5 changed files
with
32 additions
and
51 deletions
Show diff stats
app/controllers/public/content_viewer_controller.rb
@@ -99,9 +99,7 @@ class ContentViewerController < ApplicationController | @@ -99,9 +99,7 @@ class ContentViewerController < ApplicationController | ||
99 | end | 99 | end |
100 | end | 100 | end |
101 | 101 | ||
102 | - comments = @page.comments.without_spam | ||
103 | - @comments = comments.as_thread | ||
104 | - @comments_count = comments.count | 102 | + @comments = @page.comments.without_spam.without_reply.paginate(:per_page => per_page, :page => params[:comment_page] ) |
105 | if params[:slideshow] | 103 | if params[:slideshow] |
106 | render :action => 'slideshow', :layout => 'slideshow' | 104 | render :action => 'slideshow', :layout => 'slideshow' |
107 | end | 105 | end |
app/models/comment.rb
@@ -17,6 +17,7 @@ class Comment < ActiveRecord::Base | @@ -17,6 +17,7 @@ class Comment < ActiveRecord::Base | ||
17 | belongs_to :reply_of, :class_name => 'Comment', :foreign_key => 'reply_of_id' | 17 | belongs_to :reply_of, :class_name => 'Comment', :foreign_key => 'reply_of_id' |
18 | 18 | ||
19 | named_scope :without_spam, :conditions => ['spam IS NULL OR spam = ?', false] | 19 | named_scope :without_spam, :conditions => ['spam IS NULL OR spam = ?', false] |
20 | + named_scope :without_reply, :conditions => ['reply_of_id IS NULL'] | ||
20 | named_scope :spam, :conditions => ['spam = ?', true] | 21 | named_scope :spam, :conditions => ['spam = ?', true] |
21 | 22 | ||
22 | # unauthenticated authors: | 23 | # unauthenticated authors: |
@@ -152,21 +153,6 @@ class Comment < ActiveRecord::Base | @@ -152,21 +153,6 @@ class Comment < ActiveRecord::Base | ||
152 | @replies = comments_list | 153 | @replies = comments_list |
153 | end | 154 | end |
154 | 155 | ||
155 | - def self.as_thread | ||
156 | - result = {} | ||
157 | - root = [] | ||
158 | - order(:id).each do |c| | ||
159 | - c.replies = [] | ||
160 | - result[c.id] ||= c | ||
161 | - if result[c.reply_of_id] | ||
162 | - result[c.reply_of_id].replies << c | ||
163 | - else | ||
164 | - root << c | ||
165 | - end | ||
166 | - end | ||
167 | - root | ||
168 | - end | ||
169 | - | ||
170 | include ApplicationHelper | 156 | include ApplicationHelper |
171 | def reported_version(options = {}) | 157 | def reported_version(options = {}) |
172 | comment = self | 158 | comment = self |
app/views/content_viewer/view_page.rhtml
@@ -88,18 +88,19 @@ | @@ -88,18 +88,19 @@ | ||
88 | 88 | ||
89 | <div class="comments" id="comments_list"> | 89 | <div class="comments" id="comments_list"> |
90 | 90 | ||
91 | - <% if @page.accept_comments? || @comments_count > 0 %> | ||
92 | - <h3 <%= 'class="no-comments-yet"' if @comments_count == 0 %>> | 91 | + <% if @page.accept_comments? || @comments.count > 0 %> |
92 | + <h3 <%= 'class="no-comments-yet"' if @comments.count == 0 %>> | ||
93 | <%= number_of_comments(@page) %> | 93 | <%= number_of_comments(@page) %> |
94 | </h3> | 94 | </h3> |
95 | <% end %> | 95 | <% end %> |
96 | 96 | ||
97 | - <% if @page.accept_comments? && @comments_count > 1 %> | 97 | + <% if @page.accept_comments? && @comments.count > 1 %> |
98 | <%= link_to(_('Post a comment'), '#', :class => 'display-comment-form', :id => 'top-post-comment-button', :onclick => "jQuery('#page-comment-form .display-comment-form').first().click();") %> | 98 | <%= link_to(_('Post a comment'), '#', :class => 'display-comment-form', :id => 'top-post-comment-button', :onclick => "jQuery('#page-comment-form .display-comment-form').first().click();") %> |
99 | <% end %> | 99 | <% end %> |
100 | 100 | ||
101 | <ul class="article-comments-list"> | 101 | <ul class="article-comments-list"> |
102 | <%= render :partial => 'comment/comment', :collection => @comments %> | 102 | <%= render :partial => 'comment/comment', :collection => @comments %> |
103 | + <%= pagination_links @comments, :param_name => 'comment_page' %> | ||
103 | </ul> | 104 | </ul> |
104 | 105 | ||
105 | <% if @page.accept_comments? %> | 106 | <% if @page.accept_comments? %> |
test/functional/content_viewer_controller_test.rb
@@ -1084,13 +1084,13 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -1084,13 +1084,13 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
1084 | article.save! | 1084 | article.save! |
1085 | comment1 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello') | 1085 | comment1 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello') |
1086 | comment1.save! | 1086 | comment1.save! |
1087 | - comment2 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello', :reply_of_id => comment1.id) | 1087 | + comment2 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello') |
1088 | comment2.save! | 1088 | comment2.save! |
1089 | get :view_page, :profile => 'testuser', :page => [ 'test' ] | 1089 | get :view_page, :profile => 'testuser', :page => [ 'test' ] |
1090 | assert_tag :tag => 'a', :attributes => { :id => 'top-post-comment-button' } | 1090 | assert_tag :tag => 'a', :attributes => { :id => 'top-post-comment-button' } |
1091 | end | 1091 | end |
1092 | 1092 | ||
1093 | - should 'store number of comments' do | 1093 | + should 'not show a post comment button on top if there are one comment and one reply' do |
1094 | profile = create_user('testuser').person | 1094 | profile = create_user('testuser').person |
1095 | article = profile.articles.build(:name => 'test') | 1095 | article = profile.articles.build(:name => 'test') |
1096 | article.save! | 1096 | article.save! |
@@ -1099,7 +1099,7 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -1099,7 +1099,7 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
1099 | comment2 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello', :reply_of_id => comment1.id) | 1099 | comment2 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello', :reply_of_id => comment1.id) |
1100 | comment2.save! | 1100 | comment2.save! |
1101 | get :view_page, :profile => 'testuser', :page => [ 'test' ] | 1101 | get :view_page, :profile => 'testuser', :page => [ 'test' ] |
1102 | - assert_equal 2, assigns(:comments_count) | 1102 | + assert_no_tag :tag => 'a', :attributes => { :id => 'top-post-comment-button' } |
1103 | end | 1103 | end |
1104 | 1104 | ||
1105 | should 'suggest article link displayed into article-actions div' do | 1105 | should 'suggest article link displayed into article-actions div' do |
@@ -1178,11 +1178,22 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -1178,11 +1178,22 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
1178 | 1178 | ||
1179 | should 'not display comments marked as spam' do | 1179 | should 'not display comments marked as spam' do |
1180 | article = fast_create(Article, :profile_id => profile.id) | 1180 | article = fast_create(Article, :profile_id => profile.id) |
1181 | - ham = fast_create(Comment, :source_id => article.id, :source_type => 'Article') | ||
1182 | - spam = fast_create(Comment, :source_id => article.id, :source_type => 'Article', :spam => true) | 1181 | + ham = fast_create(Comment, :source_id => article.id, :source_type => 'Article', :title => 'some content') |
1182 | + spam = fast_create(Comment, :source_id => article.id, :source_type => 'Article', :spam => true, :title => 'this is a spam') | ||
1183 | 1183 | ||
1184 | get 'view_page', :profile => profile.identifier, :page => article.path.split('/') | 1184 | get 'view_page', :profile => profile.identifier, :page => article.path.split('/') |
1185 | - assert_equal 1, assigns(:comments_count) | 1185 | + assert_no_tag :tag => 'h4', :content => /spam/ |
1186 | end | 1186 | end |
1187 | 1187 | ||
1188 | + should 'display pagination links of comments' do | ||
1189 | + article = fast_create(Article, :profile_id => profile.id) | ||
1190 | + for n in 1..15 | ||
1191 | + article.comments.create!(:author => profile, :title => "some title #{n}", :body => 'some body #{n}') | ||
1192 | + end | ||
1193 | + assert_equal 15, article.comments.count | ||
1194 | + | ||
1195 | + get 'view_page', :profile => profile.identifier, :page => article.path.split('/') | ||
1196 | + assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_page=2", :rel => 'next' } | ||
1197 | + end | ||
1198 | + | ||
1188 | end | 1199 | end |
test/unit/comment_test.rb
@@ -285,21 +285,6 @@ class CommentTest < ActiveSupport::TestCase | @@ -285,21 +285,6 @@ class CommentTest < ActiveSupport::TestCase | ||
285 | assert_equal [c1,c3], c.reload.children | 285 | assert_equal [c1,c3], c.reload.children |
286 | end | 286 | end |
287 | 287 | ||
288 | - should "return comments as a thread" do | ||
289 | - a = fast_create(Article) | ||
290 | - c0 = fast_create(Comment, :source_id => a.id) | ||
291 | - c1 = fast_create(Comment, :reply_of_id => c0.id, :source_id => a.id) | ||
292 | - c2 = fast_create(Comment, :reply_of_id => c1.id, :source_id => a.id) | ||
293 | - c3 = fast_create(Comment, :reply_of_id => c0.id, :source_id => a.id) | ||
294 | - c4 = fast_create(Comment, :source_id => a.id) | ||
295 | - result = a.comments.as_thread | ||
296 | - assert_equal c0.id, result[0].id | ||
297 | - assert_equal [c1.id, c3.id], result[0].replies.map(&:id) | ||
298 | - assert_equal [c2.id], result[0].replies[0].replies.map(&:id) | ||
299 | - assert_equal c4.id, result[1].id | ||
300 | - assert result[1].replies.empty? | ||
301 | - end | ||
302 | - | ||
303 | should "return activities comments as a thread" do | 288 | should "return activities comments as a thread" do |
304 | person = fast_create(Person) | 289 | person = fast_create(Person) |
305 | a = TextileArticle.create!(:profile => person, :name => 'My article', :body => 'Article body') | 290 | a = TextileArticle.create!(:profile => person, :name => 'My article', :body => 'Article body') |
@@ -515,15 +500,6 @@ class CommentTest < ActiveSupport::TestCase | @@ -515,15 +500,6 @@ class CommentTest < ActiveSupport::TestCase | ||
515 | assert_equal c, SpamNotification.marked_as_ham | 500 | assert_equal c, SpamNotification.marked_as_ham |
516 | end | 501 | end |
517 | 502 | ||
518 | - should 'ignore spam when constructing threads' do | ||
519 | - original = create_comment | ||
520 | - response = create_comment(:reply_of_id => original.id) | ||
521 | - original.spam! | ||
522 | - | ||
523 | - assert_equivalent [response], Comment.without_spam.as_thread | ||
524 | - end | ||
525 | - | ||
526 | - | ||
527 | should 'store User-Agent' do | 503 | should 'store User-Agent' do |
528 | c = Comment.new(:user_agent => 'foo') | 504 | c = Comment.new(:user_agent => 'foo') |
529 | assert_equal 'foo', c.user_agent | 505 | assert_equal 'foo', c.user_agent |
@@ -700,6 +676,15 @@ class CommentTest < ActiveSupport::TestCase | @@ -700,6 +676,15 @@ class CommentTest < ActiveSupport::TestCase | ||
700 | assert_equal c1, c1.comment_root | 676 | assert_equal c1, c1.comment_root |
701 | end | 677 | end |
702 | 678 | ||
679 | + should 'be able to select non-reply comments' do | ||
680 | + c1 = fast_create(Comment) | ||
681 | + c2 = fast_create(Comment, :reply_of_id => c1.id) | ||
682 | + c3 = fast_create(Comment, :reply_of_id => c2.id) | ||
683 | + c4 = fast_create(Comment) | ||
684 | + | ||
685 | + assert_equivalent [c1,c4], Comment.without_reply | ||
686 | + end | ||
687 | + | ||
703 | private | 688 | private |
704 | 689 | ||
705 | def create_comment(args = {}) | 690 | def create_comment(args = {}) |