diff options
author | Rémy Coutable <remy@rymai.me> | 2018-01-25 13:51:37 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-01-25 13:51:37 +0000 |
commit | d7d6bad11387ecc8efadb6c94d1a94c95b63f086 (patch) | |
tree | f663bdddfad65136f0cf066c7c301738c91dff0a | |
parent | 1f5af51b476a36a72759e7560c125c2b9602b145 (diff) | |
parent | b02f9b6141627127acd027ed9491dd7c0ca5dd2a (diff) | |
download | gitlab-ce-d7d6bad11387ecc8efadb6c94d1a94c95b63f086.tar.gz |
Merge branch '42377-fix-merge-request-can-be-reverted' into 'master'
Look at system notes created just before merge as well as after, when deciding if an MR can be reverted
Closes #42377
See merge request gitlab-org/gitlab-ce!16678
-rw-r--r-- | app/models/merge_request.rb | 8 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 14 |
2 files changed, 19 insertions, 3 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8028ff3875b..4accb08eaf9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -989,8 +989,14 @@ class MergeRequest < ActiveRecord::Base merged_at = metrics&.merged_at notes_association = notes_with_associations + # It is not guaranteed that Note#created_at will be strictly later than + # MergeRequestMetric#merged_at. Nanoseconds on MySQL may break this + # comparison, as will a HA environment if clocks are not *precisely* + # synchronized. Add a minute's leeway to compensate for both possibilities + cutoff = merged_at - 1.minute + if merged_at - notes_association = notes_association.where('created_at > ?', merged_at) + notes_association = notes_association.where('created_at >= ?', cutoff) end !merge_commit.has_been_reverted?(current_user, notes_association) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c76f32b3989..429b6615131 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1127,9 +1127,19 @@ describe MergeRequest do end end - context 'when the revert commit is mentioned in a note before the MR was merged' do + context 'when the revert commit is mentioned in a note just before the MR was merged' do before do - subject.notes.last.update!(created_at: subject.metrics.merged_at - 1.second) + subject.notes.last.update!(created_at: subject.metrics.merged_at - 30.seconds) + end + + it 'returns false' do + expect(subject.can_be_reverted?(current_user)).to be_falsey + end + end + + context 'when the revert commit is mentioned in a note long before the MR was merged' do + before do + subject.notes.last.update!(created_at: subject.metrics.merged_at - 2.minutes) end it 'returns true' do |