Fixing validation error when try to approve published events

- Event.body should be string as all other article types - Removed Event.description - Moving existing data in description field to body (ActionItem1865)

Fixing validation error when try to approve published events
- Event.body should be string as all other article types - Removed Event.description - Moving existing data in description field to body (ActionItem1865)
Joenio Costa · Daniela Feitosa
1 parent 49e084f8
Showing 6 changed files with 72 additions and 42 deletions Show diff stats
app/models/event.rb
app/views/cms/_event.rhtml
db/migrate/20110215153624_move_data_serialized_hash_to_setting_field_for_articles.rb
db/schema.rb
test/unit/approve_article_test.rb
test/unit/event_test.rb
 class Event < Article
-  acts_as_having_settings :field => :body
-
-  settings_items :description, :type => :string
-  settings_items :link, :type => :string
   settings_items :address, :type => :string
+  def link=(value)
+    self.setting[:link] = maybe_add_http(value)
+  end
+
+  def link
+    maybe_add_http(self.setting[:link])
+  end
+
   xss_terminate :only => [ :link ], :on => 'validation'
-  xss_terminate :only => [ :description, :link, :address ], :with => 'white_list', :on => 'validation'
+  xss_terminate :only => [ :body, :link, :address ], :with => 'white_list', :on => 'validation'
   def initialize(*args)
     super(*args)
@@ -27,7 +31,7 @@ class Event &lt; Article
   }
   include WhiteListFilter
-  filter_iframes :description, :link, :address, :whitelist => lambda { profile && profile.environment && profile.environment.trusted_sites_for_iframe }
+  filter_iframes :body, :link, :address, :whitelist => lambda { profile && profile.environment && profile.environment.trusted_sites_for_iframe }
   def self.description
     _('A calendar event')
@@ -96,26 +100,18 @@ class Event &lt; Article
         }
       }
-      if self.description
+      if self.body
         html.div('_____XXXX_DESCRIPTION_GOES_HERE_XXXX_____', :class => 'event-description') 
       end
     }
-    if self.description
-      result.sub!('_____XXXX_DESCRIPTION_GOES_HERE_XXXX_____', self.description)
+    if self.body
+      result.sub!('_____XXXX_DESCRIPTION_GOES_HERE_XXXX_____', self.body)
     end
     result
   end
-  def link=(value)
-    self.body[:link] = maybe_add_http(value)
-  end
-
-  def link
-    maybe_add_http(self.body[:link])
-  end
-
   def event?
     true
   end
@@ -15,5 +15,5 @@
 <%= labelled_form_field(_('Address:'), text_field(:article, :address)) %>
-<%= labelled_form_field(_('Information about the event:'), text_area(:article, :description, :cols => 64)) %>
+<%= labelled_form_field(_('Information about the event:'), text_area(:article, :body, :cols => 64)) %>
@@ -0,0 +1,26 @@
+class MoveDataSerializedHashToSettingFieldForArticles < ActiveRecord::Migration
+  def self.up
+    select_all("SELECT id FROM tasks WHERE type = 'ApproveArticle' AND status = 1").each do |data|
+      article = Task.find(data['id']).article
+      next unless article.kind_of?(Event)
+      body = ''
+      begin
+        body = YAML.load(article.body)
+      rescue
+        # do nothing
+        next
+      end
+      if body.kind_of?(Hash)
+        settings = article.setting.merge(body)
+        body = ActiveRecord::Base.sanitize_sql_for_assignment(:body => settings[:description])
+        update("UPDATE articles set %s WHERE id = %d" % [body, article.id])
+        setting = ActiveRecord::Base.sanitize_sql_for_assignment(:setting => settings.to_yaml)
+        update("UPDATE articles set %s WHERE id = %d" % [setting, article.id])
+      end
+    end
+  end
+
+  def self.down
+    say "Nothing to undo"
+  end
+end
@@ -9,7 +9,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20110203160153) do
+ActiveRecord::Schema.define(:version => 20110215153624) do
   create_table "action_tracker", :force => true do |t|
     t.integer  "user_id"
@@ -362,4 +362,12 @@ class ApproveArticleTest &lt; ActiveSupport::TestCase
     assert_match(/#{task.requestor.name} wants to publish the article: #{article.name}/, email.subject)
   end
+  should 'approve an event' do
+    event = fast_create(Event, :profile_id => profile.id, :name => 'Event test', :slug => 'event-test', :abstract => 'Lead of article', :body => 'This is my event')
+    task = ApproveArticle.create!(:name => 'Event test', :article => event, :target => community, :requestor => profile)
+    assert_difference event.class, :count do
+      task.finish
+    end
+  end
+
 end
@@ -14,9 +14,9 @@ class EventTest &lt; ActiveSupport::TestCase
     assert_kind_of String, Event.short_description
   end
-  should 'have a description' do
-    e = Event.new(:description => 'some useful description')
-    assert_equal 'some useful description', e.description
+  should 'have a body' do
+    e = Event.new(:body => 'some useful description')
+    assert_equal 'some useful description', e.body
   end
   should 'have a link' do
@@ -62,9 +62,9 @@ class EventTest &lt; ActiveSupport::TestCase
     assert_includes Event.find_by_contents('surprisingly'), e
   end
-  should 'be indexed by description' do
+  should 'be indexed by body' do
     profile = create_user('testuser').person
-    e = Event.create!(:name => 'bli', :start_date => Date.new(2008, 06, 06), :profile => profile, :description => 'my surprisingly long description about my freaking nice event')
+    e = Event.create!(:name => 'bli', :start_date => Date.new(2008, 06, 06), :profile => profile, :body => 'my surprisingly long description about my freaking nice event')
     assert_includes Event.find_by_contents('surprisingly'), e
   end
@@ -123,7 +123,7 @@ class EventTest &lt; ActiveSupport::TestCase
   end
   should 'provide nice display format' do
-    e = Event.new(:start_date => Date.new(2008,1,1), :end_date => Date.new(2008,1,1), :link => 'http://www.myevent.org', :description => 'my somewhat short description')
+    e = Event.new(:start_date => Date.new(2008,1,1), :end_date => Date.new(2008,1,1), :link => 'http://www.myevent.org', :body => 'my somewhat short description')
     assert_tag_in_string e.to_html, :content => Regexp.new("January 1, 2008")
     assert_tag_in_string e.to_html, :content => 'my somewhat short description'
@@ -131,9 +131,9 @@ class EventTest &lt; ActiveSupport::TestCase
   end
-  should 'not crash when description is blank' do
+  should 'not crash when body is blank' do
     e = Event.new
-    assert_nil e.description
+    assert_nil e.body
     assert_no_match(/_____XXXX_DESCRIPTION_GOES_HERE_XXXX_____/, e.to_html)
   end
@@ -144,30 +144,30 @@ class EventTest &lt; ActiveSupport::TestCase
   should 'add http:// when reading link' do
     a = Event.new
-    a.body[:link] = 'www.gnu.org'
+    a.setting[:link] = 'www.gnu.org'
     assert_equal 'http://www.gnu.org', a.link
   end
   should 'not add http:// to empty link' do
     a = Event.new
-    a.body[:link] = ''
+    a.setting[:link] = ''
     assert_equal '', a.link
-    a.body[:link] = nil
+    a.setting[:link] = nil
     assert_equal '', a.link
   end
-  should 'not escape HTML in description' do
-    a = Event.new(:description => '<p>a paragraph of text</p>', :link => 'www.gnu.org')
+  should 'not escape HTML in body' do
+    a = Event.new(:body => '<p>a paragraph of text</p>', :link => 'www.gnu.org')
     assert_match '<p>a paragraph of text</p>', a.to_html
   end
-  should 'filter HTML in description' do
+  should 'filter HTML in body' do
     profile = create_user('testuser').person
-    e = Event.create!(:profile => profile, :name => 'test', :description => '<p>a paragraph (valid)</p><script type="text/javascript">/* this is invalid */</script>"', :link => 'www.colivre.coop.br', :start_date => Date.today)
+    e = Event.create!(:profile => profile, :name => 'test', :body => '<p>a paragraph (valid)</p><script type="text/javascript">/* this is invalid */</script>"', :link => 'www.colivre.coop.br', :start_date => Date.today)
-    assert_tag_in_string e.description, :tag => 'p', :content => 'a paragraph (valid)'
-    assert_no_tag_in_string e.description, :tag => 'script'
+    assert_tag_in_string e.body, :tag => 'p', :content => 'a paragraph (valid)'
+    assert_no_tag_in_string e.body, :tag => 'script'
   end
   should 'nil to link' do
@@ -238,31 +238,31 @@ class EventTest &lt; ActiveSupport::TestCase
   should 'filter fields with white_list filter' do
     event = Event.new
-    event.description = "<h1> Description </h1>"
+    event.body = "<h1> Description </h1>"
     event.address = "<strong> Address <strong>"
     event.valid?
-    assert_equal "<h1> Description </h1>", event.description
+    assert_equal "<h1> Description </h1>", event.body
     assert_equal "<strong> Address <strong>", event.address
   end
   should 'escape malformed html tags' do
     event = Event.new
-    event.description = "<h1<< Description >>/h1>"
+    event.body = "<h1<< Description >>/h1>"
     event.address = "<strong>><< Address <strong>"
     event.valid?
-    assert_no_match /[<>]/, event.description
+    assert_no_match /[<>]/, event.body
     assert_no_match /[<>]/, event.address
   end
   should 'not sanitize html comments' do
     event = Event.new
-    event.description = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
+    event.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
     event.address = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
     event.valid?
-    assert_match  /<!-- .* --> <h1> Wellformed html code <\/h1>/, event.description
+    assert_match  /<!-- .* --> <h1> Wellformed html code <\/h1>/, event.body
     assert_match  /<!-- .* --> <h1> Wellformed html code <\/h1>/, event.address
   end
	@@ -15,5 +15,5 @@		@@ -15,5 +15,5 @@
15		15
16	<%= labelled_form_field(_('Address:'), text_field(:article, :address)) %>	16	<%= labelled_form_field(_('Address:'), text_field(:article, :address)) %>
17		17
18	-<%= labelled_form_field(_('Information about the event:'), text_area(:article, :description, :cols => 64)) %>	18	+<%= labelled_form_field(_('Information about the event:'), text_area(:article, :body, :cols => 64)) %>
19		19
@@ -0,0 +1,26 @@		@@ -0,0 +1,26 @@
	1	+class MoveDataSerializedHashToSettingFieldForArticles < ActiveRecord::Migration
	2	+ def self.up
	3	+ select_all("SELECT id FROM tasks WHERE type = 'ApproveArticle' AND status = 1").each do \|data\|
	4	+ article = Task.find(data['id']).article
	5	+ next unless article.kind_of?(Event)
	6	+ body = ''
	7	+ begin
	8	+ body = YAML.load(article.body)
	9	+ rescue
	10	+ # do nothing
	11	+ next
	12	+ end
	13	+ if body.kind_of?(Hash)
	14	+ settings = article.setting.merge(body)
	15	+ body = ActiveRecord::Base.sanitize_sql_for_assignment(:body => settings[:description])
	16	+ update("UPDATE articles set %s WHERE id = %d" % [body, article.id])
	17	+ setting = ActiveRecord::Base.sanitize_sql_for_assignment(:setting => settings.to_yaml)
	18	+ update("UPDATE articles set %s WHERE id = %d" % [setting, article.id])
	19	+ end
	20	+ end
	21	+ end
	22	+
	23	+ def self.down
	24	+ say "Nothing to undo"
	25	+ end
	26	+end
	@@ -9,7 +9,7 @@		@@ -9,7 +9,7 @@
9	#	9	#
10	# It's strongly recommended to check this file into your version control system.	10	# It's strongly recommended to check this file into your version control system.
11		11
12	-ActiveRecord::Schema.define(:version => 20110203160153) do	12	+ActiveRecord::Schema.define(:version => 20110215153624) do
13		13
14	create_table "action_tracker", :force => true do \|t\|	14	create_table "action_tracker", :force => true do \|t\|
15	t.integer "user_id"	15	t.integer "user_id"
	@@ -362,4 +362,12 @@ class ApproveArticleTest < ActiveSupport::TestCase		@@ -362,4 +362,12 @@ class ApproveArticleTest < ActiveSupport::TestCase
362	assert_match(/#{task.requestor.name} wants to publish the article: #{article.name}/, email.subject)	362	assert_match(/#{task.requestor.name} wants to publish the article: #{article.name}/, email.subject)
363	end	363	end
364		364
		365	+ should 'approve an event' do
		366	+ event = fast_create(Event, :profile_id => profile.id, :name => 'Event test', :slug => 'event-test', :abstract => 'Lead of article', :body => 'This is my event')
		367	+ task = ApproveArticle.create!(:name => 'Event test', :article => event, :target => community, :requestor => profile)
		368	+ assert_difference event.class, :count do
		369	+ task.finish
		370	+ end
		371	+ end
		372	+
365	end	373	end