Skip to content

Commit

Permalink
Statuses can no longer have a NULL default done ratio
Browse files Browse the repository at this point in the history
It is 0 by default. This is to ensure that the total work, total
remaining work, and total % Complete are always consistent. Without it,
we could have Work being set and Remaining work being unset, which is
bad for the ancestors totals.

See https://community.openproject.org/wp/40749 for the detailed
specification.
  • Loading branch information
cbliard committed Mar 29, 2024
1 parent 82d28c5 commit 19198a8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 50 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class ChangeStatusDefaultDoneRatioToZero < ActiveRecord::Migration[7.1]
def change
change_column_default :statuses, :default_done_ratio, from: nil, to: 0
reversible do |dir|
dir.up do
execute "UPDATE statuses SET default_done_ratio = 0 WHERE default_done_ratio IS NULL"
end
end
change_column_null :statuses, :default_done_ratio, false
end
end
31 changes: 8 additions & 23 deletions spec/services/work_packages/set_attributes_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

RSpec.describe WorkPackages::SetAttributesService,
type: :model do
shared_let(:status_no_pct_complete) { create(:status, default_done_ratio: nil, name: "no % complete") }
shared_let(:status_0_pct_complete) { create(:status, default_done_ratio: 0, name: "0% complete") }
shared_let(:status_50_pct_complete) { create(:status, default_done_ratio: 50, name: "50% complete") }
shared_let(:status_70_pct_complete) { create(:status, default_done_ratio: 70, name: "70% complete") }

Expand All @@ -43,7 +43,7 @@
p
end
let(:work_package) do
wp = build_stubbed(:work_package, project:, status: status_no_pct_complete)
wp = build_stubbed(:work_package, project:, status: status_0_pct_complete)
wp.type = initial_type
wp.send(:clear_changes_information)

Expand Down Expand Up @@ -184,33 +184,25 @@
it_behaves_like "service call", description: "recomputes remaining work accordingly"
end

context "when another status with a default % complete value is set" do
context "when another status is set" do
let(:call_attributes) { { status: status_70_pct_complete } }
let(:expected_attributes) { { remaining_hours: 3.0 } }

it_behaves_like "service call",
description: "recomputes remaining work according to the % complete value of the new status"
end

context "when another status without any default % complete value is set" do
let(:call_attributes) { { status: status_no_pct_complete } }
let(:expected_attributes) { { remaining_hours: nil } }

it_behaves_like "service call",
description: "unsets remaining work"
end
end

context "given a work package with work and remaining work unset, and a status with no % complete" do
context "given a work package with work and remaining work unset, and a status with 0% complete" do
before do
work_package.status = status_no_pct_complete
work_package.status = status_0_pct_complete
work_package.done_ratio = work_package.status.default_done_ratio
work_package.estimated_hours = nil
work_package.remaining_hours = nil
work_package.send(:clear_changes_information)
end

context "when another status with a default % complete value is set" do
context "when another status with another % complete value is set" do
let(:call_attributes) { { status: status_70_pct_complete } }
let(:expected_attributes) { { remaining_hours: nil } }

Expand All @@ -220,10 +212,10 @@

context "when work is set" do
let(:call_attributes) { { estimated_hours: 10.0 } }
let(:expected_attributes) { { remaining_hours: nil } }
let(:expected_attributes) { { remaining_hours: 10.0 } }

it_behaves_like "service call",
description: "remaining work remains unset"
description: "remaining work is updated accordingly from work and % complete value of the status"
end
end
end
Expand All @@ -246,13 +238,6 @@

it_behaves_like "service call", description: "sets the % complete value to the status default % complete value"
end

context "when another status with no % complete value is set" do
let(:call_attributes) { { status: status_no_pct_complete } }
let(:expected_attributes) { { done_ratio: nil } }

it_behaves_like "service call", description: "unsets the % complete value"
end
end
end

Expand Down
31 changes: 4 additions & 27 deletions spec/services/work_packages/update_ancestors_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ def set_attributes_on(work_package, attributes)

describe "done_ratio/estimated_hours/remaining_hours propagation" do
context "when setting the status of a work package" do
shared_let(:open_status) { create(:status) }
shared_let(:new_status_with_done_ratio) { create(:status, default_done_ratio: 100) }
shared_let(:new_status_without_done_ratio) { create(:status, default_done_ratio: nil) }
shared_let(:open_status) { create(:status, name: "open", default_done_ratio: 0) }
shared_let(:complete_status_with_100p_done_ratio) { create(:status, name: "complete", default_done_ratio: 100) }

context 'when using the "status-based" % complete mode',
with_settings: { work_package_done_ratio: "status" } do
Expand Down Expand Up @@ -92,8 +91,8 @@ def call_update_ancestors_service(work_package)
it "recomputes child remaining work and update ancestors total % complete accordingly" do
value =
case field
when :status then new_status_with_done_ratio
when :status_id then new_status_with_done_ratio.id
when :status then complete_status_with_100p_done_ratio
when :status_id then complete_status_with_100p_done_ratio.id
end
set_attributes_on(child, field => value)
call_update_ancestors_service(child)
Expand All @@ -107,28 +106,6 @@ def call_update_ancestors_service(work_package)
end
end
end

context "when changing child status to a status without any default done ratio" do
%i[status status_id].each do |field|
context "with the #{field} field" do
it "unsets child remaining work and update ancestors total % complete accordingly" do
value =
case field
when :status then new_status_without_done_ratio
when :status_id then new_status_without_done_ratio.id
end
set_attributes_on(child, field => value)
call_update_ancestors_service(child)

expect_work_packages([parent, child], <<~TABLE)
| subject | work | total work | remaining work | total remaining work | % complete | total % complete |
| parent | 10h | 15h | 10h | 10h | 0% | 33% |
| child | 5h | 5h | | | | |
TABLE
end
end
end
end
end
end
end
Expand Down

0 comments on commit 19198a8

Please sign in to comment.