Add background migration to backfill the relative position for designs

In https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37835 we've
introduced the `relative_position` column and are setting it for every
newly created design.

The post migration queries all issues having designs, and bulk-queues
background migrations for them.

The feature flag can be enabled after all migrations have completed.
If it's enabled before then, the ordering will be temporarily incorrect,
but otherwise this won't cause any harm.

This means:

- We don't need to worry about newly created issues, or issues that
  previously didn't have designs but do now, since they will already
  have a relative position set.

- We don't need to do any clean up later.

changelogs/unreleased/221167-background-migration.yml

+---
+title: Backfill relative positions on designs
+merge_request: 37837
+author:
+type: changed

db/post_migrate/20200724130639_backfill_designs_relative_position.rb

+# frozen_string_literal: true
+
+class BackfillDesignsRelativePosition < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INTERVAL = 2.minutes
+  BATCH_SIZE = 1000
+  MIGRATION = 'BackfillDesignsRelativePosition'
+
+  disable_ddl_transaction!
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+
+    has_many :designs
+  end
+
+  class Design < ActiveRecord::Base
+    self.table_name = 'design_management_designs'
+  end
+
+  def up
+    issues_with_designs = Issue.where(id: Design.select(:issue_id))
+
+    issues_with_designs.each_batch(of: BATCH_SIZE) do |relation, index|
+      issue_ids = relation.pluck(:id)
+      delay = INTERVAL * index
+
+      migrate_in(delay, MIGRATION, [issue_ids])
+    end
+  end
+
+  def down
+  end
+end

db/schema_migrations/20200724130639

+9d711a0c4f785660c0a2317e598e427d5e2f91b177e4c0b96cef2958f787aa6e
\ No newline at end of file

lib/gitlab/background_migration/backfill_designs_relative_position.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Backfill `relative_position` column in `design_management_designs` table
+    class BackfillDesignsRelativePosition
+      # Define the issue model
+      class Issue < ActiveRecord::Base
+        self.table_name = 'issues'
+      end
+
+      # Define the design model
+      class Design < ActiveRecord::Base
+        include RelativePositioning if defined?(RelativePositioning)
+
+        self.table_name = 'design_management_designs'
+
+        def self.relative_positioning_query_base(design)
+          where(issue_id: design.issue_id)
+        end
+
+        def self.relative_positioning_parent_column
+          :issue_id
+        end
+
+        def self.move_nulls_to_start(designs)
+          if defined?(super)
+            super(designs)
+          else
+            logger.error "BackfillDesignsRelativePosition failed because move_nulls_to_start is no longer included in the RelativePositioning concern"
+          end
+        end
+      end
+
+      def perform(issue_ids)
+        issue_ids.each do |issue_id|
+          migrate_issue(issue_id)
+        end
+      end
+
+      private
+
+      def migrate_issue(issue_id)
+        issue = Issue.find_by(id: issue_id)
+        return unless issue
+
+        designs = Design.where(issue_id: issue.id).order(:id)
+        return unless designs.any?
+
+        Design.move_nulls_to_start(designs)
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/backfill_designs_relative_position_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillDesignsRelativePosition do
+  let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') }
+  let(:project) { table(:projects).create!(namespace_id: namespace.id) }
+  let(:issues) { table(:issues) }
+  let(:designs) { table(:design_management_designs) }
+
+  before do
+    issues.create!(id: 1, project_id: project.id)
+    issues.create!(id: 2, project_id: project.id)
+    issues.create!(id: 3, project_id: project.id)
+    issues.create!(id: 4, project_id: project.id)
+
+    designs.create!(id: 1, issue_id: 1, project_id: project.id, filename: 'design1.jpg')
+    designs.create!(id: 2, issue_id: 1, project_id: project.id, filename: 'design2.jpg')
+    designs.create!(id: 3, issue_id: 2, project_id: project.id, filename: 'design3.jpg')
+    designs.create!(id: 4, issue_id: 2, project_id: project.id, filename: 'design4.jpg')
+    designs.create!(id: 5, issue_id: 3, project_id: project.id, filename: 'design5.jpg')
+  end
+
+  describe '#perform' do
+    it 'backfills the position for the designs in each issue' do
+      expect(described_class::Design).to receive(:move_nulls_to_start).with(
+        a_collection_containing_exactly(
+          an_object_having_attributes(id: 1, issue_id: 1),
+          an_object_having_attributes(id: 2, issue_id: 1)
+        )
+      ).ordered.and_call_original
+
+      expect(described_class::Design).to receive(:move_nulls_to_start).with(
+        a_collection_containing_exactly(
+          an_object_having_attributes(id: 3, issue_id: 2),
+          an_object_having_attributes(id: 4, issue_id: 2)
+        )
+      ).ordered.and_call_original
+
+      # We only expect calls to `move_nulls_to_start` with issues 1 and 2:
+      # - Issue 3 should be skipped because we're not passing its ID
+      # - Issue 4 should be skipped because it doesn't have any designs
+      # - Issue 0 should be skipped because it doesn't exist
+      subject.perform([1, 2, 4, 0])
+
+      expect(designs.find(1).relative_position).to be < designs.find(2).relative_position
+      expect(designs.find(3).relative_position).to be < designs.find(4).relative_position
+      expect(designs.find(5).relative_position).to be_nil
+    end
+  end
+end

spec/migrations/backfill_designs_relative_position_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200724130639_backfill_designs_relative_position.rb')
+
+RSpec.describe BackfillDesignsRelativePosition do
+  let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') }
+  let(:project) { table(:projects).create!(namespace_id: namespace.id) }
+  let(:issues) { table(:issues) }
+  let(:designs) { table(:design_management_designs) }
+
+  before do
+    issues.create!(id: 1, project_id: project.id)
+    issues.create!(id: 2, project_id: project.id)
+    issues.create!(id: 3, project_id: project.id)
+    issues.create!(id: 4, project_id: project.id)
+
+    designs.create!(issue_id: 1, project_id: project.id, filename: 'design1.jpg')
+    designs.create!(issue_id: 2, project_id: project.id, filename: 'design2.jpg')
+    designs.create!(issue_id: 4, project_id: project.id, filename: 'design3.jpg')
+
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+  end
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(2.minutes, [1, 2])
+
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(4.minutes, [4])
+
+        # Issue 3 should be skipped because it doesn't have any designs
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end