Commit 4d801a70 authored by Nick Thomas's avatar Nick Thomas

Inline the new checks into normal update error handling

parent b96dbc67
...@@ -7,7 +7,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -7,7 +7,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
include RendersCommits include RendersCommits
include ToggleAwardEmoji include ToggleAwardEmoji
include IssuableCollections include IssuableCollections
include MarkupHelper
skip_before_action :merge_request, only: [:index, :bulk_update] skip_before_action :merge_request, only: [:index, :bulk_update]
before_action :whitelist_query_limiting, only: [:assign_related_issues, :update] before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
...@@ -123,21 +122,23 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -123,21 +122,23 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
respond_to do |format| respond_to do |format|
format.html do format.html do
check_branch_conflict if @merge_request.errors.present?
if @merge_request.valid?
redirect_to([@merge_request.target_project.namespace.becomes(Namespace), @merge_request.target_project, @merge_request])
else
define_edit_vars define_edit_vars
render :edit render :edit
else
redirect_to project_merge_request_path(@merge_request.target_project, @merge_request)
end end
end end
format.json do format.json do
if merge_request.errors.present?
render json: @merge_request.errors, status: :bad_request
else
render json: serializer.represent(@merge_request, serializer: 'basic') render json: serializer.represent(@merge_request, serializer: 'basic')
end end
end end
end
rescue ActiveRecord::StaleObjectError rescue ActiveRecord::StaleObjectError
define_edit_vars if request.format.html? define_edit_vars if request.format.html?
...@@ -258,12 +259,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -258,12 +259,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@merge_request.check_if_can_be_merged @merge_request.check_if_can_be_merged
end end
def check_branch_conflict
if @merge_request.errors[:validate_branches]
flash[:alert] = markdown(@merge_request.errors[:validate_branches].to_sentence, pipeline: :single_line)
end
end
def merge! def merge!
# Disable the CI check if merge_when_pipeline_succeeds is enabled since we have # Disable the CI check if merge_when_pipeline_succeeds is enabled since we have
# to wait until CI completes to know # to wait until CI completes to know
......
...@@ -539,15 +539,26 @@ class MergeRequest < ActiveRecord::Base ...@@ -539,15 +539,26 @@ class MergeRequest < ActiveRecord::Base
def validate_branches def validate_branches
if target_project == source_project && target_branch == source_branch 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 end
if opened? 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 = target_project
similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id .merge_requests
if similar_mrs.any? .where(source_branch: source_branch, target_branch: target_branch)
errors.add :validate_branches, .where(source_project_id: source_project&.id)
"Another open Merge Request already exists for this source branch: !#{similar_mrs.first.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 end
end end
......
...@@ -291,14 +291,16 @@ describe Projects::MergeRequestsController do ...@@ -291,14 +291,16 @@ describe Projects::MergeRequestsController do
it_behaves_like 'update invalid issuable', MergeRequest it_behaves_like 'update invalid issuable', MergeRequest
end end
context 'there are two merge requests with the same source branch' do context 'two merge requests with the same source branch' do
it "does not allow to reopen a closed merge request if another one is open" do it 'does not allow a closed merge request to be reopened if another one is open' do
merge_request.close! merge_request.close!
create(:merge_request, source_project: merge_request.source_project, source_branch: merge_request.source_branch) create(:merge_request, source_project: merge_request.source_project, source_branch: merge_request.source_branch)
update_merge_request(state_event: 'reopen') update_merge_request(state_event: 'reopen')
expect(controller).to set_flash[:alert].to(/Another open Merge Request already exists for this source branch/) 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 expect(merge_request.reload).to be_closed
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment