add -r flag for to update command to add reviewers to new PRs

With reference to #145, add a command-line flag to `git spr update` to
add reviewers to any newly created PRs.

    git spr update -r ejoffe

This will, if possible in the context of the repo, request from Eitan
on the newly created PR.

In the case of existing PRs, it will instead print a warning that the
flag has not been applied to a specific pre-existing PR.

    warning: not updating reviewers for PR #1

This is done using the [requestReviews] graphql API call. Since
[requestReviews] operates on the opaque [ID] values, first pull down
the [Repository].`assignableUsers` value and use that to find the [ID]
mapping for each user's login name.

[requestReviews]: https://docs.github.com/en/github-ae@latest/graphql/reference/mutations#requestreviews
[ID]: https://docs.github.com/en/github-ae@latest/graphql/reference/scalars#id
[Repository]: https://docs.github.com/en/github-ae@latest/graphql/reference/objects#repository

cmd/spr/main.go

@@ -136,11 +136,16 @@ VERSION: {{.Version}}
 				Aliases: []string{"u", "up"},
 				Usage:   "Update and create pull requests for updated commits in the stack",
 				Action: func(c *cli.Context) error {
-					stackedpr.UpdatePullRequests(ctx)
+					stackedpr.UpdatePullRequests(ctx, c.StringSlice("reviewer"))
 					return nil
 				},
 				Flags: []cli.Flag{
 					detailFlag,
+					&cli.StringSliceFlag{
+						Name:    "reviewer",
+						Aliases: []string{"r"},
+						Usage:   "Add the specified reviewer to newly created pull requests",
+					},
 				},
 			},
 			{
@@ -154,7 +159,7 @@ VERSION: {{.Version}}
 					} else {
 						stackedpr.MergePullRequests(ctx, nil)
 					}
-					stackedpr.UpdatePullRequests(ctx)
+					stackedpr.UpdatePullRequests(ctx, nil)
 					return nil
 				},
 				Flags: []cli.Flag{

github/githubclient/client.go

@@ -275,6 +275,49 @@ func (c *client) GetInfo(ctx context.Context, gitcmd git.GitInterface) *github.G
 	return info
 }
 
+// GetAssignableUsers is taken from github.com/cli/cli/api and is the approach used by the official gh
+// client to resolve user IDs to "ID" values for the update PR API calls. See api.RepoAssignableUsers.
+func (c *client) GetAssignableUsers(ctx context.Context) []github.RepoAssignee {
+	if c.config.User.LogGitHubCalls {
+		fmt.Printf("> github get assignable users\n")
+	}
+	type responseData struct {
+		Repository struct {
+			AssignableUsers struct {
+				Nodes    []github.RepoAssignee
+				PageInfo struct {
+					HasNextPage bool
+					EndCursor   string
+				}
+			} `graphql:"assignableUsers(first: 100, after: $endCursor)"`
+		} `graphql:"repository(owner: $owner, name: $name)"`
+	}
+
+	variables := map[string]interface{}{
+		"owner":     githubv4.String(c.config.Repo.GitHubRepoOwner),
+		"name":      githubv4.String(c.config.Repo.GitHubRepoName),
+		"endCursor": (*githubv4.String)(nil),
+	}
+
+	users := []github.RepoAssignee{}
+	for {
+		var query responseData
+		err := c.api.Query(ctx, &query, variables)
+		if err != nil {
+			log.Fatal().Err(err).Msg("get assignable users failed")
+			return nil
+		}
+
+		users = append(users, query.Repository.AssignableUsers.Nodes...)
+		if !query.Repository.AssignableUsers.PageInfo.HasNextPage {
+			break
+		}
+		variables["endCursor"] = githubv4.String(query.Repository.AssignableUsers.PageInfo.EndCursor)
+	}
+
+	return users
+}
+
 func (c *client) CreatePullRequest(ctx context.Context,
 	info *github.GitHubInfo, commit git.Commit, prevCommit *git.Commit) *github.PullRequest {
 
@@ -438,6 +481,48 @@ func (c *client) UpdatePullRequest(ctx context.Context,
 	}
 }
 
+func ghIds(s []string) *[]githubv4.ID {
+	ids := make([]githubv4.ID, len(s))
+	for i, v := range s {
+		ids[i] = v
+	}
+	return &ids
+}
+
+// AddReviewers adds reviewers to the provided pull request using the requestReviews() API call. It
+// takes github user IDs (ID type) as its input. These can be found by first querying the AssignableUsers
+// for the repo, and then mapping login name to ID.
+func (c *client) AddReviewers(ctx context.Context, pr *github.PullRequest, userIDs []string) {
+	log.Debug().Strs("userIDs", userIDs).Msg("AddReviewers")
+	if c.config.User.LogGitHubCalls {
+		fmt.Printf("> github add reviewers %d - %s - %+v\n", pr.Number, pr.Title, userIDs)
+	}
+	var mutation struct {
+		RequestReviews struct {
+			PullRequest struct {
+				ID string
+			}
+		} `graphql:"requestReviews(input: $input)"`
+	}
+	union := githubv4.Boolean(false)
+	params := githubv4.RequestReviewsInput{
+		PullRequestID: pr.ID,
+		Union:         &union,
+		UserIDs:       ghIds(userIDs),
+	}
+	// variables := map[string]interface{}{"input": params}
+	err := c.api.Mutate(context.Background(), &mutation, params, nil)
+	if err != nil {
+		log.Fatal().
+			Str("id", pr.ID).
+			Int("number", pr.Number).
+			Str("title", pr.Title).
+			Strs("userIDs", userIDs).
+			Err(err).
+			Msg("add reviewers failed")
+	}
+}
+
 func (c *client) CommentPullRequest(ctx context.Context, pr *github.PullRequest, comment string) {
 	var updatepr struct {
 		PullRequest struct {

github/interface.go

@@ -9,8 +9,10 @@ import (
 
 type GitHubInterface interface {
 	GetInfo(ctx context.Context, gitcmd git.GitInterface) *GitHubInfo
+	GetAssignableUsers(ctx context.Context) []RepoAssignee
 	CreatePullRequest(ctx context.Context, info *GitHubInfo, commit git.Commit, prevCommit *git.Commit) *PullRequest
 	UpdatePullRequest(ctx context.Context, info *GitHubInfo, pr *PullRequest, commit git.Commit, prevCommit *git.Commit)
+	AddReviewers(ctx context.Context, pr *PullRequest, userIDs []string)
 	CommentPullRequest(ctx context.Context, pr *PullRequest, comment string)
 	MergePullRequest(ctx context.Context, pr *PullRequest, mergeMethod githubv4.PullRequestMergeMethod)
 	ClosePullRequest(ctx context.Context, pr *PullRequest)
@@ -22,3 +24,9 @@ type GitHubInfo struct {
 	LocalBranch  string
 	PullRequests []*PullRequest
 }
+
+type RepoAssignee struct {
+	ID    string
+	Login string
+	Name  string
+}

github/mockclient/mockclient.go

@@ -11,6 +11,11 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const (
+	NobodyUserID = "U_kgDOBb2UmA"
+	NobodyLogin  = "nobody"
+)
+
 // NewMockClient creates a new mock client
 func NewMockClient(t *testing.T) *MockClient {
 	return &MockClient{
@@ -32,6 +37,20 @@ func (c *MockClient) GetInfo(ctx context.Context, gitcmd git.GitInterface) *gith
 	return c.Info
 }
 
+func (c *MockClient) GetAssignableUsers(ctx context.Context) []github.RepoAssignee {
+	fmt.Printf("HUB: GetAssignableUsers\n")
+	c.verifyExpectation(expectation{
+		op: getAssignableUsersOP,
+	})
+	return []github.RepoAssignee{
+		{
+			ID:    NobodyUserID,
+			Login: NobodyLogin,
+			Name:  "No Body",
+		},
+	}
+}
+
 func (c *MockClient) CreatePullRequest(ctx context.Context, info *github.GitHubInfo,
 	commit git.Commit, prevCommit *git.Commit) *github.PullRequest {
 	fmt.Printf("HUB: CreatePullRequest\n")
@@ -69,6 +88,13 @@ func (c *MockClient) UpdatePullRequest(ctx context.Context, info *github.GitHubI
 	})
 }
 
+func (c *MockClient) AddReviewers(ctx context.Context, pr *github.PullRequest, userIDs []string) {
+	c.verifyExpectation(expectation{
+		op:      addReviewersOP,
+		userIDs: userIDs,
+	})
+}
+
 func (c *MockClient) CommentPullRequest(ctx context.Context, pr *github.PullRequest, comment string) {
 	fmt.Printf("HUB: CommentPullRequest\n")
 	c.verifyExpectation(expectation{
@@ -101,6 +127,12 @@ func (c *MockClient) ExpectGetInfo() {
 	})
 }
 
+func (c *MockClient) ExpectGetAssignableUsers() {
+	c.expect = append(c.expect, expectation{
+		op: getAssignableUsersOP,
+	})
+}
+
 func (c *MockClient) ExpectCreatePullRequest(commit git.Commit, prev *git.Commit) {
 	c.expect = append(c.expect, expectation{
 		op:     createPullRequestOP,
@@ -117,6 +149,13 @@ func (c *MockClient) ExpectUpdatePullRequest(commit git.Commit, prev *git.Commit
 	})
 }
 
+func (c *MockClient) ExpectAddReviewers(userIDs []string) {
+	c.expect = append(c.expect, expectation{
+		op:      addReviewersOP,
+		userIDs: userIDs,
+	})
+}
+
 func (c *MockClient) ExpectCommentPullRequest(commit git.Commit) {
 	c.expect = append(c.expect, expectation{
 		op:     commentPullRequestOP,
@@ -149,8 +188,10 @@ type operation string
 
 const (
 	getInfoOP            operation = "GetInfo"
+	getAssignableUsersOP operation = "GetAssignableUsers"
 	createPullRequestOP  operation = "CreatePullRequest"
 	updatePullRequestOP  operation = "UpdatePullRequest"
+	addReviewersOP       operation = "AddReviewers"
 	commentPullRequestOP operation = "CommentPullRequest"
 	mergePullRequestOP   operation = "MergePullRequest"
 	closePullRequestOP   operation = "ClosePullRequest"
@@ -161,4 +202,5 @@ type expectation struct {
 	commit      git.Commit
 	prev        *git.Commit
 	mergeMethod githubv4.PullRequestMergeMethod
+	userIDs     []string
 }

spr/spr.go

@@ -77,14 +77,33 @@ func (sd *stackediff) AmendCommit(ctx context.Context) {
 	sd.mustgit("rebase -i --autosquash --autostash", nil)
 }
 
+func (sd *stackediff) addReviewers(ctx context.Context, pr *github.PullRequest, reviewers []string) {
+	assignable := sd.github.GetAssignableUsers(ctx)
+	userIDs := make([]string, 0, len(reviewers))
+	for _, r := range reviewers {
+		found := false
+		for _, u := range assignable {
+			if strings.EqualFold(r, u.Login) {
+				found = true
+				userIDs = append(userIDs, u.ID)
+				break
+			}
+		}
+		if !found {
+			check(fmt.Errorf("unable to add reviewer, user %q not found", r))
+		}
+	}
+	sd.github.AddReviewers(ctx, pr, userIDs)
+}
+
 // UpdatePullRequests implements a stacked diff workflow on top of github.
 //  Each time it's called it compares the local branch unmerged commits
 //   with currently open pull requests in github.
 //  It will create a new pull request for all new commits, and update the
 //   pull request if a commit has been amended.
 //  In the case where commits are reordered, the corresponding pull requests
 //   will also be reordered to match the commit stack order.
-func (sd *stackediff) UpdatePullRequests(ctx context.Context) {
+func (sd *stackediff) UpdatePullRequests(ctx context.Context, reviewers []string) {
 	sd.profiletimer.Step("UpdatePullRequests::Start")
 	githubInfo := sd.fetchAndGetGitHubInfo(ctx)
 	if githubInfo == nil {
@@ -148,6 +167,9 @@ func (sd *stackediff) UpdatePullRequests(ctx context.Context) {
 				}
 				updateQueue = append(updateQueue, prUpdate{pr, c, prevCommit})
 				pr.Commit = c
+				if len(reviewers) != 0 {
+					fmt.Fprintf(sd.output, "warning: not updating reviewers for PR #%d\n", pr.Number)
+				}
 				break
 			}
 		}
@@ -161,6 +183,9 @@ func (sd *stackediff) UpdatePullRequests(ctx context.Context) {
 			pr := sd.github.CreatePullRequest(ctx, githubInfo, c, prevCommit)
 			githubInfo.PullRequests = append(githubInfo.PullRequests, pr)
 			updateQueue = append(updateQueue, prUpdate{pr, c, prevCommit})
+			if len(reviewers) != 0 {
+				sd.addReviewers(ctx, pr, reviewers)
+			}
 		}
 	}
 	sd.profiletimer.Step("UpdatePullRequests::updatePullRequests")

spr/spr_test.go

@@ -79,9 +79,11 @@ func TestSPRBasicFlowFourCommits(t *testing.T) {
 	gitmock.ExpectLogAndRespond([]*git.Commit{&c1})
 	gitmock.ExpectPushCommits([]*git.Commit{&c1})
 	githubmock.ExpectCreatePullRequest(c1, nil)
+	githubmock.ExpectGetAssignableUsers()
+	githubmock.ExpectAddReviewers([]string{mockclient.NobodyUserID})
 	githubmock.ExpectUpdatePullRequest(c1, nil)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, []string{mockclient.NobodyLogin})
 	fmt.Printf("OUT: %s\n", output.String())
 	assert.Equal("[✔✔✔✔]   1 : test commit 1\n", output.String())
 	output.Reset()
@@ -92,14 +94,17 @@ func TestSPRBasicFlowFourCommits(t *testing.T) {
 	gitmock.ExpectLogAndRespond([]*git.Commit{&c2, &c1})
 	gitmock.ExpectPushCommits([]*git.Commit{&c2})
 	githubmock.ExpectCreatePullRequest(c2, &c1)
+	githubmock.ExpectGetAssignableUsers()
+	githubmock.ExpectAddReviewers([]string{mockclient.NobodyUserID})
 	githubmock.ExpectUpdatePullRequest(c1, nil)
 	githubmock.ExpectUpdatePullRequest(c2, &c1)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, []string{mockclient.NobodyLogin})
 	lines := strings.Split(output.String(), "\n")
 	fmt.Printf("OUT: %s\n", output.String())
-	assert.Equal("[✔✔✔✔]   1 : test commit 2", lines[0])
-	assert.Equal("[✔✔✔✔]   1 : test commit 1", lines[1])
+	assert.Equal("warning: not updating reviewers for PR #1", lines[0])
+	assert.Equal("[✔✔✔✔]   1 : test commit 2", lines[1])
+	assert.Equal("[✔✔✔✔]   1 : test commit 1", lines[2])
 	output.Reset()
 
 	// 'git spr -u' :: UpdatePullRequest :: commits=[c1, c2, c3, c4]
@@ -114,7 +119,7 @@ func TestSPRBasicFlowFourCommits(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c3, &c2)
 	githubmock.ExpectUpdatePullRequest(c4, &c3)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	lines = strings.Split(output.String(), "\n")
 	fmt.Printf("OUT: %s\n", output.String())
 	assert.Equal("[✔✔✔✔]   1 : test commit 4", lines[0])
@@ -175,7 +180,7 @@ func TestSPRAmendCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c1, nil)
 	githubmock.ExpectUpdatePullRequest(c2, &c1)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	fmt.Printf("OUT: %s\n", output.String())
 	lines := strings.Split(output.String(), "\n")
 	assert.Equal("[✔✔✔✔]   1 : test commit 2", lines[0])
@@ -192,7 +197,7 @@ func TestSPRAmendCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c1, nil)
 	githubmock.ExpectUpdatePullRequest(c2, &c1)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	lines = strings.Split(output.String(), "\n")
 	fmt.Printf("OUT: %s\n", output.String())
 	assert.Equal("[✔✔✔✔]   1 : test commit 2", lines[0])
@@ -210,7 +215,7 @@ func TestSPRAmendCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c1, nil)
 	githubmock.ExpectUpdatePullRequest(c2, &c1)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	lines = strings.Split(output.String(), "\n")
 	fmt.Printf("OUT: %s\n", output.String())
 	assert.Equal("[✔✔✔✔]   1 : test commit 2", lines[0])
@@ -279,7 +284,7 @@ func TestSPRReorderCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c3, &c2)
 	githubmock.ExpectUpdatePullRequest(c4, &c3)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	fmt.Printf("OUT: %s\n", output.String())
 	lines := strings.Split(output.String(), "\n")
 	assert.Equal("[✔✔✔✔]   1 : test commit 4", lines[0])
@@ -307,7 +312,7 @@ func TestSPRReorderCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c1, &c4)
 	githubmock.ExpectUpdatePullRequest(c3, &c1)
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	fmt.Printf("OUT: %s\n", output.String())
 	// TODO : Need to update pull requests in GetInfo expect to get this check to work
 	// lines = strings.Split(output.String(), "\n")
@@ -367,7 +372,7 @@ func TestSPRDeleteCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c4, &c3)
 	githubmock.ExpectGetInfo()
 
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	fmt.Printf("OUT: %s\n", output.String())
 	lines := strings.Split(output.String(), "\n")
 	assert.Equal("[✔✔✔✔]   1 : test commit 4", lines[0])
@@ -391,7 +396,7 @@ func TestSPRDeleteCommit(t *testing.T) {
 	githubmock.ExpectUpdatePullRequest(c4, &c1)
 	gitmock.ExpectPushCommits([]*git.Commit{&c1, &c4})
 	githubmock.ExpectGetInfo()
-	s.UpdatePullRequests(ctx)
+	s.UpdatePullRequests(ctx, nil)
 	fmt.Printf("OUT: %s\n", output.String())
 	// TODO : Need to update pull requests in GetInfo expect to get this check to work
 	// lines = strings.Split(output.String(), "\n")