Commit cedbb336 authored by Stan Hu's avatar Stan Hu

Fix API /project/:id/branches not returning correct merge status

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24034 introduced
a regression where only the first 20 branches were used to determine
whether a branch has been merged because the pagination was applied
incorrectly. Requesting the second page of branches via the API would
always have the wrong merge status. We fix this by properly paginating
the branches before requesting their merge status.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/56250
parent 50a1e01f
---
title: Fix API /project/:id/branches not returning correct merge status
merge_request: 26785
author:
type: fixed
...@@ -34,11 +34,11 @@ module API ...@@ -34,11 +34,11 @@ module API
repository = user_project.repository repository = user_project.repository
branches = BranchesFinder.new(repository, declared_params(include_missing: false)).execute branches = BranchesFinder.new(repository, declared_params(include_missing: false)).execute
branches = ::Kaminari.paginate_array(branches) branches = paginate(::Kaminari.paginate_array(branches))
merged_branch_names = repository.merged_branch_names(branches.map(&:name)) merged_branch_names = repository.merged_branch_names(branches.map(&:name))
present( present(
paginate(branches), branches,
with: Entities::Branch, with: Entities::Branch,
current_user: current_user, current_user: current_user,
project: user_project, project: user_project,
......
...@@ -36,10 +36,30 @@ describe API::Branches do ...@@ -36,10 +36,30 @@ describe API::Branches do
expect(branch_names).to match_array(project.repository.branch_names) expect(branch_names).to match_array(project.repository.branch_names)
end end
def check_merge_status(json_response)
merged, unmerged = json_response.partition { |branch| branch['merged'] }
merged_branches = merged.map { |branch| branch['name'] }
unmerged_branches = unmerged.map { |branch| branch['name'] }
expect(Set.new(merged_branches)).to eq(project.repository.merged_branch_names(merged_branches + unmerged_branches))
expect(project.repository.merged_branch_names(unmerged_branches)).to be_empty
end
it 'determines only a limited number of merged branch names' do it 'determines only a limited number of merged branch names' do
expect(API::Entities::Branch).to receive(:represent).with(anything, has_merged_branch_names_count(2)) expect(API::Entities::Branch).to receive(:represent).with(anything, has_merged_branch_names_count(1)).and_call_original
get api(route, current_user), params: { per_page: 2 } get api(route, current_user), params: { per_page: 2 }
expect(response).to have_gitlab_http_status(200)
check_merge_status(json_response)
end
it 'merge status matches reality on paginated input' do
get api(route, current_user), params: { per_page: 20, page: 2 }
expect(response).to have_gitlab_http_status(200)
check_merge_status(json_response)
end end
context 'when repository is disabled' do context 'when repository is disabled' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment