Merge branch 'kassio/backfill-dates-source-to-issues' into 'master'

Backfill Issues[start_date,due_date] with WorkItems::DatesSource

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157993



Merged-by: Adam Hegyi <ahegyi@gitlab.com>
Approved-by: Felipe Artur <fcardozo@gitlab.com>
Approved-by: Adam Hegyi <ahegyi@gitlab.com>
Reviewed-by: Felipe Artur <fcardozo@gitlab.com>
Reviewed-by: Kassio Borges <kborges@gitlab.com>
Reviewed-by: Alexandru Croitor <acroitor@gitlab.com>
Reviewed-by: charlie ablett <cablett@gitlab.com>
Co-authored-by: Kassio Borges <kassioborgesm@gmail.com>
Co-authored-by: Kassio Borges <kborges@gitlab.com>

db/docs/batched_background_migrations/backfill_issues_dates_with_work_item_dates_sources.yml

+---
+migration_job_name: BackfillIssuesDatesWithWorkItemDatesSources
+description: Backfill issues dates with data from work_item_dates_sources table
+feature_category: product_planning
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157993
+milestone: '17.2'
+queued_migration_version: 20240702212015
+finalize_after: '2024-08-05'

db/migrate/20240701153843_add_work_items_dates_sources_sync_to_issues_trigger.rb

+# frozen_string_literal: true
+
+class AddWorkItemsDatesSourcesSyncToIssuesTrigger < Gitlab::Database::Migration[2.2]
+  milestone '17.2'
+
+  include Gitlab::Database::SchemaHelpers
+
+  enable_lock_retries!
+
+  WORK_ITEM_DATES_SOURCE_TABLE_NAME = 'work_item_dates_sources'
+
+  TRIGGER_FUNCTION_NAME = 'sync_issues_dates_with_work_item_dates_sources'
+  TRIGGER_NAME = "trigger_#{TRIGGER_FUNCTION_NAME}"
+
+  def up
+    create_trigger_function(TRIGGER_FUNCTION_NAME, replace: true) do
+      <<~SQL
+        UPDATE
+          issues
+        SET
+          start_date = NEW.start_date,
+          due_date = NEW.due_date
+        WHERE
+          issues.id = NEW.issue_id;
+
+        RETURN NULL;
+      SQL
+    end
+
+    create_trigger(
+      WORK_ITEM_DATES_SOURCE_TABLE_NAME,
+      TRIGGER_NAME,
+      TRIGGER_FUNCTION_NAME,
+      fires: "AFTER INSERT OR UPDATE OF start_date, due_date"
+    )
+  end
+
+  def down
+    drop_trigger(WORK_ITEM_DATES_SOURCE_TABLE_NAME, TRIGGER_NAME)
+    drop_function(TRIGGER_FUNCTION_NAME)
+  end
+end

db/post_migrate/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources.rb

+# frozen_string_literal: true
+
+class QueueBackfillIssuesDatesWithWorkItemDatesSources < Gitlab::Database::Migration[2.2]
+  milestone '17.2'
+
+  restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+  MIGRATION = "BackfillIssuesDatesWithWorkItemDatesSources"
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 100
+
+  def up
+    queue_batched_background_migration(
+      MIGRATION,
+      :work_item_dates_sources,
+      :issue_id,
+      job_interval: DELAY_INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+  end
+
+  def down
+    delete_batched_background_migration(MIGRATION, :work_item_dates_sources, :issue_id, [])
+  end
+end

db/schema_migrations/20240701153843

+a86234cca3400e87012ea211d8445e114520d06f7fc79c4dcb2baf7e59415ff5
\ No newline at end of file

db/schema_migrations/20240702212015

+e818fec9c1bb9cb7b0f9e71a7d970da48bf10ec9bc26f384abfccfd90a39a7d0
\ No newline at end of file

db/structure.sql

@@ -613,6 +613,23 @@ RETURN NULL;
 END
 $$;
+CREATE FUNCTION sync_issues_dates_with_work_item_dates_sources() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+UPDATE
+  issues
+SET
+  start_date = NEW.start_date,
+  due_date = NEW.due_date
+WHERE
+  issues.id = NEW.issue_id;
+
+RETURN NULL;
+
+END
+$$;
+
 CREATE FUNCTION table_sync_function_0992e728d3() RETURNS trigger
     LANGUAGE plpgsql
     AS $$
@@ -31735,6 +31752,8 @@ CREATE TRIGGER trigger_projects_parent_id_on_insert AFTER INSERT ON projects FOR
 CREATE TRIGGER trigger_projects_parent_id_on_update AFTER UPDATE ON projects FOR EACH ROW WHEN ((old.namespace_id IS DISTINCT FROM new.namespace_id)) EXECUTE FUNCTION insert_projects_sync_event();
+CREATE TRIGGER trigger_sync_issues_dates_with_work_item_dates_sources AFTER INSERT OR UPDATE OF start_date, due_date ON work_item_dates_sources FOR EACH ROW EXECUTE FUNCTION sync_issues_dates_with_work_item_dates_sources();
+
 CREATE TRIGGER trigger_update_details_on_namespace_insert AFTER INSERT ON namespaces FOR EACH ROW WHEN (((new.type)::text <> 'Project'::text)) EXECUTE FUNCTION update_namespace_details_from_namespaces();
 CREATE TRIGGER trigger_update_details_on_namespace_update AFTER UPDATE ON namespaces FOR EACH ROW WHEN ((((new.type)::text <> 'Project'::text) AND (((old.description)::text IS DISTINCT FROM (new.description)::text) OR (old.description_html IS DISTINCT FROM new.description_html) OR (old.cached_markdown_version IS DISTINCT FROM new.cached_markdown_version)))) EXECUTE FUNCTION update_namespace_details_from_namespaces();

lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class BackfillIssuesDatesWithWorkItemDatesSources < BatchedMigrationJob
+      operation_name :backfill_issues_dates_with_work_item_dates_source
+      feature_category :team_planning
+
+      def perform
+        each_sub_batch do |sub_batch|
+          inner_scope = sub_batch.select(:start_date, :due_date, :issue_id)
+
+          define_batchable_model('issues', connection: ApplicationRecord.connection).connection.execute <<~SQL
+            WITH work_item_dates_sources_date_values AS MATERIALIZED (
+              #{inner_scope.to_sql}
+            )
+            UPDATE issues
+            SET
+              start_date = work_item_dates_sources_date_values.start_date,
+              due_date = work_item_dates_sources_date_values.due_date
+            FROM
+              work_item_dates_sources_date_values
+            WHERE
+              work_item_dates_sources_date_values.issue_id = issues.id
+              AND work_item_dates_sources_date_values.start_date IS DISTINCT FROM issues.start_date
+              AND work_item_dates_sources_date_values.due_date IS DISTINCT FROM issues.due_date
+          SQL
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillIssuesDatesWithWorkItemDatesSources,
+  feature_category: :team_planning do
+    let!(:namespace) { table(:namespaces).create!(name: 'my test group1', path: 'my-test-group1') }
+    let!(:author) { table(:users).create!(username: 'tester', projects_limit: 100) }
+    let!(:epic_type_id) { table(:work_item_types).find_by(base_type: 7).id }
+
+    let!(:issue_1) { work_items(iid: 1) }
+    let!(:issue_2) { work_items(iid: 2) }
+    let!(:unassociated_issue) { work_items(iid: 3) }
+
+    let!(:date_source_1) do
+      dates_source(
+        work_item: issue_1,
+        start_date: 1.day.ago,
+        due_date: 1.day.from_now
+      )
+    end
+
+    let!(:date_source_2) do
+      dates_source(
+        work_item: issue_2,
+        start_date: 2.days.ago,
+        due_date: 2.days.from_now
+      )
+    end
+
+    subject(:migration) do
+      described_class.new(
+        start_id: WorkItems::DatesSource.minimum(:issue_id),
+        end_id: WorkItems::DatesSource.maximum(:issue_id),
+        batch_table: :work_item_dates_sources,
+        batch_column: :issue_id,
+        job_arguments: [nil],
+        sub_batch_size: 2,
+        pause_ms: 2,
+        connection: ::ApplicationRecord.connection
+      )
+    end
+
+    it 'backfills data correctly' do
+      # Because we now also have a database trigger to ensure the work_item_dates_sources
+      # dates are synced with work_items, we have to reload the objects in memory before we
+      # can update them to ensure their start_date/due_date are nil
+      issue_1.reload.update!(start_date: nil, due_date: nil)
+      issue_2.reload.update!(start_date: nil, due_date: nil)
+
+      expect { migration.perform }
+        .to change { issue_1.reload.start_date }.from(nil).to(date_source_1.start_date)
+        .and change { issue_1.reload.due_date }.from(nil).to(date_source_1.due_date)
+        .and change { issue_2.reload.start_date }.from(nil).to(date_source_2.start_date)
+        .and change { issue_2.reload.due_date }.from(nil).to(date_source_2.due_date)
+        .and not_change { unassociated_issue.reload.start_date }
+        .and not_change { unassociated_issue.reload.due_date }
+    end
+
+    private
+
+    def work_items(iid:)
+      table(:issues).create!(
+        iid: iid,
+        title: "Issue #{iid}",
+        lock_version: 1,
+        namespace_id: namespace.id,
+        author_id: author.id,
+        work_item_type_id: epic_type_id
+      )
+    end
+
+    def dates_source(work_item:, start_date: nil, due_date: nil)
+      table(:work_item_dates_sources).create!(
+        issue_id: work_item.id,
+        namespace_id: work_item.namespace_id,
+        start_date: start_date,
+        due_date: due_date
+      )
+    end
+  end

spec/migrations/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueBackfillIssuesDatesWithWorkItemDatesSources, feature_category: :team_planning do
+  let!(:batched_migration) { described_class::MIGRATION }
+
+  it 'schedules a new batched migration' do
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(batched_migration).not_to have_scheduled_batched_migration
+      }
+
+      migration.after -> {
+        expect(batched_migration).to have_scheduled_batched_migration(
+          table_name: :work_item_dates_sources,
+          column_name: :issue_id,
+          interval: described_class::DELAY_INTERVAL,
+          batch_size: described_class::BATCH_SIZE
+        )
+      }
+    end
+  end
+end

spec/models/work_items/dates_source_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe WorkItems::DatesSource, feature_category: :portfolio_management do
-  describe 'ssociations' do
+  describe 'associations' do
     it { is_expected.to belong_to(:namespace).inverse_of(:work_items_dates_source) }
     it { is_expected.to belong_to(:work_item).with_foreign_key('issue_id').inverse_of(:dates_source) }
     it { is_expected.to belong_to(:due_date_sourcing_work_item).class_name('WorkItem') }
@@ -16,8 +16,43 @@
     work_item = create(:work_item)
     date_source = described_class.new(work_item: work_item)
 
-    date_source.valid?
+    expect(date_source).to be_valid
 
     expect(date_source.namespace).to eq(work_item.namespace)
   end
+
+  context 'on database triggers' do
+    let_it_be_with_reload(:work_item) { create(:work_item) }
+
+    context 'on create' do
+      it 'ensures to keep the issues table start_date and due_date columns updated' do
+        date_source = described_class.new(
+          work_item: work_item,
+          start_date: 1.day.ago,
+          due_date: 1.day.from_now
+        )
+
+        expect { date_source.save! }
+          .to change { work_item.reload.start_date }.from(nil).to(date_source.start_date)
+          .and change { work_item.reload.due_date }.from(nil).to(date_source.due_date)
+      end
+    end
+
+    context 'on update' do
+      it 'ensures to keep the issues table start_date and due_date columns updated' do
+        start_date = 2.days.ago.to_date
+        due_date = 2.days.from_now.to_date
+
+        date_source = described_class.create!(
+          work_item: work_item,
+          start_date: start_date - 1.day,
+          due_date: due_date + 1.day
+        )
+
+        expect { date_source.update!(start_date: start_date, due_date: due_date) }
+          .to change { work_item.reload.start_date }.to(start_date)
+          .and change { work_item.reload.due_date }.to(due_date)
+      end
+    end
+  end
 end

spec/support/matchers/background_migrations_matchers.rb

@@ -89,7 +89,10 @@ def same_arrays?(arg, expected)
       Gitlab::Database::BackgroundMigration::BatchedMigration
         .where(job_class_name: migration)
 
-    expect(batched_migrations.count).to be(0)
+    expect(batched_migrations.count).to(
+      be(0),
+      "#{migration} should not be scheduled, found #{batched_migrations.count} times"
+    )
   end
 end