Merge branch '22548-reopen-error-message' into 'master'

Improve error message when attempting to reopen MR and there's a new open MR for the same branch

Closes #22548

See merge request gitlab-org/gitlab-ce!22326

5 files changed, 55 insertions, 16 deletions

diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue
index 841fcec96e8..ce56beb1e6b 100644
--- a/app/assets/javascripts/notes/components/comment_form.vue
+++ b/app/assets/javascripts/notes/components/comment_form.vue
@@ -247,15 +247,19 @@ Please check your network connection and try again.`;
       } else {
         this.reopenIssue()
           .then(() => this.enableButton())
-          .catch(() => {
+          .catch(({ data }) => {
             this.enableButton();
             this.toggleStateButtonLoading(false);
-            Flash(
-              sprintf(
-                __('Something went wrong while reopening the %{issuable}. Please try again later'),
-                { issuable: this.noteableDisplayName },
-              ),
+            let errorMessage = sprintf(
+              __('Something went wrong while reopening the %{issuable}. Please try again later'),
+              { issuable: this.noteableDisplayName },
             );
+
+            if (data) {
+              errorMessage = Object.values(data).join('\n');
+            }
+
+            Flash(errorMessage);
           });
       }
     },
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index d521db79f85..da9316d5f22 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -122,17 +122,21 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
     respond_to do |format|
       format.html do
-        if @merge_request.valid?
-          redirect_to([@merge_request.target_project.namespace.becomes(Namespace), @merge_request.target_project, @merge_request])
-        else
+        if @merge_request.errors.present?
           define_edit_vars
 
           render :edit
+        else
+          redirect_to project_merge_request_path(@merge_request.target_project, @merge_request)
         end
       end
 
       format.json do
-        render json: serializer.represent(@merge_request, serializer: 'basic')
+        if merge_request.errors.present?
+          render json: @merge_request.errors, status: :bad_request
+        else
+          render json: serializer.represent(@merge_request, serializer: 'basic')
+        end
       end
     end
   rescue ActiveRecord::StaleObjectError
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index d0811a715bc..861211ffc0a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -539,15 +539,26 @@ class MergeRequest < ActiveRecord::Base
 
   def validate_branches
     if target_project == source_project && target_branch == source_branch
-      errors.add :branch_conflict, "You can not use same project/branch for source and target"
+      errors.add :branch_conflict, "You can't use same project/branch for source and target"
+      return
     end
 
     if opened?
-      similar_mrs = self.target_project.merge_requests.where(source_branch: source_branch, target_branch: target_branch, source_project_id: source_project.try(:id)).opened
-      similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id
-      if similar_mrs.any?
-        errors.add :validate_branches,
-                   "Cannot Create: This merge request already exists: #{similar_mrs.pluck(:title)}"
+      similar_mrs = target_project
+        .merge_requests
+        .where(source_branch: source_branch, target_branch: target_branch)
+        .where(source_project_id: source_project&.id)
+        .opened
+
+      similar_mrs = similar_mrs.where.not(id: id) if persisted?
+
+      conflict = similar_mrs.first
+
+      if conflict.present?
+        errors.add(
+          :validate_branches,
+          "Another open merge request already exists for this source branch: #{conflict.to_reference}"
+        )
       end
     end
   end
diff --git a/changelogs/unreleased/22548-reopen-error-message.yml b/changelogs/unreleased/22548-reopen-error-message.yml
new file mode 100644
index 00000000000..79c20eccb12
--- /dev/null
+++ b/changelogs/unreleased/22548-reopen-error-message.yml
@@ -0,0 +1,6 @@
+---
+title: Show error message when attempting to reopen an MR and there is an open MR
+  for the same branch
+merge_request: 16447
+author: Akos Gyimesi
+type: fixed
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index e62523c65c9..7f15da859e5 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -290,6 +290,20 @@ describe Projects::MergeRequestsController do
 
       it_behaves_like 'update invalid issuable', MergeRequest
     end
+
+    context 'two merge requests with the same source branch' do
+      it 'does not allow a closed merge request to be reopened if another one is open' do
+        merge_request.close!
+        create(:merge_request, source_project: merge_request.source_project, source_branch: merge_request.source_branch)
+
+        update_merge_request(state_event: 'reopen')
+
+        errors = assigns[:merge_request].errors
+
+        expect(errors[:validate_branches]).to include(/Another open merge request already exists for this source branch/)
+        expect(merge_request.reload).to be_closed
+      end
+    end
   end
 
   describe 'POST merge' do