Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: suppport page + size as pagination params #642

Merged
merged 3 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ module Api
module Contracts
class PaginationQueryParamsSchema < BaseContract
params do
# legacy format
optional(:pageNumber).maybe(:integer).value(gteq?: 1)
optional(:pageSize).maybe(:integer).value(gteq?: 1)

# desired format
optional(:page).maybe(:integer).value(gteq?: 1)
optional(:size).maybe(:integer).value(gteq?: 1)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/pact_broker/api/decorators/branch_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class BranchDecorator < BaseDecorator
link "pb:latest-version" do | user_options |
{
title: "Latest version for branch",
href: branch_versions_url(represented, user_options.fetch(:base_url)) + "?pageSize=1"
href: branch_versions_url(represented, user_options.fetch(:base_url)) + "?size=1"
}
end

Expand Down
4 changes: 2 additions & 2 deletions lib/pact_broker/api/decorators/pagination_links.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module PaginationLinks
represented.respond_to?(:page_count) &&
represented.current_page < represented.page_count
{
href: context[:resource_url] + "?pageSize=#{represented.page_size}&pageNumber=#{represented.current_page + 1}",
href: context[:resource_url] + "?size=#{represented.page_size}&page=#{represented.current_page + 1}",
title: "Next page"
}

Expand All @@ -34,7 +34,7 @@ module PaginationLinks
link :previous do | context |
if represented.respond_to?(:first_page?) && !represented.first_page?
{
href: context[:resource_url] + "?pageSize=#{represented.page_size}&pageNumber=#{represented.current_page - 1}",
href: context[:resource_url] + "?size=#{represented.page_size}&page=#{represented.current_page - 1}",
title: "Previous page"
}
end
Expand Down
8 changes: 7 additions & 1 deletion lib/pact_broker/api/resources/pagination_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@ module PactBroker
module Api
module Resources
module PaginationMethods
# rubocop: disable Metrics/CyclomaticComplexity
def pagination_options
if request.query["pageNumber"] || request.query["pageSize"]
if request.query["page"] || request.query["size"]
{
page_number: request.query["page"]&.to_i || 1,
page_size: request.query["size"]&.to_i || 100
}
elsif request.query["pageNumber"] || request.query["pageSize"]
{
page_number: request.query["pageNumber"]&.to_i || 1,
page_size: request.query["pageSize"]&.to_i || 100
Expand Down
4 changes: 2 additions & 2 deletions spec/features/get_branch_versions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
end

context "with pagination options" do
subject { get(path, { "pageSize" => "2", "pageNumber" => "1" }) }
subject { get(path, { "size" => "2", "page" => "1" }) }

it "only returns the number of items specified in the pageSize" do
it "only returns the number of items specified in the size" do
expect(JSON.parse(subject.body).dig("_embedded", "versions").size).to eq 2
end

Expand Down
2 changes: 1 addition & 1 deletion spec/features/get_integrations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
end

context "with pagination options" do
let(:query) { { "pageSize" => "2", "pageNumber" => "1" } }
let(:query) { { "size" => "2", "page" => "1" } }

it_behaves_like "a paginated response"
end
Expand Down
4 changes: 2 additions & 2 deletions spec/features/get_pacticipant_branches_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
it_behaves_like "a page"

context "with pagination options" do
subject { get(path, { "pageSize" => "2", "pageNumber" => "1" }) }
subject { get(path, { "size" => "2", "number" => "1" }) }

it "only returns the number of items specified in the pageSize" do
it "only returns the number of items specified in the size" do
expect(response_body_hash[:_links][:"pb:branches"].size).to eq 2
end

Expand Down
4 changes: 2 additions & 2 deletions spec/features/get_pacticipants_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
end

context "with pagination options" do
subject { get(path, { "pageSize" => "2", "pageNumber" => "1" }) }
subject { get(path, { "size" => "2", "page" => "1" }) }

it "only returns the number of items specified in the pageSize" do
it "only returns the number of items specified in the page" do
expect(response_body_hash[:_links][:"pacticipants"].size).to eq 2
end

Expand Down
2 changes: 1 addition & 1 deletion spec/features/get_versions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
end

context "with pagination options" do
subject { get(path, { "pageSize" => "1", "pageNumber" => "1" }) }
subject { get(path, { "size" => "1", "page" => "1" }) }

it "paginates the response" do
expect(last_response_body[:_links][:"versions"].size).to eq 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
},
"pb:latest-version": {
"title": "Latest version for branch",
"href": "http://example.org/pacticipants/Foo/branches/main/versions?pageSize=1"
"href": "http://example.org/pacticipants/Foo/branches/main/versions?size=1"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,51 @@ module Contracts
context "with values that are not numeric" do
let(:params) do
{
"pageNumber" => "a",
"pageSize" => "3.2"
"page" => "a",
"size" => "3.2"
}
end

its([:pageNumber]) { is_expected.to contain_exactly(match("integer"))}
its([:pageSize]) { is_expected.to contain_exactly(match("integer"))}
its([:page]) { is_expected.to contain_exactly(match("integer"))}
its([:size]) { is_expected.to contain_exactly(match("integer"))}
end

context "with values that are 0" do
let(:params) do
{
"pageNumber" => "-0",
"pageSize" => "-0"
"page" => "-0",
"size" => "-0"
}
end

its([:pageNumber]) { is_expected.to contain_exactly(match(/greater.*1/))}
its([:pageSize]) { is_expected.to contain_exactly(match(/greater.*1/))}
its([:page]) { is_expected.to contain_exactly(match(/greater.*1/))}
its([:size]) { is_expected.to contain_exactly(match(/greater.*1/))}
end

context "legacy format" do
context "with values that are not numeric" do
let(:params) do
{
"pageNumber" => "a",
"pageSize" => "3.2"
}
end

its([:pageNumber]) { is_expected.to contain_exactly(match("integer"))}
its([:pageSize]) { is_expected.to contain_exactly(match("integer"))}
end

context "with values that are 0" do
let(:params) do
{
"pageNumber" => "-0",
"pageSize" => "-0"
}
end

its([:pageNumber]) { is_expected.to contain_exactly(match(/greater.*1/))}
its([:pageSize]) { is_expected.to contain_exactly(match(/greater.*1/))}
end
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/pact_broker/api/resources/dashboard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ module Resources
end

context "with pagination" do
subject { get(path, { pageNumber: 1, pageSize: 1 }) }
subject { get(path, { page: 1, size: 1 }) }

it "only returns the items for the page" do
expect(response_body_hash["items"].size).to eq 1
end
end

context "with invalid pagination" do
subject { get(path, { pageNumber: -1, pageSize: -1 }) }
subject { get(path, { page: -1, size: -1 }) }

it_behaves_like "an invalid pagination params response"
end
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/pact_broker/api/resources/integrations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module Resources
let(:errors) { {} }

let(:path) { "/integrations" }
let(:params) { { "pageNumber" => "1", "pageSize" => "2" } }
let(:params) { { "page" => "1", "size" => "2" } }

subject { get(path, params, rack_headers) }

Expand Down
8 changes: 4 additions & 4 deletions spec/lib/pact_broker/api/resources/pacticipants_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ module Resources
describe "GET" do
let(:query) do
{
"pageSize" => "10",
"pageNumber" => "1",
"size" => "10",
"page" => "1",
"q" => "search"
}
end
Expand Down Expand Up @@ -44,8 +44,8 @@ module Resources
context "with invalid pagination params" do
let(:query) do
{
"pageSize" => "0",
"pageNumber" => "0",
"size" => "0",
"page" => "0",
}
end

Expand Down
4 changes: 2 additions & 2 deletions spec/support/shared_examples_for_responses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
end

it "includes the parameter validation errors" do
expect(response_body_hash[:errors].has_key?(:pageNumber)).to be_truthy
expect(response_body_hash[:errors].has_key?(:pageSize)).to be_truthy
expect(response_body_hash[:errors].has_key?(:page)).to be_truthy
expect(response_body_hash[:errors].has_key?(:size)).to be_truthy
end
end

Expand Down