Skip to content

Commit db835b0

Browse files
committed
fix(gitlab): paginate through MergeRequest notes to locate update marker
This commits adds paging logic when listing commits in the gitlab CreateComment function. Previously, only the first 100 notes on a MR were compared when finding the note to update. Paging through the list of merge request notes ensures that if a MR has more than 100 comments we will still update the appropriate comment. This commit also creates default gitlab paging options which are used by all gitlab API calls that paginate through the response.
1 parent aac4f25 commit db835b0

File tree

2 files changed

+32
-19
lines changed

2 files changed

+32
-19
lines changed

pkg/provider/gitlab/acl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, e
7272
}
7373

7474
var nextPage int
75-
opt := &gitlab.ListMergeRequestDiscussionsOptions{Page: page}
75+
opt := &gitlab.ListMergeRequestDiscussionsOptions{Page: page, PerPage: defaultGitlabListOptions.PerPage}
7676
discussions, resp, err := v.Client().Discussions.ListMergeRequestDiscussions(v.targetProjectID, event.PullRequestNumber, opt)
7777
if err != nil || len(discussions) == 0 {
7878
return false, err

pkg/provider/gitlab/gitlab.go

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ type Provider struct {
6868
cachedChangedFiles *changedfiles.ChangedFiles
6969
}
7070

71+
var defaultGitlabListOptions = gitlab.ListOptions{
72+
PerPage: 100,
73+
}
74+
7175
func (v *Provider) Client() *gitlab.Client {
7276
providerMetrics.RecordAPIUsage(
7377
v.Logger,
@@ -99,24 +103,33 @@ func (v *Provider) CreateComment(_ context.Context, event *info.Event, commit, u
99103

100104
// List comments of the merge request
101105
if updateMarker != "" {
102-
comments, _, err := v.Client().Notes.ListMergeRequestNotes(event.TargetProjectID, event.PullRequestNumber, &gitlab.ListMergeRequestNotesOptions{
103-
ListOptions: gitlab.ListOptions{
104-
Page: 1,
105-
PerPage: 100,
106-
},
107-
})
108-
if err != nil {
109-
return err
110-
}
106+
options := []gitlab.RequestOptionFunc{}
111107

112-
re := regexp.MustCompile(updateMarker)
113-
for _, comment := range comments {
114-
if re.MatchString(comment.Body) {
115-
_, _, err := v.Client().Notes.UpdateMergeRequestNote(event.TargetProjectID, event.PullRequestNumber, comment.ID, &gitlab.UpdateMergeRequestNoteOptions{
116-
Body: &commit,
117-
})
108+
for {
109+
comments, resp, err := v.Client().Notes.ListMergeRequestNotes(event.TargetProjectID, event.PullRequestNumber, &gitlab.ListMergeRequestNotesOptions{ListOptions: defaultGitlabListOptions}, options...)
110+
if err != nil {
118111
return err
119112
}
113+
114+
re := regexp.MustCompile(updateMarker)
115+
for _, comment := range comments {
116+
if re.MatchString(comment.Body) {
117+
_, _, err := v.Client().Notes.UpdateMergeRequestNote(event.TargetProjectID, event.PullRequestNumber, comment.ID, &gitlab.UpdateMergeRequestNoteOptions{
118+
Body: &commit,
119+
})
120+
return err
121+
}
122+
}
123+
124+
// Exit the loop when we've seen all pages.
125+
if resp.NextLink == "" {
126+
break
127+
}
128+
129+
// Otherwise, set param to query the next page
130+
options = []gitlab.RequestOptionFunc{
131+
gitlab.WithKeysetPaginationParameters(resp.NextLink),
132+
}
120133
}
121134
}
122135

@@ -386,7 +399,7 @@ func (v *Provider) GetTektonDir(_ context.Context, event *info.Event, path, prov
386399
ListOptions: gitlab.ListOptions{
387400
OrderBy: "id",
388401
Pagination: "keyset",
389-
PerPage: 20,
402+
PerPage: defaultGitlabListOptions.PerPage,
390403
Sort: "asc",
391404
},
392405
}
@@ -524,7 +537,7 @@ func (v *Provider) fetchChangedFiles(_ context.Context, runevent *info.Event) (c
524537
ListOptions: gitlab.ListOptions{
525538
OrderBy: "id",
526539
Pagination: "keyset",
527-
PerPage: 20,
540+
PerPage: defaultGitlabListOptions.PerPage,
528541
Sort: "asc",
529542
},
530543
}
@@ -564,7 +577,7 @@ func (v *Provider) fetchChangedFiles(_ context.Context, runevent *info.Event) (c
564577
}
565578
}
566579
case triggertype.Push:
567-
options := gitlab.GetCommitDiffOptions{}
580+
options := gitlab.GetCommitDiffOptions{ListOptions: defaultGitlabListOptions}
568581
pageOpts := []gitlab.RequestOptionFunc{}
569582

570583
for {

0 commit comments

Comments
 (0)