Commit 920fa740 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'revert-28513' into 'master'

Revert "Automatically update MR merge-ref along merge status"

See merge request gitlab-org/gitlab-ce!29492
parents 821b8058 00a273d3
...@@ -33,7 +33,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -33,7 +33,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
def show def show
close_merge_request_if_no_source_project close_merge_request_if_no_source_project
@merge_request.check_mergeability mark_merge_request_mergeable
respond_to do |format| respond_to do |format|
format.html do format.html do
...@@ -251,6 +251,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -251,6 +251,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@merge_request.has_no_commits? && !@merge_request.target_branch_exists? @merge_request.has_no_commits? && !@merge_request.target_branch_exists?
end end
def mark_merge_request_mergeable
@merge_request.check_if_can_be_merged
end
def merge! def merge!
# Disable the CI check if auto_merge_strategy is specified since we have # Disable the CI check if auto_merge_strategy is specified since we have
# to wait until CI completes to know # to wait until CI completes to know
......
...@@ -725,16 +725,19 @@ class MergeRequest < ApplicationRecord ...@@ -725,16 +725,19 @@ class MergeRequest < ApplicationRecord
MergeRequests::ReloadDiffsService.new(self, current_user).execute MergeRequests::ReloadDiffsService.new(self, current_user).execute
end end
def check_mergeability
MergeRequests::MergeabilityCheckService.new(self).execute
end
# rubocop: enable CodeReuse/ServiceClass # rubocop: enable CodeReuse/ServiceClass
# Returns boolean indicating the merge_status should be rechecked in order to def check_if_can_be_merged
# switch to either can_be_merged or cannot_be_merged. return unless self.class.state_machines[:merge_status].check_state?(merge_status) && Gitlab::Database.read_write?
def recheck_merge_status?
self.class.state_machines[:merge_status].check_state?(merge_status) can_be_merged =
!broken? && project.repository.can_be_merged?(diff_head_sha, target_branch)
if can_be_merged
mark_as_mergeable
else
mark_as_unmergeable
end
end end
def merge_event def merge_event
...@@ -760,7 +763,7 @@ class MergeRequest < ApplicationRecord ...@@ -760,7 +763,7 @@ class MergeRequest < ApplicationRecord
def mergeable?(skip_ci_check: false) def mergeable?(skip_ci_check: false)
return false unless mergeable_state?(skip_ci_check: skip_ci_check) return false unless mergeable_state?(skip_ci_check: skip_ci_check)
check_mergeability check_if_can_be_merged
can_be_merged? && !should_be_rebased? can_be_merged? && !should_be_rebased?
end end
...@@ -775,6 +778,15 @@ class MergeRequest < ApplicationRecord ...@@ -775,6 +778,15 @@ class MergeRequest < ApplicationRecord
true true
end end
def mergeable_to_ref?
return false unless mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
# Given the `merge_ref_path` will have the same
# state the `target_branch` would have. Ideally
# we need to check if it can be merged to it.
project.repository.can_be_merged?(diff_head_sha, target_branch)
end
def ff_merge_possible? def ff_merge_possible?
project.repository.ancestor?(target_branch_sha, diff_head_sha) project.repository.ancestor?(target_branch_sha, diff_head_sha)
end end
...@@ -1087,12 +1099,6 @@ class MergeRequest < ApplicationRecord ...@@ -1087,12 +1099,6 @@ class MergeRequest < ApplicationRecord
target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path) target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path)
end end
# Returns the current merge-ref HEAD commit.
#
def merge_ref_head
project.repository.commit(merge_ref_path)
end
def ref_path def ref_path
"refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head" "refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head"
end end
......
...@@ -20,14 +20,20 @@ module MergeRequests ...@@ -20,14 +20,20 @@ module MergeRequests
raise_error('Conflicts detected during merge') unless commit_id raise_error('Conflicts detected during merge') unless commit_id
success(commit_id: commit_id) commit = project.commit(commit_id)
rescue MergeError, ArgumentError => error target_id, source_id = commit.parent_ids
success(commit_id: commit.id,
target_id: target_id,
source_id: source_id)
rescue MergeError => error
error(error.message) error(error.message)
end end
private private
def validate! def validate!
authorization_check!
error_check! error_check!
end end
...@@ -37,13 +43,21 @@ module MergeRequests ...@@ -37,13 +43,21 @@ module MergeRequests
error = error =
if !hooks_validation_pass?(merge_request) if !hooks_validation_pass?(merge_request)
hooks_validation_error(merge_request) hooks_validation_error(merge_request)
elsif source.blank? elsif !@merge_request.mergeable_to_ref?
"Merge request is not mergeable to #{target_ref}"
elsif !source
'No source for merge' 'No source for merge'
end end
raise_error(error) if error raise_error(error) if error
end end
def authorization_check!
unless Ability.allowed?(current_user, :admin_merge_request, project)
raise_error("You are not allowed to merge to this ref")
end
end
def target_ref def target_ref
merge_request.merge_ref_path merge_request.merge_ref_path
end end
......
# frozen_string_literal: true
module MergeRequests
class MergeabilityCheckService < ::BaseService
include Gitlab::Utils::StrongMemoize
delegate :project, to: :@merge_request
delegate :repository, to: :project
def initialize(merge_request)
@merge_request = merge_request
end
# Updates the MR merge_status. Whenever it switches to a can_be_merged state,
# the merge-ref is refreshed.
#
# Returns a ServiceResponse indicating merge_status is/became can_be_merged
# and the merge-ref is synced. Success in case of being/becoming mergeable,
# error otherwise.
def execute
return ServiceResponse.error(message: 'Invalid argument') unless merge_request
return ServiceResponse.error(message: 'Unsupported operation') if Gitlab::Database.read_only?
update_merge_status
unless merge_request.can_be_merged?
return ServiceResponse.error(message: 'Merge request is not mergeable')
end
unless payload.fetch(:merge_ref_head)
return ServiceResponse.error(message: 'Merge ref was not found')
end
ServiceResponse.success(payload: payload)
end
private
attr_reader :merge_request
def payload
strong_memoize(:payload) do
{
merge_ref_head: merge_ref_head_payload
}
end
end
def merge_ref_head_payload
commit = merge_request.merge_ref_head
return unless commit
target_id, source_id = commit.parent_ids
{
commit_id: commit.id,
source_id: source_id,
target_id: target_id
}
end
def update_merge_status
return unless merge_request.recheck_merge_status?
if can_git_merge?
merge_to_ref && merge_request.mark_as_mergeable
else
merge_request.mark_as_unmergeable
end
end
def can_git_merge?
!merge_request.broken? && repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch)
end
def merge_to_ref
result = MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
result[:status] == :success
end
end
end
# frozen_string_literal: true # frozen_string_literal: true
class ServiceResponse class ServiceResponse
def self.success(message: nil, payload: {}) def self.success(message: nil)
new(status: :success, message: message, payload: payload) new(status: :success, message: message)
end end
def self.error(message:, payload: {}, http_status: nil) def self.error(message:, http_status: nil)
new(status: :error, message: message, payload: payload, http_status: http_status) new(status: :error, message: message, http_status: http_status)
end end
attr_reader :status, :message, :http_status, :payload attr_reader :status, :message, :http_status
def initialize(status:, message: nil, payload: {}, http_status: nil) def initialize(status:, message: nil, http_status: nil)
self.status = status self.status = status
self.message = message self.message = message
self.payload = payload
self.http_status = http_status self.http_status = http_status
end end
...@@ -28,5 +27,5 @@ class ServiceResponse ...@@ -28,5 +27,5 @@ class ServiceResponse
private private
attr_writer :status, :message, :http_status, :payload attr_writer :status, :message, :http_status
end end
---
title: Sync merge ref upon mergeability check
merge_request: 28513
author:
type: added
...@@ -1191,29 +1191,33 @@ Parameters: ...@@ -1191,29 +1191,33 @@ Parameters:
} }
``` ```
## Returns the up to date merge-ref HEAD commit ## Merge to default merge ref path
Merge the changes between the merge request source and target branches into `refs/merge-requests/:iid/merge` Merge the changes between the merge request source and target branches into `refs/merge-requests/:iid/merge`
ref, of the target project repository, if possible. This ref will have the state the target branch would have if ref, of the target project repository. This ref will have the state the target branch would have if
a regular merge action was taken. a regular merge action was taken.
This is not a regular merge action given it doesn't change the merge request target branch state in any manner. This is not a regular merge action given it doesn't change the merge request state in any manner.
This ref (`refs/merge-requests/:iid/merge`) isn't necessarily overwritten when submitting This ref (`refs/merge-requests/:iid/merge`) is **always** overwritten when submitting
requests to this API, though it'll make sure the ref has the latest possible state. requests to this API, so none of its state is kept or used in the process.
If the merge request has conflicts, is empty or already merged, you'll get a `400` and a descriptive error message. If the merge request has conflicts, is empty or already merged,
you'll get a `400` and a descriptive error message. If you don't have permissions to do so,
you'll get a `403`.
It returns the HEAD commit of `refs/merge-requests/:iid/merge` in the response body in case of `200`. It returns the HEAD commit of `refs/merge-requests/:iid/merge` in the response body in
case of `200`.
``` ```
GET /projects/:id/merge_requests/:merge_request_iid/merge_ref PUT /projects/:id/merge_requests/:merge_request_iid/merge_to_ref
``` ```
Parameters: Parameters:
- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
- `merge_request_iid` (required) - Internal ID of MR - `merge_request_iid` (required) - Internal ID of MR
- `merge_commit_message` (optional) - Custom merge commit message
```json ```json
{ {
......
...@@ -701,7 +701,7 @@ module API ...@@ -701,7 +701,7 @@ module API
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/42344 for more # See https://gitlab.com/gitlab-org/gitlab-ce/issues/42344 for more
# information. # information.
expose :merge_status do |merge_request| expose :merge_status do |merge_request|
merge_request.check_mergeability merge_request.check_if_can_be_merged
merge_request.merge_status merge_request.merge_status
end end
expose :diff_head_sha, as: :sha expose :diff_head_sha, as: :sha
......
...@@ -397,16 +397,28 @@ module API ...@@ -397,16 +397,28 @@ module API
present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
end end
desc 'Returns the up to date merge-ref HEAD commit' desc 'Merge a merge request to its default temporary merge ref path'
get ':id/merge_requests/:merge_request_iid/merge_ref' do params do
optional :merge_commit_message, type: String, desc: 'Custom merge commit message'
end
put ':id/merge_requests/:merge_request_iid/merge_to_ref' do
merge_request = find_project_merge_request(params[:merge_request_iid]) merge_request = find_project_merge_request(params[:merge_request_iid])
result = ::MergeRequests::MergeabilityCheckService.new(merge_request).execute authorize! :admin_merge_request, user_project
merge_params = {
commit_message: params[:merge_commit_message]
}
result = ::MergeRequests::MergeToRefService
.new(merge_request.target_project, current_user, merge_params)
.execute(merge_request)
if result.success? if result[:status] == :success
present :commit_id, result.payload.dig(:merge_ref_head, :commit_id) present result.slice(:commit_id), 200
else else
render_api_error!(result.message, 400) http_status = result[:http_status] || 400
render_api_error!(result[:message], http_status)
end end
end end
......
...@@ -1996,6 +1996,57 @@ describe MergeRequest do ...@@ -1996,6 +1996,57 @@ describe MergeRequest do
end end
end end
describe '#check_if_can_be_merged' do
let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) }
shared_examples 'checking if can be merged' do
context 'when it is not broken and has no conflicts' do
before do
allow(subject).to receive(:broken?) { false }
allow(project.repository).to receive(:can_be_merged?).and_return(true)
end
it 'is marked as mergeable' do
expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged')
end
end
context 'when broken' do
before do
allow(subject).to receive(:broken?) { true }
allow(project.repository).to receive(:can_be_merged?).and_return(false)
end
it 'becomes unmergeable' do
expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged')
end
end
context 'when it has conflicts' do
before do
allow(subject).to receive(:broken?) { false }
allow(project.repository).to receive(:can_be_merged?).and_return(false)
end
it 'becomes unmergeable' do
expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged')
end
end
end
context 'when merge_status is unchecked' do
subject { create(:merge_request, source_project: project, merge_status: :unchecked) }
it_behaves_like 'checking if can be merged'
end
context 'when merge_status is unchecked' do
subject { create(:merge_request, source_project: project, merge_status: :cannot_be_merged_recheck) }
it_behaves_like 'checking if can be merged'
end
end
describe '#mergeable?' do describe '#mergeable?' do
let(:project) { create(:project) } let(:project) { create(:project) }
...@@ -2009,7 +2060,7 @@ describe MergeRequest do ...@@ -2009,7 +2060,7 @@ describe MergeRequest do
it 'return true if #mergeable_state? is true and the MR #can_be_merged? is true' do it 'return true if #mergeable_state? is true and the MR #can_be_merged? is true' do
allow(subject).to receive(:mergeable_state?) { true } allow(subject).to receive(:mergeable_state?) { true }
expect(subject).to receive(:check_mergeability) expect(subject).to receive(:check_if_can_be_merged)
expect(subject).to receive(:can_be_merged?) { true } expect(subject).to receive(:can_be_merged?) { true }
expect(subject.mergeable?).to be_truthy expect(subject.mergeable?).to be_truthy
...@@ -2023,7 +2074,7 @@ describe MergeRequest do ...@@ -2023,7 +2074,7 @@ describe MergeRequest do
it 'checks if merge request can be merged' do it 'checks if merge request can be merged' do
allow(subject).to receive(:mergeable_ci_state?) { true } allow(subject).to receive(:mergeable_ci_state?) { true }
expect(subject).to receive(:check_mergeability) expect(subject).to receive(:check_if_can_be_merged)
subject.mergeable? subject.mergeable?
end end
...@@ -3091,6 +3142,38 @@ describe MergeRequest do ...@@ -3091,6 +3142,38 @@ describe MergeRequest do
end end
end end
describe '#mergeable_to_ref?' do
it 'returns true when merge request is mergeable' do
subject = create(:merge_request)
expect(subject.mergeable_to_ref?).to be(true)
end
it 'returns false when merge request is already merged' do
subject = create(:merge_request, :merged)
expect(subject.mergeable_to_ref?).to be(false)
end
it 'returns false when merge request is closed' do
subject = create(:merge_request, :closed)
expect(subject.mergeable_to_ref?).to be(false)
end
it 'returns false when merge request is work in progress' do
subject = create(:merge_request, title: 'WIP: The feature')
expect(subject.mergeable_to_ref?).to be(false)
end
it 'returns false when merge request has no commits' do
subject = create(:merge_request, source_branch: 'empty-branch', target_branch: 'master')
expect(subject.mergeable_to_ref?).to be(false)
end
end
describe '#merge_participants' do describe '#merge_participants' do
it 'contains author' do it 'contains author' do
expect(subject.merge_participants).to eq([subject.author]) expect(subject.merge_participants).to eq([subject.author])
......
...@@ -1546,67 +1546,54 @@ describe API::MergeRequests do ...@@ -1546,67 +1546,54 @@ describe API::MergeRequests do
end end
end end
describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref" do describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge_to_ref" do
before do let(:pipeline) { create(:ci_pipeline_without_jobs) }
merge_request.mark_as_unchecked!
end
let(:merge_request_iid) { merge_request.iid }
let(:url) do let(:url) do
"/projects/#{project.id}/merge_requests/#{merge_request_iid}/merge_ref" "/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge_to_ref"
end end
it 'returns the generated ID from the merge service in case of success' do it 'returns the generated ID from the merge service in case of success' do
get api(url, user) put api(url, user), params: { merge_commit_message: 'Custom message' }
commit = project.commit(json_response['commit_id'])
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['commit_id']).to eq(merge_request.merge_ref_head.sha) expect(json_response['commit_id']).to be_present
expect(commit.message).to eq('Custom message')
end end
it "returns 400 if branch can't be merged" do it "returns 400 if branch can't be merged" do
merge_request.update!(merge_status: 'cannot_be_merged') merge_request.update!(state: 'merged')
get api(url, user) put api(url, user)
expect(response).to have_gitlab_http_status(400) expect(response).to have_gitlab_http_status(400)
expect(json_response['message']).to eq('Merge request is not mergeable') expect(json_response['message'])
.to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}")
end end
context 'when user has no access to the MR' do it 'returns 403 if user has no permissions to merge to the ref' do
let(:project) { create(:project, :private) } user2 = create(:user)
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } project.add_reporter(user2)
it 'returns 404' do
project.add_guest(user)
get api(url, user) put api(url, user2)
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(403)
expect(json_response['message']).to eq('404 Not found') expect(json_response['message']).to eq('403 Forbidden')
end
end end
context 'when invalid merge request IID' do it 'returns 404 for an invalid merge request IID' do
let(:merge_request_iid) { '12345' } put api("/projects/#{project.id}/merge_requests/12345/merge_to_ref", user)
it 'returns 404' do
get api(url, user)
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
end
context 'when merge request ID is used instead IID' do it "returns 404 if the merge request id is used instead of iid" do
let(:merge_request_iid) { merge_request.id } put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
it 'returns 404' do
get api(url, user)
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
end end
end
describe "PUT /projects/:id/merge_requests/:merge_request_iid" do describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
context 'updates force_remove_source_branch properly' do context 'updates force_remove_source_branch properly' do
......
...@@ -32,8 +32,10 @@ describe MergeRequests::MergeToRefService do ...@@ -32,8 +32,10 @@ describe MergeRequests::MergeToRefService do
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
expect(result[:commit_id]).to be_present expect(result[:commit_id]).to be_present
expect(result[:source_id]).to eq(merge_request.source_branch_sha)
expect(result[:target_id]).to eq(merge_request.target_branch_sha)
expect(repository.ref_exists?(target_ref)).to be(true) expect(repository.ref_exists?(target_ref)).to be(true)
expect(ref_head.sha).to eq(result[:commit_id]) expect(ref_head.id).to eq(result[:commit_id])
end end
end end
...@@ -70,6 +72,10 @@ describe MergeRequests::MergeToRefService do ...@@ -70,6 +72,10 @@ describe MergeRequests::MergeToRefService do
let(:merge_request) { create(:merge_request, :simple) } let(:merge_request) { create(:merge_request, :simple) }
let(:project) { merge_request.project } let(:project) { merge_request.project }
before do
project.add_maintainer(user)
end
describe '#execute' do describe '#execute' do
let(:service) do let(:service) do
described_class.new(project, user, commit_message: 'Awesome message', described_class.new(project, user, commit_message: 'Awesome message',
...@@ -86,12 +92,6 @@ describe MergeRequests::MergeToRefService do ...@@ -86,12 +92,6 @@ describe MergeRequests::MergeToRefService do
it_behaves_like 'successfully evaluates pre-condition checks' it_behaves_like 'successfully evaluates pre-condition checks'
context 'commit history comparison with regular MergeService' do context 'commit history comparison with regular MergeService' do
before do
# The merge service needs an authorized user while merge-to-ref
# doesn't.
project.add_maintainer(user)
end
let(:merge_ref_service) do let(:merge_ref_service) do
described_class.new(project, user, {}) described_class.new(project, user, {})
end end
...@@ -136,9 +136,9 @@ describe MergeRequests::MergeToRefService do ...@@ -136,9 +136,9 @@ describe MergeRequests::MergeToRefService do
let(:merge_method) { :merge } let(:merge_method) { :merge }
it 'returns error' do it 'returns error' do
allow(project).to receive_message_chain(:repository, :merge_to_ref) { nil } allow(merge_request).to receive(:mergeable_to_ref?) { false }
error_message = 'Conflicts detected during merge' error_message = "Merge request is not mergeable to #{merge_request.merge_ref_path}"
result = service.execute(merge_request) result = service.execute(merge_request)
...@@ -170,5 +170,28 @@ describe MergeRequests::MergeToRefService do ...@@ -170,5 +170,28 @@ describe MergeRequests::MergeToRefService do
it { expect(todo).not_to be_done } it { expect(todo).not_to be_done }
end end
context 'when merge request is WIP state' do
it 'fails to merge' do
merge_request = create(:merge_request, title: 'WIP: The feature')
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}")
end
end
it 'returns error when user has no authorization to admin the merge request' do
unauthorized_user = create(:user)
project.add_reporter(unauthorized_user)
service = described_class.new(project, unauthorized_user)
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('You are not allowed to merge to this ref')
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::MergeabilityCheckService do
shared_examples_for 'unmergeable merge request' do
it 'updates or keeps merge status as cannot_be_merged' do
subject
expect(merge_request.merge_status).to eq('cannot_be_merged')
end
it 'does not change the merge ref HEAD' do
expect { subject }.not_to change(merge_request, :merge_ref_head)
end
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result).to be_error
end
end
shared_examples_for 'mergeable merge request' do
it 'updates or keeps merge status as can_be_merged' do
subject
expect(merge_request.merge_status).to eq('can_be_merged')
end
it 'updates the merge ref' do
expect { subject }.to change(merge_request, :merge_ref_head).from(nil)
end
it 'returns ServiceResponse.success' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result).to be_success
end
it 'ServiceResponse has merge_ref_head payload' do
result = subject
expect(result.payload.keys).to contain_exactly(:merge_ref_head)
expect(result.payload[:merge_ref_head].keys)
.to contain_exactly(:commit_id, :target_id, :source_id)
end
end
describe '#execute' do
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, merge_status: :unchecked, source_project: project, target_project: project) }
let(:repo) { project.repository }
subject { described_class.new(merge_request).execute }
before do
project.add_developer(merge_request.author)
end
it_behaves_like 'mergeable merge request'
context 'when multiple calls to the service' do
it 'returns success' do
subject
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.success?).to be(true)
end
it 'second call does not change the merge-ref' do
expect { subject }.to change(merge_request, :merge_ref_head).from(nil)
expect { subject }.not_to change(merge_request, :merge_ref_head)
end
end
context 'when broken' do
before do
allow(merge_request).to receive(:broken?) { true }
allow(project.repository).to receive(:can_be_merged?) { false }
end
it_behaves_like 'unmergeable merge request'
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.error?).to be(true)
expect(result.message).to eq('Merge request is not mergeable')
end
end
context 'when it has conflicts' do
before do
allow(merge_request).to receive(:broken?) { false }
allow(project.repository).to receive(:can_be_merged?) { false }
end
it_behaves_like 'unmergeable merge request'
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.error?).to be(true)
expect(result.message).to eq('Merge request is not mergeable')
end
end
context 'when MR cannot be merged and has no merge ref' do
before do
merge_request.mark_as_unmergeable!
end
it_behaves_like 'unmergeable merge request'
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.error?).to be(true)
expect(result.message).to eq('Merge request is not mergeable')
end
end
context 'when MR cannot be merged and has outdated merge ref' do
before do
MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
merge_request.mark_as_unmergeable!
end
it_behaves_like 'unmergeable merge request'
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.error?).to be(true)
expect(result.message).to eq('Merge request is not mergeable')
end
end
context 'when merge request is not given' do
subject { described_class.new(nil).execute }
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.message).to eq('Invalid argument')
end
end
context 'when read only DB' do
it 'returns ServiceResponse.error' do
allow(Gitlab::Database).to receive(:read_only?) { true }
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.message).to eq('Unsupported operation')
end
end
context 'when MR is mergeable but merge-ref does not exists' do
before do
merge_request.mark_as_mergeable!
end
it 'keeps merge status as can_be_merged' do
expect { subject }.not_to change(merge_request, :merge_status).from('can_be_merged')
end
it 'returns ServiceResponse.error' do
result = subject
expect(result).to be_a(ServiceResponse)
expect(result.error?).to be(true)
expect(result.message).to eq('Merge ref was not found')
end
end
end
end
...@@ -16,13 +16,6 @@ describe ServiceResponse do ...@@ -16,13 +16,6 @@ describe ServiceResponse do
expect(response).to be_success expect(response).to be_success
expect(response.message).to eq('Good orange') expect(response.message).to eq('Good orange')
end end
it 'creates a successful response with payload' do
response = described_class.success(payload: { good: 'orange' })
expect(response).to be_success
expect(response.payload).to eq(good: 'orange')
end
end end
describe '.error' do describe '.error' do
...@@ -40,15 +33,6 @@ describe ServiceResponse do ...@@ -40,15 +33,6 @@ describe ServiceResponse do
expect(response.message).to eq('Bad apple') expect(response.message).to eq('Bad apple')
expect(response.http_status).to eq(400) expect(response.http_status).to eq(400)
end end
it 'creates a failed response with payload' do
response = described_class.error(message: 'Bad apple',
payload: { bad: 'apple' })
expect(response).to be_error
expect(response.message).to eq('Bad apple')
expect(response.payload).to eq(bad: 'apple')
end
end end
describe '#success?' do describe '#success?' do
......
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