From 129b83bef4985b0f4d35522152235e934b723d18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Tue, 17 Feb 2026 18:31:05 +0100 Subject: [PATCH 1/2] feat: implement pagination for MR discussions This uses the gitlab.Scan function to iterate over all discussions and handle errors after the iterator is exhausted. The test for non-200s is no longer needed as the Scan function transforms such responses to standard errors. --- cmd/app/list_discussions.go | 13 ++++++------- cmd/app/list_discussions_test.go | 11 ----------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/cmd/app/list_discussions.go b/cmd/app/list_discussions.go index d0f3fe76..4b1848e0 100644 --- a/cmd/app/list_discussions.go +++ b/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 diff --git a/cmd/app/list_discussions_test.go b/cmd/app/list_discussions_test.go index 5d43f1d9..ab142a4c 100644 --- a/cmd/app/list_discussions_test.go +++ b/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( From 946b7182b0b95e6119ffbae6e841bc5ed8be819e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Tue, 17 Feb 2026 18:35:35 +0100 Subject: [PATCH 2/2] perf: only fetch emojis for filtered discussions --- cmd/app/list_discussions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/app/list_discussions.go b/cmd/app/list_discussions.go index 4b1848e0..74a9a333 100644 --- a/cmd/app/list_discussions.go +++ b/cmd/app/list_discussions.go @@ -123,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) }