Add latest changes from gitlab-org/gitlab@master

app/services/issue_rebalancing_service.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+class IssueRebalancingService
+  MAX_ISSUE_COUNT = 10_000
+  TooManyIssues = Class.new(StandardError)
+
+  def initialize(issue)
+    @issue = issue
+    @base = Issue.relative_positioning_query_base(issue)
+  end
+
+  def execute
+    gates = [issue.project, issue.project.group].compact
+    return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) }
+
+    raise TooManyIssues, "#{issue_count} issues" if issue_count > MAX_ISSUE_COUNT
+
+    start = RelativePositioning::START_POSITION - (gaps / 2) * gap_size
+
+    Issue.transaction do
+      indexed_ids.each_slice(100) { |pairs| assign_positions(start, pairs) }
+    end
+  end
+
+  private
+
+  attr_reader :issue, :base
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def indexed_ids
+    base.reorder(:relative_position, :id).pluck(:id).each_with_index
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def assign_positions(start, positions)
+    values = positions.map do |id, index|
+      "(#{id}, #{start + (index * gap_size)})"
+    end.join(', ')
+
+    Issue.connection.exec_query(<<~SQL, "rebalance issue positions")
+      WITH cte(cte_id, new_pos) AS (
+       SELECT *
+       FROM (VALUES #{values}) as t (id, pos)
+      )
+      UPDATE #{Issue.table_name}
+      SET relative_position = cte.new_pos
+      FROM cte
+      WHERE cte_id = id
+    SQL
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
+  def issue_count
+    @issue_count ||= base.count
+  end
+
+  def gaps
+    issue_count - 1
+  end
+
+  def gap_size
+    # We could try to split the available range over the number of gaps we need,
+    # but IDEAL_DISTANCE * MAX_ISSUE_COUNT is only 0.1% of the available range,
+    # so we are guaranteed not to exhaust it by using this static value.
+    #
+    # If we raise MAX_ISSUE_COUNT or IDEAL_DISTANCE significantly, this may
+    # change!
+    RelativePositioning::IDEAL_DISTANCE
+  end
+end

app/services/issues/base_service.rb

@@ -19,6 +19,19 @@ module Issues
 
     private
 
+    NO_REBALANCING_NEEDED = ((RelativePositioning::MIN_POSITION * 0.9999)..(RelativePositioning::MAX_POSITION * 0.9999)).freeze
+
+    def rebalance_if_needed(issue)
+      return unless issue
+      return if issue.relative_position.nil?
+      return if NO_REBALANCING_NEEDED.cover?(issue.relative_position)
+
+      gates = [issue.project, issue.project.group].compact
+      return unless gates.any? { |gate| Feature.enabled?(:rebalance_issues, gate) }
+
+      IssueRebalancingWorker.perform_async(issue.id)
+    end
+
     def create_assignee_note(issue, old_assignees)
       SystemNoteService.change_issuable_assignees(
         issue, issue.project, current_user, old_assignees)

app/services/issues/create_service.rb

@@ -30,6 +30,7 @@ module Issues
       user_agent_detail_service.create
       resolve_discussions_with_issue(issuable)
       delete_milestone_total_issue_counter_cache(issuable.milestone)
+      rebalance_if_needed(issuable)
 
       super
     end

app/services/issues/update_service.rb

@@ -83,6 +83,7 @@ module Issues
       raise ActiveRecord::RecordNotFound unless issue_before || issue_after
 
       issue.move_between(issue_before, issue_after)
+      rebalance_if_needed(issue)
     end
 
     # rubocop: disable CodeReuse/ActiveRecord

app/workers/all_queues.yml

@@ -1436,6 +1436,14 @@
   :weight: 1
   :idempotent: 
   :tags: []
+- :name: issue_rebalancing
+  :feature_category: :issue_tracking
+  :has_external_dependencies: 
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: mailers
   :feature_category: 
   :has_external_dependencies: 

app/workers/issue_rebalancing_worker.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class IssueRebalancingWorker
+  include ApplicationWorker
+
+  idempotent!
+  urgency :low
+  feature_category :issue_tracking
+
+  def perform(issue_id)
+    issue = Issue.find(issue_id)
+
+    IssueRebalancingService.new(issue).execute
+  rescue ActiveRecord::RecordNotFound, IssueRebalancingService::TooManyIssues => e
+    Gitlab::ErrorTracking.log_exception(e, issue_id: issue_id)
+  end
+end

changelogs/unreleased/ajk-relative-positioning-worker.yml

@@ -0,0 +1,5 @@
+---
+title: Add background worker to rebalance issues
+merge_request: 40124
+author:
+type: added

config/feature_flags/development/rebalance_issues.yml

@@ -0,0 +1,7 @@
+---
+name: rebalance_issues
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40124 
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/239344
+group: 'group::project management'
+type: development
+default_enabled: false

config/sidekiq_queues.yml

@@ -138,6 +138,8 @@
   - 2
 - - irker
   - 1
+- - issue_rebalancing
+  - 1
 - - jira_connect
   - 1
 - - jira_importer

doc/user/project/issues/index.md

@@ -112,8 +112,6 @@ and modify them if you have the necessary [permissions](../../permissions.md).
 #### Real-time sidebar **(CORE ONLY)**
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/17589) in GitLab 13.3.
-> - It cannot be enabled or disabled per-project.
-> - It's not recommended for production use.
 
 Assignees in the sidebar are updated in real time. This feature is **disabled by default**.
 To enable, you need to enable [ActionCable in-app mode](https://docs.gitlab.com/omnibus/settings/actioncable.html).

doc/user/project/merge_requests/squash_and_merge.md

@@ -65,14 +65,27 @@ meaningful commit messages and:
 ## Enabling squash for a merge request
 
 Anyone who can create or edit a merge request can choose for it to be squashed
-on the merge request form:
+on the merge request form. Users can select or unselect the checkbox at the moment
+they are creating the merge request:
 
 ![Squash commits checkbox on edit form](img/squash_edit_form.png)
 
-This can then be overridden at the time of accepting the merge request:
+After the merge request is submitted, Squash and Merge can still be enabled or disabled
+by editing the merge request description:
+
+1. Scroll to the top of the merge request page and click **Edit**.
+1. Scroll down to the end of the merge request form and select the checkbox
+**Squash commits when merge request is accepted**.
+
+This setting can then be overridden at the time of accepting the merge request.
+At the end of the merge request widget, next to the **Merge** button, the **Squash commits** checkbox
+can be either selected or unselected:
 
 ![Squash commits checkbox on accept merge request form](img/squash_mr_widget.png)
 
+Note that Squash and Merge might not be available depending on the project's configuration
+for [Squash Commit Options](#squash-commits-options).
+
 ## Commit metadata for squashed commits
 
 The squashed commit has the following metadata:

qa/qa/specs/features/browser_ui/3_create/repository/move_project_create_fork_spec.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 module QA
-  RSpec.describe 'Create', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/217002', type: :investigating } do
-    describe 'Gitaly repository storage', :orchestrated, :repository_storage, :requires_admin do
+  RSpec.describe 'Create', :orchestrated, :repository_storage, :requires_admin, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/217002', type: :investigating } do
+    describe 'Gitaly repository storage' do
       let(:user) { Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_1, Runtime::Env.gitlab_qa_password_1) }
       let(:parent_project) do
         Resource::Project.fabricate_via_api! do |project|

spec/services/issue_rebalancing_service_spec.rb

@@ -0,0 +1,101 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IssueRebalancingService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { project.creator }
+  let_it_be(:start) { RelativePositioning::START_POSITION }
+  let_it_be(:max_pos) { RelativePositioning::MAX_POSITION }
+  let_it_be(:min_pos) { RelativePositioning::MIN_POSITION }
+  let_it_be(:clump_size) { 300 }
+
+  let_it_be(:unclumped) do
+    (0..clump_size).to_a.map do |i|
+      create(:issue, project: project, author: user, relative_position: start + (1024 * i))
+    end
+  end
+
+  let_it_be(:end_clump) do
+    (0..clump_size).to_a.map do |i|
+      create(:issue, project: project, author: user, relative_position: max_pos - i)
+    end
+  end
+
+  let_it_be(:start_clump) do
+    (0..clump_size).to_a.map do |i|
+      create(:issue, project: project, author: user, relative_position: min_pos + i)
+    end
+  end
+
+  def issues_in_position_order
+    project.reload.issues.reorder(relative_position: :asc).to_a
+  end
+
+  it 'rebalances a set of issues with clumps at the end and start' do
+    all_issues = start_clump + unclumped + end_clump.reverse
+    service = described_class.new(project.issues.first)
+
+    expect { service.execute }.not_to change { issues_in_position_order.map(&:id) }
+
+    all_issues.each(&:reset)
+
+    gaps = all_issues.take(all_issues.count - 1).zip(all_issues.drop(1)).map do |a, b|
+      b.relative_position - a.relative_position
+    end
+
+    expect(gaps).to all(be > RelativePositioning::MIN_GAP)
+    expect(all_issues.first.relative_position).to be > (RelativePositioning::MIN_POSITION * 0.9999)
+    expect(all_issues.last.relative_position).to be < (RelativePositioning::MAX_POSITION * 0.9999)
+  end
+
+  it 'is idempotent' do
+    service = described_class.new(project.issues.first)
+
+    expect do
+      service.execute
+      service.execute
+    end.not_to change { issues_in_position_order.map(&:id) }
+  end
+
+  it 'does nothing if the feature flag is disabled' do
+    stub_feature_flags(rebalance_issues: false)
+    issue = project.issues.first
+    issue.project
+    issue.project.group
+    old_pos = issue.relative_position
+
+    service = described_class.new(issue)
+
+    expect { service.execute }.not_to exceed_query_limit(0)
+    expect(old_pos).to eq(issue.reload.relative_position)
+  end
+
+  it 'acts if the flag is enabled for the project' do
+    issue = create(:issue, project: project, author: user, relative_position: max_pos)
+    stub_feature_flags(rebalance_issues: issue.project)
+
+    service = described_class.new(issue)
+
+    expect { service.execute }.to change { issue.reload.relative_position }
+  end
+
+  it 'acts if the flag is enabled for the group' do
+    issue = create(:issue, project: project, author: user, relative_position: max_pos)
+    project.update!(group: create(:group))
+    stub_feature_flags(rebalance_issues: issue.project.group)
+
+    service = described_class.new(issue)
+
+    expect { service.execute }.to change { issue.reload.relative_position }
+  end
+
+  it 'aborts if there are too many issues' do
+    issue = project.issues.first
+    base = double(count: 10_001)
+
+    allow(Issue).to receive(:relative_positioning_query_base).with(issue).and_return(base)
+
+    expect { described_class.new(issue).execute }.to raise_error(described_class::TooManyIssues)
+  end
+end

spec/services/issues/create_service_spec.rb

@@ -75,6 +75,37 @@ RSpec.describe Issues::CreateService do
         expect(Todo.where(attributes).count).to eq 1
       end
 
+      it 'rebalances if needed' do
+        create(:issue, project: project, relative_position: RelativePositioning::MAX_POSITION)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(Integer)
+
+        expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
+      end
+
+      it 'does not rebalance if the flag is disabled' do
+        stub_feature_flags(rebalance_issues: false)
+
+        create(:issue, project: project, relative_position: RelativePositioning::MAX_POSITION)
+        expect(IssueRebalancingWorker).not_to receive(:perform_async).with(Integer)
+
+        expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
+      end
+
+      it 'does rebalance if the flag is enabled for the project' do
+        stub_feature_flags(rebalance_issues: project)
+
+        create(:issue, project: project, relative_position: RelativePositioning::MAX_POSITION)
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(Integer)
+
+        expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
+      end
+
+      it 'does not rebalance unless needed' do
+        expect(IssueRebalancingWorker).not_to receive(:perform_async)
+
+        expect(issue.relative_position).to eq(project.issues.maximum(:relative_position))
+      end
+
       context 'when label belongs to project group' do
         let(:group) { create(:group) }
         let(:group_labels) { create_pair(:group_label, group: group) }

spec/services/issues/update_service_spec.rb

@@ -106,7 +106,7 @@ RSpec.describe Issues::UpdateService, :mailer do
 
         [issue, issue1, issue2].each do |issue|
           issue.move_to_end
-          issue.save
+          issue.save!
         end
 
         opts[:move_between_ids] = [issue1.id, issue2.id]
@@ -116,6 +116,66 @@ RSpec.describe Issues::UpdateService, :mailer do
         expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
       end
 
+      it 'does not rebalance even if needed if the flag is disabled' do
+        stub_feature_flags(rebalance_issues: false)
+
+        range = described_class::NO_REBALANCING_NEEDED
+        issue1 = create(:issue, project: project, relative_position: range.first - 100)
+        issue2 = create(:issue, project: project, relative_position: range.first)
+        issue.update!(relative_position: RelativePositioning::START_POSITION)
+
+        opts[:move_between_ids] = [issue1.id, issue2.id]
+
+        expect(IssueRebalancingWorker).not_to receive(:perform_async).with(issue.id)
+
+        update_issue(opts)
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
+
+      it 'rebalances if needed if the flag is enabled for the project' do
+        stub_feature_flags(rebalance_issues: project)
+
+        range = described_class::NO_REBALANCING_NEEDED
+        issue1 = create(:issue, project: project, relative_position: range.first - 100)
+        issue2 = create(:issue, project: project, relative_position: range.first)
+        issue.update!(relative_position: RelativePositioning::START_POSITION)
+
+        opts[:move_between_ids] = [issue1.id, issue2.id]
+
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+
+        update_issue(opts)
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
+
+      it 'rebalances if needed on the left' do
+        range = described_class::NO_REBALANCING_NEEDED
+        issue1 = create(:issue, project: project, relative_position: range.first - 100)
+        issue2 = create(:issue, project: project, relative_position: range.first)
+        issue.update!(relative_position: RelativePositioning::START_POSITION)
+
+        opts[:move_between_ids] = [issue1.id, issue2.id]
+
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+
+        update_issue(opts)
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
+
+      it 'rebalances if needed on the right' do
+        range = described_class::NO_REBALANCING_NEEDED
+        issue1 = create(:issue, project: project, relative_position: range.last)
+        issue2 = create(:issue, project: project, relative_position: range.last + 100)
+        issue.update!(relative_position: RelativePositioning::START_POSITION)
+
+        opts[:move_between_ids] = [issue1.id, issue2.id]
+
+        expect(IssueRebalancingWorker).to receive(:perform_async).with(issue.id)
+
+        update_issue(opts)
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
+
       context 'when moving issue between issues from different projects' do
         let(:group) { create(:group) }
         let(:subgroup) { create(:group, parent: group) }

spec/workers/issue_rebalancing_worker_spec.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IssueRebalancingWorker do
+  describe '#perform' do
+    let_it_be(:issue) { create(:issue) }
+
+    it 'runs an instance of IssueRebalancingService' do
+      service = double(execute: nil)
+      expect(IssueRebalancingService).to receive(:new).with(issue).and_return(service)
+
+      described_class.new.perform(issue.id)
+    end
+
+    it 'anticipates the inability to find the issue' do
+      expect(Gitlab::ErrorTracking).to receive(:log_exception).with(ActiveRecord::RecordNotFound, include(issue_id: -1))
+      expect(IssueRebalancingService).not_to receive(:new)
+
+      described_class.new.perform(-1)
+    end
+
+    it 'anticipates there being too many issues' do
+      service = double
+      allow(service).to receive(:execute) { raise IssueRebalancingService::TooManyIssues }
+      expect(IssueRebalancingService).to receive(:new).with(issue).and_return(service)
+      expect(Gitlab::ErrorTracking).to receive(:log_exception).with(IssueRebalancingService::TooManyIssues, include(issue_id: issue.id))
+
+      described_class.new.perform(issue.id)
+    end
+  end
+end