summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/commit.rb5
-rw-r--r--app/models/merge_request.rb11
-rw-r--r--changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml6
-rw-r--r--spec/models/commit_range_spec.rb6
-rw-r--r--spec/models/merge_request_spec.rb77
5 files changed, 99 insertions, 6 deletions
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 39d7f5b159d..ede8ad301e4 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -342,10 +342,11 @@ class Commit
@merged_merge_request_hash[current_user]
end
- def has_been_reverted?(current_user, noteable = self)
+ def has_been_reverted?(current_user, notes_association = nil)
ext = all_references(current_user)
+ notes_association ||= notes_with_associations
- noteable.notes_with_associations.system.each do |note|
+ notes_association.system.each do |note|
note.all_references(current_user, extractor: ext)
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ef58816937c..8efe7d41f37 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -982,7 +982,16 @@ class MergeRequest < ActiveRecord::Base
end
def can_be_reverted?(current_user)
- merge_commit && !merge_commit.has_been_reverted?(current_user, self)
+ return false unless merge_commit
+
+ merged_at = metrics&.merged_at
+ notes_association = notes_with_associations
+
+ if merged_at
+ notes_association = notes_association.where('created_at > ?', merged_at)
+ end
+
+ !merge_commit.has_been_reverted?(current_user, notes_association)
end
def can_be_cherry_picked?
diff --git a/changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml b/changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml
new file mode 100644
index 00000000000..146ae12afbd
--- /dev/null
+++ b/changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml
@@ -0,0 +1,6 @@
+---
+title: Speed up loading merged merge requests when they contained a lot of commits
+ before merging
+merge_request: 16320
+author:
+type: performance
diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb
index 38829773599..f2efcd9d0e9 100644
--- a/spec/models/commit_range_spec.rb
+++ b/spec/models/commit_range_spec.rb
@@ -151,11 +151,11 @@ describe CommitRange do
.with(commit1, user)
.and_return(true)
- expect(commit1.has_been_reverted?(user, issue)).to eq(true)
+ expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(true)
end
- it 'returns false a commit has not been reverted' do
- expect(commit1.has_been_reverted?(user, issue)).to eq(false)
+ it 'returns false if the commit has not been reverted' do
+ expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(false)
end
end
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 07b3e1c1758..02ccef9becc 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1035,6 +1035,83 @@ describe MergeRequest do
end
end
+ describe '#can_be_reverted?' do
+ context 'when there is no merged_at for the MR' do
+ before do
+ subject.metrics.update!(merged_at: nil)
+ end
+
+ it 'returns false' do
+ expect(subject.can_be_reverted?(nil)).to be_falsey
+ end
+ end
+
+ context 'when there is no merge_commit for the MR' do
+ before do
+ subject.metrics.update!(merged_at: Time.now.utc)
+ end
+
+ it 'returns false' do
+ expect(subject.can_be_reverted?(nil)).to be_falsey
+ end
+ end
+
+ context 'when the MR has been merged' do
+ before do
+ MergeRequests::MergeService
+ .new(subject.target_project, subject.author)
+ .execute(subject)
+ end
+
+ context 'when there is no revert commit' do
+ it 'returns true' do
+ expect(subject.can_be_reverted?(nil)).to be_truthy
+ end
+ end
+
+ context 'when there is a revert commit' do
+ let(:current_user) { subject.author }
+ let(:branch) { subject.target_branch }
+ let(:project) { subject.target_project }
+
+ let(:revert_commit_id) do
+ params = {
+ commit: subject.merge_commit,
+ branch_name: branch,
+ start_branch: branch
+ }
+
+ Commits::RevertService.new(project, current_user, params).execute[:result]
+ end
+
+ before do
+ project.add_master(current_user)
+
+ ProcessCommitWorker.new.perform(project.id,
+ current_user.id,
+ project.commit(revert_commit_id).to_hash,
+ project.default_branch == branch)
+ end
+
+ context 'when the revert commit is mentioned in a note after the MR was merged' do
+ 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 before the MR was merged' do
+ before do
+ subject.notes.last.update!(created_at: subject.metrics.merged_at - 1.second)
+ end
+
+ it 'returns true' do
+ expect(subject.can_be_reverted?(current_user)).to be_truthy
+ end
+ end
+ end
+ end
+ end
+
describe '#participants' do
let(:project) { create(:project, :public) }