Merge branch '54796-add-tiebreaker-to-sorts' into 'master'
Added: Include order by ID desc as tie break in pagination Closes #54796 See merge request gitlab-org/gitlab-ce!25311
This commit is contained in:
commit
2f768281b8
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: 'API: Sort tie breaker with id DESC'
|
||||
merge_request: 25311
|
||||
author: Nermin Vehabovic
|
||||
type: changed
|
|
@ -299,6 +299,12 @@ module API
|
|||
items.search(text)
|
||||
end
|
||||
|
||||
def order_options_with_tie_breaker
|
||||
order_options = { params[:order_by] => params[:sort] }
|
||||
order_options['id'] ||= 'desc'
|
||||
order_options
|
||||
end
|
||||
|
||||
# error helpers
|
||||
|
||||
def forbidden!(reason = nil)
|
||||
|
@ -393,7 +399,7 @@ module API
|
|||
|
||||
# rubocop: disable CodeReuse/ActiveRecord
|
||||
def reorder_projects(projects)
|
||||
projects.reorder(params[:order_by] => params[:sort])
|
||||
projects.reorder(order_options_with_tie_breaker)
|
||||
end
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
|
|
|
@ -29,8 +29,7 @@ module API
|
|||
|
||||
issues = IssuesFinder.new(current_user, args).execute
|
||||
.preload(:assignees, :labels, :notes, :timelogs, :project, :author, :closed_by)
|
||||
|
||||
issues.reorder(args[:order_by] => args[:sort])
|
||||
issues.reorder(order_options_with_tie_breaker)
|
||||
end
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
|
|
|
@ -38,7 +38,7 @@ module API
|
|||
args[:scope] = args[:scope].underscore if args[:scope]
|
||||
|
||||
merge_requests = MergeRequestsFinder.new(current_user, args).execute
|
||||
.reorder(args[:order_by] => args[:sort])
|
||||
.reorder(order_options_with_tie_breaker)
|
||||
merge_requests = paginate(merge_requests)
|
||||
.preload(:source_project, :target_project)
|
||||
|
||||
|
|
|
@ -39,7 +39,7 @@ module API
|
|||
# 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.
|
||||
raw_notes = noteable.notes.with_metadata.reorder(params[:order_by] => params[:sort])
|
||||
raw_notes = noteable.notes.with_metadata.reorder(order_options_with_tie_breaker)
|
||||
notes =
|
||||
# paginate() only works with a relation. This could lead to a
|
||||
# mismatch between the pagination headers info and the actual notes
|
||||
|
|
|
@ -26,7 +26,7 @@ module API
|
|||
# rubocop: disable CodeReuse/ActiveRecord
|
||||
def reorder_users(users)
|
||||
if params[:order_by] && params[:sort]
|
||||
users.reorder(params[:order_by] => params[:sort])
|
||||
users.reorder(order_options_with_tie_breaker)
|
||||
else
|
||||
users
|
||||
end
|
||||
|
|
|
@ -359,12 +359,40 @@ describe API::Issues do
|
|||
expect_paginated_array_response([])
|
||||
end
|
||||
|
||||
context 'without sort params' do
|
||||
it 'sorts by created_at descending by default' do
|
||||
get api('/issues', user)
|
||||
|
||||
expect_paginated_array_response([issue.id, closed_issue.id])
|
||||
end
|
||||
|
||||
context 'with 2 issues with same created_at' do
|
||||
let!(:closed_issue2) do
|
||||
create :closed_issue,
|
||||
author: user,
|
||||
assignees: [user],
|
||||
project: project,
|
||||
milestone: milestone,
|
||||
created_at: closed_issue.created_at,
|
||||
updated_at: 1.hour.ago,
|
||||
title: issue_title,
|
||||
description: issue_description
|
||||
end
|
||||
|
||||
it 'page breaks first page correctly' do
|
||||
get api('/issues?per_page=2', user)
|
||||
|
||||
expect_paginated_array_response([issue.id, closed_issue2.id])
|
||||
end
|
||||
|
||||
it 'page breaks second page correctly' do
|
||||
get api('/issues?per_page=2&page=2', user)
|
||||
|
||||
expect_paginated_array_response([closed_issue.id])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'sorts ascending when requested' do
|
||||
get api('/issues?sort=asc', user)
|
||||
|
||||
|
@ -613,12 +641,40 @@ describe API::Issues do
|
|||
expect_paginated_array_response(group_confidential_issue.id)
|
||||
end
|
||||
|
||||
context 'without sort params' do
|
||||
it 'sorts by created_at descending by default' do
|
||||
get api(base_url, user)
|
||||
|
||||
expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id, group_issue.id])
|
||||
end
|
||||
|
||||
context 'with 2 issues with same created_at' do
|
||||
let!(:group_issue2) do
|
||||
create :issue,
|
||||
author: user,
|
||||
assignees: [user],
|
||||
project: group_project,
|
||||
milestone: group_milestone,
|
||||
updated_at: 1.hour.ago,
|
||||
title: issue_title,
|
||||
description: issue_description,
|
||||
created_at: group_issue.created_at
|
||||
end
|
||||
|
||||
it 'page breaks first page correctly' do
|
||||
get api("#{base_url}?per_page=3", user)
|
||||
|
||||
expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id, group_issue2.id])
|
||||
end
|
||||
|
||||
it 'page breaks second page correctly' do
|
||||
get api("#{base_url}?per_page=3&page=2", user)
|
||||
|
||||
expect_paginated_array_response([group_issue.id])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'sorts ascending when requested' do
|
||||
get api("#{base_url}?sort=asc", user)
|
||||
|
||||
|
@ -828,12 +884,40 @@ describe API::Issues do
|
|||
expect_paginated_array_response([issue.id, closed_issue.id])
|
||||
end
|
||||
|
||||
context 'without sort params' do
|
||||
it 'sorts by created_at descending by default' do
|
||||
get api("#{base_url}/issues", user)
|
||||
|
||||
expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue.id])
|
||||
end
|
||||
|
||||
context 'with 2 issues with same created_at' do
|
||||
let!(:closed_issue2) do
|
||||
create :closed_issue,
|
||||
author: user,
|
||||
assignees: [user],
|
||||
project: project,
|
||||
milestone: milestone,
|
||||
created_at: closed_issue.created_at,
|
||||
updated_at: 1.hour.ago,
|
||||
title: issue_title,
|
||||
description: issue_description
|
||||
end
|
||||
|
||||
it 'page breaks first page correctly' do
|
||||
get api("#{base_url}/issues?per_page=3", user)
|
||||
|
||||
expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue2.id])
|
||||
end
|
||||
|
||||
it 'page breaks second page correctly' do
|
||||
get api("#{base_url}/issues?per_page=3&page=2", user)
|
||||
|
||||
expect_paginated_array_response([closed_issue.id])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'sorts ascending when requested' do
|
||||
get api("#{base_url}/issues", user), params: { sort: :asc }
|
||||
|
||||
|
|
|
@ -257,6 +257,38 @@ shared_examples 'merge requests list' do
|
|||
expect(response_dates).to eq(response_dates.sort.reverse)
|
||||
end
|
||||
|
||||
context '2 merge requests with equal created_at' do
|
||||
let!(:closed_mr2) do
|
||||
create :merge_request,
|
||||
state: 'closed',
|
||||
milestone: milestone1,
|
||||
author: user,
|
||||
assignee: user,
|
||||
source_project: project,
|
||||
target_project: project,
|
||||
title: "Test",
|
||||
created_at: @mr_earlier.created_at
|
||||
end
|
||||
|
||||
it 'page breaks first page correctly' do
|
||||
get api("#{endpoint_path}?sort=desc&per_page=4", user)
|
||||
|
||||
response_ids = json_response.map { |merge_request| merge_request['id'] }
|
||||
|
||||
expect(response_ids).to include(closed_mr2.id)
|
||||
expect(response_ids).not_to include(@mr_earlier.id)
|
||||
end
|
||||
|
||||
it 'page breaks second page correctly' do
|
||||
get api("#{endpoint_path}?sort=desc&per_page=4&page=2", user)
|
||||
|
||||
response_ids = json_response.map { |merge_request| merge_request['id'] }
|
||||
|
||||
expect(response_ids).not_to include(closed_mr2.id)
|
||||
expect(response_ids).to include(@mr_earlier.id)
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns an array of merge_requests ordered by updated_at' do
|
||||
path = endpoint_path + '?order_by=updated_at'
|
||||
|
||||
|
|
|
@ -8,6 +8,7 @@ shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
|
|||
create_list(:note, 3, params)
|
||||
end
|
||||
|
||||
context 'without sort params' do
|
||||
it 'sorts by created_at in descending order by default' do
|
||||
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user)
|
||||
|
||||
|
@ -17,6 +18,37 @@ shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
|
|||
expect(response_dates).to eq(response_dates.sort.reverse)
|
||||
end
|
||||
|
||||
context '2 notes with equal created_at' do
|
||||
before do
|
||||
@first_note = Note.first
|
||||
|
||||
params = { noteable: noteable, author: user }
|
||||
params[:project] = parent if parent.is_a?(Project)
|
||||
params[:created_at] = @first_note.created_at
|
||||
|
||||
@note2 = create(:note, params)
|
||||
end
|
||||
|
||||
it 'page breaks first page correctly' do
|
||||
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?per_page=4", user)
|
||||
|
||||
response_ids = json_response.map { |note| note['id'] }
|
||||
|
||||
expect(response_ids).to include(@note2.id)
|
||||
expect(response_ids).not_to include(@first_note.id)
|
||||
end
|
||||
|
||||
it 'page breaks second page correctly' do
|
||||
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?per_page=4&page=2", user)
|
||||
|
||||
response_ids = json_response.map { |note| note['id'] }
|
||||
|
||||
expect(response_ids).not_to include(@note2.id)
|
||||
expect(response_ids).to include(@first_note.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'sorts by ascending order when requested' do
|
||||
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?sort=asc", user)
|
||||
|
||||
|
|
Loading…
Reference in New Issue