Merge branch 'Michi302/gitlab-ce-fix/pagination-links' into 'master'

Fix add_pagination_headers to keep request parameters in Link header

Originally opened at !2321. Closes #5904. /cc @Michi302

See merge request !2674

3 files changed, 19 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7a70516173c..f207e0c5316 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,6 +19,7 @@ v 8.5.0 (unreleased)
   - Update the ExternalIssue regex pattern (Blake Hitchcock)
   - Optimized performance of finding issues to be closed by a merge request
   - Revert "Add IP check against DNSBLs at account sign-up"
+  - Fix API to keep request parameters in Link header (Michael Potthoff)
   - Deprecate API "merge_request/:merge_request_id/comments". Use "merge_requests/:merge_request_id/notes" instead
   - Deprecate API "merge_request/:merge_request_id/...". Use "merge_requests/:merge_request_id/..." instead
   - Mark inline difference between old and new paths when a file is renamed
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 9dacf7c1e86..a72044e8058 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -30,7 +30,7 @@ module API
     end
 
     def sudo_identifier()
-      identifier ||= params[SUDO_PARAM] ||= env[SUDO_HEADER]
+      identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER]
 
       # Regex for integers
       if !!(identifier =~ /^[0-9]+$/)
@@ -344,12 +344,22 @@ module API
 
     def pagination_links(paginated_data)
       request_url = request.url.split('?').first
+      request_params = params.clone
+      request_params[:per_page] = paginated_data.limit_value
 
       links = []
-      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")
+
+      request_params[:page] = paginated_data.current_page - 1
+      links << %(<#{request_url}?#{request_params.to_query}>; rel="prev") unless paginated_data.first_page?
+
+      request_params[:page] = paginated_data.current_page + 1
+      links << %(<#{request_url}?#{request_params.to_query}>; rel="next") unless paginated_data.last_page?
+
+      request_params[:page] = 1
+      links << %(<#{request_url}?#{request_params.to_query}>; rel="first")
+
+      request_params[:page] = paginated_data.total_pages
+      links << %(<#{request_url}?#{request_params.to_query}>; rel="last")
 
       links.join(', ')
     end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 5e65ad18c0e..410e684ee3b 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -46,10 +46,10 @@ describe API::API, api: true  do
         expect(json_response.first['title']).to eq(issue.title)
       end
 
-      it "should add pagination headers" do
-        get api("/issues?per_page=3", user)
+      it "should add pagination headers and keep query params" do
+        get api("/issues?state=closed&per_page=3", user)
         expect(response.headers['Link']).to eq(
-          '<http://www.example.com/api/v3/issues?page=1&per_page=3>; rel="first", <http://www.example.com/api/v3/issues?page=1&per_page=3>; rel="last"'
+          '<http://www.example.com/api/v3/issues?page=1&per_page=3&private_token=%s&state=closed>; rel="first", <http://www.example.com/api/v3/issues?page=1&per_page=3&private_token=%s&state=closed>; rel="last"' % [user.private_token, user.private_token]
         )
       end