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

githubapi: use AuthorAssociation to determine ReviewState better

This produces more accurate results and is consistent with similar
code elsewhere.

This change also improves handling of other PullRequestReviewState
values.
dmitshur committed 5 months ago commit a1ef85d7455ffbeb260718aa501ac5c3eea3707c
githubapi/githubapi.go
@@ -354,20 +354,21 @@ 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          *githubV4Actor
-						PublishedAt     githubv4.DateTime
-						LastEditedAt    *githubv4.DateTime
-						Editor          *githubV4Actor
-						State           githubv4.PullRequestReviewState
-						Body            string
-						ReactionGroups  reactionGroups
-						ViewerCanUpdate bool
-						Comments        struct {
+						DatabaseID        uint64
+						Author            *githubV4Actor
+						AuthorAssociation githubv4.CommentAuthorAssociation
+						PublishedAt       githubv4.DateTime
+						LastEditedAt      *githubv4.DateTime
+						Editor            *githubV4Actor
+						State             githubv4.PullRequestReviewState
+						Body              string
+						ReactionGroups    reactionGroups
+						ViewerCanUpdate   bool
+						Comments          struct {
 							Nodes []struct {
 								DatabaseID       uint64
 								Path             string
 								OriginalPosition int // The original line index in the diff to which the comment applies.
 								Body             string
@@ -434,11 +435,11 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 				Editable:  comment.ViewerCanUpdate,
 			})
 		}
 		if variables["firstPage"].(githubv4.Boolean) {
 			for _, review := range q.Repository.PullRequest.Reviews.Nodes {
-				state, ok := ghPRReviewState(review.State)
+				state, ok := ghPRReviewState(review.State, review.AuthorAssociation)
 				if !ok {
 					continue
 				}
 				var edited *change.Edited
 				if review.LastEditedAt != nil {
@@ -875,27 +876,36 @@ func ghPRState(state githubv4.PullRequestState) change.State {
 		panic("unreachable")
 	}
 }
 
 // ghPRReviewState converts a GitHub PullRequestReviewState to state.Review, if it's supported.
-//
-// TODO: Consider using author association or so to tell approved by someone with
-//       push rights (+2) apart from approved by someone without push rights (+1), etc.
-func ghPRReviewState(st githubv4.PullRequestReviewState) (_ state.Review, ok bool) {
-	switch st {
-	case githubv4.PullRequestReviewStateApproved:
+func ghPRReviewState(st githubv4.PullRequestReviewState, aa githubv4.CommentAuthorAssociation) (_ state.Review, ok bool) {
+	// TODO: This is a heuristic. Author can be a member of the organization that
+	// owns the repository, but it's not known whether they have push access or not.
+	// TODO: Use https://developer.github.com/v3/repos/collaborators/#review-a-users-permission-level perhaps?
+	// Or wait for equivalent to be available via API v4?
+	approver := aa == githubv4.CommentAuthorAssociationOwner ||
+		aa == githubv4.CommentAuthorAssociationCollaborator ||
+		aa == githubv4.CommentAuthorAssociationMember
+
+	switch {
+	case st == githubv4.PullRequestReviewStateApproved && approver:
 		return state.ReviewPlus2, true
-	case githubv4.PullRequestReviewStateCommented:
+	case st == githubv4.PullRequestReviewStateApproved && !approver:
+		return state.ReviewPlus1, true
+	case st == githubv4.PullRequestReviewStateCommented:
 		return state.ReviewNoScore, true
-	case githubv4.PullRequestReviewStateChangesRequested:
+	case st == githubv4.PullRequestReviewStateChangesRequested && !approver:
+		return state.ReviewMinus1, true
+	case st == githubv4.PullRequestReviewStateChangesRequested && approver:
 		return state.ReviewMinus2, true
-	case githubv4.PullRequestReviewStateDismissed:
+	case st == githubv4.PullRequestReviewStateDismissed:
 		// PullRequestReviewStateDismissed are reviews that have been retroactively dismissed.
 		// Display them as a regular comment review for now (we can't know the original state).
 		// THINK: Consider displaying these more distinctly.
 		return state.ReviewNoScore, true
-	case githubv4.PullRequestReviewStatePending:
+	case st == githubv4.PullRequestReviewStatePending:
 		// PullRequestReviewStatePending are reviews that are pending (haven't been posted yet).
 		// TODO: Consider displaying pending review comments. Figure this out
 		//       when adding ability to leave reviews.
 		return 0, false
 	default: