🐛 Fix invalid code review

checks/code_review.go

@@ -118,14 +118,15 @@ func githubCodeReview(c *checker.CheckRequest) (int, string, error) {
 		// Check if the PR is committed by someone other than author. this is kind
 		// of equivalent to a review and is done several times on small prs to save
 		// time on clicking the approve button.
-		if !foundApprovedReview {
-			if !pr.MergeCommit.AuthoredByCommitter {
-				c.Dlogger.Debug3(&checker.LogMessage{
-					Text: fmt.Sprintf("found pr with committer different than author: %d", pr.Number),
-				})
-				totalReviewed++
-			}
+		if !foundApprovedReview &&
+			pr.MergeCommit.Committer.Login != pr.Author.Login {
+			c.Dlogger.Debug3(&checker.LogMessage{
+				Text: fmt.Sprintf("found PR#%d with committer (%s) different from author (%s)",
+					pr.Number, pr.Author.Login, pr.MergeCommit.Committer.Login),
+			})
+			totalReviewed++
 		}
+
 	}
 
 	return createReturn("GitHub", totalReviewed, totalMerged)

clients/commit.go

@@ -18,14 +18,8 @@ import "time"
 
 // Commit represents a Git commit.
 type Commit struct {
-	CommittedDate       time.Time
-	Message             string
-	SHA                 string
-	Committer           User
-	AuthoredByCommitter bool
-}
-
-// User represents a Git user.
-type User struct {
-	Login string
+	CommittedDate time.Time
+	Message       string
+	SHA           string
+	Committer     User
 }

clients/githubrepo/graphql.go

@@ -56,22 +56,29 @@ type graphqlData struct {
 		}
 		PullRequests struct {
 			Nodes []struct {
+				Author struct {
+					Login githubv4.String
+				}
 				Number      githubv4.Int
 				HeadRefOid  githubv4.String
 				MergeCommit struct {
-					AuthoredByCommitter githubv4.Boolean
+					Author struct {
+						User struct {
+							Login githubv4.String
+						}
+					}
 				}
 				MergedAt githubv4.DateTime
 				Labels   struct {
 					Nodes []struct {
 						Name githubv4.String
 					}
 				} `graphql:"labels(last: $labelsToAnalyze)"`
-				LatestReviews struct {
+				Reviews struct {
 					Nodes []struct {
 						State githubv4.String
 					}
-				} `graphql:"latestReviews(last: $reviewsToAnalyze)"`
+				} `graphql:"reviews(last: $reviewsToAnalyze)"`
 			}
 		} `graphql:"pullRequests(last: $pullRequestsToAnalyze, states: MERGED)"`
 	} `graphql:"repository(owner: $owner, name: $name)"`
@@ -142,21 +149,27 @@ func (handler *graphqlHandler) isArchived() (bool, error) {
 
 func pullRequestsFrom(data *graphqlData) []clients.PullRequest {
 	ret := make([]clients.PullRequest, len(data.Repository.PullRequests.Nodes))
-	for i, pr := range data.Repository.PullRequests.Nodes {
+	for i := range data.Repository.PullRequests.Nodes {
+		pr := data.Repository.PullRequests.Nodes[i]
 		toAppend := clients.PullRequest{
 			Number:   int(pr.Number),
 			HeadSHA:  string(pr.HeadRefOid),
 			MergedAt: pr.MergedAt.Time,
+			Author: clients.User{
+				Login: string(pr.Author.Login),
+			},
 			MergeCommit: clients.Commit{
-				AuthoredByCommitter: bool(pr.MergeCommit.AuthoredByCommitter),
+				Committer: clients.User{
+					Login: string(pr.MergeCommit.Author.User.Login),
+				},
 			},
 		}
 		for _, label := range pr.Labels.Nodes {
 			toAppend.Labels = append(toAppend.Labels, clients.Label{
 				Name: string(label.Name),
 			})
 		}
-		for _, review := range pr.LatestReviews.Nodes {
+		for _, review := range pr.Reviews.Nodes {
 			toAppend.Reviews = append(toAppend.Reviews, clients.Review{
 				State: string(review.State),
 			})

clients/pull_request.go

@@ -27,6 +27,7 @@ type PullRequest struct {
 	HeadSHA     string
 	Labels      []Label
 	Reviews     []Review
+	Author      User
 }
 
 // Label represents a PR label.

clients/user.go

@@ -0,0 +1,20 @@
+// Copyright 2021 Security Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package clients
+
+// User represents a Git user.
+type User struct {
+	Login string
+}