From 391d65b3aafa99ea070309a9bac5621a479157ce Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Wed, 27 Nov 2024 12:14:59 +0100 Subject: [PATCH] [59539] Keep manual scheduling mode in migration The rule is: it never switches from manual to automatic scheduling mode. It only switches from automatic to manual, and only if keeping automatic is not possible because it would mean losing the dates. The code and specs have been updated to reflect this. A materialized view is used to reuse the data in multiple different queries. --- .../automatic_mode/migrate_values_job.rb | 91 ++++++-------- .../update_scheduling_mode_and_lags_spec.rb | 116 ++++++++++++++---- 2 files changed, 133 insertions(+), 74 deletions(-) diff --git a/app/workers/work_packages/automatic_mode/migrate_values_job.rb b/app/workers/work_packages/automatic_mode/migrate_values_job.rb index 39293396afe6..0fee67054806 100644 --- a/app/workers/work_packages/automatic_mode/migrate_values_job.rb +++ b/app/workers/work_packages/automatic_mode/migrate_values_job.rb @@ -28,10 +28,8 @@ class WorkPackages::AutomaticMode::MigrateValuesJob < ApplicationJob def perform - with_temporary_table do - change_scheduling_mode_to_manual - change_scheduling_mode_to_automatic_for_followers - change_scheduling_mode_to_automatic_for_parents + with_temporary_tables do + change_independent_childless_work_packages_scheduling_mode_to_manual set_lags_for_follows_relations copy_values_to_work_packages_and_update_journals end @@ -39,16 +37,16 @@ def perform private - def with_temporary_table + def with_temporary_tables WorkPackage.transaction do - create_temporary_table + create_temporary_tables yield ensure - drop_temporary_table + drop_temporary_tables end end - def create_temporary_table + def create_temporary_tables execute(<<~SQL.squish) CREATE UNLOGGED TABLE temp_wp_values AS SELECT @@ -58,42 +56,44 @@ def create_temporary_table schedule_manually FROM work_packages SQL - end - - def drop_temporary_table execute(<<~SQL.squish) - DROP TABLE temp_wp_values + CREATE MATERIALIZED VIEW follows_relations + AS SELECT + relations.id as id, + relations.from_id as succ_id, + COALESCE(wp_pred.due_date, wp_pred.start_date) as pred_date, + COALESCE(wp_succ.start_date, wp_succ.due_date) as succ_date, + wp_succ.schedule_manually as succ_schedule_manually + FROM relations + LEFT JOIN work_packages wp_pred ON relations.to_id = wp_pred.id + LEFT JOIN work_packages wp_succ ON relations.from_id = wp_succ.id + WHERE relation_type = 'follows' SQL + execute("CREATE INDEX ON follows_relations (succ_id)") end - def change_scheduling_mode_to_manual - execute(<<~SQL.squish) - UPDATE temp_wp_values - SET schedule_manually = true - SQL + def drop_temporary_tables + execute("DROP TABLE temp_wp_values") + execute("DROP MATERIALIZED VIEW follows_relations") end - def change_scheduling_mode_to_automatic_for_followers + # Change the scheduling mode to manual for: + # - non-successor (independent) and non-parent (childless) work packages + # - successor work packages with dates but without any predecessor with dates + def change_independent_childless_work_packages_scheduling_mode_to_manual execute(<<~SQL.squish) UPDATE temp_wp_values - SET schedule_manually = false - WHERE EXISTS ( + SET schedule_manually = true + WHERE NOT EXISTS ( + SELECT 1 + FROM follows_relations + WHERE follows_relations.succ_id = temp_wp_values.id + AND (follows_relations.pred_date IS NOT NULL + OR follows_relations.succ_date IS NULL) + ) AND NOT EXISTS ( SELECT 1 - FROM relations - WHERE relations.from_id = temp_wp_values.id - AND relations.relation_type = 'follows' - ) - SQL - end - - def change_scheduling_mode_to_automatic_for_parents - execute(<<~SQL.squish) - UPDATE temp_wp_values - SET schedule_manually = false - WHERE id IN ( - SELECT DISTINCT parent_id FROM work_packages - WHERE parent_id IS NOT NULL + WHERE work_packages.parent_id = temp_wp_values.id ) SQL end @@ -109,32 +109,21 @@ def set_lags_for_follows_relations # - Use both information to count the number of working days between # predecessor and successor dates and update the lag with it execute(<<~SQL.squish) - WITH follows_relations_with_dates AS ( - SELECT - relations.id as id, - relations.from_id as succ_id, - COALESCE(wp_pred.due_date, wp_pred.start_date) as pred_date, - COALESCE(wp_succ.start_date, wp_succ.due_date) as succ_date - FROM relations - LEFT JOIN work_packages wp_pred ON relations.to_id = wp_pred.id - LEFT JOIN work_packages wp_succ ON relations.from_id = wp_succ.id - WHERE relation_type = 'follows' - AND COALESCE(wp_pred.due_date, wp_pred.start_date) IS NOT NULL - AND COALESCE(wp_succ.start_date, wp_succ.due_date) IS NOT NULL - ), - closest_follows_relations_with_dates AS ( + WITH closest_follows_relations_with_dates AS ( SELECT DISTINCT ON (succ_id) id, pred_date, succ_date - FROM follows_relations_with_dates + FROM follows_relations + WHERE pred_date IS NOT NULL + AND succ_date IS NOT NULL ORDER BY succ_id, pred_date DESC ), working_dates AS ( SELECT date::date FROM generate_series( - (SELECT MIN(pred_date) FROM follows_relations_with_dates), - (SELECT MAX(succ_date) FROM follows_relations_with_dates), + (SELECT MIN(pred_date) FROM closest_follows_relations_with_dates), + (SELECT MAX(succ_date) FROM closest_follows_relations_with_dates), '1 day'::interval ) AS date WHERE EXTRACT(ISODOW FROM date)::integer IN (#{working_days.join(',')}) diff --git a/spec/migrations/update_scheduling_mode_and_lags_spec.rb b/spec/migrations/update_scheduling_mode_and_lags_spec.rb index 0dcf3ae5209a..bab471f28052 100644 --- a/spec/migrations/update_scheduling_mode_and_lags_spec.rb +++ b/spec/migrations/update_scheduling_mode_and_lags_spec.rb @@ -94,8 +94,11 @@ end end - # spec from #42388, "Migration from an earlier version" section - context "for work packages with no predecessors" do + # spec from #59539, "Migration from an earlier version" section: + # + # > - For work packages with no predecessors (or with no relations at all), they will be + # > switched to manual scheduling. + context "for work packages with no predecessors nor children" do let_work_packages(<<~TABLE) subject | start date | due date | scheduling mode wp automatic 1 | 2024-11-20 | 2024-11-21 | automatic @@ -115,43 +118,110 @@ end end - # spec from #42388, "Migration from an earlier version" section - context "for work packages following another one" do + # spec from #59539, "Migration from an earlier version" section: + # + # > - Manually scheduled work packages remain so. + context "for manually scheduled work packages following another one" do let_work_packages(<<~TABLE) subject | start date | due date | scheduling mode | properties - main | 2024-11-19 | 2024-11-19 | manual | - wp automatic 1 | 2024-11-20 | 2024-11-21 | automatic | follows main - wp automatic 2 | | 2024-11-21 | automatic | follows main - wp automatic 3 | 2024-11-20 | | automatic | follows main - wp automatic 4 | | | automatic | follows main - wp manual 1 | 2024-11-20 | 2024-11-21 | manual | follows main - wp manual 2 | | 2024-11-21 | manual | follows main - wp manual 3 | 2024-11-20 | | manual | follows main - wp manual 4 | | | manual | follows main + main | | | manual | + wp 1 | 2024-11-20 | 2024-11-21 | manual | follows main + wp 2 | | 2024-11-21 | manual | follows main + wp 3 | 2024-11-20 | | manual | follows main + wp 4 | | | manual | follows main + TABLE + + it "remains manually scheduled" do + run_migration + + expect(table_work_packages).to all(be_schedule_manually) + end + end + + # spec from #59539, "Migration from an earlier version" section + # + # > - If the successor is in automatic scheduling mode, has dates and some predecessors + # > have dates too: + # > - The successor remains in automatic mode + context "for automatically scheduled work packages following another one having dates" do + let_work_packages(<<~TABLE) + subject | start date | due date | scheduling mode | properties + pred with dates | 2024-11-19 | 2024-11-19 | manual | + pred without dates | | | manual | + wp 1 | 2024-11-20 | 2024-11-21 | automatic | follows pred with dates, follows pred without dates + wp 2 | | 2024-11-21 | automatic | follows pred with dates, follows pred without dates + wp 3 | 2024-11-20 | | automatic | follows pred with dates, follows pred without dates + wp 4 | | | automatic | follows pred with dates, follows pred without dates + TABLE + + it "remains automatically scheduled" do + run_migration + + expect([wp1, wp2, wp3, wp4]).to all(be_schedule_automatically) + end + end + + # spec from #59539, "Migration from an earlier version" section + # > - If the successor is in automatic scheduling mode and has no dates + # > - The successor remains in automatic mode and continues to have no dates, + # > regardless of having predecessor with dates or not. + # > - If the successor is in automatic scheduling mode, has dates and none of the + # > predecessors have any dates + # > - The successor is switched to manual mode to preserve its dates and duration + context "for automatically scheduled work packages without dates following another one" do + let_work_packages(<<~TABLE) + subject | start date | due date | scheduling mode | properties + pred without dates | | | manual | + succ | | | automatic | follows pred without dates + TABLE + + it "remains automatically scheduled and continues to have no dates" do + run_migration + + expect(succ).to be_schedule_automatically + end + end + + # spec from #59539, "Migration from an earlier version" section + # + # > - If the successor is in automatic scheduling mode, has dates and none of the + # > predecessors have any dates + # > - The successor is switched to manual mode to preserve its dates and duration + context "for automatically scheduled work packages following another one having no dates" do + let_work_packages(<<~TABLE) + subject | start date | due date | scheduling mode | properties + pred without dates | | | manual | + succ 1 | 2024-11-20 | 2024-11-21 | automatic | follows pred without dates + succ 2 | | 2024-11-21 | automatic | follows pred without dates + succ 3 | 2024-11-20 | | automatic | follows pred without dates TABLE - # TODO: should work packages without any dates really be switched to manual like the specs say? - it "switches to automatic scheduling" do + it "switches to manual scheduling to preserve its dates and duration" do run_migration - expect(main).to be_schedule_manually - expect(table_work_packages - [main]).to all(be_schedule_automatically) + expect([succ1, succ2, succ3]).to all(be_schedule_manually) end end # spec from #42388, "Migration from an earlier version" section + # + # > - Manually scheduled work packages remain so. + # > - If the relationship is parent-child, there are no changes to dates; the parent + # > remains in automatic mode. context "for parent work packages" do let_work_packages(<<~TABLE) - hierarchy | scheduling mode | - parent | manual | - child | manual | + hierarchy | scheduling mode | + parent_automatic | automatic | + child1 | manual | + parent_manual | manual | + child2 | manual | TABLE - it "switches to automatic scheduling" do + it "keep their scheduling mode" do run_migration - expect(parent).to be_schedule_automatically - expect(child).to be_schedule_manually + expect(parent_automatic).to be_schedule_automatically + expect(parent_manual).to be_schedule_manually end end