diff --git a/app/controllers/deliverables_controller.rb b/app/controllers/deliverables_controller.rb index 9e56b73..e399f9c 100644 --- a/app/controllers/deliverables_controller.rb +++ b/app/controllers/deliverables_controller.rb @@ -14,6 +14,7 @@ class DeliverablesController < InheritedResources::Base end def create + remove_empty_budget_items(params) @deliverable = begin_of_association_chain.deliverables.build(params[:deliverable]) if params[:deliverable] && params[:deliverable][:type] && Deliverable.valid_types.include?(params[:deliverable][:type]) @deliverable.type = params[:deliverable][:type] @@ -24,6 +25,7 @@ class DeliverablesController < InheritedResources::Base def update @deliverable = begin_of_association_chain.deliverables.find_by_id(params[:id]) params[:deliverable] = params[:fixed_deliverable] || params[:hourly_deliverable] || params[:retainer_deliverable] + remove_empty_budget_items(params) update!(:notice => l(:text_flash_deliverable_updated, :name => @deliverable.title)) { contract_url(@project, @contract) } end @@ -75,4 +77,27 @@ class DeliverablesController < InheritedResources::Base period end + # Remove empty budgets. Will prevent validation errors + # from empty fields submitted from the bulk adding form. + # + # LSS Clients #6714 + def remove_empty_budget_items(params) + params["deliverable"]["labor_budgets_attributes"].reject! {|key, b| budget_item_empty?(b) } + params["deliverable"]["overhead_budgets_attributes"].reject! {|key, b| budget_item_empty?(b) } + params["deliverable"]["fixed_budgets_attributes"].reject! {|key, b| fixed_budget_item_empty?(b) } + end + + def budget_item_empty?(item) + (item["time_entry_activity_id"].blank?) && + (item["hours"].blank? || item["hours"].to_f == 0.0) && + (item["budget"].blank? || item["budget"].gsub('$','').to_f == 0.0) + end + + def fixed_budget_item_empty?(item) + (item["title"].blank?) && + (item["budget"].blank? || item["budget"].gsub('$','').to_f == 0.0) && + (item["markup"].blank? || item["markup"].gsub('$','').to_d == 0.0) + + end + end diff --git a/test/integration/deliverables_edit_test.rb b/test/integration/deliverables_edit_test.rb index 0f5f155..720323d 100644 --- a/test/integration/deliverables_edit_test.rb +++ b/test/integration/deliverables_edit_test.rb @@ -118,6 +118,34 @@ class DeliverablesEditTest < ActionController::IntegrationTest assert_equal 1000.0, @overhead_budget.budget end + should "not create new budget items for the deliverable if the activity, hours, and dollars are not all blank" do + TimeEntryActivity.destroy_all + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@hourly_deliverable.id}", 'Edit' + assert_response :success + assert_template 'deliverables/edit' + + within("#deliverable-labor") do + fill_in "hrs", :with => '' + fill_in "$", :with => '$0' + end + + within("#deliverable-overhead") do + fill_in "hrs", :with => '0' + fill_in "$", :with => '' + end + + click_button "Save" + + assert_response :success + assert_template 'contracts/show' + + @hourly_deliverable.reload + assert_equal 0, @hourly_deliverable.labor_budgets.count + assert_equal 0, @hourly_deliverable.overhead_budgets.count + assert_equal 0, @hourly_deliverable.fixed_budgets.count + end + should "show allow editing the Deliverable Finances section for each Retainer period" do @retainer_deliverable = RetainerDeliverable.spawn(:contract => @contract, :manager => @manager, :title => "Retainer") @retainer_deliverable.labor_budgets << @labor_budget = LaborBudget.spawn(:deliverable => @retainer_deliverable, :budget => 1000, :hours => 10) @@ -476,8 +504,8 @@ class DeliverablesEditTest < ActionController::IntegrationTest assert_equal [100, nil, nil], @retainer_deliverable.overhead_budgets.collect(&:hours) assert_equal [100, nil, nil], @retainer_deliverable.overhead_budgets.collect(&:budget) - assert_equal 3, @retainer_deliverable.fixed_budgets.count - assert_equal [600, nil, nil], @retainer_deliverable.fixed_budgets.collect(&:budget) + assert_equal 1, @retainer_deliverable.fixed_budgets.count + assert_equal [600], @retainer_deliverable.fixed_budgets.collect(&:budget) end context "locked deliverable" do diff --git a/test/integration/deliverables_new_test.rb b/test/integration/deliverables_new_test.rb index 37dae03..6ea2e6e 100644 --- a/test/integration/deliverables_new_test.rb +++ b/test/integration/deliverables_new_test.rb @@ -278,4 +278,50 @@ class DeliverablesNewTest < ActionController::IntegrationTest end + should "not create new budget items for the deliverable if the activity, hours, and dollars are all blank" do + @manager = User.generate! + @role = Role.generate! + User.add_to_project(@manager, @project, @role) + TimeEntryActivity.destroy_all + + visit_contract_page(@contract) + click_link 'Add New' + assert_response :success + + within("#deliverable-details") do + fill_in "Title", :with => 'A New Deliverable with blanks' + select "Hourly", :from => "Type" + select @manager.name, :from => "Manager" + end + + within("#deliverable-labor") do + # no activity selected + fill_in "hrs", :with => '' + fill_in "$", :with => '$0' + end + + within("#deliverable-overhead") do + # no activity selected + fill_in "hrs", :with => '0' + fill_in "$", :with => '' + end + + within("#deliverable-fixed") do + fill_in "title", :with => '' + fill_in "budget", :with => '$0' + fill_in "markup", :with => '' + end + + click_button "Save" + + assert_response :success + assert_template 'contracts/show' + + @deliverable = Deliverable.last + assert_equal "A New Deliverable with blanks", @deliverable.title + assert_equal 0, @deliverable.labor_budgets.count + assert_equal 0, @deliverable.overhead_budgets.count + assert_equal 0, @deliverable.fixed_budgets.count + end + end