diff --git a/app/workers/work_packages/apply_working_days_change_job.rb b/app/workers/work_packages/apply_working_days_change_job.rb index ddee93e1ca..97370b00b7 100644 --- a/app/workers/work_packages/apply_working_days_change_job.rb +++ b/app/workers/work_packages/apply_working_days_change_job.rb @@ -44,20 +44,32 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob def each_applicable_work_package WorkPackage .where(ignore_non_working_days: false) - .order(start_date: :asc) + .where.not(start_date: nil, due_date: nil) + .order(WorkPackage.arel_table[:start_date].asc.nulls_first, + WorkPackage.arel_table[:due_date].asc) .pluck(:id) .each do |id| work_package = WorkPackage.find(id) - next if dates_and_duration_match?(work_package) + next unless dates_and_duration_mismatch?(work_package) yield work_package end end - def dates_and_duration_match?(work_package) - days.working?(work_package.start_date) \ - && days.working?(work_package.due_date) \ - && days.duration(work_package.start_date, work_package.due_date) == work_package.duration + def dates_and_duration_mismatch?(work_package) + # precondition: ignore_non_working_days is false + non_working?(work_package.start_date) \ + || non_working?(work_package.due_date) \ + || wrong_duration?(work_package) + end + + def non_working?(date) + date && !days.working?(date) + end + + def wrong_duration?(work_package) + computed_duration = days.duration(work_package.start_date, work_package.due_date) + computed_duration && work_package.duration != computed_duration end def days diff --git a/spec/workers/work_packages/apply_working_days_change_job_spec.rb b/spec/workers/work_packages/apply_working_days_change_job_spec.rb index d19c19df4b..0c0a624cec 100644 --- a/spec/workers/work_packages/apply_working_days_change_job_spec.rb +++ b/spec/workers/work_packages/apply_working_days_change_job_spec.rb @@ -42,9 +42,13 @@ RSpec.describe WorkPackages::ApplyWorkingDaysChangeJob do end context 'when a work package includes a date that is now a non-working day' do - let_schedule(<<~CHART, ignore_non_working_days: false) - days | MTWTFSS | - work_package | XXXX | + let_schedule(<<~CHART) + days | MTWTFSS | + work_package | XXXX | + work_package_on_start | XX | + work_package_on_due | XXX | + wp_start_only | [ | + wp_due_only | ] | CHART before do @@ -53,14 +57,59 @@ RSpec.describe WorkPackages::ApplyWorkingDaysChangeJob do it 'moves the finish date to the corresponding number of now-excluded days to maintain duration [#31992]' do job.perform_now(user_id: user.id) + expect(WorkPackage.all).to match_schedule(<<~CHART) + days | MTWTFSS | + work_package | XX.XX | + work_package_on_start | XX | + work_package_on_due | XX.X | + wp_start_only | [ | + wp_due_only | ] | + CHART + end + end + + context 'when a work package was scheduled to start on a date that is now a non-working day' do + let_schedule(<<~CHART) + days | MTWTFSS | + work_package | XX | + CHART + + before do + set_non_working_week_days('wednesday') + end + + it 'moves the start date to the earliest working day in the future, ' \ + 'and the finish date changes by consequence [#31992]' do + job.perform_now(user_id: user.id) expect(WorkPackage.all).to match_schedule(<<~CHART) days | MTWTFSS | - work_package | XX.XX | + work_package | XX | CHART end end - context 'when a work package includes a date that is now a non-working day, but has working days include weekends' do + context 'when a follower has a predecessor with dates covering a day that is now a non-working day' do + let_schedule(<<~CHART) + days | MTWTFSS | + predecessor | XX | working days work week + follower | XXX | working days include weekends, follows predecessor + CHART + + before do + set_non_working_week_days('wednesday') + end + + it 'moves the follower start date by consequence of the predecessor dates shift [#31992]' do + job.perform_now(user_id: user.id) + expect(WorkPackage.all).to match_schedule(<<~CHART) + days | MTWTFSS | + predecessor | X.X | working days work week + follower | XXX | working days include weekends + CHART + end + end + + context 'when a work package has working days include weekends, and includes a date that is now a non-working day' do let_schedule(<<~CHART) days | MTWTFSS | work_package | XXXX | working days include weekends @@ -79,35 +128,24 @@ RSpec.describe WorkPackages::ApplyWorkingDaysChangeJob do end end - context 'when having multiple work packages' do - let_schedule(<<~CHART, ignore_non_working_days: false) - days | MTWTFSS | - wp1 | XX | - wp2 | XX | - wp3 | XX | - wp4 | XX | - wp5 | XXXXX | + context 'when a work package only has a duration' do + let_schedule(<<~CHART) + days | MTWTFSS | + work_package | | duration 3 days CHART before do set_non_working_week_days('wednesday') end - it 'updates all impacted work packages' do + it 'does not change anything' do job.perform_now(user_id: user.id) - expect(WorkPackage.all).to match_schedule(<<~CHART) - days | MTWTFSS | - wp1 | XX | - wp2 | X.X | - wp3 | XX | - wp4 | XX | - wp5 | XX.XX..X | - CHART + expect(work_package.duration).to eq(3) end end context 'when having multiple work packages following each other' do - let_schedule(<<~CHART, ignore_non_working_days: false) + let_schedule(<<~CHART) days | MTWTFSS | wp1 | X..XX | follows wp2 wp2 | X | follows wp3 @@ -132,43 +170,28 @@ RSpec.describe WorkPackages::ApplyWorkingDaysChangeJob do end end - context 'when a work package was scheduled to start on a date that is now a non-working day' do - let_schedule(<<~CHART, ignore_non_working_days: false) - days | MTWTFSS | - work_package | XX | - CHART - - before do - set_non_working_week_days('wednesday') - end - - it 'moves the start date to the earliest working day in the future, ' \ - 'and the finish date changes by consequence [#31992]' do - job.perform_now(user_id: user.id) - expect(WorkPackage.all).to match_schedule(<<~CHART) - days | MTWTFSS | - work_package | XX | - CHART - end - end - - context 'when a follower has a predecessor with dates covering a day that is now a non-working day' do + context 'when having multiple work packages following each other and first one only has a due date' do let_schedule(<<~CHART) - days | MTWTFSS | - predecessor | XX | working days work week - follower | XXX | working days include weekends, follows predecessor + days | MTWTFSS | + wp1 | X..XX | follows wp2 + wp2 | XX | follows wp3 + wp3 | ] | CHART before do - set_non_working_week_days('wednesday') + set_non_working_week_days('tuesday', 'wednesday', 'friday') end - it 'moves the follower start date by consequence of the predecessor dates shift [#31992]' do - job.perform_now(user_id: user.id) + it 'updates them only once' do + expect { job.perform_now(user_id: user.id) } + .to change { WorkPackage.pluck(:lock_version) } + .from([0, 0, 0]) + .to([1, 1, 1]) expect(WorkPackage.all).to match_schedule(<<~CHART) - days | MTWTFSS | - predecessor | X.X | working days work week - follower | XXX | working days include weekends + days | MTWTFSSm t ssm t ssm | + wp1 | X..X...X | + wp2 | X..X | + wp3 | ] | CHART end end