diff --git a/api/queries_repo.go b/api/queries_repo.go index e5806b91e..6754a1542 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -314,6 +314,47 @@ func FetchRepository(client *Client, repo ghrepo.Interface, fields []string) (*R return InitRepoHostname(result.Repository, repo.RepoHost()), nil } +// IssueRepoInfo fetches only the repository fields needed for issue operations such as +// issue creation and transfer, avoiding fields like defaultBranchRef that require additional +// token permissions. +func IssueRepoInfo(client *Client, repo ghrepo.Interface) (*Repository, error) { + query := ` + query IssueRepositoryInfo($owner: String!, $name: String!) { + repository(owner: $owner, name: $name) { + id + name + owner { login } + hasIssuesEnabled + viewerPermission + } + }` + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "name": repo.RepoName(), + } + + var result struct { + Repository *Repository + } + if err := client.GraphQL(repo.RepoHost(), query, variables, &result); err != nil { + return nil, err + } + // The GraphQL API should have returned an error in case of a missing repository, but this isn't + // guaranteed to happen when an authentication token with insufficient permissions is being used. + if result.Repository == nil { + return nil, GraphQLError{ + GraphQLError: &ghAPI.GraphQLError{ + Errors: []ghAPI.GraphQLErrorItem{{ + Type: "NOT_FOUND", + Message: fmt.Sprintf("Could not resolve to a Repository with the name '%s/%s'.", repo.RepoOwner(), repo.RepoName()), + }}, + }, + } + } + + return InitRepoHostname(result.Repository, repo.RepoHost()), nil +} + func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { query := ` fragment repo on Repository { diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index ae00a98b2..4fe7074f1 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -26,15 +26,181 @@ func TestGitHubRepo_notFound(t *testing.T) { client := newTestClient(httpReg) repo, err := GitHubRepo(client, ghrepo.New("OWNER", "REPO")) - if err == nil { - t.Fatal("GitHubRepo did not return an error") - } - if wants := "GraphQL: Could not resolve to a Repository with the name 'OWNER/REPO'."; err.Error() != wants { - t.Errorf("GitHubRepo error: want %q, got %q", wants, err.Error()) - } - if repo != nil { - t.Errorf("GitHubRepo: expected nil repo, got %v", repo) + require.EqualError(t, err, "GraphQL: Could not resolve to a Repository with the name 'OWNER/REPO'.") + assert.Nil(t, repo) +} + +func TestGitHubRepo_success(t *testing.T) { + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "hasIssuesEnabled": true, + "description": "a cool repo", + "hasWikiEnabled": true, + "viewerPermission": "ADMIN", + "defaultBranchRef": {"name": "main"}, + "parent": null, + "mergeCommitAllowed": true, + "rebaseMergeAllowed": true, + "squashMergeAllowed": false + } } }`)) + + client := newTestClient(httpReg) + repo, err := GitHubRepo(client, ghrepo.New("OWNER", "REPO")) + require.NoError(t, err) + assert.Equal(t, &Repository{ + ID: "REPOID", + Name: "REPO", + Owner: RepositoryOwner{Login: "OWNER"}, + HasIssuesEnabled: true, + Description: "a cool repo", + HasWikiEnabled: true, + ViewerPermission: "ADMIN", + DefaultBranchRef: BranchRef{Name: "main"}, + MergeCommitAllowed: true, + RebaseMergeAllowed: true, + hostname: "github.com", + }, repo) + assert.True(t, repo.ViewerCanPush()) + assert.True(t, repo.ViewerCanTriage()) +} + +func TestGitHubRepo_withParent(t *testing.T) { + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "hasIssuesEnabled": true, + "description": "", + "hasWikiEnabled": false, + "viewerPermission": "READ", + "defaultBranchRef": {"name": "main"}, + "parent": { + "id": "PARENTID", + "name": "PARENT-REPO", + "owner": {"login": "PARENT-OWNER"}, + "hasIssuesEnabled": true, + "description": "parent repo", + "hasWikiEnabled": true, + "viewerPermission": "READ", + "defaultBranchRef": {"name": "develop"} + }, + "mergeCommitAllowed": false, + "rebaseMergeAllowed": false, + "squashMergeAllowed": true + } } }`)) + + client := newTestClient(httpReg) + repo, err := GitHubRepo(client, ghrepo.New("OWNER", "REPO")) + require.NoError(t, err) + wantParent := &Repository{ + ID: "PARENTID", + Name: "PARENT-REPO", + Owner: RepositoryOwner{Login: "PARENT-OWNER"}, + HasIssuesEnabled: true, + Description: "parent repo", + HasWikiEnabled: true, + ViewerPermission: "READ", + DefaultBranchRef: BranchRef{Name: "develop"}, + hostname: "github.com", } + assert.Equal(t, &Repository{ + ID: "REPOID", + Name: "REPO", + Owner: RepositoryOwner{Login: "OWNER"}, + HasIssuesEnabled: true, + ViewerPermission: "READ", + DefaultBranchRef: BranchRef{Name: "main"}, + Parent: wantParent, + SquashMergeAllowed: true, + hostname: "github.com", + }, repo) + assert.False(t, repo.ViewerCanPush()) + assert.False(t, repo.ViewerCanTriage()) +} + +func TestIssueRepoInfo_notFound(t *testing.T) { + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query IssueRepositoryInfo\b`), + httpmock.StringResponse(`{ "data": { "repository": null } }`)) + + client := newTestClient(httpReg) + repo, err := IssueRepoInfo(client, ghrepo.New("OWNER", "REPO")) + require.EqualError(t, err, "GraphQL: Could not resolve to a Repository with the name 'OWNER/REPO'.") + assert.Nil(t, repo) +} + +func TestIssueRepoInfo_success(t *testing.T) { + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query IssueRepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } }`)) + + client := newTestClient(httpReg) + repo, err := IssueRepoInfo(client, ghrepo.New("OWNER", "REPO")) + require.NoError(t, err) + assert.Equal(t, &Repository{ + ID: "REPOID", + Name: "REPO", + Owner: RepositoryOwner{Login: "OWNER"}, + HasIssuesEnabled: true, + ViewerPermission: "WRITE", + hostname: "github.com", + }, repo) + assert.True(t, repo.ViewerCanTriage()) +} + +func TestIssueRepoInfo_issuesDisabled(t *testing.T) { + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query IssueRepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "hasIssuesEnabled": false, + "viewerPermission": "READ" + } } }`)) + + client := newTestClient(httpReg) + repo, err := IssueRepoInfo(client, ghrepo.New("OWNER", "REPO")) + require.NoError(t, err) + assert.Equal(t, &Repository{ + ID: "REPOID", + Name: "REPO", + Owner: RepositoryOwner{Login: "OWNER"}, + ViewerPermission: "READ", + hostname: "github.com", + }, repo) + assert.False(t, repo.ViewerCanTriage()) } func Test_RepoMetadata(t *testing.T) { diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index f9fab11a1..4346bd5cd 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -234,7 +234,7 @@ func createRun(opts *CreateOptions) (err error) { fmt.Fprintf(opts.IO.ErrOut, "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) } - repo, err := api.GitHubRepo(apiClient, baseRepo) + repo, err := api.IssueRepoInfo(apiClient, baseRepo) if err != nil { return } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 57de4c9b5..d595f34cd 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -411,7 +411,7 @@ func Test_createRun(t *testing.T) { name: "editor", httpStubs: func(r *httpmock.Registry) { r.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -440,7 +440,7 @@ func Test_createRun(t *testing.T) { name: "editor and template", httpStubs: func(r *httpmock.Registry) { r.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -522,7 +522,7 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(r *httpmock.Registry) { r.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -595,7 +595,7 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(r *httpmock.Registry) { r.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -727,7 +727,7 @@ func TestIssueCreate(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -760,7 +760,7 @@ func TestIssueCreate_recover(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -844,7 +844,7 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -907,7 +907,7 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -953,7 +953,7 @@ func TestIssueCreate_metadata(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.StubRepoInfoResponse("OWNER", "REPO", "main") + http.StubIssueRepoInfoResponse("OWNER", "REPO") http.Register( httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` @@ -1064,7 +1064,7 @@ func TestIssueCreate_disabledIssues(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -1091,7 +1091,7 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { `), ) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -1134,7 +1134,7 @@ func TestIssueCreate_AtCopilotAssignee(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "REPOID", @@ -1176,7 +1176,7 @@ func TestIssueCreate_projectsV2(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.StubRepoInfoResponse("OWNER", "REPO", "main") + http.StubIssueRepoInfoResponse("OWNER", "REPO") http.Register( httpmock.GraphQL(`query RepositoryProjectList\b`), httpmock.StringResponse(` @@ -1269,7 +1269,7 @@ func TestProjectsV1Deprecation(t *testing.T) { ios, _, _, _ := iostreams.Test() reg := &httpmock.Registry{} - reg.StubRepoInfoResponse("OWNER", "REPO", "main") + reg.StubIssueRepoInfoResponse("OWNER", "REPO") reg.Register( // ( is required to avoid matching projectsV2 httpmock.GraphQL(`projects\(`), @@ -1306,7 +1306,7 @@ func TestProjectsV1Deprecation(t *testing.T) { ios, _, _, _ := iostreams.Test() reg := &httpmock.Registry{} - reg.StubRepoInfoResponse("OWNER", "REPO", "main") + reg.StubIssueRepoInfoResponse("OWNER", "REPO") // ( is required to avoid matching projectsV2 reg.Exclude(t, httpmock.GraphQL(`projects\(`)) diff --git a/pkg/cmd/issue/transfer/transfer.go b/pkg/cmd/issue/transfer/transfer.go index a6dfb9b23..8ac1ff3fe 100644 --- a/pkg/cmd/issue/transfer/transfer.go +++ b/pkg/cmd/issue/transfer/transfer.go @@ -105,7 +105,7 @@ func issueTransfer(httpClient *http.Client, issueID string, destRepo ghrepo.Inte destinationRepoID = r.ID } else { apiClient := api.NewClientFromHTTP(httpClient) - r, err := api.GitHubRepo(apiClient, destRepo) + r, err := api.IssueRepoInfo(apiClient, destRepo) if err != nil { return "", err } diff --git a/pkg/cmd/issue/transfer/transfer_test.go b/pkg/cmd/issue/transfer/transfer_test.go index 2b12db944..12faad1b6 100644 --- a/pkg/cmd/issue/transfer/transfer_test.go +++ b/pkg/cmd/issue/transfer/transfer_test.go @@ -169,7 +169,7 @@ func Test_transferRunSuccessfulIssueTransfer(t *testing.T) { } } }`)) http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.GraphQL(`query IssueRepositoryInfo\b`), httpmock.StringResponse(` { "data": { "repository": { "id": "dest-id", diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go index ad92d0572..1484734f3 100644 --- a/pkg/httpmock/legacy.go +++ b/pkg/httpmock/legacy.go @@ -22,6 +22,20 @@ func (r *Registry) StubRepoInfoResponse(owner, repo, branch string) { `, repo, owner, branch))) } +func (r *Registry) StubIssueRepoInfoResponse(owner, repo string) { + r.Register( + GraphQL(`query IssueRepositoryInfo\b`), + StringResponse(fmt.Sprintf(` + { "data": { "repository": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `, repo, owner))) +} + func (r *Registry) StubRepoResponse(owner, repo string) { r.StubRepoResponseWithPermission(owner, repo, "WRITE") }