Merge branch 'stable'

Antonio Terceiro
2 parents 3c321efd f442790a
Showing 12 changed files with 86 additions and 23 deletions Show diff stats
app/models/approve_article.rb
app/models/task.rb
app/views/cms/media_listing.rhtml
lib/feed_handler.rb
public/javascripts/application.js
public/javascripts/lightbox.js
test/unit/approve_article_test.rb
test/unit/feed_handler_test.rb
test/unit/task_test.rb
test/unit/text_article_test.rb
test/unit/tiny_mce_article_test.rb
vendor/plugins/white_list_sanitizer_unescape_before_reescape/init.rb
@@ -68,6 +68,7 @@ class ApproveArticle &lt; Task
   end
   def target_notification_message
+    return nil if target.organization? && !target.moderated_articles?
     description + "\n\n" +
     _('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name }
   end
@@ -59,7 +59,7 @@ class Task &lt; ActiveRecord::Base
     begin
       target_msg = task.target_notification_message
-      TaskMailer.deliver_target_notification(task, target_msg)
+      TaskMailer.deliver_target_notification(task, target_msg) if target_msg
     rescue NotImplementedError => ex
       RAILS_DEFAULT_LOGGER.info ex.to_s
     end
@@ -46,6 +46,17 @@
     <% end %>
   </div><!-- id='media-listing-upload' -->
   <hr/>
+
+  <script type='text/javascript'>
+    document.observe("dom:loaded", function() {
+      Event.addBehavior.reassignAfterAjax = true;
+      Event.addBehavior({
+        'div#pagination-images .pagination a' : Remote.Link,
+        'div#pagination-documents .pagination a' : Remote.Link
+      })
+    });
+  </script>
+
   <div id='media-listing'>
     <h3><%= _('Folders') %></h3>
     <p><%= _('Drag images and documents to add them to the text. If needed, resize images by clicking the tree icon on editor.') %></p>
@@ -15,7 +15,12 @@ class FeedHandler
   def fetch(address)
     begin
       content = ""
-      open(address) do |s| content = s.read end
+      block = lambda { |s| content = s.read }
+      content = if is_web_address?(address)
+        open( address, "User-Agent" => "Noosfero/#{Noosfero::VERSION}", &block )
+      else
+        open_uri_original_open(address, &block)
+      end
       return content
     rescue Exception => ex
       raise FeedHandler::FetchError, ex.to_s
@@ -39,4 +44,14 @@ class FeedHandler
   class ParseError < Exception; end
   class FetchError < Exception; end
+  protected
+
+  # extracted from the open implementation in the open-uri library
+  def is_web_address?(address)
+    address.respond_to?(:open) ||
+      address.respond_to?(:to_str) &&
+      (%r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ address) &&
+      URI.parse(address).respond_to?(:open)
+  end
+
 end
@@ -45,12 +45,3 @@ function convToValidIdentifier( str, sep ) {
             .replace( /ç/g,       "c" )
             .replace( /[^-_a-z0-9.]+/g, sep )
 }
-
-document.observe("dom:loaded", function() {
-  Event.addBehavior.reassignAfterAjax = true;
-  Event.addBehavior({
-    'div#pagination-images .pagination a' : Remote.Link,
-    'div#pagination-documents .pagination a' : Remote.Link
-  })
-});
-
@@ -53,7 +53,6 @@ function checkIt(string) {
 Event.observe(window, 'load', initialize, false);
 Event.observe(window, 'load', getBrowserInfo, false);
-Event.observe(window, 'unload', Event.unloadCache, false);
 var lightbox = Class.create();
@@ -2,8 +2,15 @@ require File.dirname(__FILE__) + &#39;/../test_helper&#39;
 class ApproveArticleTest < ActiveSupport::TestCase
+  def setup
+    ActionMailer::Base.delivery_method = :test
+    ActionMailer::Base.perform_deliveries = true
+    ActionMailer::Base.deliveries = []
+    @profile = create_user('test_user').person
+  end
+  attr_reader :profile
+
   should 'have name, reference article and profile' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile)
@@ -14,7 +21,6 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'create published article when finished' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile)
@@ -24,7 +30,7 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'override target notification message method from Task' do
-    p1 = create_user('testuser1').person
+    p1 = profile
     p2 = create_user('testuser2').person
     task = AddFriend.new(:person => p1, :friend => p2)
     assert_nothing_raised NotImplementedError do
@@ -33,7 +39,6 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'have parent if defined' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     folder = profile.articles.create!(:name => 'test folder', :type => 'Folder')
@@ -43,7 +48,6 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'not have parent if not defined' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile)
@@ -52,7 +56,6 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'alert when reference article is removed' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile)
@@ -64,7 +67,6 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'preserve article_parent' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     a = ApproveArticle.new(:article_parent => article)
@@ -72,7 +74,6 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
   end
   should 'handle blank names' do
-    profile = create_user('test_user').person
     article = profile.articles.create!(:name => 'test article')
     community = Community.create!(:name => 'test comm')
     a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile)
@@ -81,4 +82,19 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
       a.finish
     end
   end
+
+  should 'notify target if group is moderated' do
+    article = profile.articles.create!(:name => 'test article')
+    community = Community.create!(:name => 'test comm', :moderated_articles => true)
+    a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile)
+    assert !ActionMailer::Base.deliveries.empty?
+  end
+
+  should 'not notify target if group is not moderated' do
+    article = profile.articles.create!(:name => 'test article')
+    community = Community.create!(:name => 'test comm', :moderated_articles => false)
+    a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile)
+    assert ActionMailer::Base.deliveries.empty?
+  end
+
 end
@@ -73,4 +73,10 @@ class FeedHandlerTest &lt; Test::Unit::TestCase
     handler.process(container)
   end
+  should 'identifies itself as noosfero user agent' do
+    handler = FeedHandler.new
+    handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"}, anything).returns('bli content')
+    assert_equal 'bli content', handler.fetch('http://site.org/feed.xml')
+  end
+
 end
@@ -207,6 +207,13 @@ class TaskTest &lt; Test::Unit::TestCase
     end
   end
+  should 'not notify target if message is nil' do
+    task = Task.new
+    task.stubs(:target_notification_message).returns(nil)
+    TaskMailer.expects(:deliver_target_notification).never
+    task.save!
+  end
+
   protected
   def sample_user
@@ -19,5 +19,11 @@ class TextArticleTest &lt; Test::Unit::TestCase
     article = TextileArticle.create!(:name => 'found article test', :profile => person)
     assert_equal TextileArticle.find_by_contents('found'), TextArticle.find_by_contents('found')
   end
-  
+
+  should 'remove comments from TextArticle body' do
+    person = create_user('testuser').person
+    article = TextArticle.create!(:profile => person, :name => 'article', :body => "the <!-- comment --> article ...")
+    assert_equal "the  article ...", article.body
+  end
+
 end
@@ -35,8 +35,12 @@ class TinyMceArticleTest &lt; Test::Unit::TestCase
   should 'not translate & to amp; over times' do
     article = TinyMceArticle.create!(:name => 'link', :body => "<a href='www.invalid.com?param1=value&param2=value'>link</a>", :profile => profile)
     assert article.save
-    assert_no_match /&amp;amp;/, article.body
-    assert_match /&amp;/, article.body
+    assert_no_match(/&amp;amp;/, article.body)
+    assert_match(/&amp;/, article.body)
   end
+  should 'not escape comments from tiny mce article body' do
+    article = TinyMceArticle.create!(:profile => profile, :name => 'article', :abstract => 'abstract', :body => "the <!-- comment --> article ...")
+    assert_equal "the <!-- comment --> article ...", article.body
+  end
 end
@@ -4,6 +4,13 @@
 # this was solved in rails 2.2.1, then remove this patch when upgrade to it
 HTML::WhiteListSanitizer.module_eval do
+  
+  def sanitize_with_filter_comments(*args, &block)
+    text = sanitize_without_filter_comments(*args, &block)
+    text.gsub(/&lt;!--/, '<!--') if text
+  end
+  alias_method_chain :sanitize, :filter_comments
+
   # unescape before reescape to avoid:
   # & -> &amp; -> &amp;amp; -> &amp;amp;amp; -> &amp;amp;amp;amp; -> etc
   protected
	@@ -68,6 +68,7 @@ class ApproveArticle < Task		@@ -68,6 +68,7 @@ class ApproveArticle < Task
68	end	68	end
69		69
70	def target_notification_message	70	def target_notification_message
		71	+ return nil if target.organization? && !target.moderated_articles?
71	description + "\n\n" +	72	description + "\n\n" +
72	_('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name }	73	_('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name }
73	end	74	end
	@@ -59,7 +59,7 @@ class Task < ActiveRecord::Base		@@ -59,7 +59,7 @@ class Task < ActiveRecord::Base
59		59
60	begin	60	begin
61	target_msg = task.target_notification_message	61	target_msg = task.target_notification_message
62	- TaskMailer.deliver_target_notification(task, target_msg)	62	+ TaskMailer.deliver_target_notification(task, target_msg) if target_msg
63	rescue NotImplementedError => ex	63	rescue NotImplementedError => ex
64	RAILS_DEFAULT_LOGGER.info ex.to_s	64	RAILS_DEFAULT_LOGGER.info ex.to_s
65	end	65	end
	@@ -46,6 +46,17 @@		@@ -46,6 +46,17 @@
46	<% end %>	46	<% end %>
47	</div><!-- id='media-listing-upload' -->	47	</div><!-- id='media-listing-upload' -->
48	<hr/>	48	<hr/>
		49	+
		50	+ <script type='text/javascript'>
		51	+ document.observe("dom:loaded", function() {
		52	+ Event.addBehavior.reassignAfterAjax = true;
		53	+ Event.addBehavior({
		54	+ 'div#pagination-images .pagination a' : Remote.Link,
		55	+ 'div#pagination-documents .pagination a' : Remote.Link
		56	+ })
		57	+ });
		58	+ </script>
		59	+
49	<div id='media-listing'>	60	<div id='media-listing'>
50	<h3><%= _('Folders') %></h3>	61	<h3><%= _('Folders') %></h3>
51	<p><%= _('Drag images and documents to add them to the text. If needed, resize images by clicking the tree icon on editor.') %></p>	62	<p><%= _('Drag images and documents to add them to the text. If needed, resize images by clicking the tree icon on editor.') %></p>
	@@ -15,7 +15,12 @@ class FeedHandler		@@ -15,7 +15,12 @@ class FeedHandler
15	def fetch(address)	15	def fetch(address)
16	begin	16	begin
17	content = ""	17	content = ""
18	- open(address) do \|s\| content = s.read end	18	+ block = lambda { \|s\| content = s.read }
		19	+ content = if is_web_address?(address)
		20	+ open( address, "User-Agent" => "Noosfero/#{Noosfero::VERSION}", &block )
		21	+ else
		22	+ open_uri_original_open(address, &block)
		23	+ end
19	return content	24	return content
20	rescue Exception => ex	25	rescue Exception => ex
21	raise FeedHandler::FetchError, ex.to_s	26	raise FeedHandler::FetchError, ex.to_s
	@@ -39,4 +44,14 @@ class FeedHandler		@@ -39,4 +44,14 @@ class FeedHandler
39	class ParseError < Exception; end	44	class ParseError < Exception; end
40	class FetchError < Exception; end	45	class FetchError < Exception; end
41		46
		47	+ protected
		48	+
		49	+ # extracted from the open implementation in the open-uri library
		50	+ def is_web_address?(address)
		51	+ address.respond_to?(:open) \|\|
		52	+ address.respond_to?(:to_str) &&
		53	+ (%r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ address) &&
		54	+ URI.parse(address).respond_to?(:open)
		55	+ end
		56	+
42	end	57	end
	@@ -45,12 +45,3 @@ function convToValidIdentifier( str, sep ) {		@@ -45,12 +45,3 @@ function convToValidIdentifier( str, sep ) {
45	.replace( /ç/g, "c" )	45	.replace( /ç/g, "c" )
46	.replace( /[^-_a-z0-9.]+/g, sep )	46	.replace( /[^-_a-z0-9.]+/g, sep )
47	}	47	}
48	-
49	-document.observe("dom:loaded", function() {
50	- Event.addBehavior.reassignAfterAjax = true;
51	- Event.addBehavior({
52	- 'div#pagination-images .pagination a' : Remote.Link,
53	- 'div#pagination-documents .pagination a' : Remote.Link
54	- })
55	-});
56	-
	@@ -53,7 +53,6 @@ function checkIt(string) {		@@ -53,7 +53,6 @@ function checkIt(string) {
53		53
54	Event.observe(window, 'load', initialize, false);	54	Event.observe(window, 'load', initialize, false);
55	Event.observe(window, 'load', getBrowserInfo, false);	55	Event.observe(window, 'load', getBrowserInfo, false);
56	-Event.observe(window, 'unload', Event.unloadCache, false);
57		56
58	var lightbox = Class.create();	57	var lightbox = Class.create();
59		58
	@@ -73,4 +73,10 @@ class FeedHandlerTest < Test::Unit::TestCase		@@ -73,4 +73,10 @@ class FeedHandlerTest < Test::Unit::TestCase
73	handler.process(container)	73	handler.process(container)
74	end	74	end
75		75
		76	+ should 'identifies itself as noosfero user agent' do
		77	+ handler = FeedHandler.new
		78	+ handler.expects(:open).with('http://site.org/feed.xml', {"User-Agent" => "Noosfero/#{Noosfero::VERSION}"}, anything).returns('bli content')
		79	+ assert_equal 'bli content', handler.fetch('http://site.org/feed.xml')
		80	+ end
		81	+
76	end	82	end
	@@ -207,6 +207,13 @@ class TaskTest < Test::Unit::TestCase		@@ -207,6 +207,13 @@ class TaskTest < Test::Unit::TestCase
207	end	207	end
208	end	208	end
209		209
		210	+ should 'not notify target if message is nil' do
		211	+ task = Task.new
		212	+ task.stubs(:target_notification_message).returns(nil)
		213	+ TaskMailer.expects(:deliver_target_notification).never
		214	+ task.save!
		215	+ end
		216	+
210	protected	217	protected
211		218
212	def sample_user	219	def sample_user
	@@ -19,5 +19,11 @@ class TextArticleTest < Test::Unit::TestCase		@@ -19,5 +19,11 @@ class TextArticleTest < Test::Unit::TestCase
19	article = TextileArticle.create!(:name => 'found article test', :profile => person)	19	article = TextileArticle.create!(:name => 'found article test', :profile => person)
20	assert_equal TextileArticle.find_by_contents('found'), TextArticle.find_by_contents('found')	20	assert_equal TextileArticle.find_by_contents('found'), TextArticle.find_by_contents('found')
21	end	21	end
22	-	22	+
		23	+ should 'remove comments from TextArticle body' do
		24	+ person = create_user('testuser').person
		25	+ article = TextArticle.create!(:profile => person, :name => 'article', :body => "the <!-- comment --> article ...")
		26	+ assert_equal "the article ...", article.body
		27	+ end
		28	+
23	end	29	end
	@@ -4,6 +4,13 @@		@@ -4,6 +4,13 @@
4	# this was solved in rails 2.2.1, then remove this patch when upgrade to it	4	# this was solved in rails 2.2.1, then remove this patch when upgrade to it
5		5
6	HTML::WhiteListSanitizer.module_eval do	6	HTML::WhiteListSanitizer.module_eval do
		7	+
		8	+ def sanitize_with_filter_comments(*args, &block)
		9	+ text = sanitize_without_filter_comments(*args, &block)
		10	+ text.gsub(/<!--/, '<!--') if text
		11	+ end
		12	+ alias_method_chain :sanitize, :filter_comments
		13	+
7	# unescape before reescape to avoid:	14	# unescape before reescape to avoid:
8	# & -> & -> &amp; -> &amp;amp; -> &amp;amp;amp; -> etc	15	# & -> & -> &amp; -> &amp;amp; -> &amp;amp;amp; -> etc
9	protected	16	protected