summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2016-01-14 15:20:16 +0000
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2016-01-14 15:20:16 +0000
commit0eef82761fe3a100c4f22c7a1abea3a34dc76edf (patch)
treeb68717b4dcf92243adef44ac26e8a027d58071a3
parent0430c00cf02cc62a376545ae7333a06f78913aca (diff)
parent1b08cd811abeed18c3601e1b997b0566a243662c (diff)
downloadgitlab-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--CHANGELOG1
-rw-r--r--lib/api/helpers.rb30
-rw-r--r--spec/requests/api/commit_status_spec.rb6
-rw-r--r--spec/requests/api/notes_spec.rb4
-rw-r--r--spec/support/api/pagination_shared_examples.rb20
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