Merge pull request #9488 from opf/fix/37775-Work-package-in-inconsistent-state-when-deleting-value-already-set-to-work-package

[37775] Work package in inconsistent state when deleting value already set to work package

modules/budgets/app/controllers/budgets_controller.rb

@@ -29,8 +29,9 @@
 class BudgetsController < ApplicationController
   include AttachableServiceCall
 
-  before_action :find_budget, only: %i[show edit update copy]
+  before_action :find_budget, only: %i[show edit update copy destroy_info]
   before_action :find_budgets, only: :destroy
+  before_action :check_and_update_belonging_work_packages, only: :destroy
   before_action :find_project, only: %i[new create update_material_budget_item update_labor_budget_item]
   before_action :find_optional_project, only: :index
 
@@ -141,6 +142,10 @@ class BudgetsController < ApplicationController
     redirect_to action: 'index', project_id: @project
   end
 
+  def destroy_info
+    @possible_other_budgets = @project.budgets.where.not(id: @budget.id)
+  end
+
   def update_material_budget_item
     @element_id = params[:element_id]
 
@@ -258,4 +263,27 @@ class BudgetsController < ApplicationController
       .page(page_param)
       .per_page(per_page_param)
   end
+
+  def check_and_update_belonging_work_packages
+    if params[:todo]
+      update_belonging_work_packages
+    end
+
+    budget = Budget.find(params[:id])
+    if budget.work_packages.any?
+      redirect_to destroy_info_budget_path(budget)
+    end
+  end
+
+  def update_belonging_work_packages
+    reassign_to_id = params[:reassign_to_id]
+    budget_id = params[:id]
+
+    budget_exists = Budget.visible(current_user).exists?(reassign_to_id) if params[:todo] == 'reassign'
+    reassign_to = budget_exists ? reassign_to_id : nil
+
+    WorkPackage
+      .where(budget_id: budget_id)
+      .update_all(budget_id: reassign_to, updated_at: DateTime.now)
+  end
 end

modules/budgets/app/views/budgets/destroy_info.html.erb

@@ -0,0 +1,63 @@
+<%#-- copyright
+OpenProject is an open source project management software.
+Copyright (C) 2012-2021 the OpenProject GmbH
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License version 3.
+
+OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+Copyright (C) 2006-2013 Jean-Philippe Lang
+Copyright (C) 2010-2013 the ChiliProject Team
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+See docs/COPYRIGHT.rdoc for more details.
+
+++#%>
+
+<% html_title "#{t(:button_delete)} #{t(:label_budget_id, id: @budget.id)}: #{@budget.subject}" %>
+<%= toolbar title: "#{t(:button_delete)} #{t(:label_budget_id, id: @budget.id)}: #{@budget.subject}" %>
+
+<%= styled_form_tag(budget_path(@budget), method: :delete) do %>
+  <section class="form--section">
+    <p><strong><%= t(:text_budget_destroy_assigned_wp, count: @budget.work_packages.count) %></strong></p>
+
+    <div class="form--field">
+      <label class="form--label" for="todo_delete"><%= t(:text_budget_delete) %></label>
+      <div class="form--field-container">
+        <%= styled_radio_button_tag 'todo', 'delete', false %>
+      </div>
+    </div>
+
+    <% if @possible_other_budgets.any? %>
+      <div class="form--field">
+        <label class="form--label" for="todo_reassign"><%= t(:text_budget_reassign_to) %></label>
+        <div class="form--field-container">
+          <%= styled_radio_button_tag 'todo', 'reassign', true %>
+        </div>
+
+        <%= label_tag "reassign_to_id", t(:text_budget_reassign_to), class: "hidden-for-sighted" %>
+        <%= styled_select_tag 'reassign_to_id', options_from_collection_for_select(@possible_other_budgets, 'id', 'name'), container_class: '-middle' %>
+
+        <% csp_onclick("document.getElementById('todo_reassign').checked = true", '#reassign_to_id') %>
+      </div>
+    <% end %>
+  </section>
+
+  <%= styled_submit_tag t(:button_apply), class: '-highlight' %>
+  <%= link_to t(:button_cancel),
+              budget_path(@budget),
+              class: 'button' %>
+<% end %>

modules/budgets/app/views/budgets/show.html.erb

@@ -45,9 +45,9 @@ See docs/COPYRIGHT.rdoc for more details.
       <% end %>
     </li>
   <% end %>
-  <% if authorize_for(:budgets, :copy) %>
+  <% if authorize_for(:budgets, :destroy) %>
     <li class="toolbar-item">
-      <%= link_to({ controller: 'budgets', action: 'destroy', id: @budget }, class: 'button', method: :delete, data: { confirm: t(:text_are_you_sure)}) do %>
+      <%= link_to({ controller: 'budgets', action: 'destroy', id: @budget }, class: 'button', method: :delete) do %>
         <%= op_icon('button--icon icon-delete') %>
         <span class="button--text"><%= t(:button_delete) %></span>
       <% end %>

modules/budgets/config/locales/en.yml

@@ -83,3 +83,7 @@ en:
   permission_edit_budgets: "Edit budgets"
   permission_view_budgets: "View budgets"
   project_module_budgets: "Budgets"
+
+  text_budget_reassign_to: "Reassign them to this budget:"
+  text_budget_delete: "Delete the budget from all work packages"
+  text_budget_destroy_assigned_wp: "There are %{count} work packages assigned to this budget. What do you want to do?"

modules/budgets/config/routes.rb

@@ -38,5 +38,6 @@ OpenProject::Application.routes.draw do
 
   resources :budgets, only: %i[show update destroy edit] do
     get :copy, on: :member
+    get :destroy_info, on: :member
   end
 end

modules/budgets/lib/budgets/engine.rb

@@ -8,7 +8,7 @@ module Budgets
              name: 'Budgets' do
       project_module :budgets do
         permission :view_budgets, { budgets: %i[index show] }
-        permission :edit_budgets, { budgets: %i[index show edit update destroy new create copy] }
+        permission :edit_budgets, { budgets: %i[index show edit update destroy destroy_info new create copy] }
       end
 
       menu :project_menu,

modules/budgets/spec/features/budgets/delete_budget_spec.rb

@@ -0,0 +1,140 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require File.expand_path("#{File.dirname(__FILE__)}/../../spec_helper.rb")
+
+describe 'Deleting a budget', type: :feature, js: true do
+  let(:project) { FactoryBot.create :project, enabled_module_names: %i[budgets costs] }
+  let(:user) { FactoryBot.create :admin }
+  let(:budget_subject) { "A budget subject" }
+  let(:budget_description) { "A budget description" }
+  let!(:budget) do
+    FactoryBot.create :budget,
+                      subject: budget_subject,
+                      description: budget_description,
+                      author: user,
+                      project: project
+  end
+
+  let(:budget_page) { Pages::EditBudget.new budget.id }
+  let(:budget_index_page) { Pages::IndexBudget.new project }
+
+  before do
+    login_as(user)
+    budget_page.visit!
+  end
+
+  context 'when no WP are assigned to this budget' do
+    it 'simply deletes the budget without additional checks' do
+      # Delete the budget
+      budget_page.click_delete
+
+      # Get directly back to index page and the budget is deleted
+      budget_index_page.expect_budget_not_listed budget_subject
+    end
+  end
+
+  context 'when WPs are assigned to this budget' do
+    let(:wp1) { FactoryBot.create :work_package, project: project, budget: budget }
+    let(:wp2) { FactoryBot.create :work_package, project: project, budget: budget }
+    let(:budget_destroy_info_page) { Pages::DestroyInfo.new budget }
+
+    before do
+      wp1
+      wp2
+    end
+
+    context 'with no other budget to assign to' do
+      before do
+        # When deleting with WPs assigned we get to the destroy_info page
+        budget_page.click_delete
+        budget_destroy_info_page.expect_loaded
+
+        # In any case the delete option is shown
+        budget_destroy_info_page.expect_delete_option
+      end
+
+      it 'deletes the budget from the WPs' do
+        # Select to delete the budget from the WPs
+        budget_destroy_info_page.expect_no_reassign_option
+        budget_destroy_info_page.select_delete_option
+
+        # Delete the budget
+        budget_destroy_info_page.delete
+
+        # Get back to index page and the budget is deleted
+        budget_index_page.expect_budget_not_listed budget_subject
+
+        # Both WPs are updated correctly
+        wp1.reload
+        wp2.reload
+        expect(wp1.budget).to eq nil
+        expect(wp2.budget).to eq nil
+      end
+    end
+
+    context 'with another budget to assign to' do
+      let(:budget2) do
+        FactoryBot.create :budget,
+                          subject: 'Another budget',
+                          description: budget_description,
+                          author: user,
+                          project: project
+      end
+
+      before do
+        budget2
+
+        # When deleting with WPs assigned we get to the destroy_info page
+        budget_page.click_delete
+        budget_destroy_info_page.expect_loaded
+
+        # In any case the delete option is shown
+        budget_destroy_info_page.expect_delete_option
+      end
+
+      it 'reassigns the WP to another budget' do
+        # Select reassign
+        budget_destroy_info_page.expect_reassign_option
+        budget_destroy_info_page.select_reassign_option budget2.subject
+
+        # Delete the budget
+        budget_destroy_info_page.delete
+
+        # Get back to index page and the budget is deleted
+        budget_index_page.expect_budget_not_listed budget_subject
+
+        # Both WPs are updated correctly
+        wp1.reload
+        wp2.reload
+        expect(wp1.budget.id).to eq budget2.id
+        expect(wp2.budget.id).to eq budget2.id
+      end
+    end
+  end
+end

modules/budgets/spec/support/pages/destroy_info.rb

@@ -0,0 +1,75 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'support/pages/page'
+
+module Pages
+  class DestroyInfo < Page
+    attr_accessor :budget
+
+    def initialize(budget)
+      self.budget = budget
+    end
+
+    def expect_loaded
+      expect(page)
+        .to have_content("#{I18n.t(:button_delete)} #{I18n.t(:label_budget_id, id: budget.id)}: #{budget.subject}")
+    end
+
+    def expect_reassign_option
+      expect(page)
+        .to have_field('todo_reassign')
+    end
+
+    def expect_no_reassign_option
+      expect(page)
+        .not_to have_field('todo_reassign')
+    end
+
+    def select_reassign_option(budget_name)
+      select(budget_name, from: 'reassign_to_id')
+    end
+
+    def expect_delete_option
+      expect(page)
+        .to have_field('todo_delete')
+    end
+
+    def select_delete_option
+      choose('todo_delete')
+    end
+
+    def delete
+      click_button 'Apply'
+    end
+
+    def path
+      destroy_info_budget_path(budget)
+    end
+  end
+end

modules/budgets/spec/support/pages/edit_budget.rb

@@ -45,6 +45,12 @@ module Pages
       end
     end
 
+    def click_delete
+      within '.toolbar-items' do
+        click_link 'Delete'
+      end
+    end
+
     def path
       "/budgets/#{budget_id}"
     end

modules/budgets/spec/support/pages/index_budget.rb

@@ -0,0 +1,48 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'support/pages/page'
+
+module Pages
+  class IndexBudget < Page
+    attr_accessor :project
+
+    def initialize(project)
+      self.project = project
+    end
+
+    def expect_budget_not_listed(budget_name)
+      expect(page)
+        .not_to have_content(budget_name)
+    end
+
+    def path
+      budgets_path(project)
+    end
+  end
+end