Commit 0e54aafd8482c58802e931956e3aeec9aabf2adc
Committed by
Daniela Feitosa
Daniela Feitosa
1 parent
0f3973a7
Exists in
master
and in
22 other branches
Fixing create_enterprise task crash
(ActionItem1893)
Showing
2 changed files
with
35 additions
and
13 deletions
Show diff stats
app/controllers/my_profile/tasks_controller.rb
| @@ -16,16 +16,18 @@ class TasksController < MyProfileController | @@ -16,16 +16,18 @@ class TasksController < MyProfileController | ||
| 16 | def close | 16 | def close |
| 17 | failed = {} | 17 | failed = {} |
| 18 | 18 | ||
| 19 | - params[:tasks].each do |id, value| | ||
| 20 | - decision = value[:decision] | ||
| 21 | - if request.post? && VALID_DECISIONS.include?(decision) && id && decision != 'skip' | ||
| 22 | - task = profile.find_in_all_tasks(id) | ||
| 23 | - task.update_attributes!(value[:task]) | ||
| 24 | - begin | ||
| 25 | - task.send(decision) | ||
| 26 | - rescue Exception => ex | ||
| 27 | - message = "#{task.title} (#{task.requestor ? task.requestor.name : task.author_name})" | ||
| 28 | - failed[ex.clean_message] ? failed[ex.clean_message] << message : failed[ex.clean_message] = [message] | 19 | + if params[:tasks] |
| 20 | + params[:tasks].each do |id, value| | ||
| 21 | + decision = value[:decision] | ||
| 22 | + if request.post? && VALID_DECISIONS.include?(decision) && id && decision != 'skip' | ||
| 23 | + task = profile.find_in_all_tasks(id) | ||
| 24 | + begin | ||
| 25 | + task.update_attributes(value[:task]) | ||
| 26 | + task.send(decision) | ||
| 27 | + rescue Exception => ex | ||
| 28 | + message = "#{task.title} (#{task.requestor ? task.requestor.name : task.author_name})" | ||
| 29 | + failed[ex.clean_message] ? failed[ex.clean_message] << message : failed[ex.clean_message] = [message] | ||
| 30 | + end | ||
| 29 | end | 31 | end |
| 30 | end | 32 | end |
| 31 | end | 33 | end |
test/functional/tasks_controller_test.rb
| @@ -217,16 +217,18 @@ class TasksControllerTest < Test::Unit::TestCase | @@ -217,16 +217,18 @@ class TasksControllerTest < Test::Unit::TestCase | ||
| 217 | assert_includes c_blog2.children(true), p_article | 217 | assert_includes c_blog2.children(true), p_article |
| 218 | end | 218 | end |
| 219 | 219 | ||
| 220 | - should 'raise error if there is an enterprise with the same identifier and keep the task active' do | 220 | + should 'display error if there is an enterprise with the same identifier and keep the task active' do |
| 221 | e = Environment.default | 221 | e = Environment.default |
| 222 | e.add_admin(profile) | 222 | e.add_admin(profile) |
| 223 | task = CreateEnterprise.create!(:name => "My Enterprise", :identifier => "my-enterprise", :requestor => profile, :target => e) | 223 | task = CreateEnterprise.create!(:name => "My Enterprise", :identifier => "my-enterprise", :requestor => profile, :target => e) |
| 224 | enterprise = fast_create(Enterprise, :name => "My Enterprise", :identifier => "my-enterprise") | 224 | enterprise = fast_create(Enterprise, :name => "My Enterprise", :identifier => "my-enterprise") |
| 225 | 225 | ||
| 226 | - assert_raise ActiveRecord::RecordInvalid do | ||
| 227 | - post :close, :tasks => {task.id => { :task => {:reject_explanation => "Bla bla"}, :decision => "cancel"}} | 226 | + assert_nothing_raised do |
| 227 | + post :close, :tasks => {task.id => {:decision => "finish"}} | ||
| 228 | end | 228 | end |
| 229 | 229 | ||
| 230 | + assert_match /Validation.failed/, @response.body | ||
| 231 | + | ||
| 230 | task.reload | 232 | task.reload |
| 231 | assert_equal Task::Status::ACTIVE, task.status | 233 | assert_equal Task::Status::ACTIVE, task.status |
| 232 | end | 234 | end |
| @@ -280,4 +282,22 @@ class TasksControllerTest < Test::Unit::TestCase | @@ -280,4 +282,22 @@ class TasksControllerTest < Test::Unit::TestCase | ||
| 280 | assert_equal 'new source', TinyMceArticle.find(:first).source_name | 282 | assert_equal 'new source', TinyMceArticle.find(:first).source_name |
| 281 | end | 283 | end |
| 282 | 284 | ||
| 285 | + should "not crash if accessing close without tasks parameter" do | ||
| 286 | + assert_nothing_raised do | ||
| 287 | + post :close | ||
| 288 | + end | ||
| 289 | + end | ||
| 290 | + | ||
| 291 | + should 'close create enterprise if trying to cancel even if there is already an existing identifier' do | ||
| 292 | + identifier = "common-identifier" | ||
| 293 | + task = CreateEnterprise.create!(:identifier => identifier, :name => identifier, :requestor => profile, :target => profile) | ||
| 294 | + fast_create(Profile, :identifier => identifier) | ||
| 295 | + | ||
| 296 | + assert_nothing_raised do | ||
| 297 | + post :close, :tasks => {task.id => {:task => {:reject_explanation => "Some explanation"}, :decision => 'cancel'}} | ||
| 298 | + end | ||
| 299 | + | ||
| 300 | + task.reload | ||
| 301 | + assert_equal Task::Status::CANCELLED, task.status | ||
| 302 | + end | ||
| 283 | end | 303 | end |