From cedbb3366bf3dd9bafe95dde366c1e28ee70c615 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 29 Mar 2019 15:16:31 -0700 Subject: 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 --- .../sh-fix-project-branches-merge-status.yml | 5 +++++ lib/api/branches.rb | 4 ++-- spec/requests/api/branches_spec.rb | 22 +++++++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/sh-fix-project-branches-merge-status.yml 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 -- cgit v1.2.1