diff options
| author | Stan Hu <stanhu@gmail.com> | 2019-03-29 15:16:31 -0700 |
|---|---|---|
| committer | Stan Hu <stanhu@gmail.com> | 2019-03-30 05:20:29 -0700 |
| commit | cedbb3366bf3dd9bafe95dde366c1e28ee70c615 (patch) | |
| tree | e41816fc09a503b3d13c8037473a07b133972af8 | |
| parent | 50a1e01fa8959b08df8bfc18940f9310876873b3 (diff) | |
| download | gitlab-ce-cedbb3366bf3dd9bafe95dde366c1e28ee70c615.tar.gz | |
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
| -rw-r--r-- | changelogs/unreleased/sh-fix-project-branches-merge-status.yml | 5 | ||||
| -rw-r--r-- | lib/api/branches.rb | 4 | ||||
| -rw-r--r-- | spec/requests/api/branches_spec.rb | 22 |
3 files changed, 28 insertions, 3 deletions
diff --git a/changelogs/unreleased/sh-fix-project-branches-merge-status.yml b/changelogs/unreleased/sh-fix-project-branches-merge-status.yml new file mode 100644 index 00000000000..65f41b3faf9 --- /dev/null +++ b/changelogs/unreleased/sh-fix-project-branches-merge-status.yml @@ -0,0 +1,5 @@ +--- +title: Fix API /project/:id/branches not returning correct merge status +merge_request: 26785 +author: +type: fixed diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 07f529b01bb..5c98b0ad56c 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -34,11 +34,11 @@ module API repository = user_project.repository 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)) present( - paginate(branches), + branches, with: Entities::Branch, current_user: current_user, project: user_project, diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index b38cd66986f..75dcedabf96 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -36,10 +36,30 @@ describe API::Branches do expect(branch_names).to match_array(project.repository.branch_names) 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 - 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 } + + 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 context 'when repository is disabled' do |