From 92ed8d0cc8819ae43561d35445d118a68912d517 Mon Sep 17 00:00:00 2001 From: Joenio Costa Date: Tue, 15 Feb 2011 17:02:56 -0300 Subject: [PATCH] Fixing validation error when try to approve published events --- app/models/event.rb | 30 +++++++++++++----------------- app/views/cms/_event.rhtml | 2 +- db/migrate/20110215153624_move_data_serialized_hash_to_setting_field_for_articles.rb | 26 ++++++++++++++++++++++++++ db/schema.rb | 2 +- test/unit/approve_article_test.rb | 8 ++++++++ test/unit/event_test.rb | 46 +++++++++++++++++++++++----------------------- 6 files changed, 72 insertions(+), 42 deletions(-) create mode 100644 db/migrate/20110215153624_move_data_serialized_hash_to_setting_field_for_articles.rb diff --git a/app/models/event.rb b/app/models/event.rb index cb200b2..53840c1 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,13 +1,17 @@ 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 < 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 < 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 diff --git a/app/views/cms/_event.rhtml b/app/views/cms/_event.rhtml index f5fd5a6..fedd5aa 100644 --- a/app/views/cms/_event.rhtml +++ b/app/views/cms/_event.rhtml @@ -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)) %> diff --git a/db/migrate/20110215153624_move_data_serialized_hash_to_setting_field_for_articles.rb b/db/migrate/20110215153624_move_data_serialized_hash_to_setting_field_for_articles.rb new file mode 100644 index 0000000..e53cf6f --- /dev/null +++ b/db/migrate/20110215153624_move_data_serialized_hash_to_setting_field_for_articles.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index bbcfcd7..8b8c470 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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" diff --git a/test/unit/approve_article_test.rb b/test/unit/approve_article_test.rb index a835628..fee663a 100644 --- a/test/unit/approve_article_test.rb +++ b/test/unit/approve_article_test.rb @@ -362,4 +362,12 @@ class ApproveArticleTest < 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 diff --git a/test/unit/event_test.rb b/test/unit/event_test.rb index e613110..f6d361b 100644 --- a/test/unit/event_test.rb +++ b/test/unit/event_test.rb @@ -14,9 +14,9 @@ class EventTest < 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 < 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 < 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 < 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 < 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 => '

a paragraph of text

', :link => 'www.gnu.org') + should 'not escape HTML in body' do + a = Event.new(:body => '

a paragraph of text

', :link => 'www.gnu.org') assert_match '

a paragraph of text

', 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 => '

a paragraph (valid)

"', :link => 'www.colivre.coop.br', :start_date => Date.today) + e = Event.create!(:profile => profile, :name => 'test', :body => '

a paragraph (valid)

"', :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 < ActiveSupport::TestCase should 'filter fields with white_list filter' do event = Event.new - event.description = "

Description

" + event.body = "

Description

" event.address = " Address " event.valid? - assert_equal "

Description

", event.description + assert_equal "

Description

", event.body assert_equal " Address ", event.address end should 'escape malformed html tags' do event = Event.new - event.description = ">/h1>" + event.body = ">/h1>" event.address = "><< Address " 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 = '

Wellformed html code

' + event.body = '

Wellformed html code

' event.address = '

Wellformed html code

' event.valid? - assert_match /

Wellformed html code <\/h1>/, event.description + assert_match /

Wellformed html code <\/h1>/, event.body assert_match /

Wellformed html code <\/h1>/, event.address end -- libgit2 0.21.2