Merge pull request #529 from jakubbortlik/fix/use-pagination-to-list-discussions

fix: use pagination to list MR discussions

Author: harrisoncramer

cmd/app/list_discussions.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"net/http"
+	"slices"
 	"sort"
 	"sync"
 	"time"
@@ -90,18 +91,16 @@ func (a discussionsListerService) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		},
 	}
 
-	discussions, res, err := a.client.ListMergeRequestDiscussions(a.projectInfo.ProjectId, a.projectInfo.MergeId, &mergeRequestDiscussionOptions)
+	it, hasErr := gitlab.Scan(func(p gitlab.PaginationOptionFunc) ([]*gitlab.Discussion, *gitlab.Response, error) {
+		return a.client.ListMergeRequestDiscussions(a.projectInfo.ProjectId, a.projectInfo.MergeId, &mergeRequestDiscussionOptions, p)
+	})
+	discussions := slices.Collect(it)
 
-	if err != nil {
+	if err := hasErr(); err != nil {
 		handleError(w, err, "Could not list discussions", http.StatusInternalServerError)
 		return
 	}
 
-	if res.StatusCode >= 300 {
-		handleError(w, GenericError{r.URL.Path}, "Could not list discussions", res.StatusCode)
-		return
-	}
-
 	/* Filter out any discussions started by a blacklisted user
 	and system discussions, then return them sorted by created date */
 	var unlinkedDiscussions []*gitlab.Discussion
@@ -124,7 +123,7 @@ func (a discussionsListerService) ServeHTTP(w http.ResponseWriter, r *http.Reque
 
 	/* Collect IDs in order to fetch emojis */
 	var noteIds []int64
-	for _, discussion := range discussions {
+	for _, discussion := range slices.Concat(linkedDiscussions, unlinkedDiscussions) {
 		for _, note := range discussion.Notes {
 			noteIds = append(noteIds, note.ID)
 		}

cmd/app/list_discussions_test.go

@@ -127,17 +127,6 @@ func TestListDiscussions(t *testing.T) {
 		data, _ := getFailData(t, svc, request)
 		checkErrorFromGitlab(t, data, "Could not list discussions")
 	})
-	t.Run("Handles non-200s from Gitlab client", func(t *testing.T) {
-		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}})
-		svc := middleware(
-			discussionsListerService{testProjectData, fakeDiscussionsLister{testBase: testBase{status: http.StatusSeeOther}}},
-			withMr(testProjectData, fakeMergeRequestLister{}),
-			withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}),
-			withMethodCheck(http.MethodPost),
-		)
-		data, _ := getFailData(t, svc, request)
-		checkNon200(t, data, "Could not list discussions", "/mr/discussions/list")
-	})
 	t.Run("Handles error from emoji service", func(t *testing.T) {
 		request := makeRequest(t, http.MethodPost, "/mr/discussions/list", DiscussionsRequest{Blacklist: []string{}})
 		svc := middleware(