diff --git a/app/models/deliverable.rb b/app/models/deliverable.rb index e52aeb6..079bbc6 100644 --- a/app/models/deliverable.rb +++ b/app/models/deliverable.rb @@ -20,6 +20,7 @@ class Deliverable < ActiveRecord::Base validates_presence_of :type validates_presence_of :manager validates_inclusion_of :status, :in => ["open","locked","closed"], :allow_blank => true, :allow_nil => true + validate :validate_update_allowed # Accessors include DollarizedAttribute @@ -63,6 +64,10 @@ class Deliverable < ActiveRecord::Base update_attribute(:status, "closed") end + def open? + self.status == "open" + end + def locked? self.status == "locked" end @@ -71,6 +76,28 @@ class Deliverable < ActiveRecord::Base self.status == "closed" end + def editable? + (new_record? || open?) + end + + def validate_update_allowed + unless new_record? + if changes.keys == ["status"] + noop("Allow changes to the status only") + elsif changes["status"].present? && changes["status"].second == "open" + noop("Allow any changes when going to 'open'") + elsif changes["status"].present? + errors.add_to_base(:cant_update_locked_deliverable) if locked? + errors.add_to_base(:cant_update_closed_deliverable) if closed? + end + end + end + + # No operation method, useful to clean up logic with an optional message + # for documentation + def noop(message="") + end + def to_s title end diff --git a/app/views/deliverables/_form.html.erb b/app/views/deliverables/_form.html.erb index b824160..54ddded 100644 --- a/app/views/deliverables/_form.html.erb +++ b/app/views/deliverables/_form.html.erb @@ -2,6 +2,12 @@ <%= javascript_tag("var i18nEndDateEmpty = '#{l(:text_end_date_empty)}'") %> <%= javascript_tag("var i18nChangedPeriodMessage = '#{l(:text_changed_period_message)}'") %> +<% if resource.locked? || resource.closed? %> +
+

<%= resource.locked? ? l(:text_deliverable_locked_warning) : l(:text_deliverable_closed_warning) %>

+
+<% end %> +
<% form.inputs :name => l(:text_deliverable_details_legend), :id => 'deliverable-details' do %> <%# Used by jquery to check if this is a new or existing record %> diff --git a/config/locales/en.yml b/config/locales/en.yml index dec6e89..5c260b7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -6,6 +6,8 @@ en: cant_create_time_on_closed_contract: "Can't create a time entry on a closed contract" cant_assign_to_closed_deliverable: "Can't assign issue to a closed deliverable" cant_assign_to_locked_deliverable: "Can't assign issue to a locked deliverable" + cant_update_locked_deliverable: "Can't update a locked deliverable" + cant_update_closed_deliverable: "Can't update a closed deliverable" field_end_date: End Date field_executed: Executed @@ -89,4 +91,5 @@ en: 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." + text_deliverable_closed_warning: "This deliverable is closed and cannot be saved without changing it's status to Open." diff --git a/test/integration/deliverables_edit_test.rb b/test/integration/deliverables_edit_test.rb index cde9edc..55a5c98 100644 --- a/test/integration/deliverables_edit_test.rb +++ b/test/integration/deliverables_edit_test.rb @@ -9,7 +9,7 @@ class DeliverablesEditTest < ActionController::IntegrationTest @manager = User.generate! @role = Role.generate! User.add_to_project(@manager, @project, @role) - @fixed_deliverable = FixedDeliverable.generate!(:contract => @contract, :manager => @manager, :title => 'The Title') + @fixed_deliverable = FixedDeliverable.generate!(:contract => @contract, :manager => @manager, :title => 'The Title', :notes => "", :feature_sign_off => false, :warranty_sign_off => false) @hourly_deliverable = HourlyDeliverable.generate!(:contract => @contract, :manager => @manager, :title => 'An Hourly') @user = User.generate_user_with_permission_to_manage_budget(:project => @project) @@ -475,4 +475,168 @@ class DeliverablesEditTest < ActionController::IntegrationTest assert_equal 3, @retainer_deliverable.fixed_budgets.count assert_equal [600, nil, nil], @retainer_deliverable.fixed_budgets.collect(&:budget) end + + context "locked deliverable" do + setup do + assert @fixed_deliverable.lock! + end + + should "block edits to locked deliverables" do + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + fill_in "Title", :with => 'An updated title' + end + + click_button "Save" + + assert_response :success + assert_template 'deliverables/edit' + + assert_not_equal "An updated title", @fixed_deliverable.reload.title + end + + should "block edits to locked deliverables even when status changes to closed" do + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + fill_in "Title", :with => 'An updated title' + select "Closed", :from => "Status" + end + + click_button "Save" + + assert_response :success + assert_template 'deliverables/edit' + + assert_not_equal "An updated title", @fixed_deliverable.reload.title + assert @fixed_deliverable.reload.locked? + end + + should "be allowed to change the status on a locked deliverables to open" do + assert @fixed_deliverable.lock! + + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + select "Open", :from => "Status" + end + + click_button "Save" + + assert_response :success + assert_template 'contracts/show' + + assert @fixed_deliverable.reload.open? + end + + should "be allowed to change the status on a locked deliverables to closed" do + assert @fixed_deliverable.lock! + + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + select "Closed", :from => "Status" + end + + click_button "Save" + + assert_response :success + assert_template 'contracts/show' + + assert @fixed_deliverable.reload.closed? + end + + end + + context "closed deliverable" do + setup do + assert @fixed_deliverable.close! + end + + should "block edits to closed deliverables" do + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + fill_in "Title", :with => 'An updated title' + end + + click_button "Save" + + assert_response :success + assert_template 'deliverables/edit' + + assert_not_equal "An updated title", @fixed_deliverable.reload.title + end + + should "block edits to closed deliverables even when the status is changed to locked" do + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + fill_in "Title", :with => 'An updated title' + select "Locked", :from => "Status" + end + + click_button "Save" + + assert_response :success + assert_template 'deliverables/edit' + + assert_not_equal "An updated title", @fixed_deliverable.reload.title + assert @fixed_deliverable.reload.closed? + end + + should "be allowed to change the status on a closed deliverables to open" do + assert @fixed_deliverable.close! + + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + select "Open", :from => "Status" + end + + click_button "Save" + + assert_response :success + assert_template 'contracts/show' + + assert @fixed_deliverable.reload.open? + end + + should "be allowed to change the status on a closed deliverables to Locked" do + assert @fixed_deliverable.lock! + + visit_contract_page(@contract) + click_link_within "#deliverable_details_#{@fixed_deliverable.id}", 'Edit' + assert_response :success + + within("#deliverable-details") do + select "Locked", :from => "Status" + end + + click_button "Save" + + assert_response :success + assert_template 'contracts/show' + + assert @fixed_deliverable.reload.locked? + end + end + + + end