diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2016-01-14 15:20:16 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2016-01-14 15:20:16 +0000 |
commit | 0eef82761fe3a100c4f22c7a1abea3a34dc76edf (patch) | |
tree | b68717b4dcf92243adef44ac26e8a027d58071a3 | |
parent | 0430c00cf02cc62a376545ae7333a06f78913aca (diff) | |
parent | 1b08cd811abeed18c3601e1b997b0566a243662c (diff) | |
download | gitlab-ce-0eef82761fe3a100c4f22c7a1abea3a34dc76edf.tar.gz |
Merge branch 'add-pagination-headers-to-api' into 'master'
Add pagination headers to already paginated API resources
Following #3045, I've added pagination headers using already available Kaminari methods.
## Pagination headers in action
![Screen_Shot_2016-01-14_at_12.14.53](/uploads/88807f754dd9aafbb24da85f6bdf9521/Screen_Shot_2016-01-14_at_12.14.53.png)
---
I've also added a shared example to test paginated API resources.
A possible next step would be to paginate all the API resources that return an array.
See merge request !2426
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | lib/api/helpers.rb | 30 | ||||
-rw-r--r-- | spec/requests/api/commit_status_spec.rb | 6 | ||||
-rw-r--r-- | spec/requests/api/notes_spec.rb | 4 | ||||
-rw-r--r-- | spec/support/api/pagination_shared_examples.rb | 20 |
5 files changed, 49 insertions, 12 deletions
diff --git a/CHANGELOG b/CHANGELOG index 262d1f03ce5..1ae3520b4ff 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.5.0 (unreleased) - Remove gray background from layout in UI v 8.4.0 (unreleased) + - Add pagination headers to already paginated API resources - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages - Autocomplete data is now always loaded, instead of when focusing a comment text area (Yorick Peterse) - Improved performance of finding issues for an entire group (Yorick Peterse) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index d46b5c42967..6d2380cf47d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -97,11 +97,9 @@ module API end def paginate(relation) - per_page = params[:per_page].to_i - paginated = relation.page(params[:page]).per(per_page) - add_pagination_headers(paginated, per_page) - - paginated + relation.page(params[:page]).per(params[:per_page].to_i).tap do |data| + add_pagination_headers(data) + end end def authenticate! @@ -329,16 +327,26 @@ module API private - def add_pagination_headers(paginated, per_page) + def add_pagination_headers(paginated_data) + header 'X-Total', paginated_data.total_count.to_s + header 'X-Total-Pages', paginated_data.total_pages.to_s + header 'X-Per-Page', paginated_data.limit_value.to_s + header 'X-Page', paginated_data.current_page.to_s + header 'X-Next-Page', paginated_data.next_page.to_s + header 'X-Prev-Page', paginated_data.prev_page.to_s + header 'Link', pagination_links(paginated_data) + end + + def pagination_links(paginated_data) request_url = request.url.split('?').first links = [] - links << %(<#{request_url}?page=#{paginated.current_page - 1}&per_page=#{per_page}>; rel="prev") unless paginated.first_page? - links << %(<#{request_url}?page=#{paginated.current_page + 1}&per_page=#{per_page}>; rel="next") unless paginated.last_page? - links << %(<#{request_url}?page=1&per_page=#{per_page}>; rel="first") - links << %(<#{request_url}?page=#{paginated.total_pages}&per_page=#{per_page}>; rel="last") + links << %(<#{request_url}?page=#{paginated_data.current_page - 1}&per_page=#{paginated_data.limit_value}>; rel="prev") unless paginated_data.first_page? + links << %(<#{request_url}?page=#{paginated_data.current_page + 1}&per_page=#{paginated_data.limit_value}>; rel="next") unless paginated_data.last_page? + links << %(<#{request_url}?page=1&per_page=#{paginated_data.limit_value}>; rel="first") + links << %(<#{request_url}?page=#{paginated_data.total_pages}&per_page=#{paginated_data.limit_value}>; rel="last") - header 'Link', links.join(', ') + links.join(', ') end def abilities diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_status_spec.rb index a28607bd240..21482fc1070 100644 --- a/spec/requests/api/commit_status_spec.rb +++ b/spec/requests/api/commit_status_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::API, api: true do +describe API::CommitStatus, api: true do include ApiHelpers let(:user) { create(:user) } let(:user2) { create(:user) } @@ -12,6 +12,10 @@ describe API::API, api: true do let(:commit_status) { create(:commit_status, commit: ci_commit) } describe "GET /projects/:id/repository/commits/:sha/statuses" do + it_behaves_like 'a paginated resources' do + let(:request) { get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", user) } + end + context "reporter user" do let(:statuses_id) { json_response.map { |status| status['id'] } } diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index d8bbd107269..39f9a06fe1b 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -32,6 +32,10 @@ describe API::API, api: true do before { project.team << [user, :reporter] } describe "GET /projects/:id/noteable/:noteable_id/notes" do + it_behaves_like 'a paginated resources' do + let(:request) { get api("/projects/#{project.id}/issues/#{issue.id}/notes", user) } + end + context "when noteable is an Issue" do it "should return an array of issue notes" do get api("/projects/#{project.id}/issues/#{issue.id}/notes", user) diff --git a/spec/support/api/pagination_shared_examples.rb b/spec/support/api/pagination_shared_examples.rb new file mode 100644 index 00000000000..352a6eeec79 --- /dev/null +++ b/spec/support/api/pagination_shared_examples.rb @@ -0,0 +1,20 @@ +# Specs for paginated resources. +# +# Requires an API request: +# let(:request) { get api("/projects/#{project.id}/repository/branches", user) } +shared_examples 'a paginated resources' do + before do + # Fires the request + request + end + + it 'has pagination headers' do + expect(response.headers).to include('X-Total') + expect(response.headers).to include('X-Total-Pages') + expect(response.headers).to include('X-Per-Page') + expect(response.headers).to include('X-Page') + expect(response.headers).to include('X-Next-Page') + expect(response.headers).to include('X-Prev-Page') + expect(response.headers).to include('Link') + end +end |