Merge branch 'revert-9269c695' into 'master'

Revert mergeability check changes

See merge request gitlab-org/gitlab!84669

app/presenters/merge_request_presenter.rb

@@ -191,17 +191,13 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def mergeable_discussions_state
-    if Feature.enabled?(:change_response_code_merge_status, project, default_enabled: :yaml)
+    # This avoids calling MergeRequest#mergeable_discussions_state without
+    # considering the state of the MR first. If a MR isn't mergeable, we can
+    # safely short-circuit it.
+    if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
       merge_request.mergeable_discussions_state?
     else
-      # This avoids calling MergeRequest#mergeable_discussions_state without
-      # considering the state of the MR first. If a MR isn't mergeable, we can
-      # safely short-circuit it.
-      if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
-        merge_request.mergeable_discussions_state?
-      else
-        false
-      end
+      false
     end
   end
 

app/serializers/merge_request_poll_widget_entity.rb

@@ -33,17 +33,13 @@ class MergeRequestPollWidgetEntity < Grape::Entity
 
   # Booleans
   expose :mergeable_discussions_state?, as: :mergeable_discussions_state do |merge_request|
-    if Feature.enabled?(:change_response_code_merge_status, merge_request.project, default_enabled: :yaml)
+    # This avoids calling MergeRequest#mergeable_discussions_state without
+    # considering the state of the MR first. If a MR isn't mergeable, we can
+    # safely short-circuit it.
+    if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
       merge_request.mergeable_discussions_state?
     else
-      # This avoids calling MergeRequest#mergeable_discussions_state without
-      # considering the state of the MR first. If a MR isn't mergeable, we can
-      # safely short-circuit it.
-      if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
-        merge_request.mergeable_discussions_state?
-      else
-        false
-      end
+      false
     end
   end
 

config/feature_flags/development/change_response_code_merge_status.yml

----
-name: change_response_code_merge_status
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82465/
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356930
-milestone: '14.10'
-type: development
-group: group::code review
-default_enabled: false

doc/api/merge_requests.md

@@ -1629,7 +1629,7 @@ This API returns specific HTTP status codes on failure:
 |:------------|--------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|
 | `401`       | `Unauthorized`                             | This user does not have permission to accept this merge request.                                                                         |
 | `405`       | `Method Not Allowed`                       | The merge request cannot be accepted because it is `Draft`, `Closed`, `Pipeline Pending Completion`, or `Failed`. `Success` is required. |
-| `406`       | `Branch cannot be merged`                  | The merge request can not be merged.                                                                                           |
+| `406`       | `Branch cannot be merged`                  | The branch has conflicts and cannot be merged.                                                                                           |
 | `409`       | `SHA does not match HEAD of source branch` | The provided `sha` parameter does not match the HEAD of the source.                                                                      |
 
 For additional important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes).

ee/app/models/ee/merge_request.rb

@@ -117,8 +117,8 @@ module EE
       super.concat(merge_request_approval_variables)
     end
 
-    override :mergeable_state?
-    def mergeable_state?(skip_ci_check: false, skip_discussions_check: false)
+    override :mergeable?
+    def mergeable?(skip_ci_check: false, skip_discussions_check: false)
       return false unless approved?
       return false if has_denied_policies?
       return false if merge_blocked_by_other_mrs?

ee/spec/models/merge_request_spec.rb

@@ -1155,13 +1155,15 @@ RSpec.describe MergeRequest do
     end
   end
 
-  describe '#mergeable_state?' do
-    subject { merge_request.mergeable_state? }
+  describe '#mergeable?' do
+    subject { merge_request.mergeable? }
 
     context 'when using approvals' do
       let(:user) { create(:user) }
 
       before do
+        allow(merge_request).to receive(:mergeable_state?).and_return(true)
+
         merge_request.target_project.update!(approvals_before_merge: 1)
         project.add_developer(user)
       end
@@ -1214,26 +1216,6 @@ RSpec.describe MergeRequest do
         end
       end
     end
-
-    context 'when blocking merge requests' do
-      before do
-        stub_licensed_features(blocking_merge_requests: true)
-      end
-
-      context 'when the merge request is blocked' do
-        let(:merge_request) { create(:merge_request, :blocked, source_project: project, target_project: project) }
-
-        it 'is not mergeable' do
-          is_expected.to be_falsey
-        end
-      end
-
-      context 'when merge request is not blocked' do
-        it 'is mergeable' do
-          is_expected.to be_truthy
-        end
-      end
-    end
   end
 
   describe '#on_train?' do

ee/spec/requests/api/merge_requests_spec.rb

@@ -178,7 +178,8 @@ RSpec.describe API::MergeRequests do
 
       put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
 
-      expect(response).to have_gitlab_http_status(:method_not_allowed)
+      expect(response).to have_gitlab_http_status(:not_acceptable)
+      expect(json_response['message']).to eq('Branch cannot be merged')
     end
 
     it 'returns 200 if merge request was approved' do

ee/spec/services/ee/merge_requests/refresh_service_spec.rb

@@ -263,7 +263,6 @@ RSpec.describe MergeRequests::RefreshService do
 
     describe 'Pipelines for merge requests', :sidekiq_inline do
       let(:service) { described_class.new(project: project, current_user: current_user) }
-      let(:project) { create(:project, :repository, namespace: group, reset_approvals_on_push: true) }
       let(:current_user) { merge_request.author }
 
       let(:config) do

lib/api/merge_requests.rb

@@ -458,11 +458,7 @@ module API
 
         not_allowed! if !immediately_mergeable && !automatically_mergeable
 
-        if Feature.enabled?(:change_response_code_merge_status, user_project, default_enabled: :yaml)
-          render_api_error!('Branch cannot be merged', 422) unless merge_request.mergeable?(skip_ci_check: automatically_mergeable)
-        else
-          render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable?(skip_ci_check: automatically_mergeable)
-        end
+        render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable?(skip_ci_check: automatically_mergeable)
 
         check_sha_param!(params, merge_request)
 

spec/requests/api/merge_requests_spec.rb

@@ -2554,32 +2554,14 @@ RSpec.describe API::MergeRequests do
       expect(response).to have_gitlab_http_status(:ok)
     end
 
-    context 'when change_response_code_merge_status is enabled' do
-      it "returns 422 if branch can't be merged" do
-        allow_any_instance_of(MergeRequest)
-          .to receive(:can_be_merged?).and_return(false)
+    it "returns 406 if branch can't be merged" do
+      allow_any_instance_of(MergeRequest)
+        .to receive(:can_be_merged?).and_return(false)
 
-        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
-
-        expect(response).to have_gitlab_http_status(:unprocessable_entity)
-        expect(json_response['message']).to eq('Branch cannot be merged')
-      end
-    end
-
-    context 'when change_response_code_merge_status is disabled' do
-      before do
-        stub_feature_flags(change_response_code_merge_status: false)
-      end
-
-      it "returns 406 if branch can't be merged" do
-        allow_any_instance_of(MergeRequest)
-          .to receive(:can_be_merged?).and_return(false)
-
-        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
 
-        expect(response).to have_gitlab_http_status(:not_acceptable)
-        expect(json_response['message']).to eq('Branch cannot be merged')
-      end
+      expect(response).to have_gitlab_http_status(:not_acceptable)
+      expect(json_response['message']).to eq('Branch cannot be merged')
     end
 
     it "returns 405 if merge_request is not open" do

spec/serializers/merge_request_poll_widget_entity_spec.rb

@@ -182,40 +182,4 @@ RSpec.describe MergeRequestPollWidgetEntity do
       end
     end
   end
-
-  describe '#mergeable_discussions_state?' do
-    context 'when change_response_code_merge_status is true' do
-      before do
-        stub_feature_flags(change_response_code_merge_status: true)
-      end
-
-      it 'returns mergeable discussions state' do
-        expect(subject[:mergeable_discussions_state]).to eq(true)
-      end
-    end
-
-    context 'when change_response_code_merge_status is false' do
-      context 'when merge request is in a mergeable state' do
-        before do
-          stub_feature_flags(change_response_code_merge_status: false)
-          allow(resource).to receive(:mergeable_discussions_state?).and_return(true)
-        end
-
-        it 'returns mergeable discussions state' do
-          expect(subject[:mergeable_discussions_state]).to eq(true)
-        end
-      end
-
-      context 'when merge request is not in a mergeable state' do
-        before do
-          stub_feature_flags(change_response_code_merge_status: false)
-          allow(resource).to receive(:mergeable_discussions_state?).and_return(false)
-        end
-
-        it 'returns mergeable discussions state' do
-          expect(subject[:mergeable_discussions_state]).to eq(false)
-        end
-      end
-    end
-  end
 end