Commit dc84313e authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Return more consistent values for merge_status on MR API

parent c4904d57
......@@ -56,6 +56,7 @@ class MergeRequest < ActiveRecord::Base
after_create :ensure_merge_request_diff, unless: :importing?
after_update :clear_memoized_shas
after_update :reload_diff_if_branch_changed
after_update :mark_as_unchecked_if_target_branch_changed
# When this attribute is true some MR validation is ignored
# It allows us to close or modify broken merge requests
......@@ -561,6 +562,12 @@ class MergeRequest < ActiveRecord::Base
end
end
def mark_as_unchecked_if_target_branch_changed
return unless target_branch_changed?
mark_as_unchecked
end
def reload_diff(current_user = nil)
return unless open?
......
---
title: Update merge_status on MR creation/update/get on the API
merge_request:
author:
type: fixed
......@@ -507,7 +507,16 @@ module API
expose :work_in_progress?, as: :work_in_progress
expose :milestone, using: Entities::Milestone
expose :merge_when_pipeline_succeeds
expose :merge_status
# Ideally we should deprecate `MergeRequest#merge_status` exposure and
# use `MergeRequest#mergeable?` instead (boolean).
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/42344 for more
# information.
expose :merge_status do |merge_request|
# In order to avoid having a breaking change for users, we keep returning the
# expected values from MergeRequest#merge_status state machine.
merge_request.mergeable? ? 'can_be_merged' : 'cannot_be_merged'
end
expose :diff_head_sha, as: :sha
expose :merge_commit_sha
expose :user_notes_count
......
......@@ -77,6 +77,23 @@ describe MergeRequest do
expect(MergeRequest::Metrics.count).to eq(1)
end
end
describe '#mark_as_unchecked_if_target_branch_changed' do
let(:merge_request) { create(:merge_request, merge_status: :can_be_merged) }
it 'marks MR as unchecked if target_branch changes' do
expect { merge_request.update!(target_branch: 'bar') }
.to change(merge_request, :merge_status)
.from('can_be_merged')
.to('unchecked')
end
it 'does not marks MR as unchecked when target_branch does not changes' do
expect { merge_request.update!(title: 'foo') }
.not_to change(merge_request, :merge_status)
.from('can_be_merged')
end
end
end
describe 'respond to' 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