Download as
Browse Code »

Commit 5004be38d09b839ed2329cadd573f613dd60cb7e

Authored by Rodrigo Souto
2 parents af128233 9a88a3f3
Exists in master and in 29 other branches add_member_task_reject_details, admin_visible_profile, article-list-template, community_notifications, contact_admin_translation, event_fixes, fix_comments_pagination, fix_rails4_organization_ratings, fix_sign_up_form, follow_step_fix, forum_topic_creation, mirror_block_improvements, multi_env_on_remote_user, new_security, noosfero_spb_ci, organization_ratings_improvements, organization_ratings_link_to_profile_stable-spb-1.3, organization_ratings_translations_fix, profile_api_improvements, ratings_minor_fixes, remote_user_fix, send_email_to_admins, stable-spb-1.3, stable-spb-1.3-fixes, stable-spb-1.4, stable-spb-1.5, suggest_rejected_value, web_steps_improvements, xss_terminate_custom_options

Merge commit 'refs/merge-requests/343' of git://gitorious.org/noosfero/noosfero … 

…into merge-requests/343

Also replacing the load_comments method by a filter that receives a
scope.

Conflicts:
	app/controllers/public/content_viewer_controller.rb
	app/models/comment.rb
	app/views/content_viewer/view_page.rhtml
	test/functional/content_viewer_controller_test.rb
	test/unit/comment_test.rb
Showing 11 changed files with 89 additions and 195 deletions   Show diff stats
app/controllers/public/content_viewer_controller.rb
  Diff comments   View file @5004be3
... ... @@ -99,11 +99,10 @@ class ContentViewerController < ApplicationController
99 99 end
100 100 end
101 101  
102   - @comments = @plugins.dispatch_first(:load_comments, @page)
103   - @comments ||= @page.comments.without_spam.as_thread
104   - @comments = [@comments] unless @comments.is_a?(Array)
105   -
106   - @comments_count = Comment.count_thread(@comments)
  102 + @comments = @page.comments.without_spam.without_reply
  103 + @comments = @plugins.filter(:unavailable_comments, @comments)
  104 + @comments_count = @comments.count
  105 + @comments = @comments.paginate(:per_page => per_page, :page => params[:comment_page] )
107 106  
108 107 if params[:slideshow]
109 108 render :action => 'slideshow', :layout => 'slideshow'
... ...
app/models/comment.rb
  Diff comments   View file @5004be3
... ... @@ -17,6 +17,7 @@ class Comment < ActiveRecord::Base
17 17 belongs_to :reply_of, :class_name => 'Comment', :foreign_key => 'reply_of_id'
18 18  
19 19 named_scope :without_spam, :conditions => ['spam IS NULL OR spam = ?', false]
  20 + named_scope :without_reply, :conditions => ['reply_of_id IS NULL']
20 21 named_scope :spam, :conditions => ['spam = ?', true]
21 22  
22 23 # unauthenticated authors:
... ... @@ -152,29 +153,6 @@ class Comment < ActiveRecord::Base
152 153 @replies = comments_list
153 154 end
154 155  
155   - def self.count_thread(comments)
156   - count = comments.length
157   - comments.each do |c|
158   - count+=count_thread(c.replies)
159   - end
160   - count
161   - end
162   -
163   - def self.as_thread
164   - result = {}
165   - root = []
166   - order(:id).each do |c|
167   - c.replies = []
168   - result[c.id] ||= c
169   - if result[c.reply_of_id]
170   - result[c.reply_of_id].replies << c
171   - else
172   - root << c
173   - end
174   - end
175   - root
176   - end
177   -
178 156 include ApplicationHelper
179 157 def reported_version(options = {})
180 158 comment = self
... ...
app/views/content_viewer/view_page.rhtml
  Diff comments   View file @5004be3
... ... @@ -94,12 +94,13 @@
94 94 </h3>
95 95 <% end %>
96 96  
97   - <% if @page.accept_comments? && @comments_count > 1 %>
  97 + <% if @page.accept_comments? && @comments.count > 1 %>
98 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 99 <% end %>
100 100  
101 101 <ul class="article-comments-list">
102 102 <%= render :partial => 'comment/comment', :collection => @comments %>
  103 + <%= pagination_links @comments, :param_name => 'comment_page' %>
103 104 </ul>
104 105  
105 106 <% if @page.accept_comments? %>
... ...
lib/noosfero/plugin.rb
  Diff comments   View file @5004be3
... ... @@ -289,16 +289,16 @@ class Noosfero::Plugin
289 289 def filter_comment(comment)
290 290 end
291 291  
292   - # Define custom logic to load the article comments.
  292 + # Define custom logic to filter loaded comments.
293 293 #
294 294 # Example:
295 295 #
296   - # def load_comments(article)
297   - # article.comments.find(:all, :conditions => ['spam IS NULL OR spam = ?', false])
  296 + # def unavailable_comments(scope)
  297 + # scope.without_spams
298 298 # end
299 299 #
300   - def load_comments(article)
301   - nil
  300 + def unavailable_comments(scope)
  301 + scope
302 302 end
303 303  
304 304 # This method is called by the CommentHandler background job before sending
... ...
lib/noosfero/plugin/manager.rb
  Diff comments   View file @5004be3
... ... @@ -63,6 +63,10 @@ class Noosfero::Plugin::Manager
63 63 args.length < 2 ? args.first : args
64 64 end
65 65  
  66 + def filter(property, data)
  67 + inject(data) {|data, plugin| data = plugin.send(property, data)}
  68 + end
  69 +
66 70 def parse_macro(macro_name, macro, source = nil)
67 71 macro_instance = enabled_macros[macro_name] || default_macro
68 72 macro_instance.convert(macro, source)
... ...
plugins/comment_group/lib/comment_group_plugin.rb
  Diff comments   View file @5004be3
... ... @@ -11,8 +11,8 @@ class CommentGroupPlugin &lt; Noosfero::Plugin
11 11 _("A plugin that display comment groups.")
12 12 end
13 13  
14   - def load_comments(article)
15   - article.comments.without_spam.without_group.as_thread
  14 + def unavailable_comments(scope)
  15 + scope.without_group
16 16 end
17 17  
18 18 def comment_form_extra_contents(args)
... ...
plugins/comment_group/test/unit/comment_group_plugin_test.rb
  Diff comments   View file @5004be3
... ... @@ -10,37 +10,15 @@ class CommentGroupPluginTest &lt; ActiveSupport::TestCase
10 10  
11 11 attr_reader :environment
12 12  
13   - should 'load_comments returns all the comments wihout group of an article passed as parameter' do
  13 + should 'filter_comments returns all the comments wihout group of an article passed as parameter' do
14 14 article = fast_create(Article)
15 15 c1 = fast_create(Comment, :source_id => article.id, :group_id => 1)
16 16 c2 = fast_create(Comment, :source_id => article.id)
17 17 c3 = fast_create(Comment, :source_id => article.id)
18 18  
19 19 plugin = CommentGroupPlugin.new
20   - assert_equal [], [c2, c3] - plugin.load_comments(article)
21   - assert_equal [], plugin.load_comments(article) - [c2, c3]
22   - end
23   -
24   - should 'load_comments not returns spam comments' do
25   - article = fast_create(Article)
26   - c1 = fast_create(Comment, :source_id => article.id, :group_id => 1)
27   - c2 = fast_create(Comment, :source_id => article.id)
28   - c3 = fast_create(Comment, :source_id => article.id, :spam => true)
29   -
30   - plugin = CommentGroupPlugin.new
31   - assert_equal [], [c2] - plugin.load_comments(article)
32   - assert_equal [], plugin.load_comments(article) - [c2]
33   - end
34   -
35   - should 'load_comments returns only root comments of article' do
36   - article = fast_create(Article)
37   - c1 = fast_create(Comment, :source_id => article.id, :group_id => 1)
38   - c2 = fast_create(Comment, :source_id => article.id)
39   - c3 = fast_create(Comment, :source_id => article.id, :reply_of_id => c2.id)
40   -
41   - plugin = CommentGroupPlugin.new
42   - assert_equal [], [c2] - plugin.load_comments(article)
43   - assert_equal [], plugin.load_comments(article) - [c2]
  20 + assert_equal [], [c2, c3] - plugin.filter_comments(article.comments)
  21 + assert_equal [], plugin.filter_comments(article.comments) - [c2, c3]
44 22 end
45 23  
46 24 end
... ...
test/functional/content_viewer_controller_test.rb
  Diff comments   View file @5004be3
... ... @@ -1084,13 +1084,13 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
1084 1084 article.save!
1085 1085 comment1 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello')
1086 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 1088 comment2.save!
1089 1089 get :view_page, :profile => 'testuser', :page => [ 'test' ]
1090 1090 assert_tag :tag => 'a', :attributes => { :id => 'top-post-comment-button' }
1091 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 1094 profile = create_user('testuser').person
1095 1095 article = profile.articles.build(:name => 'test')
1096 1096 article.save!
... ... @@ -1099,7 +1099,7 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
1099 1099 comment2 = article.comments.build(:author => profile, :title => 'hi', :body => 'hello', :reply_of_id => comment1.id)
1100 1100 comment2.save!
1101 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 1103 end
1104 1104  
1105 1105 should 'suggest article link displayed into article-actions div' do
... ... @@ -1178,11 +1178,11 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
1178 1178  
1179 1179 should 'not display comments marked as spam' do
1180 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 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 1186 end
1187 1187  
1188 1188 should 'add extra content on comment form from plugins' do
... ... @@ -1212,91 +1212,43 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
1212 1212 assert_tag :tag => 'input', :attributes => {:name => 'comment[another_field_id]', :type => 'hidden'}
1213 1213 end
1214 1214  
1215   - should 'collect comments as plugin definition' do
1216   - class Plugin1 < Noosfero::Plugin
1217   - def load_comments(page)
1218   - [page.comments.find(4)]
1219   - end
1220   - end
1221   - Environment.default.enable_plugin(Plugin1.name)
1222   - Comment.delete_all
1223   - page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text')
1224   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 1' ).save!
1225   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 2' ).save!
1226   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 3' ).save!
1227   - c = page.comments.build(:author => profile, :title => 'hi', :body => 'hello 4' )
1228   - c.save!
1229   -
1230   -
1231   - get :view_page, :profile => profile.identifier, :page => [ 'myarticle' ]
1232   -
1233   - assert_equal [c], assigns(:comments)
1234   - end
1235   -
1236   - should 'not be a problem if loaded comments of plugins not be an array' do
  1215 + should 'filter comments with scope defined by the plugins' do
1237 1216 class Plugin1 < Noosfero::Plugin
1238   - def load_comments(page)
1239   - page.comments.find(4)
  1217 + def unavailable_comments(scope)
  1218 + scope.where(:user_agent => 'Jack')
1240 1219 end
1241 1220 end
1242   - Environment.default.enable_plugin(Plugin1.name)
1243   - Comment.delete_all
1244   - page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text')
1245   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 1' ).save!
1246   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 2' ).save!
1247   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 3' ).save!
1248   - c = page.comments.build(:author => profile, :title => 'hi', :body => 'hello 4' )
1249   - c.save!
1250   -
1251   -
1252   - get :view_page, :profile => profile.identifier, :page => [ 'myarticle' ]
1253   -
1254   - assert_equal [c], assigns(:comments)
1255   - end
1256 1221  
1257   -
1258   - should 'take in consideration only the first plugin comments definition' do
1259   - class Plugin1 < Noosfero::Plugin
1260   - def load_comments(page)
1261   - page.comments.first
1262   - end
1263   - end
1264 1222 class Plugin2 < Noosfero::Plugin
1265   - def load_comments(page)
1266   - page.comments.last
  1223 + def unavailable_comments(scope)
  1224 + scope.where(:referrer => 'kernel.org')
1267 1225 end
1268 1226 end
1269   - Environment.default.enable_plugin(Plugin1.name)
1270   - Environment.default.enable_plugin(Plugin2.name)
1271 1227  
1272   - page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text')
1273   - c1 = page.comments.build(:author => profile, :title => 'hi', :body => 'hello 1' )
1274   - c1.save!
1275   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 2' ).save!
1276   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 3' ).save!
1277   - c2 = page.comments.build(:author => profile, :title => 'hi', :body => 'hello 4' )
1278   - c2.save!
  1228 + Environment.default.enable_plugin(Plugin1)
  1229 + Environment.default.enable_plugin(Plugin2)
  1230 + profile = fast_create(Profile)
  1231 + article = fast_create(Article, :profile_id => profile.id)
  1232 + c1 = fast_create(Comment, :source_id => article.id, :user_agent => 'Jack', :referrer => 'kernel.org')
  1233 + c2 = fast_create(Comment, :source_id => article.id, :user_agent => 'Rose', :referrer => 'kernel.org')
  1234 + c3 = fast_create(Comment, :source_id => article.id, :user_agent => 'Jack', :referrer => 'google.com')
1279 1235  
1280   - get :view_page, :profile => profile.identifier, :page => [ 'myarticle' ]
  1236 + get :view_page, :profile => profile.identifier, :page => [ article.path ]
1281 1237  
1282   - assert_equal [c1], assigns(:comments)
  1238 + assert_includes assigns(:comments), c1
  1239 + assert_not_includes assigns(:comments), c2
  1240 + assert_not_includes assigns(:comments), c3
1283 1241 end
1284 1242  
1285   - should 'empty array of comments collected by plugin make the comments variable be an empty array' do
1286   - class Plugin1 < Noosfero::Plugin
1287   - def load_comments(page)
1288   - []
1289   - end
  1243 + should 'display pagination links of comments' do
  1244 + article = fast_create(Article, :profile_id => profile.id)
  1245 + for n in 1..15
  1246 + article.comments.create!(:author => profile, :title => "some title #{n}", :body => 'some body #{n}')
1290 1247 end
1291   - Environment.default.enable_plugin(Plugin1.name)
1292   -
1293   - page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text')
1294   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 1' ).save!
1295   - page.comments.build(:author => profile, :title => 'hi', :body => 'hello 2' ).save!
1296   -
  1248 + assert_equal 15, article.comments.count
1297 1249  
1298   - get :view_page, :profile => profile.identifier, :page => [ 'myarticle' ]
  1250 + get 'view_page', :profile => profile.identifier, :page => article.path.split('/')
  1251 + assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_page=2", :rel => 'next' }
  1252 + end
1299 1253  
1300   - assert_equal [], assigns(:comments)
1301   - end
1302 1254 end
... ...
test/unit/comment_test.rb
  Diff comments   View file @5004be3
... ... @@ -285,21 +285,6 @@ class CommentTest &lt; ActiveSupport::TestCase
285 285 assert_equal [c1,c3], c.reload.children
286 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 288 should "return activities comments as a thread" do
304 289 person = fast_create(Person)
305 290 a = TextileArticle.create!(:profile => person, :name => 'My article', :body => 'Article body')
... ... @@ -515,15 +500,6 @@ class CommentTest &lt; ActiveSupport::TestCase
515 500 assert_equal c, SpamNotification.marked_as_ham
516 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 503 should 'store User-Agent' do
528 504 c = Comment.new(:user_agent => 'foo')
529 505 assert_equal 'foo', c.user_agent
... ... @@ -700,19 +676,13 @@ class CommentTest &lt; ActiveSupport::TestCase
700 676 assert_equal c1, c1.comment_root
701 677 end
702 678  
703   - should 'count a thread of comments' do
704   - Comment.delete_all
705   - comments = []
706   - comments.push(create_comment)
707   - c1 = create_comment
708   - comments.push(c1)
709   - create_comment(:reply_of_id => c1.id)
710   - create_comment(:reply_of_id => c1.id)
711   - c2 = create_comment
712   - comments.push(c2)
713   - create_comment(:reply_of_id => c2.id)
714   -
715   - assert_equal 6, Comment.count_thread(comments)
  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
716 686 end
717 687  
718 688 private
... ...
test/unit/plugin_manager_test.rb
  Diff comments   View file @5004be3
... ... @@ -257,4 +257,23 @@ class PluginManagerTest &lt; ActiveSupport::TestCase
257 257 end
258 258 end
259 259  
  260 + should 'filter a property' do
  261 + class Plugin1 < Noosfero::Plugin
  262 + def invalid_numbers(numbers)
  263 + numbers.reject {|n| n%2==0}
  264 + end
  265 + end
  266 +
  267 + class Plugin2 < Noosfero::Plugin
  268 + def invalid_numbers(numbers)
  269 + numbers.reject {|n| n<=5}
  270 + end
  271 + end
  272 +
  273 + environment.enable_plugin(Plugin1)
  274 + environment.enable_plugin(Plugin2)
  275 +
  276 + assert_equal [7,9], manager.filter(:invalid_numbers, [1,2,3,4,5,6,7,8,9,10])
  277 + end
  278 +
260 279 end
... ...
test/unit/plugin_test.rb
  Diff comments   View file @5004be3
... ... @@ -28,30 +28,23 @@ class PluginTest &lt; ActiveSupport::TestCase
28 28 assert_equal({:controller => 'plugin_test/plugin1_admin', :action => 'index'}, Plugin1.admin_url)
29 29 end
30 30  
31   - should 'load_comments return nil by default' do
32   -
33   - class Plugin1 < Noosfero::Plugin; end;
34   -
35   - environment.stubs(:enabled_plugins).returns([Plugin1.to_s])
36   -
37   - profile = fast_create(Profile, :name => 'test profile', :identifier => 'test_profile')
38   - a = fast_create(Article, :name => 'my article', :profile_id => profile.id)
39   - assert_nil plugins.dispatch_first(:load_comments, a)
40   - end
41   -
42   - should 'load_comments return the value defined by plugin' do
43   -
  31 + should 'filter comments with scope defined by plugin' do
44 32 class Plugin1 < Noosfero::Plugin
45   - def load_comments(page)
46   - 'some value'
  33 + def filter_comments(scope)
  34 + scope.without_spam
47 35 end
48 36 end
49 37  
50   - environment.stubs(:enabled_plugins).returns([Plugin1.to_s])
  38 + article = fast_create(Article)
  39 + c1 = fast_create(Comment, :source_id => article.id, :group_id => 1)
  40 + c2 = fast_create(Comment, :source_id => article.id)
  41 + c3 = fast_create(Comment, :source_id => article.id, :spam => true)
51 42  
52   - profile = fast_create(Profile, :name => 'test profile', :identifier => 'test_profile')
53   - a = fast_create(Article, :name => 'my article', :profile_id => profile.id)
54   - assert_equal 'some value', plugins.dispatch_first(:load_comments, a)
  43 + plugin = Plugin1.new
  44 + comments = plugin.filter_comments(article.comments)
  45 + assert_includes comments, c1
  46 + assert_includes comments, c2
  47 + assert_not_includes comments, c3
55 48 end
56 49  
57 50 should 'returns empty hash for class method extra_blocks by default if no blocks are defined on plugin' do
... ...