- Link to Foo Barrit + Link to Foo Barrit
EXPECTED end diff --git a/spec/lib/open_project/text_formatting/markdown/setting_variable_spec.rb b/spec/lib/open_project/text_formatting/markdown/setting_variable_spec.rb index 3ce2081762d..1e014970436 100644 --- a/spec/lib/open_project/text_formatting/markdown/setting_variable_spec.rb +++ b/spec/lib/open_project/text_formatting/markdown/setting_variable_spec.rb @@ -60,11 +60,11 @@ RSpec.describe OpenProject::TextFormatting, Inline reference to base_url variable: #{Rails.application.root_url}- Saved and transformed link with setting
diff --git a/spec/lib/open_project/text_formatting/markdown/user_provided_links_spec.rb b/spec/lib/open_project/text_formatting/markdown/user_provided_links_spec.rb index 4b50c7bad7a..b3aa10aa4e6 100644 --- a/spec/lib/open_project/text_formatting/markdown/user_provided_links_spec.rb +++ b/spec/lib/open_project/text_formatting/markdown/user_provided_links_spec.rb @@ -121,7 +121,7 @@ RSpec.describe OpenProject::TextFormatting, let(:expected) do <<~EXPECTED
- Link to relative path + Link to relative path
EXPECTED end diff --git a/spec/migrations/update_progress_calculation_spec.rb b/spec/migrations/update_progress_calculation_spec.rb index 1895179a257..8ce33af5d5b 100644 --- a/spec/migrations/update_progress_calculation_spec.rb +++ b/spec/migrations/update_progress_calculation_spec.rb @@ -78,10 +78,18 @@ RSpec.describe UpdateProgressCalculation, type: :model do run_migration end - it "does not create a journal entry" do - table_work_packages.each do |wp| - expect(wp.journals.count).to eq(1) - end + it "does create a journal entry only for the work package with a total % complete" do + expect(wp_all_unset.journals.count).to eq(1) + expect(wp_only_pc_set.journals.count).to eq(1) + + # This one will receive a journal entry since we treat the total % complete field + # as if it was introduced by the migration (OP 14.0) even though the field existed before. + # But the calculation was off and we did not present the activity on the field anyway. + expect(wp_all_set_consistent.journals.count).to eq(2) + + expect(wp_all_set_consistent.last_journal.get_changes) + .to include("derived_done_ratio" => [nil, 60], + "cause" => [nil, { "feature" => "progress_calculation_changed", "type" => "system_update" }]) end end @@ -635,6 +643,26 @@ RSpec.describe UpdateProgressCalculation, type: :model do ) end end + + context "when parent is open without any work or remaining work set, " \ + "and children have a 100% complete status" do + it "sets parent total % complete to 100%" do + expect_migrates( + from: <<~TABLE, + hierarchy | status | work | remaining work | % complete + parent | To do (0%) | | | 0% + child1 | Done (100%) | 10h | 0h | 100% + child2 | Done (100%) | 10h | 0h | 100% + TABLE + to: <<~TABLE + hierarchy | status | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete + parent | To do (0%) | | | 0% | 20h | 0h | 100% + child1 | Done (100%) | 10h | 0h | 100% | 10h | 0h | 100% + child2 | Done (100%) | 10h | 0h | 100% | 10h | 0h | 100% + TABLE + ) + end + end end describe "totals computation" do @@ -689,6 +717,148 @@ RSpec.describe UpdateProgressCalculation, type: :model do ) end end + + context "when parent does not have any work or remaining work set, " \ + "and children have a 100% complete status" do + it "sets parent total % complete to 100%" do + expect_migrates( + from: <<~TABLE, + hierarchy | work | remaining work | % complete + parent | | | + child1 | 10h | 0h | 100% + child2 | 10h | 0h | 100% + TABLE + to: <<~TABLE + hierarchy | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete + parent | | | | 20h | 0h | 100% + child1 | 10h | 0h | 100% | 10h | 0h | 100% + child2 | 10h | 0h | 100% | 10h | 0h | 100% + TABLE + ) + end + end + + context "when work and remaining work are unset" do + it "does not set total work, total remaining work, and total % complete" do + expect_migrates( + from: <<~TABLE, + subject | work | remaining work | % complete + wp all unset | | | + wp 0% | | | 0% + wp 30% | | | 30% + wp 100% | | | 100% + TABLE + to: <<~TABLE + subject | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete + wp all unset | | | | | | + wp 0% | | | 0% | | | + wp 30% | | | 30% | | | + wp 100% | | | 100% | | | + TABLE + ) + end + end + + context "when work is set to 0h and remaining work is unset" do + it "sets remaining work, total work, and total remaining work to 0h, unsets % complete, and keeps total % complete unset" do + expect_migrates( + from: <<~TABLE, + subject | work | remaining work | % complete + wp all unset | 0h | | + wp 0% | 0h | | 0% + wp 30% | 0h | | 30% + wp 100% | 0h | | 100% + TABLE + to: <<~TABLE + subject | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete + wp all unset | 0h | 0h | | 0h | 0h | + wp 0% | 0h | 0h | | 0h | 0h | + wp 30% | 0h | 0h | | 0h | 0h | + wp 100% | 0h | 0h | | 0h | 0h | + TABLE + ) + end + end + + context "when work and remaining work are set and not 0h" do + it "sets total work, total remaining work, and total % complete accordingly same as work, remaining work and % complete" do + expect_migrates( + from: <<~TABLE, + subject | work | remaining work | + wp w set | 10h | | + wp rw set | | 5h | + wp w rw set | 10h | 5h | + TABLE + to: <<~TABLE + subject | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete + wp w set | 10h | 10h | 0% | 10h | 10h | 0% + wp rw set | 5h | 5h | 0% | 5h | 5h | 0% + wp w rw set | 10h | 5h | 50% | 10h | 5h | 50% + TABLE + ) + end + end + + context "when ∑ % complete has had some values (including wrong ones)" do + let_work_packages(<<~TABLE) + hierarchy | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete | + wp zero | | | 0 | | | 0 | + wp correct | 100h | 50h | 50 | 100h | 50h | 50 | + wp wrong | | | 90 | 100h | 50h | 90 | + wp wrong child | 100h | 50h | 20 | 10h | 10h | 20 | + TABLE + + before do + run_migration + end + + it "fixes the total values and sets ∑ % complete to nil (not 0) but keeps % complete (unless wrong)" do + expect_work_packages(WorkPackage.all, <<~TABLE) + subject | work | remaining work | % complete | ∑ work | ∑ remaining work | ∑ % complete | + wp zero | | | 0 | | | | + wp correct | 100h | 50h | 50 | 100h | 50h | 50 | + wp wrong | | | 90 | 100h | 50h | 50 | + wp wrong child | 100h | 50h | 50 | 100h | 50h | 50 | + TABLE + end + + it "creates no journal for the work package transitioning from 0 to nil and reworks the pre migration journals" do + expect(wp_zero.journals.count).to eq(1) + + expect(wp_zero.journals.first.get_changes.keys) + .not_to include("derived_done_ratio") + end + + it "creates a journal for the correct work package as the old ∑ % complete value has been set to null during the job" do + expect(wp_correct.journals.count).to eq(2) + + expect(wp_correct.journals.first.get_changes.keys) + .not_to include("derived_done_ratio") + + expect(wp_correct.journals.last.get_changes["derived_done_ratio"]) + .to eql [nil, 50] + end + + it "creates a journal for the work package transitioning from 90 to 50 and reworks the pre migration journals" do + expect(wp_wrong.journals.count).to eq(2) + + expect(wp_wrong.journals.first.get_changes.keys) + .not_to include("derived_done_ratio") + + expect(wp_wrong.journals.last.get_changes["derived_done_ratio"]) + .to eql [nil, 50] + end + + it "creates a journal for the work package transitioning from 20 to 50 and reworks the pre migration journals" do + expect(wp_wrong_child.journals.count).to eq(2) + + expect(wp_wrong_child.journals.first.get_changes.keys) + .not_to include("derived_done_ratio") + + expect(wp_wrong_child.journals.last.get_changes["derived_done_ratio"]) + .to eql [nil, 50] + end + end end describe "error during job execution" do diff --git a/spec/models/custom_actions/actions/done_ratio_spec.rb b/spec/models/custom_actions/actions/done_ratio_spec.rb index 320ac891804..4c8a1efe79f 100644 --- a/spec/models/custom_actions/actions/done_ratio_spec.rb +++ b/spec/models/custom_actions/actions/done_ratio_spec.rb @@ -88,14 +88,7 @@ RSpec.describe CustomActions::Actions::DoneRatio do end describe ".all" do - context "with field disabled", with_settings: { work_package_done_ratio: "disabled" } do - it "is empty" do - expect(described_class.all) - .to be_empty - end - end - - context "with field derived", with_settings: { work_package_done_ratio: "status" } do + context "in status-based progress calculation mode", with_settings: { work_package_done_ratio: "status" } do it "is empty" do expect(described_class.all) .to be_empty diff --git a/spec/models/queries/work_packages/selects/property_select_spec.rb b/spec/models/queries/work_packages/selects/property_select_spec.rb index e7b0f40e582..ad7868b4673 100644 --- a/spec/models/queries/work_packages/selects/property_select_spec.rb +++ b/spec/models/queries/work_packages/selects/property_select_spec.rb @@ -35,24 +35,8 @@ RSpec.describe Queries::WorkPackages::Selects::PropertySelect do it_behaves_like "query column" describe "instances" do - context "when done_ratio disabled" do - it "the done ratio column does not exist" do - allow(WorkPackage) - .to receive(:done_ratio_disabled?) - .and_return(true) - - expect(described_class.instances.map(&:name)).not_to include :done_ratio - end - end - - context "when done_ratio enabled" do - it "the done ratio column exists" do - allow(WorkPackage) - .to receive(:done_ratio_disabled?) - .and_return(false) - - expect(described_class.instances.map(&:name)).to include :done_ratio - end + it "the done_ratio column exists" do + expect(described_class.instances.map(&:name)).to include :done_ratio end context "when duration feature flag enabled" do diff --git a/spec/models/query_spec.rb b/spec/models/query_spec.rb index f1e7855090a..3dae47bb06f 100644 --- a/spec/models/query_spec.rb +++ b/spec/models/query_spec.rb @@ -319,20 +319,8 @@ RSpec.describe Query, end describe "#available_columns" do - context "with work_package_done_ratio NOT disabled" do - it "includes the done_ratio column" do - expect(query.displayable_columns.map(&:name)).to include :done_ratio - end - end - - context "with work_package_done_ratio disabled" do - before do - allow(WorkPackage).to receive(:done_ratio_disabled?).and_return(true) - end - - it "does not include the done_ratio column" do - expect(query.displayable_columns.map(&:name)).not_to include :done_ratio - end + it "includes the done_ratio column" do + expect(query.displayable_columns.map(&:name)).to include :done_ratio end context "results caching" do diff --git a/spec/requests/api/v3/render_resource_spec.rb b/spec/requests/api/v3/render_resource_spec.rb index 25eba4518ae..9de125e4384 100644 --- a/spec/requests/api/v3/render_resource_spec.rb +++ b/spec/requests/api/v3/render_resource_spec.rb @@ -73,6 +73,7 @@ RSpec.describe "API v3 Render resource" doHello World! This is markdown with a link and ümläutß.
diff --git a/spec/services/work_packages/update_ancestors_service_spec.rb b/spec/services/work_packages/update_ancestors_service_spec.rb index 05c495d178f..ce44ef03c83 100644 --- a/spec/services/work_packages/update_ancestors_service_spec.rb +++ b/spec/services/work_packages/update_ancestors_service_spec.rb @@ -32,8 +32,8 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do shared_association_default(:author, factory_name: :user) { create(:user) } shared_association_default(:project_with_types) { create(:project_with_types) } shared_association_default(:priority) { create(:priority) } - shared_association_default(:open_status, factory_name: :status) { create(:status) } - shared_let(:closed_status) { create(:closed_status) } + shared_association_default(:open_status, factory_name: :status) { create(:status, name: "Open", default_done_ratio: 0) } + shared_let(:closed_status) { create(:closed_status, name: "Closed", default_done_ratio: 100) } shared_let(:user) { create(:user) } let(:estimated_hours) { [nil, nil, nil] } @@ -56,11 +56,14 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do 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, 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 + def call_update_ancestors_service(work_package) + changed_attributes = work_package.changes.keys.map(&:to_sym) + described_class.new(user:, work_package:) + .call(changed_attributes) + end + context "with both parent and children having estimated hours set" do shared_let(:parent) do create(:work_package, @@ -80,19 +83,14 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do status: open_status) end - def call_update_ancestors_service(work_package) - changed_attributes = work_package.changes.keys.map(&:to_sym) - described_class.new(user:, work_package:) - .call(changed_attributes) - end context "when changing child status to a status with a default done ratio" do %i[status status_id].each do |field| context "with the #{field} field" do it "recomputes child remaining work and update ancestors total % complete accordingly" do value = case field - when :status then complete_status_with_100p_done_ratio - when :status_id then complete_status_with_100p_done_ratio.id + when :status then closed_status + when :status_id then closed_status.id end set_attributes_on(child, field => value) call_update_ancestors_service(child) @@ -100,13 +98,40 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do 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 | 0h | | 100% | | + | child | 5h | 5h | 0h | 0h | 100% | 100% | TABLE end end end end end + + context "with parent having nothing set, and 2 children having values set (bug #54179)" do + let_work_packages(<<~TABLE) + hierarchy | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | Open | | 15h | | 10h | 0% | 33% + child1 | Open | 10h | | 10h | | 0% | + child2 | Closed | 5h | | 0h | | 100% | + TABLE + + context "when changing children to all have 100% complete" do + before do + set_attributes_on(child1, status: closed_status) + call_update_ancestors_service(child1) + end + + it "sets parent total % complete to 100% and its total remaining work to 0h, " \ + "and computes totals for the updated children too" do + table_work_packages.map(&:reload) + expect_work_packages(table_work_packages, <<~TABLE) + hierarchy | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | Open | | 15h | | 0h | 0% | 100% + child1 | Closed | 10h | 10h | 0h | 0h | 100% | 100% + child2 | Closed | 5h | | 0h | | 100% | + TABLE + end + end + end end end @@ -224,7 +249,7 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do end end - context "with all tasks having estimated hours and no tasks having any progress done yet" do + context "with all tasks having estimated hours and no tasks having any remaining hours" do it_behaves_like "attributes of parent having children" do let(:estimated_hours) do [10.0, 2.0, 3.0] @@ -237,10 +262,10 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do 15.0 end let(:aggregate_remaining_hours) do - nil # zero-values aren't accounted for + 0.0 end let(:aggregate_done_ratio) do - nil + 100 end end end @@ -431,7 +456,7 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do end end - context "with all tasks having estimated hours and no tasks having any progress done yet" do + context "with all tasks having estimated hours and no tasks having remaining hours" do it_behaves_like "attributes of parent having children" do let(:estimated_hours) do [10.0, 2.0, 3.0] @@ -444,10 +469,10 @@ RSpec.describe WorkPackages::UpdateAncestorsService, type: :model do 15.0 end let(:aggregate_remaining_hours) do - nil # zero-values aren't accounted for + 0.0 end let(:aggregate_done_ratio) do - nil + 100 end end end