diff --git a/CHANGELOG b/CHANGELOG
index f874145ec860e009d223e6c5a5a2523dd5a9ce28..160fc17177836d713b8f9a4ecb2cb4844ac4a91c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
Please view this file on the master branch, on stable branches it's out of date.
v 8.4.0 (unreleased)
+ - Add pagination headers to already paginated API resources
- 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)
- Added custom application performance measuring system powered by InfluxDB (Yorick Peterse)
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index a4df810e755ce7c50bdce1316fa1d8a1f8d40724..312ef90915fc9ee183db0e032da0b70d79be15e7 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!
@@ -327,16 +325,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/lib/api/notes.rb b/lib/api/notes.rb
index 174473f53719925adb4e8c9bd0bb99a5d7f95fc7..ebd9e97148ce0b0926aa94b172ca4bb2facbcd71 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -22,17 +22,11 @@ module API
@noteable = user_project.send(:"#{noteables_str}").find(params[:"#{noteable_id_str}"])
# We exclude notes that are cross-references and that cannot be viewed
- # by the current user. By doing this exclusion at this level and not
- # at the DB query level (which we cannot in that case), the current
- # page can have less elements than :per_page even if
- # there's more than one page.
+ # by the current user.
notes =
- # paginate() only works with a relation. This could lead to a
- # mismatch between the pagination headers info and the actual notes
- # array returned, but this is really a edge-case.
- paginate(@noteable.notes).
+ @noteable.notes.
reject { |n| n.cross_reference_not_visible_for?(current_user) }
- present notes, with: Entities::Note
+ present paginate(Kaminari.paginate_array(notes)), with: Entities::Note
end
# Get a single +noteable+ note
diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_status_spec.rb
index a28607bd2400a1f7f8f5035b5547bb3bd0bf253a..21482fc1070b40123e0baef34703349d3d740e29 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 d8bbd1072695d1d47b02e42ff7291904d4e8e9ab..39f9a06fe1ba0b2bf2106da97048bd75b99c74ec 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 0000000000000000000000000000000000000000..352a6eeec79fb7dcf84dc5bc8687529ff6026227
--- /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