Fix: Iterate over several pages to get all discussions

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)
 

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")
 	})