diff --git a/ghutil/ghutil.go b/ghutil/ghutil.go index 5866fa3..3083df4 100644 --- a/ghutil/ghutil.go +++ b/ghutil/ghutil.go @@ -35,6 +35,9 @@ const ( LabelClaYes = "cla: yes" LabelClaNo = "cla: no" LabelClaExternal = "cla: external" + + ReviewRequestChanges = "REQUEST_CHANGES" + ReviewApprove = "APPROVE" ) // OrganizationsService is the subset of `github.OrganizationsService` used by @@ -61,6 +64,7 @@ type IssuesService interface { // PullRequestsService is the subset of `github.PullRequestsService` used by // this module. type PullRequestsService interface { + CreateReview(ctx context.Context, owner string, repo string, number int, review *github.PullRequestReviewRequest) (*github.PullRequestReview, *github.Response, error) List(ctx context.Context, owner string, repo string, opt *github.PullRequestListOptions) ([]*github.PullRequest, *github.Response, error) ListCommits(ctx context.Context, owner string, repo string, number int, opt *github.ListOptions) ([]*github.RepositoryCommit, *github.Response, error) } @@ -380,15 +384,16 @@ func (ghc *GitHubClient) ProcessPullRequest(ctx context.Context, orgName string, } } - addComment := func(comment string) { - logging.Infof(" Adding comment to repo '%s/%s/ PR %d: %s", orgName, repoName, *pull.Number, comment) + addReview := func(review string, event string) { + logging.Infof(" Adding %s review to repo '%s/%s/ PR %d: %s", event, orgName, repoName, *pull.Number, review) if updateRepo { - issueComment := github.IssueComment{ - Body: &comment, + prReview := github.PullRequestReviewRequest{ + Body: &review, + Event: &event, } - _, _, err := ghc.Issues.CreateComment(ctx, orgName, repoName, *pull.Number, &issueComment) + _, _, err := ghc.PullRequests.CreateReview(ctx, orgName, repoName, *pull.Number, &prReview) if err != nil { - logging.Errorf(" Error leaving comment on PR %d: %v", *pull.Number, err) + logging.Errorf(" Error leaving review on PR %d: %v", *pull.Number, err) } } else { logging.Info(" ... but -update-repo flag is disabled; skipping") @@ -409,26 +414,23 @@ func (ghc *GitHubClient) ProcessPullRequest(ctx context.Context, orgName string, } else { logging.Infof(" No action needed: [%s] label already added", LabelClaYes) } + + addReview(pullRequestNonComplianceReason, ReviewApprove) } else /* !pullRequestIsCompliant */ { - labelsUpdatedForNonCompliance := false // if PR doesn't have [cla: no] label, add it. if !hasLabelClaNo { addLabel(LabelClaNo) - labelsUpdatedForNonCompliance = true } else { logging.Infof(" No action needed: [%s] label already added", LabelClaNo) } // if PR has [cla: yes] label, remove it. if hasLabelClaYes { removeLabel(LabelClaYes) - labelsUpdatedForNonCompliance = true } else { logging.Infof(" No action needed: [%s] label already missing", LabelClaYes) } - if labelsUpdatedForNonCompliance { - addComment(pullRequestNonComplianceReason) - } + addReview(pullRequestNonComplianceReason, ReviewRequestChanges) } } return nil diff --git a/ghutil/ghutil_test.go b/ghutil/ghutil_test.go index de14049..40acf85 100644 --- a/ghutil/ghutil_test.go +++ b/ghutil/ghutil_test.go @@ -442,3 +442,103 @@ func TestCheckPullRequestCompliance_OneCompliantOneNot(t *testing.T) { assert.Equal(t, "Committer of one or more commits is not listed as a CLA signer, either individual or as a member of an organization.", nonComplianceReason) assert.Nil(t, err) } + +func TestCheckProcessPullRequest_Compliant_Approve(t *testing.T) { + setUp(t) + defer tearDown(t) + + expectRepoLabels(orgName, repoName, true, true, false) + repoClaLabelStatus := ghc.GetRepoClaLabelStatus(ctx, orgName, repoName) + + claSigners := config.ClaSigners{ + People: []config.Account{ + { + Name: "John Doe", + Email: "john@example.com", + Login: "john-doe", + }, + }, + } + var prNum = 42 + var num *int + num = &prNum + var prTitle = "Fix Things" + var title *string + title = &prTitle + pull := &github.PullRequest{Number: num, Title: title} + + var commits []*github.RepositoryCommit + mockGhc.PullRequests.EXPECT().ListCommits(ctx, orgName, repoName, pullNumber, nil).Return(commits, nil, nil) + mockGhc.Issues.EXPECT().ListLabelsByIssue(ctx, orgName, repoName, prNum, nil).Return(nil, nil, nil) + mockGhc.Issues.EXPECT().AddLabelsToIssue(ctx, orgName, repoName, prNum, []string{"cla: yes"}).Return(nil, nil, nil) + var reviewBody = "" + var body *string + body = &reviewBody + var reviewEvent = "APPROVE" + var event *string + event = &reviewEvent + var prReview = github.PullRequestReviewRequest{ + Body: body, + Event: event, + } + mockGhc.PullRequests.EXPECT().CreateReview(ctx, orgName, repoName, prNum, &prReview).Return(nil, nil, nil) + err := ghc.ProcessPullRequest(ctx, orgName, repoName, pull, claSigners, repoClaLabelStatus, true) + assert.Nil(t, err) +} + +func TestCheckProcessPullRequest_NotCompliant_RequestChanges(t *testing.T) { + setUp(t) + defer tearDown(t) + + expectRepoLabels(orgName, repoName, true, true, false) + repoClaLabelStatus := ghc.GetRepoClaLabelStatus(ctx, orgName, repoName) + + sha1 := "12345abcde" + name1 := "John Doe" + email1 := "john@example.com" + login1 := "john-doe" + + sha2 := "abcde24680" + name2 := "Jane Doe" + email2 := "jane@example.com" + login2 := "jane-doe" + + commits := []*github.RepositoryCommit{ + createCommit(sha1, name1, email1, login1), + createCommit(sha2, name2, email2, login2), + } + + claSigners := config.ClaSigners{ + People: []config.Account{ + { + Name: name1, + Email: email1, + Login: login1, + }, + }, + } + var prNum = 42 + var num *int + num = &prNum + var prTitle = "Fix Things" + var title *string + title = &prTitle + pull := &github.PullRequest{Number: num, Title: title} + + mockGhc.PullRequests.EXPECT().ListCommits(ctx, orgName, repoName, pullNumber, nil).Return(commits, nil, nil) + mockGhc.Issues.EXPECT().ListLabelsByIssue(ctx, orgName, repoName, prNum, nil).Return(nil, nil, nil) + mockGhc.Issues.EXPECT().AddLabelsToIssue(ctx, orgName, repoName, prNum, []string{"cla: no"}).Return(nil, nil, nil) + var reviewBody = "Committer of one or more commits is not listed as a CLA signer, either individual or as a member of an organization." + var body *string + body = &reviewBody + var reviewEvent = "REQUEST_CHANGES" + var event *string + event = &reviewEvent + var prReview = github.PullRequestReviewRequest{ + Body: body, + Event: event, + } + mockGhc.PullRequests.EXPECT().CreateReview(ctx, orgName, repoName, prNum, &prReview).Return(nil, nil, nil) + err := ghc.ProcessPullRequest(ctx, orgName, repoName, pull, claSigners, repoClaLabelStatus, true) + assert.Nil(t, err) +}