Skip to content

Commit 3b49028

Browse files
Feature merge request beyond 20 (#551)
* feat(choose_merge_request): add per_page option for merge request fetching Allow users to configure the number of merge requests fetched via the `per_page` setting in `choose_merge_request`. Defaults to 100. * Remove the per_page option and now fetch all merge request internally * Implement merge request pagination using gitlab api --------- Co-authored-by: Clement Hue <clement.hue@thetrainline.com>
1 parent 286fdb5 commit 3b49028

2 files changed

Lines changed: 14 additions & 18 deletions

File tree

cmd/app/merge_requests.go

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"errors"
66
"net/http"
7+
"slices"
78

89
gitlab "gitlab.com/gitlab-org/api/client-go"
910
)
@@ -35,23 +36,18 @@ func (a mergeRequestListerService) ServeHTTP(w http.ResponseWriter, r *http.Requ
3536
payload.Scope = gitlab.Ptr("all")
3637
}
3738

38-
payload.PerPage = 100
39-
var mergeRequests []*gitlab.BasicMergeRequest
40-
for nextPage := int64(1); nextPage != 0; {
41-
payload.Page = nextPage
42-
mrs, res, err := a.client.ListProjectMergeRequests(a.projectInfo.ProjectId, payload)
43-
if err != nil {
44-
handleError(w, err, "Failed to list merge requests", http.StatusInternalServerError)
45-
return
46-
}
39+
payload.ListOptions = gitlab.ListOptions{
40+
PerPage: 100,
41+
}
4742

48-
if res.StatusCode >= 300 {
49-
handleError(w, GenericError{r.URL.Path}, "Failed to list merge requests", res.StatusCode)
50-
return
51-
}
43+
it, hasErr := gitlab.Scan(func(p gitlab.PaginationOptionFunc) ([]*gitlab.BasicMergeRequest, *gitlab.Response, error) {
44+
return a.client.ListProjectMergeRequests(a.projectInfo.ProjectId, payload, p)
45+
})
46+
mergeRequests := slices.Collect(it)
5247

53-
mergeRequests = append(mergeRequests, mrs...)
54-
nextPage = res.NextPage
48+
if err := hasErr(); err != nil {
49+
handleError(w, err, "Failed to list merge requests", http.StatusInternalServerError)
50+
return
5551
}
5652

5753
if len(mergeRequests) == 0 {

cmd/app/merge_requests_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ func TestMergeRequestHandler(t *testing.T) {
5656
t.Run("Handles non-200s from Gitlab client", func(t *testing.T) {
5757
request := makeRequest(t, http.MethodPost, "/merge_requests", testListMergeRequestsRequest)
5858
svc := middleware(
59-
mergeRequestListerService{testProjectData, fakeMergeRequestLister{testBase: testBase{status: http.StatusSeeOther}}},
59+
mergeRequestListerService{testProjectData, fakeMergeRequestLister{testBase: testBase{errFromGitlab: true}}},
6060
withPayloadValidation(methodToPayload{http.MethodPost: newPayload[gitlab.ListProjectMergeRequestsOptions]}),
6161
withMethodCheck(http.MethodPost),
6262
)
6363
data, status := getFailData(t, svc, request)
64-
checkNon200(t, data, "Failed to list merge requests", "/merge_requests")
65-
assert(t, status, http.StatusSeeOther)
64+
checkErrorFromGitlab(t, data, "Failed to list merge requests")
65+
assert(t, status, http.StatusInternalServerError)
6666
})
6767
t.Run("Should handle not having any merge requests with 404", func(t *testing.T) {
6868
request := makeRequest(t, http.MethodPost, "/merge_requests", testListMergeRequestsRequest)

0 commit comments

Comments
 (0)