Improve issue comment re: overfetching, handling PRs

- `issue comment` no longer fetches all issue fields and thus avoids the
  problem when loading failed due to token not having access to projects

- `issue comment` now accepts either issue or pull number as argument.

api/query_builder.go

@@ -176,6 +176,8 @@ var PullRequestFields = append(IssueFields,
 	"statusCheckRollup",
 )
 
+// PullRequestGraphQL constructs a GraphQL query fragment for a set of pull request fields. Since GitHub
+// pull requests are also technically issues, this function can be used to query issues as well.
 func PullRequestGraphQL(fields []string) string {
 	var q []string
 	for _, field := range fields {

pkg/cmd/issue/comment/comment.go

@@ -1,10 +1,7 @@
 package comment
 
 import (
-	"net/http"
-
 	"github.com/MakeNowJust/heredoc"
-	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/ghrepo"
 	issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
 	prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
@@ -32,7 +29,13 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
 		`),
 		Args: cobra.ExactArgs(1),
 		PreRunE: func(cmd *cobra.Command, args []string) error {
-			opts.RetrieveCommentable = retrieveIssue(f.HttpClient, f.BaseRepo, args[0])
+			opts.RetrieveCommentable = func() (prShared.Commentable, ghrepo.Interface, error) {
+				httpClient, err := f.HttpClient()
+				if err != nil {
+					return nil, nil, err
+				}
+				return issueShared.IssueFromArgWithFields(httpClient, f.BaseRepo, args[0], []string{"id", "url"})
+			}
 			return prShared.CommentablePreRun(cmd, opts)
 		},
 		RunE: func(_ *cobra.Command, args []string) error {
@@ -58,22 +61,3 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
 
 	return cmd
 }
-
-func retrieveIssue(httpClient func() (*http.Client, error),
-	baseRepo func() (ghrepo.Interface, error),
-	selector string) func() (prShared.Commentable, ghrepo.Interface, error) {
-	return func() (prShared.Commentable, ghrepo.Interface, error) {
-		httpClient, err := httpClient()
-		if err != nil {
-			return nil, nil, err
-		}
-		apiClient := api.NewClientFromHTTP(httpClient)
-
-		issue, repo, err := issueShared.IssueFromArg(apiClient, baseRepo, selector)
-		if err != nil {
-			return nil, nil, err
-		}
-
-		return issue, repo, nil
-	}
-}

pkg/cmd/issue/comment/comment_test.go

@@ -203,7 +203,6 @@ func Test_commentRun(t *testing.T) {
 				ConfirmSubmitSurvey:   func() (bool, error) { return true, nil },
 			},
 			httpStubs: func(t *testing.T, reg *httpmock.Registry) {
-				mockIssueFromNumber(t, reg)
 				mockCommentCreate(t, reg)
 			},
 			stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n",
@@ -217,9 +216,6 @@ func Test_commentRun(t *testing.T) {
 
 				OpenInBrowser: func(string) error { return nil },
 			},
-			httpStubs: func(t *testing.T, reg *httpmock.Registry) {
-				mockIssueFromNumber(t, reg)
-			},
 			stderr: "Opening github.com/OWNER/REPO/issues/123 in your browser.\n",
 		},
 		{
@@ -232,7 +228,6 @@ func Test_commentRun(t *testing.T) {
 				EditSurvey: func() (string, error) { return "comment body", nil },
 			},
 			httpStubs: func(t *testing.T, reg *httpmock.Registry) {
-				mockIssueFromNumber(t, reg)
 				mockCommentCreate(t, reg)
 			},
 			stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n",
@@ -245,7 +240,6 @@ func Test_commentRun(t *testing.T) {
 				Body:        "comment body",
 			},
 			httpStubs: func(t *testing.T, reg *httpmock.Registry) {
-				mockIssueFromNumber(t, reg)
 				mockCommentCreate(t, reg)
 			},
 			stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n",
@@ -259,14 +253,17 @@ func Test_commentRun(t *testing.T) {
 
 		reg := &httpmock.Registry{}
 		defer reg.Verify(t)
-		tt.httpStubs(t, reg)
-
-		httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }
-		baseRepo := func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }
+		if tt.httpStubs != nil {
+			tt.httpStubs(t, reg)
+		}
 
 		tt.input.IO = io
-		tt.input.HttpClient = httpClient
-		tt.input.RetrieveCommentable = retrieveIssue(tt.input.HttpClient, baseRepo, "123")
+		tt.input.HttpClient = func() (*http.Client, error) {
+			return &http.Client{Transport: reg}, nil
+		}
+		tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) {
+			return &mockCommentable{}, ghrepo.New("OWNER", "REPO"), nil
+		}
 
 		t.Run(tt.name, func(t *testing.T) {
 			err := shared.CommentableRun(tt.input)
@@ -277,15 +274,13 @@ func Test_commentRun(t *testing.T) {
 	}
 }
 
-func mockIssueFromNumber(_ *testing.T, reg *httpmock.Registry) {
-	reg.Register(
-		httpmock.GraphQL(`query IssueByNumber\b`),
-		httpmock.StringResponse(`
-			{ "data": { "repository": { "hasIssuesEnabled": true, "issue": {
-				"number": 123,
-				"url": "https://github.com/OWNER/REPO/issues/123"
-			} } } }`),
-	)
+type mockCommentable struct{}
+
+func (c mockCommentable) Identifier() string {
+	return "ISSUE-ID"
+}
+func (c mockCommentable) Link() string {
+	return "https://github.com/OWNER/REPO/issues/123"
 }
 
 func mockCommentCreate(t *testing.T, reg *httpmock.Registry) {
@@ -296,6 +291,7 @@ func mockCommentCreate(t *testing.T, reg *httpmock.Registry) {
 			"url": "https://github.com/OWNER/REPO/issues/123#issuecomment-456"
 		} } } } }`,
 			func(inputs map[string]interface{}) {
+				assert.Equal(t, "ISSUE-ID", inputs["subjectId"])
 				assert.Equal(t, "comment body", inputs["body"])
 			}),
 	)

pkg/cmd/issue/shared/lookup.go

@@ -2,6 +2,7 @@ package shared
 
 import (
 	"fmt"
+	"net/http"
 	"net/url"
 	"regexp"
 	"strconv"
@@ -11,6 +12,8 @@ import (
 	"github.com/cli/cli/v2/internal/ghrepo"
 )
 
+// IssueFromArg loads an issue with all its fields.
+// Deprecated: use IssueFromArgWithFields instead.
 func IssueFromArg(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), arg string) (*api.Issue, ghrepo.Interface, error) {
 	issueNumber, baseRepo := issueMetadataFromURL(arg)
 
@@ -30,7 +33,31 @@ func IssueFromArg(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, er
 		}
 	}
 
-	issue, err := issueFromNumber(apiClient, baseRepo, issueNumber)
+	issue, err := api.IssueByNumber(apiClient, baseRepo, issueNumber)
+	return issue, baseRepo, err
+}
+
+// IssueFromArgWithFields loads an issue or pull request with the specified fields.
+func IssueFromArgWithFields(httpClient *http.Client, baseRepoFn func() (ghrepo.Interface, error), arg string, fields []string) (*api.Issue, ghrepo.Interface, error) {
+	issueNumber, baseRepo := issueMetadataFromURL(arg)
+
+	if issueNumber == 0 {
+		var err error
+		issueNumber, err = strconv.Atoi(strings.TrimPrefix(arg, "#"))
+		if err != nil {
+			return nil, nil, fmt.Errorf("invalid issue format: %q", arg)
+		}
+	}
+
+	if baseRepo == nil {
+		var err error
+		baseRepo, err = baseRepoFn()
+		if err != nil {
+			return nil, nil, fmt.Errorf("could not determine base repo: %w", err)
+		}
+	}
+
+	issue, err := findIssueOrPR(httpClient, baseRepo, issueNumber, fields)
 	return issue, baseRepo, err
 }
 
@@ -56,6 +83,34 @@ func issueMetadataFromURL(s string) (int, ghrepo.Interface) {
 	return issueNumber, repo
 }
 
-func issueFromNumber(apiClient *api.Client, repo ghrepo.Interface, issueNumber int) (*api.Issue, error) {
-	return api.IssueByNumber(apiClient, repo, issueNumber)
+func findIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.Issue, error) {
+	type response struct {
+		Repository struct {
+			Issue *api.Issue
+		}
+	}
+
+	query := fmt.Sprintf(`
+	query IssueByNumber($owner: String!, $repo: String!, $number: Int!) {
+		repository(owner: $owner, name: $repo) {
+			issue: issueOrPullRequest(number: $number) {
+				...on Issue{%[1]s}
+				...on PullRequest{%[1]s}
+			}
+		}
+	}`, api.PullRequestGraphQL(fields))
+
+	variables := map[string]interface{}{
+		"owner":  repo.RepoOwner(),
+		"repo":   repo.RepoName(),
+		"number": number,
+	}
+
+	var resp response
+	client := api.NewClientFromHTTP(httpClient)
+	if err := client.GraphQL(repo.RepoHost(), query, variables, &resp); err != nil {
+		return nil, err
+	}
+
+	return resp.Repository.Issue, nil
 }