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

githubapi: Sync various improvements from issues/githubapi.

Detect ghost actors/users via them missing entirely, rather than
checking for zero DatabaseID. This is consistent with how GitHub
GraphQL returns them at this time.

Fix minor miscounting issue in reactions method.

Remove unneeded error return parameter from reactions method.

These changes help keep the code more consistent with
issues/githubapi package.
dmitshur committed 2 years ago commit 2fb0e89b2ed38899ef391fdf285fad155e9bc230
githubapi/githubapi.go
@@ -80,11 +80,11 @@ func (s service) List(ctx context.Context, rs string, opt change.ListOptions) ([
 						Nodes []struct {
 							Name  string
 							Color string
 						}
 					} `graphql:"labels(first:100)"`
-					Author    githubqlActor
+					Author    *githubqlActor
 					CreatedAt githubql.DateTime
 					Comments  struct {
 						TotalCount int
 					}
 				}
@@ -166,11 +166,11 @@ func (s service) Get(ctx context.Context, rs string, id uint64) (change.Change,
 		Repository struct {
 			PullRequest struct {
 				Number    uint64
 				State     githubql.PullRequestState
 				Title     string
-				Author    githubqlActor
+				Author    *githubqlActor
 				CreatedAt githubql.DateTime
 				Commits   struct {
 					TotalCount int
 				}
 			} `graphql:"pullRequest(number:$prNumber)"`
@@ -219,11 +219,11 @@ func (s service) ListCommits(ctx context.Context, rs string, id uint64) ([]chang
 	var commits []change.Commit
 	for _, c := range cs {
 		commits = append(commits, change.Commit{
 			SHA:        *c.SHA,
 			Message:    *c.Commit.Message,
-			Author:     ghV3UserOrGitUser(c.Author, c.Commit.Author),
+			Author:     ghV3UserOrGitUser(c.Author, *c.Commit.Author),
 			AuthorTime: *c.Commit.Author.Date,
 		})
 	}
 	return commits, nil
 }
@@ -256,17 +256,17 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 		// TODO: Map to 400 Bad Request HTTP error.
 		return nil, err
 	}
 
 	type event struct { // Common fields for all events.
-		Actor     githubqlActor
+		Actor     *githubqlActor
 		CreatedAt githubql.DateTime
 	}
 	var q struct {
 		Repository struct {
 			PullRequest struct {
-				Author          githubqlActor
+				Author          *githubqlActor
 				PublishedAt     githubql.DateTime
 				LastEditedAt    *githubql.DateTime
 				Editor          *githubqlActor
 				Body            githubql.String
 				ReactionGroups  reactionGroups
@@ -275,11 +275,11 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 				Timeline struct {
 					Nodes []struct {
 						Typename     string `graphql:"__typename"`
 						IssueComment struct {
 							DatabaseID      uint64
-							Author          githubqlActor
+							Author          *githubqlActor
 							PublishedAt     githubql.DateTime
 							LastEditedAt    *githubql.DateTime
 							Editor          *githubqlActor
 							Body            string
 							ReactionGroups  reactionGroups
@@ -311,17 +311,17 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 							}
 						} `graphql:"...on UnlabeledEvent"`
 						ReviewRequestedEvent struct {
 							event
 							RequestedReviewer struct {
-								githubqlActor `graphql:"...on Actor"`
+								*githubqlActor `graphql:"...on Actor"`
 							}
 						} `graphql:"...on ReviewRequestedEvent"`
 						ReviewRequestRemovedEvent struct {
 							event
 							RequestedReviewer struct {
-								githubqlActor `graphql:"...on Actor"`
+								*githubqlActor `graphql:"...on Actor"`
 							}
 						} `graphql:"...on ReviewRequestRemovedEvent"`
 						MergedEvent struct {
 							event
 							Commit struct {
@@ -344,11 +344,11 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 				// 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 {
 						DatabaseID      uint64
-						Author          githubqlActor
+						Author          *githubqlActor
 						PublishedAt     githubql.DateTime
 						LastEditedAt    *githubql.DateTime
 						Editor          *githubqlActor
 						State           githubql.PullRequestReviewState
 						Body            string
@@ -375,62 +375,54 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 		return nil, err
 	}
 	var timeline []interface{}
 	{
 		pr := q.Repository.PullRequest
-		reactions, err := s.reactions(pr.ReactionGroups)
-		if err != nil {
-			return timeline, err
-		}
 		var edited *change.Edited
 		if pr.LastEditedAt != nil {
 			edited = &change.Edited{
-				By: ghActor(*pr.Editor),
+				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: reactions,
+			Reactions: s.reactions(pr.ReactionGroups),
 			Editable:  pr.ViewerCanUpdate,
 		})
 	}
 	for _, node := range q.Repository.PullRequest.Timeline.Nodes {
 		if node.Typename != "IssueComment" {
 			continue
 		}
 		comment := node.IssueComment
-		reactions, err := s.reactions(comment.ReactionGroups)
-		if err != nil {
-			return timeline, err
-		}
 		var edited *change.Edited
 		if comment.LastEditedAt != nil {
 			edited = &change.Edited{
-				By: ghActor(*comment.Editor),
+				By: ghActor(comment.Editor),
 				At: comment.LastEditedAt.Time,
 			}
 		}
 		timeline = append(timeline, change.Comment{
 			ID:        comment.DatabaseID,
 			User:      ghActor(comment.Author),
 			CreatedAt: comment.PublishedAt.Time,
 			Edited:    edited,
 			Body:      comment.Body,
-			Reactions: reactions,
+			Reactions: s.reactions(comment.ReactionGroups),
 			Editable:  comment.ViewerCanUpdate,
 		})
 	}
 	for _, review := range q.Repository.PullRequest.Reviews.Nodes {
 		var edited *change.Edited
 		if review.LastEditedAt != nil {
 			edited = &change.Edited{
-				By: ghActor(*review.Editor),
+				By: ghActor(review.Editor),
 				At: review.LastEditedAt.Time,
 			}
 		}
 		var cs []change.InlineComment
 		for _, comment := range review.Comments.Nodes {
@@ -575,12 +567,12 @@ type githubqlActor struct {
 	Login     string
 	AvatarURL string `graphql:"avatarUrl(size:96)"`
 	URL       string
 }
 
-func ghActor(actor githubqlActor) users.User {
-	if actor.User.DatabaseID == 0 && actor.Bot.DatabaseID == 0 {
+func ghActor(actor *githubqlActor) users.User {
+	if actor == nil {
 		return ghost // Deleted user, replace with https://github.com/ghost.
 	}
 	return users.User{
 		UserSpec: users.UserSpec{
 			ID:     actor.User.DatabaseID | actor.Bot.DatabaseID,
@@ -597,12 +589,12 @@ type githubqlUser struct {
 	Login      string
 	AvatarURL  string `graphql:"avatarUrl(size:96)"`
 	URL        string
 }
 
-func ghUser(user githubqlUser) users.User {
-	if user.DatabaseID == 0 {
+func ghUser(user *githubqlUser) users.User {
+	if user == nil {
 		return ghost // Deleted user, replace with https://github.com/ghost.
 	}
 	return users.User{
 		UserSpec: users.UserSpec{
 			ID:     user.DatabaseID,
@@ -612,22 +604,18 @@ func ghUser(user githubqlUser) users.User {
 		AvatarURL: user.AvatarURL,
 		HTMLURL:   user.URL,
 	}
 }
 
-func ghV3UserOrGitUser(user *github.User, gitUser *github.CommitAuthor) users.User {
+func ghV3UserOrGitUser(user *github.User, gitUser github.CommitAuthor) users.User {
 	if user == nil {
 		return users.User{
 			Name:      *gitUser.Name,
 			Email:     *gitUser.Email,
 			AvatarURL: "https://secure.gravatar.com/avatar?d=mm&f=y&s=96", // TODO: Use email.
 		}
 	}
-	return ghV3User(user)
-}
-
-func ghV3User(user *github.User) users.User {
 	if *user.ID == 0 {
 		return ghost // Deleted user, replace with https://github.com/ghost.
 	}
 	return users.User{
 		UserSpec: users.UserSpec{
@@ -692,18 +680,18 @@ func ghColor(hex string) issues.RGB {
 }
 
 type reactionGroups []struct {
 	Content githubql.ReactionContent
 	Users   struct {
-		Nodes      []githubqlUser
+		Nodes      []*githubqlUser
 		TotalCount int
 	} `graphql:"users(first:10)"`
 	ViewerHasReacted bool
 }
 
 // reactions converts []githubql.ReactionGroup to []reactions.Reaction.
-func (s service) reactions(rgs reactionGroups) ([]reactions.Reaction, error) {
+func (s service) reactions(rgs reactionGroups) []reactions.Reaction {
 	var rs []reactions.Reaction
 	for _, rg := range rgs {
 		if rg.Users.TotalCount == 0 {
 			continue
 		}
@@ -711,19 +699,21 @@ func (s service) reactions(rgs reactionGroups) ([]reactions.Reaction, error) {
 		// Only return the details of first few users and authed user.
 		var us []users.User
 		addedAuthedUser := false
 		for i := 0; i < rg.Users.TotalCount; i++ {
 			if i < len(rg.Users.Nodes) {
-				actor := ghUser(rg.Users.Nodes[i])
-				us = append(us, actor)
-				if s.currentUser.ID != 0 && actor.UserSpec == s.currentUser.UserSpec {
+				user := ghUser(rg.Users.Nodes[i])
+				us = append(us, user)
+				if s.currentUser.ID != 0 && user.UserSpec == s.currentUser.UserSpec {
 					addedAuthedUser = true
 				}
 			} else if i == len(rg.Users.Nodes) {
 				// Add authed user last if they've reacted, but haven't been added already.
 				if rg.ViewerHasReacted && !addedAuthedUser {
 					us = append(us, s.currentUser)
+				} else {
+					us = append(us, users.User{})
 				}
 			} else {
 				us = append(us, users.User{})
 			}
 		}
@@ -731,11 +721,11 @@ func (s service) reactions(rgs reactionGroups) ([]reactions.Reaction, error) {
 		rs = append(rs, reactions.Reaction{
 			Reaction: internalizeReaction(rg.Content),
 			Users:    us,
 		})
 	}
-	return rs, nil
+	return rs
 }
 
 // internalizeReaction converts githubql.ReactionContent to reactions.EmojiID.
 func internalizeReaction(reaction githubql.ReactionContent) reactions.EmojiID {
 	switch reaction {