diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-01-23 19:08:10 -0200 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-01-23 19:08:24 -0200 |
commit | dc84313e73da8454cd043dbd235a2b552658d8ae (patch) | |
tree | 374c8dcff81fea618b45948b5092c56f914903ca | |
parent | c4904d570c2c5094d2d2bfee5d243b39556fdc89 (diff) | |
download | gitlab-ce-dc84313e73da8454cd043dbd235a2b552658d8ae.tar.gz |
Return more consistent values for merge_status on MR API
-rw-r--r-- | app/models/merge_request.rb | 7 | ||||
-rw-r--r-- | changelogs/unreleased/osw-updates-merge-status-on-api-actions.yml | 5 | ||||
-rw-r--r-- | lib/api/entities.rb | 11 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 17 |
4 files changed, 39 insertions, 1 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8028ff3875b..4addf42325d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -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? diff --git a/changelogs/unreleased/osw-updates-merge-status-on-api-actions.yml b/changelogs/unreleased/osw-updates-merge-status-on-api-actions.yml new file mode 100644 index 00000000000..cb3b1590acf --- /dev/null +++ b/changelogs/unreleased/osw-updates-merge-status-on-api-actions.yml @@ -0,0 +1,5 @@ +--- +title: Update merge_status on MR creation/update/get on the API +merge_request: +author: +type: fixed diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3f4b62dc1b2..6af379451a6 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c76f32b3989..7cc237937f3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -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 |