Skip to content

Commit

Permalink
Merge pull request cli#178 from github/issues-disabled
Browse files Browse the repository at this point in the history
Warn about repo issues disabled on `issue status/list/create`
  • Loading branch information
Nate Smith authored Dec 19, 2019
2 parents 58a6cbc + bd9b3b9 commit ade69a4
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 80 deletions.
99 changes: 54 additions & 45 deletions api/queries_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ type IssueLabel struct {
Name string
}

type apiIssues struct {
Issues struct {
Nodes []Issue
}
}

const fragments = `
fragment issue on Issue {
number
Expand All @@ -47,12 +41,8 @@ const fragments = `
}
`

func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*Issue, error) {
repoID, err := GitHubRepoId(client, ghRepo)
if err != nil {
return nil, err
}

// IssueCreate creates an issue in a GitHub repository
func IssueCreate(client *Client, repo *Repository, params map[string]interface{}) (*Issue, error) {
query := `
mutation CreateIssue($input: CreateIssueInput!) {
createIssue(input: $input) {
Expand All @@ -63,7 +53,7 @@ func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*I
}`

inputParams := map[string]interface{}{
"repositoryId": repoID,
"repositoryId": repo.ID,
}
for key, val := range params {
inputParams[key] = val
Expand All @@ -78,7 +68,7 @@ func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*I
}
}{}

err = client.GraphQL(query, variables, &result)
err := client.GraphQL(query, variables, &result)
if err != nil {
return nil, err
}
Expand All @@ -88,36 +78,41 @@ func IssueCreate(client *Client, ghRepo Repo, params map[string]interface{}) (*I

func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) {
type response struct {
Assigned apiIssues
Mentioned apiIssues
Authored apiIssues
Repository struct {
Assigned struct {
Nodes []Issue
}
Mentioned struct {
Nodes []Issue
}
Authored struct {
Nodes []Issue
}
HasIssuesEnabled bool
}
}

query := fragments + `
query($owner: String!, $repo: String!, $viewer: String!, $per_page: Int = 10) {
assigned: repository(owner: $owner, name: $repo) {
issues(filterBy: {assignee: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
...issue
}
}
}
mentioned: repository(owner: $owner, name: $repo) {
issues(filterBy: {mentioned: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
...issue
}
}
}
authored: repository(owner: $owner, name: $repo) {
issues(filterBy: {createdBy: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
...issue
}
}
}
}
`
query($owner: String!, $repo: String!, $viewer: String!, $per_page: Int = 10) {
repository(owner: $owner, name: $repo) {
hasIssuesEnabled
assigned: issues(filterBy: {assignee: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
...issue
}
}
mentioned: issues(filterBy: {mentioned: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
...issue
}
}
authored: issues(filterBy: {createdBy: $viewer, states: OPEN}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
...issue
}
}
}
}`

owner := ghRepo.RepoOwner()
repo := ghRepo.RepoName()
Expand All @@ -133,10 +128,14 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa
return nil, err
}

if !resp.Repository.HasIssuesEnabled {
return nil, fmt.Errorf("the '%s/%s' repository has disabled issues", owner, repo)
}

payload := IssuesPayload{
Assigned: resp.Assigned.Issues.Nodes,
Mentioned: resp.Mentioned.Issues.Nodes,
Authored: resp.Authored.Issues.Nodes,
Assigned: resp.Repository.Assigned.Nodes,
Mentioned: resp.Repository.Mentioned.Nodes,
Authored: resp.Repository.Authored.Nodes,
}

return &payload, nil
Expand Down Expand Up @@ -171,6 +170,7 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig
query := fragments + `
query($owner: String!, $repo: String!, $limit: Int, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) {
repository(owner: $owner, name: $repo) {
hasIssuesEnabled
issues(first: $limit, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) {
nodes {
...issue
Expand All @@ -192,14 +192,23 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig
}

var resp struct {
Repository apiIssues
Repository struct {
Issues struct {
Nodes []Issue
}
HasIssuesEnabled bool
}
}

err := client.GraphQL(query, variables, &resp)
if err != nil {
return nil, err
}

if !resp.Repository.HasIssuesEnabled {
return nil, fmt.Errorf("the '%s/%s' repository has disabled issues", owner, repo)
}

return resp.Repository.Issues.Nodes, nil
}

Expand Down
4 changes: 2 additions & 2 deletions api/queries_pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ func PullRequestForBranch(client *Client, ghRepo Repo, branch string) (*PullRequ
}

func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{}) (*PullRequest, error) {
repoID, err := GitHubRepoId(client, ghRepo)
repo, err := GitHubRepo(client, ghRepo)
if err != nil {
return nil, err
}
Expand All @@ -381,7 +381,7 @@ func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{
}`

inputParams := map[string]interface{}{
"repositoryId": repoID,
"repositoryId": repo.ID,
}
for key, val := range params {
inputParams[key] = val
Expand Down
39 changes: 27 additions & 12 deletions api/queries_repo.go
Original file line number Diff line number Diff line change
@@ -1,31 +1,46 @@
package api

import "fmt"
import (
"fmt"

func GitHubRepoId(client *Client, ghRepo Repo) (string, error) {
"github.com/pkg/errors"
)

// Repository contains information about a GitHub repo
type Repository struct {
ID string
HasIssuesEnabled bool
}

// GitHubRepo looks up the node ID of a named repository
func GitHubRepo(client *Client, ghRepo Repo) (*Repository, error) {
owner := ghRepo.RepoOwner()
repo := ghRepo.RepoName()

query := `
query FindRepoID($owner:String!, $name:String!) {
repository(owner:$owner, name:$name) {
id
}
query($owner: String!, $name: String!) {
repository(owner: $owner, name: $name) {
id
hasIssuesEnabled
}
}`
variables := map[string]interface{}{
"owner": owner,
"name": repo,
}

result := struct {
Repository struct {
Id string
}
Repository Repository
}{}
err := client.GraphQL(query, variables, &result)
if err != nil || result.Repository.Id == "" {
return "", fmt.Errorf("failed to determine GH repo ID: %s", err)

if err != nil || result.Repository.ID == "" {
newErr := fmt.Errorf("failed to determine repository ID for '%s/%s'", owner, repo)
if err != nil {
newErr = errors.Wrap(err, newErr.Error())
}
return nil, newErr
}

return result.Repository.Id, nil
return &result.Repository, nil
}
10 changes: 9 additions & 1 deletion command/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,14 @@ func issueCreate(cmd *cobra.Command, args []string) error {
return err
}

repo, err := api.GitHubRepo(apiClient, baseRepo)
if err != nil {
return err
}
if !repo.HasIssuesEnabled {
return fmt.Errorf("the '%s/%s' repository has disabled issues", baseRepo.RepoOwner(), baseRepo.RepoName())
}

title, err := cmd.Flags().GetString("title")
if err != nil {
return errors.Wrap(err, "could not parse title")
Expand Down Expand Up @@ -291,7 +299,7 @@ func issueCreate(cmd *cobra.Command, args []string) error {
"body": body,
}

newIssue, err := api.IssueCreate(apiClient, baseRepo, params)
newIssue, err := api.IssueCreate(apiClient, repo, params)
if err != nil {
return err
}
Expand Down
71 changes: 64 additions & 7 deletions command/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ func TestIssueStatus_blankSlate(t *testing.T) {
http := initFakeHTTP()

http.StubResponse(200, bytes.NewBufferString(`
{ "data": {
"assigned": { "issues": { "nodes": [] } },
"mentioned": { "issues": { "nodes": [] } },
"authored": { "issues": { "nodes": [] } }
} }
{ "data": { "repository": {
"hasIssuesEnabled": true,
"assigned": { "nodes": [] },
"mentioned": { "nodes": [] },
"authored": { "nodes": [] }
} } }
`))

output, err := RunCommand(issueStatusCmd, "issue status")
Expand All @@ -72,6 +73,22 @@ Issues opened by you
}
}

func TestIssueStatus_disabledIssues(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()

http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"hasIssuesEnabled": false
} } }
`))

_, err := RunCommand(issueStatusCmd, "issue status")
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Errorf("error running command `issue status`: %v", err)
}
}

func TestIssueList(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()
Expand Down Expand Up @@ -100,9 +117,15 @@ func TestIssueList(t *testing.T) {
}

func TestIssueList_withFlags(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()

http.StubResponse(200, bytes.NewBufferString(`{"data": {}}`)) // Since we are testing that the flags are passed, we don't care about the response
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"hasIssuesEnabled": true,
"issues": { "nodes": [] }
} } }
`))

output, err := RunCommand(issueListCmd, "issue list -a probablyCher -l web,bug -s open")
if err != nil {
Expand All @@ -127,6 +150,22 @@ func TestIssueList_withFlags(t *testing.T) {
eq(t, reqBody.Variables.States, []string{"OPEN"})
}

func TestIssueList_disabledIssues(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()

http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"hasIssuesEnabled": false
} } }
`))

_, err := RunCommand(issueListCmd, "issue list")
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Errorf("error running command `issue list`: %v", err)
}
}

func TestIssueView(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()
Expand Down Expand Up @@ -225,7 +264,8 @@ func TestIssueCreate(t *testing.T) {

http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"id": "REPOID"
"id": "REPOID",
"hasIssuesEnabled": true
} } }
`))
http.StubResponse(200, bytes.NewBufferString(`
Expand Down Expand Up @@ -258,6 +298,23 @@ func TestIssueCreate(t *testing.T) {
eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n")
}

func TestIssueCreate_disabledIssues(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
http := initFakeHTTP()

http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": false
} } }
`))

_, err := RunCommand(issueCreateCmd, `issue create -t heres -b johnny`)
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Errorf("error running command `issue create`: %v", err)
}
}

func TestIssueCreate_web(t *testing.T) {
initBlankContext("OWNER/REPO", "master")
initFakeHTTP()
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/issueList.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"data": {
"repository": {
"hasIssuesEnabled": true,
"issues": {
"nodes": [
{
Expand Down
Loading

0 comments on commit ade69a4

Please sign in to comment.