From c40bd5fdc248b8ceccf35b9f689f871de333ec23 Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Tue, 17 Sep 2024 09:06:11 +0200 Subject: [PATCH] Recompute progress values when changing status on new work package form Because of the weird way we initialize the fake work package, it could believe that the status did not change and no recomputation was needed. This is an edge case, and I'm not satisfied with its design. --- .../derive_progress_values_status_based.rb | 3 ++- .../work_packages/progress_modal_spec.rb | 19 ++++++++++++++++ ...erive_progress_values_status_based_spec.rb | 22 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/services/work_packages/set_attributes_service/derive_progress_values_status_based.rb b/app/services/work_packages/set_attributes_service/derive_progress_values_status_based.rb index 7f140a9caf1a..00ea713c06af 100644 --- a/app/services/work_packages/set_attributes_service/derive_progress_values_status_based.rb +++ b/app/services/work_packages/set_attributes_service/derive_progress_values_status_based.rb @@ -54,7 +54,8 @@ def derive_remaining_work? end def status_percent_complete_changed? - work_package.status_id_changed? && work_package.status.default_done_ratio != work_package.done_ratio_was + work_package.status_id.present? && work_package.status_id_came_from_user? \ + && work_package.status.default_done_ratio != work_package.done_ratio_was end # Update +% complete+ from the status if the status changed. diff --git a/spec/features/work_packages/progress_modal_spec.rb b/spec/features/work_packages/progress_modal_spec.rb index 21d7ea323905..c4d76b53c481 100644 --- a/spec/features/work_packages/progress_modal_spec.rb +++ b/spec/features/work_packages/progress_modal_spec.rb @@ -265,6 +265,25 @@ def visit_progress_query_displaying_work_package work_package_create_page.save! work_package_table.expect_and_dismiss_toaster(message: "Successful creation.") end + + it "updates remaining work when status is changed" do + work_package_create_page.visit! + work_package_create_page.set_attributes({ subject: "hello" }) + + progress_popover.open + progress_popover.expect_hints(work: nil, remaining_work: nil) + progress_popover.set_values(work: "14h") + progress_popover.expect_values(remaining_work: "14h") + progress_popover.expect_hints(remaining_work: :derived) + progress_popover.save + + status_field = work_package_create_page.edit_field(:status) + status_field.update("in progress") + + progress_popover.open + progress_popover.expect_values(work: "14h", remaining_work: "7h") + progress_popover.expect_hints(remaining_work: :derived) + end end end end diff --git a/spec/services/work_packages/set_attributes_service/derive_progress_values_status_based_spec.rb b/spec/services/work_packages/set_attributes_service/derive_progress_values_status_based_spec.rb index 5cc0bd8a0722..e2587b2816b1 100644 --- a/spec/services/work_packages/set_attributes_service/derive_progress_values_status_based_spec.rb +++ b/spec/services/work_packages/set_attributes_service/derive_progress_values_status_based_spec.rb @@ -138,6 +138,28 @@ end end + context "given a work package with status and % complete not being in sync" do + before do + work_package.status = status_50_pct_complete + work_package.done_ratio = 0 + work_package.estimated_hours = 10.0 + work_package.remaining_hours = 10.0 + work_package.clear_changes_information + end + + context "when status is set again to the same value" do + let(:set_attributes) { { status: status_50_pct_complete } } + let(:expected_derived_attributes) { { remaining_hours: 5.0, done_ratio: 50 } } + let(:expected_kept_attributes) { %w[estimated_hours] } + + include_examples "update progress values", description: "updates % complete value to the status default % complete value " \ + "and derives remaining work", + expected_hints: { + remaining_work: :derived + } + end + end + context "given a work package with work and remaining work being empty, and a status with 0% complete" do before do work_package.status = status_0_pct_complete