Improve issue reopen re: overfetching, handling PRs

mislav · mislav · commit f99a149eb206 · 2021-11-23T19:39:40.000+01:00
- `issue reopen` no longer fetches all issue fields and thus avoids the
  problem when loading failed due to token not having access to projects

- `issue reopen` now accepts either issue or pull number as argument.
diff --git a/api/queries_issue.go b/api/queries_issue.go
@@ -342,27 +342,6 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e
 	return &resp.Repository.Issue, nil
 }
 
-func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error {
-	var mutation struct {
-		ReopenIssue struct {
-			Issue struct {
-				ID githubv4.ID
-			}
-		} `graphql:"reopenIssue(input: $input)"`
-	}
-
-	variables := map[string]interface{}{
-		"input": githubv4.ReopenIssueInput{
-			IssueID: issue.ID,
-		},
-	}
-
-	gql := graphQLClient(client.http, repo.RepoHost())
-	err := gql.MutateNamed(context.Background(), "IssueReopen", &mutation, variables)
-
-	return err
-}
-
 func IssueDelete(client *Client, repo ghrepo.Interface, issue Issue) error {
 	var mutation struct {
 		DeleteIssue struct {
diff --git a/api/queries_pr.go b/api/queries_pr.go
@@ -679,7 +679,7 @@ func PullRequestClose(httpClient *http.Client, repo ghrepo.Interface, prID strin
 	return gql.MutateNamed(context.Background(), "PullRequestClose", &mutation, variables)
 }
 
-func PullRequestReopen(client *Client, repo ghrepo.Interface, pr *PullRequest) error {
+func PullRequestReopen(httpClient *http.Client, repo ghrepo.Interface, prID string) error {
 	var mutation struct {
 		ReopenPullRequest struct {
 			PullRequest struct {
@@ -690,14 +690,12 @@ func PullRequestReopen(client *Client, repo ghrepo.Interface, pr *PullRequest) e
 
 	variables := map[string]interface{}{
 		"input": githubv4.ReopenPullRequestInput{
-			PullRequestID: pr.ID,
+			PullRequestID: prID,
 		},
 	}
 
-	gql := graphQLClient(client.http, repo.RepoHost())
-	err := gql.MutateNamed(context.Background(), "PullRequestReopen", &mutation, variables)
-
-	return err
+	gql := graphQLClient(httpClient, repo.RepoHost())
+	return gql.MutateNamed(context.Background(), "PullRequestReopen", &mutation, variables)
 }
 
 func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) error {
diff --git a/pkg/cmd/issue/reopen/reopen.go b/pkg/cmd/issue/reopen/reopen.go
@@ -1,15 +1,19 @@
 package reopen
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 
 	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/config"
+	"github.com/cli/cli/v2/internal/ghinstance"
 	"github.com/cli/cli/v2/internal/ghrepo"
 	"github.com/cli/cli/v2/pkg/cmd/issue/shared"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/iostreams"
+	graphql "github.com/cli/shurcooL-graphql"
+	"github.com/shurcooL/githubv4"
 	"github.com/spf13/cobra"
 )
 
@@ -58,9 +62,8 @@ func reopenRun(opts *ReopenOptions) error {
 	if err != nil {
 		return err
 	}
-	apiClient := api.NewClientFromHTTP(httpClient)
 
-	issue, baseRepo, err := shared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg)
+	issue, baseRepo, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, []string{"id", "number", "title", "state"})
 	if err != nil {
 		return err
 	}
@@ -70,7 +73,7 @@ func reopenRun(opts *ReopenOptions) error {
 		return nil
 	}
 
-	err = api.IssueReopen(apiClient, baseRepo, *issue)
+	err = apiReopen(httpClient, baseRepo, issue)
 	if err != nil {
 		return err
 	}
@@ -79,3 +82,26 @@ func reopenRun(opts *ReopenOptions) error {
 
 	return nil
 }
+
+func apiReopen(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue) error {
+	if issue.IsPullRequest() {
+		return api.PullRequestReopen(httpClient, repo, issue.ID)
+	}
+
+	var mutation struct {
+		ReopenIssue struct {
+			Issue struct {
+				ID githubv4.ID
+			}
+		} `graphql:"reopenIssue(input: $input)"`
+	}
+
+	variables := map[string]interface{}{
+		"input": githubv4.ReopenIssueInput{
+			IssueID: issue.ID,
+		},
+	}
+
+	gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient)
+	return gql.MutateNamed(context.Background(), "IssueReopen", &mutation, variables)
+}
diff --git a/pkg/cmd/issue/reopen/reopen_test.go b/pkg/cmd/issue/reopen/reopen_test.go
@@ -119,9 +119,24 @@ func TestIssueReopen_issuesDisabled(t *testing.T) {
 	http.Register(
 		httpmock.GraphQL(`query IssueByNumber\b`),
 		httpmock.StringResponse(`
-			{ "data": { "repository": {
-				"hasIssuesEnabled": false
-			} } }`),
+		{
+			"data": {
+				"repository": {
+					"hasIssuesEnabled": false,
+					"issue": null
+				}
+			},
+			"errors": [
+				{
+					"type": "NOT_FOUND",
+					"path": [
+						"repository",
+						"issue"
+					],
+					"message": "Could not resolve to an issue or pull request with the number of 2."
+				}
+			]
+		}`),
 	)
 
 	_, err := runCommand(http, true, "2")
diff --git a/pkg/cmd/pr/reopen/reopen.go b/pkg/cmd/pr/reopen/reopen.go
@@ -73,9 +73,8 @@ func reopenRun(opts *ReopenOptions) error {
 	if err != nil {
 		return err
 	}
-	apiClient := api.NewClientFromHTTP(httpClient)
 
-	err = api.PullRequestReopen(apiClient, baseRepo, pr)
+	err = api.PullRequestReopen(httpClient, baseRepo, pr.ID)
 	if err != nil {
 		return fmt.Errorf("API call failed: %w", err)
 	}

Original file line number	Diff line number	Diff line change
`@@ -73,9 +73,8 @@ func reopenRun(opts *ReopenOptions) error {`
`73`	`73`	`if err != nil {`
`74`	`74`	`return err`
`75`	`75`	`}`
`76`		`- apiClient := api.NewClientFromHTTP(httpClient)`
`77`	`76`
`78`		`- err = api.PullRequestReopen(apiClient, baseRepo, pr)`
	`77`	`+ err = api.PullRequestReopen(httpClient, baseRepo, pr.ID)`
`79`	`78`	`if err != nil {`
`80`	`79`	`return fmt.Errorf("API call failed: %w", err)`
`81`	`80`	`}`