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 6 years ago commit 2fb0e89b2ed38899ef391fdf285fad155e9bc230
Collapse all
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 {