[#6898] Check for project variable before using

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

app/views/issues/_bulk_edit_deliverable.html.erb

@@ -1,4 +1,4 @@
-<% if project.module_enabled?(:contracts) && User.current.allowed_to?(:assign_deliverable_to_issue, project) %>
+<% if project && project.module_enabled?(:contracts) && User.current.allowed_to?(:assign_deliverable_to_issue, project) %>
   <p>
     <%= label_tag(:deliverable_id, l(:field_deliverable)) %>
     <%= select_tag('deliverable_id',

assets/javascripts/contracts.js

@@ -2,7 +2,7 @@ jQuery(function($) {
   $("#ajax-indicator").ajaxStart(function(){ $(this).show().css('z-index', '9999');  });
   $("#ajax-indicator").ajaxStop(function(){ $(this).hide();  });
 
-  var right_align = $('#contract-terms .finance tr td:nth-child ~ td, .c_overview table.right tr td:nth-child ~ td, #deliverables table tr.click td:nth-child(5) ~ td, .deliverable_finance_table tr.aright td:nth-child ~ td');
+  var right_align = $('#contract-terms .finance tr td:nth-child(1) ~ td, .c_overview table.right tr td:nth-child(1) ~ td, #deliverables table tr.click td:nth-child(5) ~ td, .deliverable_finance_table tr.aright td:nth-child(1) ~ td');
 
   if (right_align.length > 0) {
     right_align.after().css("text-align", "right");

config/locales/en.yml

@@ -22,7 +22,7 @@ en:
   field_account_executive_short: "Acct. Mgr."
   field_end_date: "End Date"
   text_new_contract: "New Contract"
-  text_edit_contract_name: "Edit {{name}}"
+  text_edit_contract_name: "Edit %{name}"
   field_billable_rate: "Billable Rate"
   field_billable_rate_hint: "$"
   field_discount: "Discount"
@@ -34,7 +34,7 @@ en:
   field_details: "Details"
   button_add_new: Add New
   text_new_deliverable: New Deliverable
-  text_edit_deliverable_title: "Edit {{title}}"
+  text_edit_deliverable_title: "Edit %{title}"
   field_manager: Manager
   field_labor: Labor
   field_overhead: Overhead
@@ -43,7 +43,7 @@ en:
   field_feature_sign_off: Feature Sign Off
   field_warranty_sign_off: Warranty Sign Off
   text_deliverable_finances: Deliverable Finances
-  text_deliverable_finances_date: "Deliverable Finances - {{date}}"
+  text_deliverable_finances_date: "Deliverable Finances - %{date}"
   text_short_hours: hrs
   text_dollar_sign: '$'
   field_client_point_of_contact: "Point of Contact"
@@ -85,15 +85,15 @@ en:
   text_changed_period_message: "The period for this deliverable has been changed. Would you like to expand/shrink the Deliverable Finances?"
   field_current_period: "Current period"
   text_retainer_monthly_message: "Enter budget for a representative month. Any overrides to individual months can be done via the editor after saving."
-  text_flash_deliverable_created: "Deliverable: {{name}} was successfully created."
+  text_flash_deliverable_created: "Deliverable: %{name} was successfully created."
-  text_flash_deliverable_updated: "Deliverable: {{name}} was successfully updated."
+  text_flash_deliverable_updated: "Deliverable: %{name} was successfully updated."
-  text_flash_deliverable_deleted: "Deliverable: {{name}} was successfully deleted."
+  text_flash_deliverable_deleted: "Deliverable: %{name} was successfully deleted."
   field_budget: Budget
   field_markup: Markup
   field_paid: Paid
   field_spent: Spent
   field_profit: Profit
-  text_error_message_orphaned_time: "There is {{amount}} worth of time clocked to issues that are not assigned to any deliverables."
+  text_error_message_orphaned_time: "There is %{amount} worth of time clocked to issues that are not assigned to any deliverables."
   text_error_message_update_orphaned_time: "Please update the orphaned issues."
   field_estimated: Estimated
   text_deliverable_locked_warning: "This deliverable is locked and cannot be saved without changing it's status to Open."

lib/redmine_contracts/hooks/view_layouts_base_html_head_hook.rb

@@ -6,15 +6,25 @@ module RedmineContracts
                                     context[:controller].is_a?(ContractsController) ||
                                     context[:controller].is_a?(DeliverablesController)
                                     )
-          return stylesheet_link_tag("redmine_contracts", :plugin => "redmine_contracts", :media => "screen") +
+          tags = [stylesheet_link_tag("redmine_contracts", :plugin => "redmine_contracts", :media => "screen")]
-            stylesheet_link_tag('smoothness/jquery-ui-1.8.15.custom.css', :plugin => "redmine_contracts") +
+          tags << stylesheet_link_tag('smoothness/jquery-ui-1.8.15.custom.css', :plugin => "redmine_contracts")
 
-            javascript_include_tag('jquery-1.4.4.min.js', :plugin => 'redmine_contracts') +
+          jquery_included = begin
-            javascript_include_tag('jquery.tmpl.min.js', :plugin => 'redmine_contracts') +
+                              ChiliProject::Compatibility && ChiliProject::Compatibility.using_jquery?
-            javascript_include_tag('jquery-ui-1.8.15.custom.min.js', :plugin => "redmine_contracts") +
+                          rescue NameError
-            javascript_tag('jQuery.noConflict();') +
+                            # No compatibilty test
-            javascript_include_tag('contracts.js', :plugin => 'redmine_contracts')
+                            false
+                          end
+        unless jquery_included
+          tags << javascript_include_tag('jquery-1.4.4.min.js', :plugin => 'redmine_contracts')
+          tags << javascript_tag('jQuery.noConflict();')
+        end
+          
+          tags << javascript_include_tag('jquery.tmpl.min.js', :plugin => 'redmine_contracts')
+          tags << javascript_include_tag('jquery-ui-1.8.15.custom.min.js', :plugin => "redmine_contracts")
+          tags << javascript_include_tag('contracts.js', :plugin => 'redmine_contracts')
 
+          return tags.join(' ')
         else
           return ''
         end

lib/redmine_contracts/patches/project_patch.rb

@@ -16,27 +16,6 @@ module RedmineContracts
       end
 
       module InstanceMethods
-        def billable_activities
-          activities_sorted_by_billable[:billable]
-        end
-
-        def non_billable_activities
-          activities_sorted_by_billable[:non_billable]
-        end
-
-        private
-
-        def activities_sorted_by_billable
-          split_activities = activities.partition do |activity|
-            activity.billable?
-          end
-
-          {
-            :billable => split_activities.first,
-            :non_billable => split_activities.second
-          }
-
-        end
       end
     end
   end

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 1, @retainer_deliverable.fixed_budgets.count
-    assert_equal [600, nil, nil], @retainer_deliverable.fixed_budgets.collect(&:budget)
+    assert_equal [600], @retainer_deliverable.fixed_budgets.collect(&:budget)
   end
 
   context "locked deliverable" do

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