From d20a28c486946dcabb4446b69278444a104f2b0a Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Wed, 13 May 2020 15:47:26 -0700
Subject: [PATCH] Clear merge request error on push to source branch

If a merge error happens for some reason to a merge request, that error
will persist even after a new push to the repository. This makes it
impossible for a user to merge a merge request. The only workaround is
to close the existing merge request and open a new one.

If a user pushes to the source branch, we should clear out the merge
error since the diff would likely have changed. If a user pushes to the
target branch, we still want to keep the error present so the user can
take action. Clearing it out in that case would likely lead to confusion
because the error would disappear on unrelated changes.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/34186
---
 .../merge_requests/refresh_service.rb         |  4 +++
 .../sh-fix-merge-request-stickiness.yml       |  5 ++++
 .../merge_requests/refresh_service_spec.rb    | 25 +++++++++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 changelogs/unreleased/sh-fix-merge-request-stickiness.yml

diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index c6e1651fa262f7..56a91fa0305b3e 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -115,6 +115,10 @@ def reload_merge_requests
       filter_merge_requests(merge_requests).each do |merge_request|
         if branch_and_project_match?(merge_request) || @push.force_push?
           merge_request.reload_diff(current_user)
+          # Clear existing merge error if the push were directed at the
+          # source branch. Clearing the error when the target branch
+          # changes will hide the error from the user.
+          merge_request.merge_error = nil
         elsif merge_request.merge_request_diff.includes_any_commits?(push_commit_ids)
           merge_request.reload_diff(current_user)
         end
diff --git a/changelogs/unreleased/sh-fix-merge-request-stickiness.yml b/changelogs/unreleased/sh-fix-merge-request-stickiness.yml
new file mode 100644
index 00000000000000..796648f7eded49
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-merge-request-stickiness.yml
@@ -0,0 +1,5 @@
+---
+title: Clear merge request error on push to source branch
+merge_request: 32001
+author:
+type: fixed
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index de7b933f89d9a7..3e252212cdbfdf 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -94,6 +94,31 @@
         expect(@fork_build_failed_todo).to be_done
       end
 
+      context 'when a merge error exists' do
+        let(:error_message) { 'This is a merge error' }
+
+        before do
+          @merge_request = create(:merge_request,
+            source_project: @project,
+            source_branch: 'feature',
+            target_branch: 'master',
+            target_project: @project,
+            merge_error: error_message)
+        end
+
+        it 'clears merge errors when pushing to the source branch' do
+          expect { refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') }
+            .to change { @merge_request.reload.merge_error }
+            .from(error_message)
+            .to(nil)
+        end
+
+        it 'does not clear merge errors when pushing to the target branch' do
+          expect { refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') }
+            .not_to change { @merge_request.reload.merge_error }
+        end
+      end
+
       it 'reloads source branch MRs memoization' do
         refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
 
-- 
GitLab