Commit b8a9e184 authored by David Kim's avatar David Kim Committed by Phil Hughes

Handle out of bound batch request gracefully

parent b6e206b0
......@@ -393,8 +393,10 @@ class MergeRequestDiff < ApplicationRecord
diffs_batch = diffs_in_batch_collection(batch_page, batch_size, diff_options: diff_options)
if comparison
if diff_options[:paths].blank? && !without_files?
# Return the empty MergeRequestDiffBatch for an out of bound batch request
break diffs_batch if diffs_batch.diff_file_paths.blank?
if diff_options[:paths].blank? && !without_files? && diffs_batch.diff_file_paths.present?
diff_options.merge!(
paths: diffs_batch.diff_file_paths,
pagination_data: diffs_batch.pagination_data
......
......@@ -21,9 +21,9 @@ module Gitlab
@paginated_collection = load_paginated_collection(batch_page, batch_size, diff_options)
@pagination_data = {
current_page: batch_gradual_load? ? nil : @paginated_collection.current_page,
next_page: batch_gradual_load? ? nil : @paginated_collection.next_page,
total_pages: batch_gradual_load? ? relation.size : @paginated_collection.total_pages
current_page: current_page,
next_page: next_page,
total_pages: total_pages
}
end
......@@ -62,6 +62,24 @@ module Gitlab
@merge_request_diff.merge_request_diff_files
end
def current_page
return if @paginated_collection.blank?
batch_gradual_load? ? nil : @paginated_collection.current_page
end
def next_page
return if @paginated_collection.blank?
batch_gradual_load? ? nil : @paginated_collection.next_page
end
def total_pages
return if @paginated_collection.blank?
batch_gradual_load? ? relation.size : @paginated_collection.total_pages
end
# rubocop: disable CodeReuse/ActiveRecord
def load_paginated_collection(batch_page, batch_size, diff_options)
batch_page ||= DEFAULT_BATCH_PAGE
......
......@@ -499,6 +499,14 @@ RSpec.describe MergeRequestDiff do
expect(diffs.pagination_data).to eq(current_page: 1, next_page: 2, total_pages: 2)
end
it 'returns an empty MergeRequestBatch with empty pagination data when the batch is empty' do
diffs = diff_with_commits.diffs_in_batch(3, 10, diff_options: diff_options)
expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiffBatch)
expect(diffs.diff_files.size).to eq 0
expect(diffs.pagination_data).to eq(current_page: nil, next_page: nil, total_pages: nil)
end
context 'with gradual load enabled' do
before do
stub_feature_flags(diffs_gradual_load: true)
......@@ -512,6 +520,14 @@ RSpec.describe MergeRequestDiff do
expect(diffs.diff_files.size).to eq 10
expect(diffs.pagination_data).to eq(current_page: nil, next_page: nil, total_pages: file_count)
end
it 'returns an empty MergeRequestBatch with empty pagination data when the batch is empty' do
diffs = diff_with_commits.diffs_in_batch(30, 10, diff_options: diff_options)
expect(diffs).to be_a(Gitlab::Diff::FileCollection::MergeRequestDiffBatch)
expect(diffs.diff_files.size).to eq 0
expect(diffs.pagination_data).to eq(current_page: nil, next_page: nil, total_pages: nil)
end
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment