From ba5b978145403fb5a274da361a568ff352c2731c Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Fri, 8 Mar 2024 17:24:29 +0100 Subject: [PATCH 1/3] Make done_ratio nil by default --- .rubocop.yml | 2 + app/models/work_package/validations.rb | 2 +- .../new_framework_defaults_7_0.rb | 2 +- ...change_done_ratio_default_value_to_null.rb | 8 ++ spec/models/mail_handler_spec.rb | 2 +- spec/models/work_package_spec.rb | 78 ++++------- .../set_attributes_service_spec.rb | 128 +++++++++++++----- .../update_service_integration_spec.rb | 2 +- 8 files changed, 135 insertions(+), 89 deletions(-) create mode 100644 db/migrate/20240307094432_change_done_ratio_default_value_to_null.rb diff --git a/.rubocop.yml b/.rubocop.yml index f646e9c75e1f..ba1a5d9139bb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -257,8 +257,10 @@ RSpec/ContextWording: - as - even - for + - given - having - if + - in - 'on' - to - unless diff --git a/app/models/work_package/validations.rb b/app/models/work_package/validations.rb index 7c9fce745f0a..94f1c37475ef 100644 --- a/app/models/work_package/validations.rb +++ b/app/models/work_package/validations.rb @@ -33,7 +33,7 @@ module WorkPackage::Validations validates :subject, :priority, :project, :type, :author, :status, presence: true validates :subject, length: { maximum: 255 } - validates :done_ratio, inclusion: { in: 0..100 } + validates :done_ratio, inclusion: { in: 0..100 }, allow_nil: true validates :estimated_hours, numericality: { allow_nil: true } validates :remaining_hours, numericality: { allow_nil: true, greater_than_or_equal_to: 0 } validates :derived_remaining_hours, numericality: { allow_nil: true, greater_than_or_equal_to: 0 } diff --git a/config/initializers/new_framework_defaults_7_0.rb b/config/initializers/new_framework_defaults_7_0.rb index 863a9f1dad5b..8a10e9e8eab1 100644 --- a/config/initializers/new_framework_defaults_7_0.rb +++ b/config/initializers/new_framework_defaults_7_0.rb @@ -90,7 +90,7 @@ # This default means that all columns will be referenced in INSERT queries # regardless of whether they have a default or not. # Previous versions had true. Rails 7.0+ default is false. -# Rails.application.config.active_record.partial_inserts = false +Rails.application.config.active_record.partial_inserts = false # https://guides.rubyonrails.org/configuring.html#config-action-controller-raise-on-open-redirects # Protect from open redirect attacks in `redirect_back_or_to` and `redirect_to`. diff --git a/db/migrate/20240307094432_change_done_ratio_default_value_to_null.rb b/db/migrate/20240307094432_change_done_ratio_default_value_to_null.rb new file mode 100644 index 000000000000..a8373662d445 --- /dev/null +++ b/db/migrate/20240307094432_change_done_ratio_default_value_to_null.rb @@ -0,0 +1,8 @@ +class ChangeDoneRatioDefaultValueToNull < ActiveRecord::Migration[7.1] + def change + change_column_null :work_packages, :done_ratio, true + change_column_default :work_packages, :done_ratio, from: 0, to: nil + change_column_null :work_package_journals, :done_ratio, true + change_column_default :work_package_journals, :done_ratio, from: 0, to: nil + end +end diff --git a/spec/models/mail_handler_spec.rb b/spec/models/mail_handler_spec.rb index fd32c8f14b23..34934b1446bb 100644 --- a/spec/models/mail_handler_spec.rb +++ b/spec/models/mail_handler_spec.rb @@ -1088,7 +1088,7 @@ .to be_nil expect(subject.done_ratio) - .to be 0 + .to be_nil expect(subject.priority) .to eql priority_low diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index e356a9c6fec3..0af23f558c4e 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -333,77 +333,57 @@ def stub_shared_versions(v = nil) is_closed: false, default_done_ratio: 0) end - shared_let(:work_package1) do + shared_let(:work_package_new) do create(:work_package, status: status_new) end - shared_let(:work_package2) do + shared_let(:work_package_assigned) do create(:work_package, - project: work_package1.project, + project: work_package_new.project, status: status_assigned, done_ratio: 30) end describe '#value' do - context 'work package field' do - before { allow(Setting).to receive(:work_package_done_ratio).and_return 'field' } - - context 'work package 1' do - subject { work_package1.done_ratio } - - it { is_expected.to eq(0) } - end - - context 'work package 2' do - subject { work_package2.done_ratio } - - it { is_expected.to eq(30) } + context 'for work-based mode', + with_settings: { work_package_done_ratio: 'field' } do + it 'returns the value from work package field' do + expect(work_package_new.done_ratio).to be_nil + expect(work_package_assigned.done_ratio).to eq(30) end end - context 'work package status' do - before { allow(Setting).to receive(:work_package_done_ratio).and_return 'status' } - - context 'work package 1' do - subject { work_package1.done_ratio } - - it { is_expected.to eq(50) } - end - - context 'work package 2' do - subject { work_package2.done_ratio } - - it { is_expected.to eq(0) } + context 'for status-based mode', + with_settings: { work_package_done_ratio: 'status' } do + it 'uses the % Complete value from the work package status' do + expect(work_package_new.done_ratio).to eq(status_new.default_done_ratio) + expect(work_package_assigned.done_ratio).to eq(status_assigned.default_done_ratio) end end end describe '#update_done_ratio_from_status' do - context 'work package field' do - before do - allow(Setting).to receive(:work_package_done_ratio).and_return 'field' - - work_package1.update_done_ratio_from_status - work_package2.update_done_ratio_from_status - end - + context 'for work-based mode', + with_settings: { work_package_done_ratio: 'field' } do it 'does not update the done ratio' do - expect(work_package1.done_ratio).to eq(0) - expect(work_package2.done_ratio).to eq(30) + expect { work_package_new.update_done_ratio_from_status } + .not_to change { work_package_new[:done_ratio] } + expect { work_package_assigned.update_done_ratio_from_status } + .not_to change { work_package_assigned[:done_ratio] } end end - context 'work package status' do - before do - allow(Setting).to receive(:work_package_done_ratio).and_return 'status' - - work_package1.update_done_ratio_from_status - work_package2.update_done_ratio_from_status - end + context 'for status-based mode', + with_settings: { work_package_done_ratio: 'status' } do + it 'updates the done ratio without saving it' do + expect { work_package_new.update_done_ratio_from_status } + .to change { work_package_new[:done_ratio] } + .from(nil).to(50) + expect { work_package_assigned.update_done_ratio_from_status } + .to change { work_package_assigned[:done_ratio] } + .from(30).to(0) - it 'updates the done ratio' do - expect(work_package1.done_ratio).to eq(50) - expect(work_package2.done_ratio).to eq(0) + expect(work_package_new).to have_changes_to_save end end end diff --git a/spec/services/work_packages/set_attributes_service_spec.rb b/spec/services/work_packages/set_attributes_service_spec.rb index 65e48f508d29..8baee0caf3cd 100644 --- a/spec/services/work_packages/set_attributes_service_spec.rb +++ b/spec/services/work_packages/set_attributes_service_spec.rb @@ -30,6 +30,10 @@ 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_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') } + let(:today) { Time.zone.today } let(:user) { build_stubbed(:user) } let(:project) do @@ -39,7 +43,7 @@ p end let(:work_package) do - wp = build_stubbed(:work_package, project:) + wp = build_stubbed(:work_package, project:, status: status_no_pct_complete) wp.type = initial_type wp.send(:clear_changes_information) @@ -72,7 +76,11 @@ contract_class: mock_contract) end +<<<<<<< HEAD shared_examples_for "service call" do +======= + shared_examples_for 'service call' do |description: nil| +>>>>>>> f594acb61a (Make done_ratio nil by default) subject do allow(work_package) .to receive(:save) @@ -84,7 +92,11 @@ expect(subject).to be_success end +<<<<<<< HEAD it "sets the value" do +======= + it description || 'sets the value' do +>>>>>>> f594acb61a (Make done_ratio nil by default) next if !defined?(expected_attributes) || expected_attributes.blank? subject @@ -145,68 +157,112 @@ it_behaves_like "service call" end - context "for estimated_hours" do - context "with the status being used for done_ratio computations", - with_settings: { work_package_done_ratio: "status" } do - context "when the work package has estimated hours and remaining hours set along with a done ratio" do - let!(:status) { create(:status, default_done_ratio: 50) } - + describe 'deriving remaining work attribute (remaining_hours)' do + context 'in status-based mode', + with_settings: { work_package_done_ratio: 'status' } do + context 'given a work package with work, remaining work, and status with % complete being set' do before do - work_package.status = status + work_package.status = status_50_pct_complete + work_package.done_ratio = work_package.status.default_done_ratio work_package.estimated_hours = 10.0 work_package.remaining_hours = 5.0 work_package.send(:clear_changes_information) end - context "and estimated_hours are subsequently unset" do + context 'when work is unset' do let(:call_attributes) { { estimated_hours: nil } } - let(:expected_attributes) { { estimated_hours: nil, remaining_hours: nil, done_ratio: 50 } } + let(:expected_attributes) { { remaining_hours: nil } } - it_behaves_like "service call" + it_behaves_like 'service call', description: 'unsets remaining work' end - context "and estimated_hours are subsequently modified" do + context 'when work is modified' do let(:call_attributes) { { estimated_hours: 5.0 } } - let(:expected_attributes) { { estimated_hours: 5.0, remaining_hours: 2.5, done_ratio: 50 } } + let(:expected_attributes) { { remaining_hours: 2.5 } } - it_behaves_like "service call" + it_behaves_like 'service call', description: 'recomputes remaining work accordingly' end - context "and the status is subsequently changed" do - let!(:other_status) { create(:status, default_done_ratio: 70) } - let(:call_attributes) { {} } - let(:expected_attributes) { { estimated_hours: 10.0, remaining_hours: 3.0 } } + context 'when another status with a default % complete value is set' do + let(:call_attributes) { { status: status_70_pct_complete } } + let(:expected_attributes) { { remaining_hours: 3.0 } } - before do - work_package.status = other_status - end + 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', skip: 'TODO: not implemented yet' do + let(:call_attributes) { { status: status_no_pct_complete } } + let(:expected_attributes) { { remaining_hours: nil } } - it_behaves_like "service call" + 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 + before do + work_package.status = status_no_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 + let(:call_attributes) { { status: status_70_pct_complete } } + let(:expected_attributes) { { remaining_hours: nil } } + + it_behaves_like 'service call', + description: 'remaining work remains unset' + end + + context 'when work is set' do + let(:call_attributes) { { estimated_hours: 10.0 } } + let(:expected_attributes) { { remaining_hours: nil } } + + it_behaves_like 'service call', + description: 'remaining work remains unset' end end end end - context "for done_ratio" do - context "with the status being used for done_ratio computations", - with_settings: { work_package_done_ratio: "status" } do - let!(:status) { create(:status, default_done_ratio: 50) } - let(:call_attributes) { { estimated_hours: 10.0 } } - # remaining hours computed from estimated hours and the status's default done ratio - let(:expected_attributes) { { estimated_hours: 10.0, remaining_hours: 5.0, done_ratio: 50 } } + describe 'deriving % complete attribute (done_ratio)' do + context 'in status-based mode', + with_settings: { work_package_done_ratio: 'status' } do + context 'given a work package with a status with 50% complete' do + before do + work_package.status = status_50_pct_complete + work_package.done_ratio = work_package.status.default_done_ratio + work_package.send(:clear_changes_information) + end - before do - work_package.status = status - end + context 'when another status with another % complete value is set' do + let(:call_attributes) { { status: status_70_pct_complete } } + let(:expected_attributes) { { done_ratio: 70 } } - it_behaves_like "service call" + 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', skip: 'TODO: not implemented yet' 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 + ### + ### Below this line, it has not be rewritten / revised yet ### + ### + describe 'is "unset"' do context "when estimated_hours is unset" do let(:call_attributes) { { estimated_hours: nil } } let(:expected_attributes) do - { estimated_hours: nil, done_ratio: 0 } + { estimated_hours: nil, done_ratio: nil } end it_behaves_like "service call" @@ -215,7 +271,7 @@ context "when remaining_hours is unset" do let(:call_attributes) { { remaining_hours: nil } } let(:expected_attributes) do - { remaining_hours: nil, done_ratio: 0 } + { remaining_hours: nil, done_ratio: nil } end it_behaves_like "service call" @@ -224,7 +280,7 @@ context "when both estimated_hours and remaining_hours are unset" do let(:call_attributes) { { estimated_hours: nil, remaining_hours: nil } } let(:expected_attributes) do - { estimated_hours: nil, remaining_hours: nil, done_ratio: 0 } + { estimated_hours: nil, remaining_hours: nil, done_ratio: nil } end it_behaves_like "service call" diff --git a/spec/services/work_packages/update_service_integration_spec.rb b/spec/services/work_packages/update_service_integration_spec.rb index 9dd3dd36bfba..d59b9abb3400 100644 --- a/spec/services/work_packages/update_service_integration_spec.rb +++ b/spec/services/work_packages/update_service_integration_spec.rb @@ -495,7 +495,7 @@ # unchanged sibling1_work_package.reload expect(sibling1_work_package.done_ratio) - .to eq(0) + .to be_nil sibling2_work_package.reload expect(sibling2_work_package.done_ratio) From 08b0955f1b6f70ed5eccf7258f82ee219ec47329 Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Thu, 21 Mar 2024 11:54:36 +0100 Subject: [PATCH 2/3] Add test scenarios from wp #40749 --- .../work_packages/set_attributes_service.rb | 8 +- .../set_attributes_service_spec.rb | 215 ++++++++++++------ 2 files changed, 152 insertions(+), 71 deletions(-) diff --git a/app/services/work_packages/set_attributes_service.rb b/app/services/work_packages/set_attributes_service.rb index 874a98792e5f..bec4cf4e644a 100644 --- a/app/services/work_packages/set_attributes_service.rb +++ b/app/services/work_packages/set_attributes_service.rb @@ -290,7 +290,7 @@ def update_duration # - +estimated_hours+ # # Unless both +remaining_hours+ and +estimated_hours+ are set, +done_ratio+ will be - # considered 0. + # considered nil. def update_done_ratio if WorkPackage.use_status_for_done_ratio? return unless model.status_id_changed? @@ -302,7 +302,7 @@ def update_done_ratio return unless work_package.remaining_hours_changed? || work_package.estimated_hours_changed? work_package.done_ratio = if done_ratio_dependent_attribute_unset? - 0 + nil else compute_done_ratio end @@ -331,6 +331,10 @@ def update_remaining_hours model.remaining_hours = if model.estimated_hours remaining_hours_from_done_ratio_and_estimated_hours end + elsif WorkPackage.use_field_for_done_ratio? && + model.estimated_hours_changed? && + model.estimated_hours.nil? + model.remaining_hours = nil end end diff --git a/spec/services/work_packages/set_attributes_service_spec.rb b/spec/services/work_packages/set_attributes_service_spec.rb index 8baee0caf3cd..8b572f614af0 100644 --- a/spec/services/work_packages/set_attributes_service_spec.rb +++ b/spec/services/work_packages/set_attributes_service_spec.rb @@ -30,9 +30,9 @@ 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_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') } + shared_let(:status_no_pct_complete) { create(:status, default_done_ratio: nil, name: "no % 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") } let(:today) { Time.zone.today } let(:user) { build_stubbed(:user) } @@ -76,11 +76,7 @@ contract_class: mock_contract) end -<<<<<<< HEAD - shared_examples_for "service call" do -======= - shared_examples_for 'service call' do |description: nil| ->>>>>>> f594acb61a (Make done_ratio nil by default) + shared_examples_for "service call" do |description: nil| subject do allow(work_package) .to receive(:save) @@ -92,11 +88,7 @@ expect(subject).to be_success end -<<<<<<< HEAD - it "sets the value" do -======= - it description || 'sets the value' do ->>>>>>> f594acb61a (Make done_ratio nil by default) + it description || "sets the value" do next if !defined?(expected_attributes) || expected_attributes.blank? subject @@ -157,10 +149,11 @@ it_behaves_like "service call" end - describe 'deriving remaining work attribute (remaining_hours)' do - context 'in status-based mode', - with_settings: { work_package_done_ratio: 'status' } do - context 'given a work package with work, remaining work, and status with % complete being set' do + # Scenarios specified in https://community.openproject.org/wp/40749 + describe "deriving remaining work attribute (remaining_hours)" do + context "in status-based mode", + with_settings: { work_package_done_ratio: "status" } do + context "given a work package with work, remaining work, and status with % complete being set" do before do work_package.status = status_50_pct_complete work_package.done_ratio = work_package.status.default_done_ratio @@ -169,38 +162,38 @@ work_package.send(:clear_changes_information) end - context 'when work is unset' do + context "when work is unset" do let(:call_attributes) { { estimated_hours: nil } } let(:expected_attributes) { { remaining_hours: nil } } - it_behaves_like 'service call', description: 'unsets remaining work' + it_behaves_like "service call", description: "unsets remaining work" end - context 'when work is modified' do + context "when work is modified" do let(:call_attributes) { { estimated_hours: 5.0 } } let(:expected_attributes) { { remaining_hours: 2.5 } } - it_behaves_like 'service call', description: 'recomputes remaining work accordingly' + 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 with a default % complete value 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' + 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', skip: 'TODO: not implemented yet' do + context "when another status without any default % complete value is set", skip: "TODO: not implemented yet" do let(:call_attributes) { { status: status_no_pct_complete } } let(:expected_attributes) { { remaining_hours: nil } } - it_behaves_like 'service call', - description: 'unsets remaining work' + 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 no % complete" do before do work_package.status = status_no_pct_complete work_package.done_ratio = work_package.status.default_done_ratio @@ -209,91 +202,175 @@ work_package.send(:clear_changes_information) end - context 'when another status with a default % complete value is set' do + context "when another status with a default % complete value is set" do let(:call_attributes) { { status: status_70_pct_complete } } let(:expected_attributes) { { remaining_hours: nil } } - it_behaves_like 'service call', - description: 'remaining work remains unset' + it_behaves_like "service call", + description: "remaining work remains unset" end - context 'when work is set' do + context "when work is set" do let(:call_attributes) { { estimated_hours: 10.0 } } let(:expected_attributes) { { remaining_hours: nil } } - it_behaves_like 'service call', - description: 'remaining work remains unset' + it_behaves_like "service call", + description: "remaining work remains unset" end end end end - describe 'deriving % complete attribute (done_ratio)' do - context 'in status-based mode', - with_settings: { work_package_done_ratio: 'status' } do - context 'given a work package with a status with 50% complete' do + # Scenarios specified in https://community.openproject.org/wp/40749 + describe "deriving % complete attribute (done_ratio)" do + context "in status-based mode", + with_settings: { work_package_done_ratio: "status" } do + context "given a work package with a status with 50% complete" do before do work_package.status = status_50_pct_complete work_package.done_ratio = work_package.status.default_done_ratio work_package.send(:clear_changes_information) end - context 'when another status with another % 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) { { done_ratio: 70 } } - it_behaves_like 'service call', description: 'sets the % complete value to the status default % complete value' + 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', skip: 'TODO: not implemented yet' do + context "when another status with no % complete value is set", skip: "TODO: not implemented yet" 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' + it_behaves_like "service call", description: "unsets the % complete value" end end end - ### - ### Below this line, it has not be rewritten / revised yet ### - ### + context "in work-based mode", + with_settings: { work_package_done_ratio: "field" } do + context "given a work package with work, remaining work, and % complete being set" do + before do + work_package.estimated_hours = 10.0 + work_package.remaining_hours = 6.0 + work_package.done_ratio = 40 + work_package.send(:clear_changes_information) + end - describe 'is "unset"' do - context "when estimated_hours is unset" do - let(:call_attributes) { { estimated_hours: nil } } - let(:expected_attributes) do - { estimated_hours: nil, done_ratio: nil } + context "when work is unset" do + let(:call_attributes) { { estimated_hours: nil } } + let(:expected_attributes) { { remaining_hours: nil, done_ratio: nil } } + + it_behaves_like "service call", description: "unsets remaining work and % complete" end - it_behaves_like "service call" - end + context "when remaining work is unset" do + let(:call_attributes) { { remaining_hours: nil } } + let(:expected_attributes) { { estimated_hours: 10.0, done_ratio: nil } } - context "when remaining_hours is unset" do - let(:call_attributes) { { remaining_hours: nil } } - let(:expected_attributes) do - { remaining_hours: nil, done_ratio: nil } + it_behaves_like "service call", description: "keeps work, and unsets % complete" end - it_behaves_like "service call" - end + context "when both work and remaining work are unset" do + let(:call_attributes) { { estimated_hours: nil, remaining_hours: nil } } + let(:expected_attributes) { { done_ratio: nil } } + + it_behaves_like "service call", description: "unsets % complete" + end + + context "when work is increased", skip: "TODO: not implemented yet" do + # work changed by +10h + let(:call_attributes) { { estimated_hours: 10.0 + 10.0 } } + let(:expected_attributes) do + { remaining_hours: 6.0 + 10.0, done_ratio: 20 } + end + + it_behaves_like "service call", + description: "remaining work is increased by the same amount, and % complete is updated accordingly" + end + + context "when work is decreased", skip: "TODO: not implemented yet" do + # work changed by -2h + let(:call_attributes) { { estimated_hours: 10.0 - 2.0 } } + let(:expected_attributes) do + { remaining_hours: 6.0 - 2.0, done_ratio: 50 } + end + + it_behaves_like "service call", + description: "remaining work is decreased by the same amount, and % complete is updated accordingly" + end + + context "when work is decreased below remaining work value", skip: "TODO: not implemented yet" do + # work changed by -8h + let(:call_attributes) { { estimated_hours: 10.0 - 8.0 } } + let(:expected_attributes) do + { remaining_hours: 0, done_ratio: 100 } + end + + it_behaves_like "service call", + description: "remaining work becomes 0h, and % complete becomes 100%" + end + + context "when remaining work is changed" do + let(:call_attributes) { { remaining_hours: 2 } } + let(:expected_attributes) { { done_ratio: 80 } } + + it_behaves_like "service call", description: "updates % complete accordingly" + end - context "when both estimated_hours and remaining_hours are unset" do - let(:call_attributes) { { estimated_hours: nil, remaining_hours: nil } } - let(:expected_attributes) do - { estimated_hours: nil, remaining_hours: nil, done_ratio: nil } + context "when remaining work is changed to a value greater than work", skip: "TODO: not implemented yet" do + let(:call_attributes) { { remaining_hours: 200.0 } } + let(:expected_attributes) { "error" } + + # open question: should it be capped or produce an error? + # I would opt for the error which would then be displayed in the popover + it_behaves_like "service call", description: "produces an error" end - it_behaves_like "service call" + context "when both work and remaining work are changed" do + let(:call_attributes) { { estimated_hours: 20, remaining_hours: 2 } } + let(:expected_attributes) { { done_ratio: 90 } } + + it_behaves_like "service call", description: "updates % complete accordingly" + end end - end - context "when both estimated_hours and remaining_hours are set" do - let(:call_attributes) { { estimated_hours: 10.0, remaining_hours: 5.0 } } - let(:expected_attributes) do - { estimated_hours: 10.0, remaining_hours: 5.0, done_ratio: 50 } + context "given a work package with work and remaining work unset, and % complete being set" do + before do + work_package.estimated_hours = nil + work_package.remaining_hours = nil + work_package.done_ratio = 60 + work_package.send(:clear_changes_information) + end + + context "when work is set", skip: "TODO: not implemented yet" do + let(:call_attributes) { { estimated_hours: 10.0 } } + let(:expected_attributes) do + { remaining_hours: 4.0, done_ratio: 60 } + end + + it_behaves_like "service call", description: "% complete is kept and remaining work is updated accordingly" + end end - it_behaves_like "service call" + context "given a work package with work, remaining work, and % complete being unset" do + before do + work_package.estimated_hours = nil + work_package.remaining_hours = nil + work_package.done_ratio = nil + work_package.send(:clear_changes_information) + end + + context "when work is set", skip: "TODO: not implemented yet" do + let(:call_attributes) { { estimated_hours: 10.0 } } + let(:expected_attributes) do + { remaining_hours: 10.0, done_ratio: 0 } + end + + it_behaves_like "service call", description: "remaining work is set to the same value and % complete is set to 0%" + end + end end end From 188bae702d021aaff234804ff609bc530d21ffea Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Thu, 21 Mar 2024 14:19:58 +0100 Subject: [PATCH 3/3] Switch to double quotes for files modified on this branch --- spec/models/work_package_spec.rb | 264 +++++++++++++++---------------- 1 file changed, 132 insertions(+), 132 deletions(-) diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index 0af23f558c4e..fd27330defb1 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -require 'spec_helper' +require "spec_helper" RSpec.describe WorkPackage do shared_let(:type) { create(:type_standard) } @@ -54,23 +54,23 @@ author_id: user.id, status_id: status.id, priority:, - subject: 'test_create', - description: 'WorkPackage#create', - estimated_hours: '1:30' } + subject: "test_create", + description: "WorkPackage#create", + estimated_hours: "1:30" } end end - describe 'associations' do + describe "associations" do subject { work_package } it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:type) } it { is_expected.to belong_to(:status) } it { is_expected.to belong_to(:author) } - it { is_expected.to belong_to(:assigned_to).class_name('Principal').optional } - it { is_expected.to belong_to(:responsible).class_name('Principal').optional } + it { is_expected.to belong_to(:assigned_to).class_name("Principal").optional } + it { is_expected.to belong_to(:responsible).class_name("Principal").optional } it { is_expected.to belong_to(:version).optional } - it { is_expected.to belong_to(:priority).class_name('IssuePriority') } + it { is_expected.to belong_to(:priority).class_name("IssuePriority") } it { is_expected.to belong_to(:category).optional } it { is_expected.to have_many(:time_entries).dependent(:delete_all) } it { is_expected.to have_many(:file_links).dependent(:delete_all).class_name("Storages::FileLink") } @@ -78,28 +78,28 @@ it { is_expected.to have_and_belong_to_many(:changesets) } it { is_expected.to have_and_belong_to_many(:github_pull_requests) } it { is_expected.to have_many(:members).dependent(:destroy) } - it { is_expected.to have_many(:member_principals).through(:members).class_name('Principal').source(:principal) } + it { is_expected.to have_many(:member_principals).through(:members).class_name("Principal").source(:principal) } it { is_expected.to have_many(:meeting_agenda_items) } it { is_expected.to have_many(:meetings).through(:meeting_agenda_items).source(:meeting) } end - describe '.new' do - context 'type' do + describe ".new" do + describe "type" do let(:type2) { create(:type) } before do project.types << type2 end - context 'no project chosen' do - it 'has no type set if no project was chosen' do + context "when no project chosen" do + it "has no type set if no project was chosen" do expect(described_class.new.type) .to be_nil end end - context 'project chosen' do - it 'has the provided type if one is provided' do + context "when project chosen" do + it "has the provided type if one is provided" do expect(described_class.new(project:, type: type2).type) .to eql type2 end @@ -107,14 +107,14 @@ end end - describe 'create' do - describe '#save' do + describe "create" do + describe "#save" do subject { work_package.save } it { is_expected.to be_truthy } end - describe '#estimated_hours' do + describe "#estimated_hours" do before do work_package.save! work_package.reload @@ -125,7 +125,7 @@ it { is_expected.to eq(1.5) } end - describe 'minimal' do + describe "minimal" do let(:work_package_minimal) do described_class.new.tap do |w| w.attributes = { project_id: project.id, @@ -133,17 +133,17 @@ author_id: user.id, status_id: status.id, priority:, - subject: 'test_create' } + subject: "test_create" } end end - context 'save' do + describe "save" do subject { work_package_minimal.save } it { is_expected.to be_truthy } end - context 'description' do + describe "description" do before do work_package_minimal.save! work_package_minimal.reload @@ -155,8 +155,8 @@ end end - describe '#assigned_to' do - context 'group_assignment' do + describe "#assigned_to" do + describe "group_assignment" do let(:group) { create(:group) } subject do @@ -169,7 +169,7 @@ end end - describe '#category' do + describe "#category" do let(:user2) { create(:user, member_with_permissions: { project => %i[view_work_packages edit_work_packages] }) } let(:category) do create(:category, @@ -187,7 +187,7 @@ it { is_expected.to eq(category.assigned_to) } end - describe 'responsible' do + describe "responsible" do let(:group) { create(:group) } let!(:member) do create(:member, @@ -196,16 +196,16 @@ roles: [create(:project_role)]) end - context 'with group assigned' do + context "with group assigned" do before { work_package.responsible = group } - it 'is valid' do + it "is valid" do expect(work_package).to be_valid end end end - describe '#assignable_versions' do + describe "#assignable_versions" do let(:stub_version2) { build_stubbed(:version) } def stub_shared_versions(v = nil) @@ -220,7 +220,7 @@ def stub_shared_versions(v = nil) expect(stub_work_package.assignable_versions).to eq([stub_version]) end - it 'returns the former version if the version changed' do + it "returns the former version if the version changed" do stub_shared_versions stub_work_package.version = stub_version2 @@ -232,7 +232,7 @@ def stub_shared_versions(v = nil) expect(stub_work_package.assignable_versions).to eq([stub_version]) end - it 'returns the current version if the version did not change' do + it "returns the current version if the version did not change" do stub_shared_versions stub_work_package.version = stub_version @@ -242,7 +242,7 @@ def stub_shared_versions(v = nil) expect(stub_work_package.assignable_versions).to eq([stub_version]) end - context 'with many versions' do + context "with many versions" do let!(:work_package) do wp = create(:work_package, project:, @@ -254,38 +254,38 @@ def stub_shared_versions(v = nil) end let!(:version_current) do create(:version, - status: 'closed', + status: "closed", project:) end let!(:version_open) do create(:version, - status: 'open', + status: "open", project:) end let!(:version_locked) do create(:version, - status: 'locked', + status: "locked", project:) end let!(:version_closed) do create(:version, - status: 'closed', + status: "closed", project:) end let!(:version_other_project) do create(:version, - status: 'open', + status: "open", project: create(:project)) end - it 'returns all open versions of the project' do + it "returns all open versions of the project" do expect(work_package.assignable_versions) .to contain_exactly(version_current, version_open) end end end - describe '#destroy' do + describe "#destroy" do let!(:time_entry1) do create(:time_entry, project:, @@ -301,34 +301,34 @@ def stub_shared_versions(v = nil) work_package.destroy end - describe 'work package' do + describe "work package" do subject { described_class.find_by(id: work_package.id) } it { is_expected.to be_nil } end - describe 'time entries' do + describe "time entries" do subject { TimeEntry.find_by(work_package_id: work_package.id) } it { is_expected.to be_nil } end end - include_examples 'creates an audit trail on destroy' do + include_examples "creates an audit trail on destroy" do subject { create(:work_package) } end - describe '#done_ratio' do + describe "#done_ratio" do shared_let(:status_new) do create(:status, - name: 'New', + name: "New", is_default: true, is_closed: false, default_done_ratio: 50) end shared_let(:status_assigned) do create(:status, - name: 'Assigned', + name: "Assigned", is_default: true, is_closed: false, default_done_ratio: 0) @@ -344,28 +344,28 @@ def stub_shared_versions(v = nil) done_ratio: 30) end - describe '#value' do - context 'for work-based mode', - with_settings: { work_package_done_ratio: 'field' } do - it 'returns the value from work package field' do + describe "#value" do + context "for work-based mode", + with_settings: { work_package_done_ratio: "field" } do + it "returns the value from work package field" do expect(work_package_new.done_ratio).to be_nil expect(work_package_assigned.done_ratio).to eq(30) end end - context 'for status-based mode', - with_settings: { work_package_done_ratio: 'status' } do - it 'uses the % Complete value from the work package status' do + context "for status-based mode", + with_settings: { work_package_done_ratio: "status" } do + it "uses the % Complete value from the work package status" do expect(work_package_new.done_ratio).to eq(status_new.default_done_ratio) expect(work_package_assigned.done_ratio).to eq(status_assigned.default_done_ratio) end end end - describe '#update_done_ratio_from_status' do - context 'for work-based mode', - with_settings: { work_package_done_ratio: 'field' } do - it 'does not update the done ratio' do + describe "#update_done_ratio_from_status" do + context "for work-based mode", + with_settings: { work_package_done_ratio: "field" } do + it "does not update the done ratio" do expect { work_package_new.update_done_ratio_from_status } .not_to change { work_package_new[:done_ratio] } expect { work_package_assigned.update_done_ratio_from_status } @@ -373,9 +373,9 @@ def stub_shared_versions(v = nil) end end - context 'for status-based mode', - with_settings: { work_package_done_ratio: 'status' } do - it 'updates the done ratio without saving it' do + context "for status-based mode", + with_settings: { work_package_done_ratio: "status" } do + it "updates the done ratio without saving it" do expect { work_package_new.update_done_ratio_from_status } .to change { work_package_new[:done_ratio] } .from(nil).to(50) @@ -389,7 +389,7 @@ def stub_shared_versions(v = nil) end end - describe '#group_by' do + describe "#group_by" do shared_let(:type2) { create(:type) } shared_let(:priority2) { create(:priority) } shared_let(:project) { create(:project, types: [type, type2]) } @@ -422,72 +422,72 @@ def stub_shared_versions(v = nil) category: category2) end - shared_examples_for 'group by' do - context 'size' do + shared_examples_for "group by" do + describe "size" do subject { groups.size } it { is_expected.to eq(2) } end - context 'total' do - subject { groups.inject(0) { |sum, group| sum + group['total'].to_i } } + describe "total" do + subject { groups.inject(0) { |sum, group| sum + group["total"].to_i } } it { is_expected.to eq(2) } end end - context 'by type' do + describe "by type" do let(:groups) { described_class.by_type(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by version' do + describe "by version" do let(:groups) { described_class.by_version(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by priority' do + describe "by priority" do let(:groups) { described_class.by_priority(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by category' do + describe "by category" do let(:groups) { described_class.by_category(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by assigned to' do + describe "by assigned to" do let(:groups) { described_class.by_assigned_to(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by responsible' do + describe "by responsible" do let(:groups) { described_class.by_responsible(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by author' do + describe "by author" do let(:groups) { described_class.by_author(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end - context 'by project' do + describe "by project" do shared_let(:project2) { create(:project, parent: project) } shared_let(:work_package3) { create(:work_package, project: project2) } let(:groups) { described_class.by_author(project) } - it_behaves_like 'group by' + it_behaves_like "group by" end end - describe '#recently_updated' do + describe "#recently_updated" do let!(:work_package1) { create(:work_package) } let!(:work_package2) { create(:work_package) } @@ -498,22 +498,22 @@ def stub_shared_versions(v = nil) end end - context 'limit' do + describe "limit" do subject { described_class.recently_updated.limit(1).first } it { is_expected.to eq(work_package2) } end end - describe '#on_active_project' do + describe "#on_active_project" do shared_let(:work_package) { create(:work_package, project:) } subject { described_class.on_active_project.length } - context 'one work package in active projects' do + context "with one work package in active projects" do it { is_expected.to eq(1) } - context 'and one work package in archived projects' do + context "and one work package in archived projects" do shared_let(:work_package_in_archived_project) do create(:work_package, project: project_archived) end @@ -523,15 +523,15 @@ def stub_shared_versions(v = nil) end end - describe '#with_author' do + describe "#with_author" do shared_let(:work_package) { create(:work_package, project:, author: user1) } subject { described_class.with_author(user1).length } - context 'one work package in active projects' do + context "with one work package in active projects" do it { is_expected.to eq(1) } - context 'and one work package in archived projects' do + context "and one work package in archived projects" do shared_let(:work_package_in_archived_project) do create(:work_package, project: project_archived, author: user1) end @@ -541,75 +541,75 @@ def stub_shared_versions(v = nil) end end - describe '#add_time_entry' do - it 'returns a new time entry' do + describe "#add_time_entry" do + it "returns a new time entry" do expect(stub_work_package.add_time_entry).to be_a TimeEntry end - it 'alreadies have the project assigned' do + it "has already the project assigned" do stub_work_package.project = stub_project expect(stub_work_package.add_time_entry.project).to eq(stub_project) end - it 'alreadies have the work_package assigned' do + it "has already the work_package assigned" do expect(stub_work_package.add_time_entry.work_package).to eq(stub_work_package) end - it 'returns an usaved entry' do + it "returns an unsaved entry" do expect(stub_work_package.add_time_entry).to be_new_record end end - describe '.allowed_target_project_on_move' do + describe ".allowed_target_project_on_move" do let(:permissions) { [:move_work_packages] } let(:user) do create(:user, member_with_permissions: { project => permissions }) end - context 'when having the move_work_packages permission' do - it 'returns the project' do + context "when having the move_work_packages permission" do + it "returns the project" do expect(described_class.allowed_target_projects_on_move(user)) .to contain_exactly(project) end end - context 'when lacking the move_work_packages permission' do + context "when lacking the move_work_packages permission" do let(:permissions) { [] } - it 'does not return the project' do + it "does not return the project" do expect(described_class.allowed_target_projects_on_move(user)) .to be_empty end end end - describe '.allowed_target_project_on_create' do + describe ".allowed_target_project_on_create" do let(:permissions) { [:add_work_packages] } let(:user) do create(:user, member_with_permissions: { project => permissions }) end - context 'when having the add_work_packages permission' do - it 'returns the project' do + context "when having the add_work_packages permission" do + it "returns the project" do expect(described_class.allowed_target_projects_on_create(user)) .to contain_exactly(project) end end - context 'when lacking the add_work_packages permission' do + context "when lacking the add_work_packages permission" do let(:permissions) { [] } - it 'does not return the project' do + it "does not return the project" do expect(described_class.allowed_target_projects_on_create(user)) .to be_empty end end end - describe '#duration' do + describe "#duration" do context "when not setting a value" do - it 'is nil' do + it "is nil" do expect(work_package.duration).to be_nil end end @@ -619,13 +619,13 @@ def stub_shared_versions(v = nil) work_package.duration = 5 end - it 'is the value' do + it "is the value" do expect(work_package.duration).to eq(5) end end end - describe 'changed_since' do + describe "changed_since" do shared_let(:work_package) do Timecop.travel(5.hours.ago) do create(:work_package, project:) @@ -634,35 +634,35 @@ def stub_shared_versions(v = nil) subject { described_class.changed_since(since) } - describe 'null' do + describe "null" do let(:since) { nil } it { expect(subject).to contain_exactly(work_package) } end - describe 'now' do + describe "now" do let(:since) { DateTime.now } it { expect(subject).to be_empty } end - describe 'work package update' do + describe "work package update" do let(:since) { work_package.reload.updated_at } it { expect(subject).to contain_exactly(work_package) } end end - describe '#ignore_non_working_days' do - context 'for a new record' do - it 'is false' do + describe "#ignore_non_working_days" do + context "for a new record" do + it "is false" do expect(described_class.new.ignore_non_working_days) .to be false end end end - context 'when destroying with agenda items' do + context "when destroying with agenda items" do shared_let(:work_package) do create(:work_package, project:, type:, status:, priority:) end @@ -672,9 +672,9 @@ def stub_shared_versions(v = nil) shared_let(:other_meeting) { other_agenda_item.meeting } let(:latest_journals) do Journal - .select('DISTINCT ON (journable_id) *') - .where(journable_type: 'Meeting', journable_id: meeting_agenda_items.pluck(:meeting_id)) - .order('journable_id, updated_at DESC') + .select("DISTINCT ON (journable_id) *") + .where(journable_type: "Meeting", journable_id: meeting_agenda_items.pluck(:meeting_id)) + .order("journable_id, updated_at DESC") end subject { work_package.destroy } @@ -684,18 +684,18 @@ def stub_shared_versions(v = nil) Meeting.find_each(&:save_journals) end - it 'dissociates the agenda items' do + it "dissociates the agenda items" do expect { subject } .to change { MeetingAgendaItem.find(meeting_agenda_items).pluck(:work_package_id) } .from(Array.new(3, work_package.id)) .to(Array.new(3, nil)) end - it 'does not affect other agenda items' do + it "does not affect other agenda items" do expect { subject }.not_to change(other_agenda_item, :reload) end - it 'updates the agenda item journal' do + it "updates the agenda item journal" do expect { subject } .to change { Journal::MeetingAgendaItemJournal @@ -706,7 +706,7 @@ def stub_shared_versions(v = nil) .to(Array.new(3, nil)) end - it 'does not affect the agenda item journal' do + it "does not affect the agenda item journal" do expect { subject } .not_to change { Journal::MeetingAgendaItemJournal @@ -716,32 +716,32 @@ def stub_shared_versions(v = nil) end end - describe '#remaining_hours' do - it 'allows empty values' do + describe "#remaining_hours" do + it "allows empty values" do expect(work_package.remaining_hours).to be_nil expect(work_package).to be_valid end - it 'allows values greater than or equal to 0' do - work_package.remaining_hours = '0' + it "allows values greater than or equal to 0" do + work_package.remaining_hours = "0" expect(work_package).to be_valid - work_package.remaining_hours = '1' + work_package.remaining_hours = "1" expect(work_package).to be_valid end - it 'disallows negative values' do - work_package.remaining_hours = '-1' + it "disallows negative values" do + work_package.remaining_hours = "-1" expect(work_package).not_to be_valid end - it 'disallows string values, that are not numbers' do - work_package.remaining_hours = 'abc' + it "disallows string values, that are not numbers" do + work_package.remaining_hours = "abc" expect(work_package).not_to be_valid end - it 'allows non-integers' do - work_package.remaining_hours = '1.3' + it "allows non-integers" do + work_package.remaining_hours = "1.3" expect(work_package).to be_valid end end