Merge branch '330276-reimplement-draft-backfill-task-as-background-migration' into 'master'

Backfill draft on merge_requests via background migration

See merge request gitlab-org/gitlab!63703

db/post_migrate/20210609202501_schedule_backfill_draft_status_on_merge_requests.rb

+# frozen_string_literal: true
+
+class ScheduleBackfillDraftStatusOnMergeRequests < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  INDEX_NAME = "tmp_index_merge_requests_draft_and_status"
+  MIGRATION = 'BackfillDraftStatusOnMergeRequests'
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 100
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :merge_requests, :id,
+      where: "draft = false AND state_id = 1 AND ((title)::text ~* '^\\[draft\\]|\\(draft\\)|draft:|draft|\\[WIP\\]|WIP:|WIP'::text)",
+      name: INDEX_NAME
+
+    eligible_mrs = Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequests::MergeRequest.eligible
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      eligible_mrs,
+      MIGRATION,
+      DELAY_INTERVAL,
+      track_jobs: true,
+      batch_size: BATCH_SIZE
+    )
+  end
+
+  def down
+    remove_concurrent_index_by_name :merge_requests, INDEX_NAME
+  end
+end

db/schema_migrations/20210609202501

+93f577e2fe2dcc0daafc4ff7e15a4511a2e9f86f05f9892f5c7625f11bfce3ae
\ No newline at end of file

db/structure.sql

@@ -25143,6 +25143,8 @@ CREATE INDEX tmp_idx_deduplicate_vulnerability_occurrences ON vulnerability_occu
 
 CREATE INDEX tmp_idx_on_namespaces_delayed_project_removal ON namespaces USING btree (id) WHERE (delayed_project_removal = true);
 
+CREATE INDEX tmp_index_merge_requests_draft_and_status ON merge_requests USING btree (id) WHERE ((draft = false) AND (state_id = 1) AND ((title)::text ~* '^\[draft\]|\(draft\)|draft:|draft|\[WIP\]|WIP:|WIP'::text));
+
 CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_child_namespaces ON namespaces USING btree (id) WHERE ((parent_id IS NOT NULL) AND (traversal_ids = '{}'::integer[]));
 
 CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_root_namespaces ON namespaces USING btree (id) WHERE ((parent_id IS NULL) AND (traversal_ids = '{}'::integer[]));

lib/gitlab/background_migration/backfill_draft_status_on_merge_requests.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Backfill draft column on open merge requests based on regex parsing of
+    #   their titles.
+    #
+    class BackfillDraftStatusOnMergeRequests
+      # Migration only version of MergeRequest table
+      class MergeRequest < ActiveRecord::Base
+        include EachBatch
+
+        self.table_name = 'merge_requests'
+
+        def self.eligible
+          where(state_id: 1)
+            .where(draft: false)
+            .where("title ~* ?", '^\\[draft\\]|\\(draft\\)|draft:|draft|\\[WIP\\]|WIP:|WIP')
+        end
+      end
+
+      def perform(start_id, end_id)
+        eligible_mrs = MergeRequest.eligible.where(id: start_id..end_id).pluck(:id)
+
+        return if eligible_mrs.empty?
+
+        eligible_mrs.each_slice(10) do |slice|
+          MergeRequest.where(id: slice).update_all(draft: true)
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequests do
+  let(:namespaces)     { table(:namespaces) }
+  let(:projects)       { table(:projects) }
+  let(:merge_requests) { table(:merge_requests) }
+
+  let(:group)   { namespaces.create!(name: 'gitlab', path: 'gitlab') }
+  let(:project) { projects.create!(namespace_id: group.id) }
+
+  let(:draft_prefixes) { ["[Draft]", "(Draft)", "Draft:", "Draft", "[WIP]", "WIP:", "WIP"] }
+
+  def create_merge_request(params)
+    common_params = {
+      target_project_id: project.id,
+      target_branch: 'feature1',
+      source_branch: 'master'
+    }
+
+    merge_requests.create!(common_params.merge(params))
+  end
+
+  context "for MRs with #draft? == true titles but draft attribute false" do
+    before do
+      draft_prefixes.each do |prefix|
+        (1..4).each do |n|
+          create_merge_request(
+            title: "#{prefix} This is a title",
+            draft: false,
+            state_id: n
+          )
+        end
+      end
+    end
+
+    it "updates all open draft merge request's draft field to true" do
+      mr_count = merge_requests.all.count
+      mr_ids = merge_requests.all.collect(&:id)
+
+      expect { subject.perform(mr_ids.first, mr_ids.last) }
+        .to change { MergeRequest.where(draft: false).count }
+        .from(mr_count).to(mr_count - draft_prefixes.length)
+    end
+  end
+end

spec/migrations/schedule_backfill_draft_status_on_merge_requests_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe ScheduleBackfillDraftStatusOnMergeRequests, :sidekiq do
+  let(:namespaces)     { table(:namespaces) }
+  let(:projects)       { table(:projects) }
+  let(:merge_requests) { table(:merge_requests) }
+
+  let(:group)   { namespaces.create!(name: 'gitlab', path: 'gitlab') }
+  let(:project) { projects.create!(namespace_id: group.id) }
+
+  let(:draft_prefixes) { ["[Draft]", "(Draft)", "Draft:", "Draft", "[WIP]", "WIP:", "WIP"] }
+
+  def create_merge_request(params)
+    common_params = {
+      target_project_id: project.id,
+      target_branch: 'feature1',
+      source_branch: 'master'
+    }
+
+    merge_requests.create!(common_params.merge(params))
+  end
+
+  before do
+    draft_prefixes.each do |prefix|
+      (1..4).each do |n|
+        create_merge_request(
+          title: "#{prefix} This is a title",
+          draft: false,
+          state_id: n
+        )
+      end
+    end
+
+    stub_const("#{described_class}::BATCH_SIZE", 1)
+  end
+
+  it 'schedules BackfillDraftStatusOnMergeRequests background jobs' do
+    Sidekiq::Testing.fake! do
+      draft_mrs = Gitlab::BackgroundMigration::BackfillDraftStatusOnMergeRequests::MergeRequest.eligible
+
+      first_mr_id = draft_mrs.first.id
+      second_mr_id = draft_mrs.second.id
+
+      freeze_time do
+        migrate!
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(7)
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(2.minutes, first_mr_id, first_mr_id)
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(4.minutes, second_mr_id, second_mr_id)
+      end
+    end
+  end
+end