summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2018-01-25 13:51:37 +0000
committerRémy Coutable <remy@rymai.me>2018-01-25 13:51:37 +0000
commitd7d6bad11387ecc8efadb6c94d1a94c95b63f086 (patch)
treef663bdddfad65136f0cf066c7c301738c91dff0a
parent1f5af51b476a36a72759e7560c125c2b9602b145 (diff)
parentb02f9b6141627127acd027ed9491dd7c0ca5dd2a (diff)
downloadgitlab-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.rb8
-rw-r--r--spec/models/merge_request_spec.rb14
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