diff --git a/cmd/list_discussions.go b/cmd/list_discussions.go index 909aa1f3..f757cb33 100644 --- a/cmd/list_discussions.go +++ b/cmd/list_discussions.go @@ -62,42 +62,49 @@ func (a *api) listDiscussionsHandler(w http.ResponseWriter, r *http.Request) { handleError(w, err, "Could not unmarshal request body", http.StatusBadRequest) } - mergeRequestDiscussionOptions := gitlab.ListMergeRequestDiscussionsOptions{ - Page: 1, - PerPage: 250, - } + var unlinkedDiscussions []*gitlab.Discussion + var linkedDiscussions []*gitlab.Discussion - discussions, res, err := a.client.ListMergeRequestDiscussions(a.projectInfo.ProjectId, a.projectInfo.MergeId, &mergeRequestDiscussionOptions, nil) + /* Repeat requests for multiple pages, as the maximum number of + items per page is 100 which may not be enough in bigger MRs. */ + /* TODO: Replace the hardcoded limit by the correct number that can + be obtained from the total number of items (X-Total). */ + for i := 1; i <= 3; i++ { + mergeRequestDiscussionOptions := gitlab.ListMergeRequestDiscussionsOptions{ + Page: i, + PerPage: 100, + } - if err != nil { - handleError(w, err, "Could not list discussions", http.StatusInternalServerError) - return - } + discussions, res, err := a.client.ListMergeRequestDiscussions(a.projectInfo.ProjectId, a.projectInfo.MergeId, &mergeRequestDiscussionOptions, nil) - if res.StatusCode >= 300 { - handleError(w, GenericError{endpoint: "/mr/discussions/list"}, "Could not list discussions", res.StatusCode) - return - } + if err != nil { + handleError(w, err, "Could not list discussions", http.StatusInternalServerError) + return + } - /* Filter out any discussions started by a blacklisted user - and system discussions, then return them sorted by created date */ - var unlinkedDiscussions []*gitlab.Discussion - var linkedDiscussions []*gitlab.Discussion - for _, discussion := range discussions { - if discussion.Notes == nil || len(discussion.Notes) == 0 || Contains(requestBody.Blacklist, discussion.Notes[0].Author.Username) > -1 { - continue + if res.StatusCode >= 300 { + handleError(w, GenericError{endpoint: "/mr/discussions/list"}, "Could not list discussions", res.StatusCode) + return } - for _, note := range discussion.Notes { - if note.Type == gitlab.NoteTypeValue("DiffNote") { - linkedDiscussions = append(linkedDiscussions, discussion) - break - } else if !note.System && note.Position == nil { - unlinkedDiscussions = append(unlinkedDiscussions, discussion) - break + + /* Filter out any discussions started by a blacklisted user + and system discussions, then return them sorted by created date */ + for _, discussion := range discussions { + if discussion.Notes == nil || len(discussion.Notes) == 0 || Contains(requestBody.Blacklist, discussion.Notes[0].Author.Username) > -1 { + continue + } + for _, note := range discussion.Notes { + if note.Type == gitlab.NoteTypeValue("DiffNote") { + linkedDiscussions = append(linkedDiscussions, discussion) + break + } else if !note.System && note.Position == nil { + unlinkedDiscussions = append(unlinkedDiscussions, discussion) + break + } } } - } + } sortedLinkedDiscussions := SortableDiscussions(linkedDiscussions) sortedUnlinkedDiscussions := SortableDiscussions(unlinkedDiscussions) diff --git a/cmd/list_discussions_test.go b/cmd/list_discussions_test.go index b635eee1..74c4efb8 100644 --- a/cmd/list_discussions_test.go +++ b/cmd/list_discussions_test.go @@ -55,7 +55,11 @@ func TestListDiscussionsHandler(t *testing.T) { assert(t, data.SuccessResponse.Message, "Discussions retrieved") assert(t, data.SuccessResponse.Status, http.StatusOK) assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer2") /* Sorting applied */ - assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer") + assert(t, data.Discussions[1].Notes[0].Author.Username, "hcramer2") /* Sorting applied */ + assert(t, data.Discussions[2].Notes[0].Author.Username, "hcramer2") /* Sorting applied */ + assert(t, data.Discussions[3].Notes[0].Author.Username, "hcramer") + assert(t, data.Discussions[4].Notes[0].Author.Username, "hcramer") + assert(t, data.Discussions[5].Notes[0].Author.Username, "hcramer") }) t.Run("Uses blacklist to filter unwanted authors", func(t *testing.T) { @@ -64,7 +68,7 @@ func TestListDiscussionsHandler(t *testing.T) { data := serveRequest(t, server, request, DiscussionsResponse{}) assert(t, data.SuccessResponse.Message, "Discussions retrieved") assert(t, data.SuccessResponse.Status, http.StatusOK) - assert(t, len(data.Discussions), 1) + assert(t, len(data.Discussions), 3) assert(t, data.Discussions[0].Notes[0].Author.Username, "hcramer2") })