diff options
author | Nick Thomas <nick@gitlab.com> | 2018-01-24 15:06:50 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-01-25 11:39:46 +0000 |
commit | b02f9b6141627127acd027ed9491dd7c0ca5dd2a (patch) | |
tree | b8e8433f51f44d60d9e3c6a687d641b0db9a177e /spec/models/merge_request_spec.rb | |
parent | e2a56af930f9f7d17a6a9b638f52007a60e4cc60 (diff) | |
download | gitlab-ce-b02f9b6141627127acd027ed9491dd7c0ca5dd2a.tar.gz |
Look at notes created just before merge when deciding if an MR can be reverted
On MySQL, at least, `Note#created_at` doesn't seem to store fractional seconds,
while `MergeRequest::Metrics#merged_at` does. This breaks the optimization
assumption that we only need to search for notes created *after* the MR has
been merged.
Unsynchronized system clocks also make this a dangerous assumption to make.
Adding a minute of leeway still optimizes away most notes, but allows both
cases to be handled more gracefully. If the system clocks are more than a
minute out, we'll still be broken, of course.
Diffstat (limited to 'spec/models/merge_request_spec.rb')
-rw-r--r-- | spec/models/merge_request_spec.rb | 14 |
1 files changed, 12 insertions, 2 deletions
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 |