Merge branch '29077-remove-two_step_rebase-feature-flag' into 'master'

Resolve "Remove two_step_rebase feature flag"

Closes #29077

See merge request gitlab-org/gitlab!23546

app/models/repository.rb

@@ -1091,10 +1091,6 @@ class Repository
   end
 
   def rebase(user, merge_request, skip_ci: false)
-    if Feature.disabled?(:two_step_rebase, default_enabled: true)
-      return rebase_deprecated(user, merge_request)
-    end
-
     push_options = []
     push_options << Gitlab::PushOptions::CI_SKIP if skip_ci
 

changelogs/unreleased/29077-remove-two_step_rebase-feature-flag.yml

+---
+title: Update rebasing to use the new two-phase Gitaly Rebase RPC
+merge_request: 23546
+author:
+type: changed

spec/models/repository_spec.rb

@@ -1618,79 +1618,58 @@ describe Repository do
       end
     end
 
-    context 'when two_step_rebase feature is enabled' do
-      before do
-        stub_feature_flags(two_step_rebase: true)
-      end
-
-      it_behaves_like 'a method that can rebase successfully'
-
-      it 'executes the new Gitaly RPC' do
-        expect_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rebase)
-        expect_any_instance_of(Gitlab::GitalyClient::OperationService).not_to receive(:user_rebase)
-
-        repository.rebase(user, merge_request)
-      end
-
-      describe 'rolling back the `rebase_commit_sha`' do
-        let(:new_sha) { Digest::SHA1.hexdigest('foo') }
+    it_behaves_like 'a method that can rebase successfully'
 
-        it 'does not rollback when there are no errors' do
-          second_response = double(pre_receive_error: nil, git_error: nil)
-          mock_gitaly(second_response)
+    it 'executes the new Gitaly RPC' do
+      expect_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rebase)
+      expect_any_instance_of(Gitlab::GitalyClient::OperationService).not_to receive(:user_rebase)
 
-          repository.rebase(user, merge_request)
+      repository.rebase(user, merge_request)
+    end
 
-          expect(merge_request.reload.rebase_commit_sha).to eq(new_sha)
-        end
+    describe 'rolling back the `rebase_commit_sha`' do
+      let(:new_sha) { Digest::SHA1.hexdigest('foo') }
 
-        it 'does rollback when a PreReceiveError is encountered in the second step' do
-          second_response = double(pre_receive_error: 'my_error', git_error: nil)
-          mock_gitaly(second_response)
+      it 'does not rollback when there are no errors' do
+        second_response = double(pre_receive_error: nil, git_error: nil)
+        mock_gitaly(second_response)
 
-          expect do
-            repository.rebase(user, merge_request)
-          end.to raise_error(Gitlab::Git::PreReceiveError)
+        repository.rebase(user, merge_request)
 
-          expect(merge_request.reload.rebase_commit_sha).to be_nil
-        end
+        expect(merge_request.reload.rebase_commit_sha).to eq(new_sha)
+      end
 
-        it 'does rollback when a GitError is encountered in the second step' do
-          second_response = double(pre_receive_error: nil, git_error: 'git error')
-          mock_gitaly(second_response)
+      it 'does rollback when a PreReceiveError is encountered in the second step' do
+        second_response = double(pre_receive_error: 'my_error', git_error: nil)
+        mock_gitaly(second_response)
 
-          expect do
-            repository.rebase(user, merge_request)
-          end.to raise_error(Gitlab::Git::Repository::GitError)
+        expect do
+          repository.rebase(user, merge_request)
+        end.to raise_error(Gitlab::Git::PreReceiveError)
 
-          expect(merge_request.reload.rebase_commit_sha).to be_nil
-        end
+        expect(merge_request.reload.rebase_commit_sha).to be_nil
+      end
 
-        def mock_gitaly(second_response)
-          responses = [
-            double(rebase_sha: new_sha).as_null_object,
-            second_response
-          ]
+      it 'does rollback when a GitError is encountered in the second step' do
+        second_response = double(pre_receive_error: nil, git_error: 'git error')
+        mock_gitaly(second_response)
 
-          expect_any_instance_of(
-            Gitaly::OperationService::Stub
-          ).to receive(:user_rebase_confirmable).and_return(responses.each)
-        end
-      end
-    end
+        expect do
+          repository.rebase(user, merge_request)
+        end.to raise_error(Gitlab::Git::Repository::GitError)
 
-    context 'when two_step_rebase feature is disabled' do
-      before do
-        stub_feature_flags(two_step_rebase: false)
+        expect(merge_request.reload.rebase_commit_sha).to be_nil
       end
 
-      it_behaves_like 'a method that can rebase successfully'
-
-      it 'executes the deprecated Gitaly RPC' do
-        expect_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:user_rebase)
-        expect_any_instance_of(Gitlab::GitalyClient::OperationService).not_to receive(:rebase)
+      def mock_gitaly(second_response)
+        responses = [
+          double(rebase_sha: new_sha).as_null_object,
+          second_response
+        ]
 
-        repository.rebase(user, merge_request)
+        expect_any_instance_of(
+          Gitaly::OperationService::Stub
+        ).to receive(:user_rebase_confirmable).and_return(responses.each)
       end
     end
   end

spec/services/merge_requests/rebase_service_spec.rb

@@ -71,14 +71,6 @@ describe MergeRequests::RebaseService do
 
     it_behaves_like 'sequence of failure and success'
 
-    context 'with deprecated step rebase feature' do
-      before do
-        stub_feature_flags(two_step_rebase: false)
-      end
-
-      it_behaves_like 'sequence of failure and success'
-    end
-
     context 'when unexpected error occurs' do
       before do
         allow(repository).to receive(:gitaly_operation_client).and_raise('Something went wrong')
@@ -140,21 +132,7 @@ describe MergeRequests::RebaseService do
         end
       end
 
-      context 'when the two_step_rebase feature is enabled' do
-        before do
-          stub_feature_flags(two_step_rebase: true)
-        end
-
-        it_behaves_like 'a service that can execute a successful rebase'
-      end
-
-      context 'when the two_step_rebase feature is disabled' do
-        before do
-          stub_feature_flags(two_step_rebase: false)
-        end
-
-        it_behaves_like 'a service that can execute a successful rebase'
-      end
+      it_behaves_like 'a service that can execute a successful rebase'
 
       context 'when skip_ci flag is set' do
         let(:skip_ci) { true }