Skip to content

Commit

Permalink
Pagination bug (#250)
Browse files Browse the repository at this point in the history
* Check next_offset before making next page request

* test pagination small page

* small refactor for clarity
  • Loading branch information
pkarman authored Feb 7, 2020
1 parent 6cc4243 commit baefc1a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
3 changes: 1 addition & 2 deletions lib/vbms/service/paged_documents.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ def call(file_number:)
pages = 1

# if we need to fetch more docs, iterate till we exhaust the pages
while total_docs > documents.length
while total_docs > documents.length && next_offset > 0
next_page = client.send_request(next_request(file_number, next_offset))
(documents << next_page.map { |section| section[:documents] }).flatten!
next_offset = next_page.first[:paging][:@next_start_index].to_i
pages += 1
break if next_offset <= 0 # reality check
end

{ paging: pagination, pages: pages, documents: documents }
Expand Down
19 changes: 19 additions & 0 deletions spec/service/paged_documents_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
let(:page_size) { 20 }
let(:total_docs) { 101 }
let(:version_sets) { 2 } # MUST divide page_size evenly.
let(:small_return_set) { false }

subject { described_class.new(client: client) }

Expand All @@ -28,6 +29,11 @@ def next_response
end
documents = build_documents(num_docs)

if small_return_set
@offset = -1
documents.pop # make returned set smaller than total_docs
end

(1..num_sets).map do
{
paging: { :@next_start_index => @offset, :@total_result_count => total_docs },
Expand Down Expand Up @@ -62,5 +68,18 @@ def next_response
expect(r[:paging][:@total_result_count]).to eq total_docs
end
end

context "when the first page reports more pages than it contains" do
let(:small_return_set) { true }
let(:total_docs) { page_size - 1 }

it "believes the next_offset over the returned document count" do
r = subject.call(file_number: file_number)

expect(r[:pages]).to eq(1)
expect(r[:documents].length).to eq(total_docs - 1)
expect(r[:paging][:@total_result_count]).to eq total_docs
end
end
end
end

0 comments on commit baefc1a

Please sign in to comment.