From 00cede9e5f45f2024f611da65103e015c4764b4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Fri, 20 Dec 2019 13:07:11 +0100
Subject: [PATCH 1/2] Fix `issue list` re: issues that have an assignee

Given the GraphQL query:

    issues(filterBy: {assignee: $assignee})

It turns out that passing a query variable `"assignee": null` is NOT
equivalent to omitting the variable altogether:

- `"assignee": null` seems to filter out issues that HAVE an assignee;
- omitting `assignee` correctly returns all issues.
---
 api/queries_issue.go  | 29 ++++++++++-------------------
 command/issue_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/api/queries_issue.go b/api/queries_issue.go
index d76e1b79e..2d4e22229 100644
--- a/api/queries_issue.go
+++ b/api/queries_issue.go
@@ -154,19 +154,6 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig
 		return nil, fmt.Errorf("invalid state: %s", state)
 	}
 
-	// If you don't want to filter by lables, graphql requires you need
-	// to send nil instead of an empty array.
-	if len(labels) == 0 {
-		labels = nil
-	}
-
-	var assignee interface{}
-	if len(assigneeString) > 0 {
-		assignee = assigneeString
-	} else {
-		assignee = nil
-	}
-
 	query := fragments + `
     query($owner: String!, $repo: String!, $limit: Int, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) {
       repository(owner: $owner, name: $repo) {
@@ -183,12 +170,16 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig
 	owner := ghRepo.RepoOwner()
 	repo := ghRepo.RepoName()
 	variables := map[string]interface{}{
-		"limit":    limit,
-		"owner":    owner,
-		"repo":     repo,
-		"states":   states,
-		"labels":   labels,
-		"assignee": assignee,
+		"limit":  limit,
+		"owner":  owner,
+		"repo":   repo,
+		"states": states,
+	}
+	if len(labels) > 0 {
+		variables["labels"] = labels
+	}
+	if assigneeString != "" {
+		variables["assignee"] = assigneeString
 	}
 
 	var resp struct {
diff --git a/command/issue_test.go b/command/issue_test.go
index 50462a982..47924e865 100644
--- a/command/issue_test.go
+++ b/command/issue_test.go
@@ -150,6 +150,34 @@ func TestIssueList_withFlags(t *testing.T) {
 	eq(t, reqBody.Variables.States, []string{"OPEN"})
 }
 
+func TestIssueList_nullAssigneeLabels(t *testing.T) {
+	initBlankContext("OWNER/REPO", "master")
+	http := initFakeHTTP()
+
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "data": {	"repository": {
+		"hasIssuesEnabled": true,
+		"issues": { "nodes": [] }
+	} } }
+	`))
+
+	_, err := RunCommand(issueListCmd, "issue list")
+	if err != nil {
+		t.Errorf("error running command `issue list`: %v", err)
+	}
+
+	bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
+	reqBody := struct {
+		Variables map[string]interface{}
+	}{}
+	json.Unmarshal(bodyBytes, &reqBody)
+
+	_, assigneeDeclared := reqBody.Variables["assignee"]
+	_, labelsDeclared := reqBody.Variables["labels"]
+	eq(t, assigneeDeclared, false)
+	eq(t, labelsDeclared, false)
+}
+
 func TestIssueList_disabledIssues(t *testing.T) {
 	initBlankContext("OWNER/REPO", "master")
 	http := initFakeHTTP()

From 9c36c7bae9b753061ae13824a06080c281e76960 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Fri, 20 Dec 2019 13:17:02 +0100
Subject: [PATCH 2/2] Ensure that string flags are reset between test runs

---
 command/pr_test.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/command/pr_test.go b/command/pr_test.go
index 8333fb1ef..91ec11a13 100644
--- a/command/pr_test.go
+++ b/command/pr_test.go
@@ -50,12 +50,14 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) {
 	cmd.SetErr(&errBuf)
 
 	// Reset flag values so they don't leak between tests
+	// FIXME: change how we initialize Cobra commands to render this hack unnecessary
 	cmd.Flags().VisitAll(func(f *pflag.Flag) {
 		switch v := f.Value.(type) {
 		case pflag.SliceValue:
 			v.Replace([]string{})
 		default:
-			if v.Type() == "bool" {
+			switch v.Type() {
+			case "bool", "string":
 				v.Set(f.DefValue)
 			}
 		}