dmitri.shuralyov.com/service/change/...

githubapi: Add timeline pagination to ListTimeline.

This helps with displaying large PRs that have over 100 timeline items
(e.g., https://github.com/neelance/go/pull/7).

The implementation is based the similar issues service ListTimeline,
see https://github.com/shurcooL/issues/commit/4081aa59e957752abbfc6b6fdab72c3bbc1c1dec.
dmitshur committed 1 year ago commit 761d04bf40c2c9ef3f63f776a7e525c6290464a3
githubapi/githubapi.go
@@ -257,38 +257,34 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 	repo, err := ghRepoSpec(rs)
 	if err != nil {
 		// TODO: Map to 400 Bad Request HTTP error.
 		return nil, err
 	}
-
+	type comment struct { // Comment fields.
+		Author          *githubV4Actor
+		PublishedAt     githubv4.DateTime
+		LastEditedAt    *githubv4.DateTime
+		Editor          *githubV4Actor
+		Body            string
+		ReactionGroups  reactionGroups
+		ViewerCanUpdate bool
+	}
 	type event struct { // Common fields for all events.
 		Actor     *githubV4Actor
 		CreatedAt githubv4.DateTime
 	}
 	var q struct {
 		Repository struct {
 			PullRequest struct {
-				Author          *githubV4Actor
-				PublishedAt     githubv4.DateTime
-				LastEditedAt    *githubv4.DateTime
-				Editor          *githubV4Actor
-				Body            githubv4.String
-				ReactionGroups  reactionGroups
-				ViewerCanUpdate bool
+				comment `graphql:"...@include(if:$firstPage)"` // Fetch the PR description only on first page.
 
 				Timeline struct {
 					Nodes []struct {
 						Typename     string `graphql:"__typename"`
 						IssueComment struct {
-							DatabaseID      uint64
-							Author          *githubV4Actor
-							PublishedAt     githubv4.DateTime
-							LastEditedAt    *githubv4.DateTime
-							Editor          *githubV4Actor
-							Body            string
-							ReactionGroups  reactionGroups
-							ViewerCanUpdate bool
+							DatabaseID uint64
+							comment
 						} `graphql:"...on IssueComment"`
 						ClosedEvent struct {
 							event
 							Closer struct {
 								Typename    string `graphql:"__typename"`
@@ -360,11 +356,15 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 						// TODO: Wait for GitHub to add support.
 						//CommentDeletedEvent struct {
 						//	event
 						//} `graphql:"...on CommentDeletedEvent"`
 					}
-				} `graphql:"timeline(first:100)"` // TODO: Pagination...
+					PageInfo struct {
+						EndCursor   githubv4.String
+						HasNextPage githubv4.Boolean
+					}
+				} `graphql:"timeline(first:100,after:$timelineCursor)"`
 
 				// Need to use PullRequest.Reviews rather than PullRequest.Timeline.PullRequestReview,
 				// because the latter is missing single-inline-reply reviews (as of 2018-02-08).
 				Reviews struct {
 					Nodes []struct {
@@ -384,204 +384,214 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 								Body             string
 								ReactionGroups   reactionGroups
 							}
 						} `graphql:"comments(first:100)"` // TODO: Pagination... Figure out how to make pagination across 2 resource types work...
 					}
-				} `graphql:"reviews(first:100)"` // TODO: Pagination... Figure out how to make pagination across 2 resource types work...
+				} `graphql:"reviews(first:100)@include(if:$firstPage)"` // TODO: Pagination... Figure out how to make pagination across 2 resource types work...
 			} `graphql:"pullRequest(number:$prNumber)"`
 		} `graphql:"repository(owner:$repositoryOwner,name:$repositoryName)"`
 		Viewer githubV4User
 	}
 	variables := map[string]interface{}{
 		"repositoryOwner": githubv4.String(repo.Owner),
 		"repositoryName":  githubv4.String(repo.Repo),
 		"prNumber":        githubv4.Int(id),
+		"firstPage":       githubv4.Boolean(true),
+		"timelineCursor":  (*githubv4.String)(nil),
 	}
-	err = s.clV4.Query(ctx, &q, variables)
-	if err != nil {
-		return nil, err
-	}
-	var timeline []interface{}
-	{
-		pr := q.Repository.PullRequest
-		var edited *change.Edited
-		if pr.LastEditedAt != nil {
-			edited = &change.Edited{
-				By: ghActor(pr.Editor),
-				At: pr.LastEditedAt.Time,
-			}
-		}
-		timeline = append(timeline, change.Comment{
-			ID:        prDescriptionCommentID,
-			User:      ghActor(pr.Author),
-			CreatedAt: pr.PublishedAt.Time,
-			Edited:    edited,
-			Body:      string(pr.Body),
-			Reactions: ghReactions(pr.ReactionGroups, ghUser(&q.Viewer)),
-			Editable:  pr.ViewerCanUpdate,
-		})
-	}
-	for _, node := range q.Repository.PullRequest.Timeline.Nodes {
-		if node.Typename != "IssueComment" {
-			continue
+	var timeline []interface{} // Of type change.Comment, change.Review, change.TimelineItem.
+	for {
+		err := s.clV4.Query(ctx, &q, variables)
+		if err != nil {
+			return nil, err
 		}
-		comment := node.IssueComment
-		var edited *change.Edited
-		if comment.LastEditedAt != nil {
-			edited = &change.Edited{
-				By: ghActor(comment.Editor),
-				At: comment.LastEditedAt.Time,
+		if variables["firstPage"].(githubv4.Boolean) {
+			pr := q.Repository.PullRequest.comment // PR description comment.
+			var edited *change.Edited
+			if pr.LastEditedAt != nil {
+				edited = &change.Edited{
+					By: ghActor(pr.Editor),
+					At: pr.LastEditedAt.Time,
+				}
 			}
+			timeline = append(timeline, change.Comment{
+				ID:        prDescriptionCommentID,
+				User:      ghActor(pr.Author),
+				CreatedAt: pr.PublishedAt.Time,
+				Edited:    edited,
+				Body:      pr.Body,
+				Reactions: ghReactions(pr.ReactionGroups, ghUser(&q.Viewer)),
+				Editable:  pr.ViewerCanUpdate,
+			})
 		}
-		timeline = append(timeline, change.Comment{
-			ID:        fmt.Sprintf("c%d", comment.DatabaseID),
-			User:      ghActor(comment.Author),
-			CreatedAt: comment.PublishedAt.Time,
-			Edited:    edited,
-			Body:      comment.Body,
-			Reactions: ghReactions(comment.ReactionGroups, ghUser(&q.Viewer)),
-			Editable:  comment.ViewerCanUpdate,
-		})
-	}
-	for _, review := range q.Repository.PullRequest.Reviews.Nodes {
-		state, ok := ghPRReviewState(review.State)
-		if !ok {
-			continue
-		}
-		var edited *change.Edited
-		if review.LastEditedAt != nil {
-			edited = &change.Edited{
-				By: ghActor(review.Editor),
-				At: review.LastEditedAt.Time,
+		for _, node := range q.Repository.PullRequest.Timeline.Nodes {
+			if node.Typename != "IssueComment" {
+				continue
 			}
-		}
-		var cs []change.InlineComment
-		for _, comment := range review.Comments.Nodes {
-			cs = append(cs, change.InlineComment{
-				ID:        fmt.Sprintf("rc%d", comment.DatabaseID),
-				File:      comment.Path,
-				Line:      comment.OriginalPosition, // TODO: This isn't line in file, it's line *in the diff*. Take it into account, compute real line, etc.
+			comment := node.IssueComment
+			var edited *change.Edited
+			if comment.LastEditedAt != nil {
+				edited = &change.Edited{
+					By: ghActor(comment.Editor),
+					At: comment.LastEditedAt.Time,
+				}
+			}
+			timeline = append(timeline, change.Comment{
+				ID:        fmt.Sprintf("c%d", comment.DatabaseID),
+				User:      ghActor(comment.Author),
+				CreatedAt: comment.PublishedAt.Time,
+				Edited:    edited,
 				Body:      comment.Body,
 				Reactions: ghReactions(comment.ReactionGroups, ghUser(&q.Viewer)),
+				Editable:  comment.ViewerCanUpdate,
 			})
 		}
-		sort.Slice(cs, func(i, j int) bool {
-			if cs[i].File == cs[j].File {
-				return cs[i].Line < cs[j].Line
+		if variables["firstPage"].(githubv4.Boolean) {
+			for _, review := range q.Repository.PullRequest.Reviews.Nodes {
+				state, ok := ghPRReviewState(review.State)
+				if !ok {
+					continue
+				}
+				var edited *change.Edited
+				if review.LastEditedAt != nil {
+					edited = &change.Edited{
+						By: ghActor(review.Editor),
+						At: review.LastEditedAt.Time,
+					}
+				}
+				var cs []change.InlineComment
+				for _, comment := range review.Comments.Nodes {
+					cs = append(cs, change.InlineComment{
+						ID:        fmt.Sprintf("rc%d", comment.DatabaseID),
+						File:      comment.Path,
+						Line:      comment.OriginalPosition, // TODO: This isn't line in file, it's line *in the diff*. Take it into account, compute real line, etc.
+						Body:      comment.Body,
+						Reactions: ghReactions(comment.ReactionGroups, ghUser(&q.Viewer)),
+					})
+				}
+				sort.Slice(cs, func(i, j int) bool {
+					if cs[i].File == cs[j].File {
+						return cs[i].Line < cs[j].Line
+					}
+					return cs[i].File < cs[j].File
+				})
+				timeline = append(timeline, change.Review{
+					ID:        fmt.Sprintf("r%d", review.DatabaseID),
+					User:      ghActor(review.Author),
+					CreatedAt: review.PublishedAt.Time,
+					Edited:    edited,
+					State:     state,
+					Body:      review.Body,
+					Editable:  review.ViewerCanUpdate,
+					Comments:  cs,
+				})
 			}
-			return cs[i].File < cs[j].File
-		})
-		timeline = append(timeline, change.Review{
-			ID:        fmt.Sprintf("r%d", review.DatabaseID),
-			User:      ghActor(review.Author),
-			CreatedAt: review.PublishedAt.Time,
-			Edited:    edited,
-			State:     state,
-			Body:      review.Body,
-			Editable:  review.ViewerCanUpdate,
-			Comments:  cs,
-		})
-	}
-	for _, event := range q.Repository.PullRequest.Timeline.Nodes {
-		e := change.TimelineItem{
-			//ID: 0, // TODO.
 		}
-		switch event.Typename {
-		case "ClosedEvent":
-			e.Actor = ghActor(event.ClosedEvent.Actor)
-			e.CreatedAt = event.ClosedEvent.CreatedAt.Time
-			switch event.ClosedEvent.Closer.Typename {
-			case "PullRequest":
-				pr := event.ClosedEvent.Closer.PullRequest
-				e.Payload = change.ClosedEvent{
-					Closer: change.Change{
-						State: ghPRState(pr.State),
-						Title: pr.Title,
+		for _, event := range q.Repository.PullRequest.Timeline.Nodes {
+			e := change.TimelineItem{
+				//ID: 0, // TODO.
+			}
+			switch event.Typename {
+			case "ClosedEvent":
+				e.Actor = ghActor(event.ClosedEvent.Actor)
+				e.CreatedAt = event.ClosedEvent.CreatedAt.Time
+				switch event.ClosedEvent.Closer.Typename {
+				case "PullRequest":
+					pr := event.ClosedEvent.Closer.PullRequest
+					e.Payload = change.ClosedEvent{
+						Closer: change.Change{
+							State: ghPRState(pr.State),
+							Title: pr.Title,
+						},
+						CloserHTMLURL: s.rtr.PullRequestURL(ctx, pr.Repository.Owner.Login, pr.Repository.Name, pr.Number),
+					}
+				case "Commit":
+					c := event.ClosedEvent.Closer.Commit
+					e.Payload = change.ClosedEvent{
+						Closer: change.Commit{
+							SHA:     c.OID,
+							Message: c.Message,
+							Author:  users.User{AvatarURL: c.Author.AvatarURL},
+						},
+						CloserHTMLURL: c.URL,
+					}
+				default:
+					e.Payload = change.ClosedEvent{}
+				}
+			case "ReopenedEvent":
+				e.Actor = ghActor(event.ReopenedEvent.Actor)
+				e.CreatedAt = event.ReopenedEvent.CreatedAt.Time
+				e.Payload = change.ReopenedEvent{}
+			case "RenamedTitleEvent":
+				e.Actor = ghActor(event.RenamedTitleEvent.Actor)
+				e.CreatedAt = event.RenamedTitleEvent.CreatedAt.Time
+				e.Payload = change.RenamedEvent{
+					From: event.RenamedTitleEvent.PreviousTitle,
+					To:   event.RenamedTitleEvent.CurrentTitle,
+				}
+			case "LabeledEvent":
+				e.Actor = ghActor(event.LabeledEvent.Actor)
+				e.CreatedAt = event.LabeledEvent.CreatedAt.Time
+				e.Payload = change.LabeledEvent{
+					Label: issues.Label{
+						Name:  event.LabeledEvent.Label.Name,
+						Color: ghColor(event.LabeledEvent.Label.Color),
 					},
-					CloserHTMLURL: s.rtr.PullRequestURL(ctx, pr.Repository.Owner.Login, pr.Repository.Name, pr.Number),
 				}
-			case "Commit":
-				c := event.ClosedEvent.Closer.Commit
-				e.Payload = change.ClosedEvent{
-					Closer: change.Commit{
-						SHA:     c.OID,
-						Message: c.Message,
-						Author:  users.User{AvatarURL: c.Author.AvatarURL},
+			case "UnlabeledEvent":
+				e.Actor = ghActor(event.UnlabeledEvent.Actor)
+				e.CreatedAt = event.UnlabeledEvent.CreatedAt.Time
+				e.Payload = change.UnlabeledEvent{
+					Label: issues.Label{
+						Name:  event.UnlabeledEvent.Label.Name,
+						Color: ghColor(event.UnlabeledEvent.Label.Color),
 					},
-					CloserHTMLURL: c.URL,
 				}
+			case "ReviewRequestedEvent":
+				e.Actor = ghActor(event.ReviewRequestedEvent.Actor)
+				e.CreatedAt = event.ReviewRequestedEvent.CreatedAt.Time
+				e.Payload = change.ReviewRequestedEvent{
+					RequestedReviewer: ghUser(event.ReviewRequestedEvent.RequestedReviewer.User),
+				}
+			case "ReviewRequestRemovedEvent":
+				e.Actor = ghActor(event.ReviewRequestRemovedEvent.Actor)
+				e.CreatedAt = event.ReviewRequestRemovedEvent.CreatedAt.Time
+				e.Payload = change.ReviewRequestRemovedEvent{
+					RequestedReviewer: ghUser(event.ReviewRequestRemovedEvent.RequestedReviewer.User),
+				}
+			case "MergedEvent":
+				e.Actor = ghActor(event.MergedEvent.Actor)
+				e.CreatedAt = event.MergedEvent.CreatedAt.Time
+				e.Payload = change.MergedEvent{
+					CommitID:      event.MergedEvent.Commit.OID,
+					CommitHTMLURL: event.MergedEvent.Commit.URL,
+					RefName:       event.MergedEvent.MergeRefName,
+				}
+			case "HeadRefDeletedEvent":
+				e.Actor = ghActor(event.HeadRefDeletedEvent.Actor)
+				e.CreatedAt = event.HeadRefDeletedEvent.CreatedAt.Time
+				e.Payload = change.DeletedEvent{
+					Type: "branch",
+					Name: event.HeadRefDeletedEvent.HeadRefName,
+				}
+			// TODO: Wait for GitHub to add support.
+			//case "CommentDeletedEvent":
+			//	e.Actor = ghActor(event.CommentDeletedEvent.Actor)
+			//	e.CreatedAt = event.CommentDeletedEvent.CreatedAt.Time
 			default:
-				e.Payload = change.ClosedEvent{}
-			}
-		case "ReopenedEvent":
-			e.Actor = ghActor(event.ReopenedEvent.Actor)
-			e.CreatedAt = event.ReopenedEvent.CreatedAt.Time
-			e.Payload = change.ReopenedEvent{}
-		case "RenamedTitleEvent":
-			e.Actor = ghActor(event.RenamedTitleEvent.Actor)
-			e.CreatedAt = event.RenamedTitleEvent.CreatedAt.Time
-			e.Payload = change.RenamedEvent{
-				From: event.RenamedTitleEvent.PreviousTitle,
-				To:   event.RenamedTitleEvent.CurrentTitle,
-			}
-		case "LabeledEvent":
-			e.Actor = ghActor(event.LabeledEvent.Actor)
-			e.CreatedAt = event.LabeledEvent.CreatedAt.Time
-			e.Payload = change.LabeledEvent{
-				Label: issues.Label{
-					Name:  event.LabeledEvent.Label.Name,
-					Color: ghColor(event.LabeledEvent.Label.Color),
-				},
-			}
-		case "UnlabeledEvent":
-			e.Actor = ghActor(event.UnlabeledEvent.Actor)
-			e.CreatedAt = event.UnlabeledEvent.CreatedAt.Time
-			e.Payload = change.UnlabeledEvent{
-				Label: issues.Label{
-					Name:  event.UnlabeledEvent.Label.Name,
-					Color: ghColor(event.UnlabeledEvent.Label.Color),
-				},
-			}
-		case "ReviewRequestedEvent":
-			e.Actor = ghActor(event.ReviewRequestedEvent.Actor)
-			e.CreatedAt = event.ReviewRequestedEvent.CreatedAt.Time
-			e.Payload = change.ReviewRequestedEvent{
-				RequestedReviewer: ghUser(event.ReviewRequestedEvent.RequestedReviewer.User),
-			}
-		case "ReviewRequestRemovedEvent":
-			e.Actor = ghActor(event.ReviewRequestRemovedEvent.Actor)
-			e.CreatedAt = event.ReviewRequestRemovedEvent.CreatedAt.Time
-			e.Payload = change.ReviewRequestRemovedEvent{
-				RequestedReviewer: ghUser(event.ReviewRequestRemovedEvent.RequestedReviewer.User),
+				continue
 			}
-		case "MergedEvent":
-			e.Actor = ghActor(event.MergedEvent.Actor)
-			e.CreatedAt = event.MergedEvent.CreatedAt.Time
-			e.Payload = change.MergedEvent{
-				CommitID:      event.MergedEvent.Commit.OID,
-				CommitHTMLURL: event.MergedEvent.Commit.URL,
-				RefName:       event.MergedEvent.MergeRefName,
-			}
-		case "HeadRefDeletedEvent":
-			e.Actor = ghActor(event.HeadRefDeletedEvent.Actor)
-			e.CreatedAt = event.HeadRefDeletedEvent.CreatedAt.Time
-			e.Payload = change.DeletedEvent{
-				Type: "branch",
-				Name: event.HeadRefDeletedEvent.HeadRefName,
-			}
-		// TODO: Wait for GitHub to add support.
-		//case "CommentDeletedEvent":
-		//	e.Actor = ghActor(event.CommentDeletedEvent.Actor)
-		//	e.CreatedAt = event.CommentDeletedEvent.CreatedAt.Time
-		default:
-			continue
+			timeline = append(timeline, e)
+		}
+		if !q.Repository.PullRequest.Timeline.PageInfo.HasNextPage {
+			break
 		}
-		timeline = append(timeline, e)
+		variables["firstPage"] = githubv4.Boolean(false)
+		variables["timelineCursor"] = githubv4.NewString(q.Repository.PullRequest.Timeline.PageInfo.EndCursor)
 	}
-
-	// We can't just delegate pagination to GitHub because our timeline items don't match up 1:1,
-	// we want to skip Commit in the timeline, etc.
+	// We can't just delegate pagination to GitHub because our timeline items may not match up 1:1,
+	// e.g., we want to skip Commit in the timeline, etc. (At least for now; may reconsider later.)
 	if opt != nil {
 		start := opt.Start
 		if start > len(timeline) {
 			start = len(timeline)
 		}