Repository: cli/cli
Pull request #8384 (2023-11-27 14:06:10)

Fix project status unmarshaling
## Description

This PR fixes https://github.com/cli/cli/issues/8358

In our previous attempt to fix this, we tried some tricks in an attempt to unmarshal a fragment specified with a `graphql` struct tag into a single field with a `json` struct tag. As it turns out, `shurcool`, our graphql library does not respect `json` tags at all. In its custom JSON decoder, it expects that fragments are unmarshalled into structs (which is pretty reasonable) so it was failing to discover the field we wanted the data to be unmarshaled into, erroring like so:

```
struct field for "optionId" doesn't exist in any of 3 places to unmarshal
```

This PR extracts a new struct called `StatusFragment` to actually represent that fragment. It also introduces some tests that fake the GQL responses to see that unmarshalling works as expected and finally, it fixes a mistake in the `export_pr` tests which hid this error case. In those tests, the JSON was being unmarshaled using the stdlib JSON decoder which unmarshalled successfully where as the real implementation used `shurcool` for this specific query.  


Patch:
diff --git a/api/queries_issue.go b/api/queries_issue.go
index fdef7783f..62531e84e 100644
--- a/api/queries_issue.go
+++ b/api/queries_issue.go
@@ -102,14 +102,18 @@ type ProjectInfo struct {
 
 type ProjectV2Item struct {
 	ID      string `json:"id"`
-	Project struct {
-		ID    string `json:"id"`
-		Title string `json:"title"`
-	}
-	Status struct {
-		OptionID string `json:"optionId" graphql:"... on ProjectV2ItemFieldSingleSelectValue{optionId}"`
-		Name     string `json:"name" graphql:"... on ProjectV2ItemFieldSingleSelectValue{name}"`
-	} `graphql:"status:fieldValueByName(name: \"Status\")"`
+	Project ProjectV2ItemProject
+	Status  ProjectV2ItemStatus
+}
+
+type ProjectV2ItemProject struct {
+	ID    string `json:"id"`
+	Title string `json:"title"`
+}
+
+type ProjectV2ItemStatus struct {
+	OptionID string `json:"optionId"`
+	Name     string `json:"name"`
 }
 
 func (p ProjectCards) ProjectNames() []string {
diff --git a/api/queries_projects_v2.go b/api/queries_projects_v2.go
index a602653af..958be6616 100644
--- a/api/queries_projects_v2.go
+++ b/api/queries_projects_v2.go
@@ -61,11 +61,27 @@ func UpdateProjectV2Items(client *Client, repo ghrepo.Interface, addProjectItems
 
 // ProjectsV2ItemsForIssue fetches all ProjectItems for an issue.
 func ProjectsV2ItemsForIssue(client *Client, repo ghrepo.Interface, issue *Issue) error {
+	type projectV2ItemStatus struct {
+		StatusFragment struct {
+			OptionID string `json:"optionId"`
+			Name     string `json:"name"`
+		} `graphql:"... on ProjectV2ItemFieldSingleSelectValue"`
+	}
+
+	type projectV2Item struct {
+		ID      string `json:"id"`
+		Project struct {
+			ID    string `json:"id"`
+			Title string `json:"title"`
+		}
+		Status projectV2ItemStatus `graphql:"status:fieldValueByName(name: \"Status\")"`
+	}
+
 	type response struct {
 		Repository struct {
 			Issue struct {
 				ProjectItems struct {
-					Nodes    []*ProjectV2Item
+					Nodes    []*projectV2Item
 					PageInfo struct {
 						HasNextPage bool
 						EndCursor   string
@@ -87,7 +103,20 @@ func ProjectsV2ItemsForIssue(client *Client, repo ghrepo.Interface, issue *Issue
 		if err != nil {
 			return err
 		}
-		items.Nodes = append(items.Nodes, query.Repository.Issue.ProjectItems.Nodes...)
+		for _, projectItemNode := range query.Repository.Issue.ProjectItems.Nodes {
+			items.Nodes = append(items.Nodes, &ProjectV2Item{
+				ID: projectItemNode.ID,
+				Project: ProjectV2ItemProject{
+					ID:    projectItemNode.Project.ID,
+					Title: projectItemNode.Project.Title,
+				},
+				Status: ProjectV2ItemStatus{
+					OptionID: projectItemNode.Status.StatusFragment.OptionID,
+					Name:     projectItemNode.Status.StatusFragment.Name,
+				},
+			})
+		}
+
 		if !query.Repository.Issue.ProjectItems.PageInfo.HasNextPage {
 			break
 		}
@@ -99,11 +128,27 @@ func ProjectsV2ItemsForIssue(client *Client, repo ghrepo.Interface, issue *Issue
 
 // ProjectsV2ItemsForPullRequest fetches all ProjectItems for a pull request.
 func ProjectsV2ItemsForPullRequest(client *Client, repo ghrepo.Interface, pr *PullRequest) error {
+	type projectV2ItemStatus struct {
+		StatusFragment struct {
+			OptionID string `json:"optionId"`
+			Name     string `json:"name"`
+		} `graphql:"... on ProjectV2ItemFieldSingleSelectValue"`
+	}
+
+	type projectV2Item struct {
+		ID      string `json:"id"`
+		Project struct {
+			ID    string `json:"id"`
+			Title string `json:"title"`
+		}
+		Status projectV2ItemStatus `graphql:"status:fieldValueByName(name: \"Status\")"`
+	}
+
 	type response struct {
 		Repository struct {
 			PullRequest struct {
 				ProjectItems struct {
-					Nodes    []*ProjectV2Item
+					Nodes    []*projectV2Item
 					PageInfo struct {
 						HasNextPage bool
 						EndCursor   string
@@ -125,7 +170,21 @@ func ProjectsV2ItemsForPullRequest(client *Client, repo ghrepo.Interface, pr *Pu
 		if err != nil {
 			return err
 		}
-		items.Nodes = append(items.Nodes, query.Repository.PullRequest.ProjectItems.Nodes...)
+
+		for _, projectItemNode := range query.Repository.PullRequest.ProjectItems.Nodes {
+			items.Nodes = append(items.Nodes, &ProjectV2Item{
+				ID: projectItemNode.ID,
+				Project: ProjectV2ItemProject{
+					ID:    projectItemNode.Project.ID,
+					Title: projectItemNode.Project.Title,
+				},
+				Status: ProjectV2ItemStatus{
+					OptionID: projectItemNode.Status.StatusFragment.OptionID,
+					Name:     projectItemNode.Status.StatusFragment.Name,
+				},
+			})
+		}
+
 		if !query.Repository.PullRequest.ProjectItems.PageInfo.HasNextPage {
 			break
 		}


Test patch:
diff --git a/api/queries_projects_v2_test.go b/api/queries_projects_v2_test.go
index bf6a618ba..fae49c926 100644
--- a/api/queries_projects_v2_test.go
+++ b/api/queries_projects_v2_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/cli/cli/v2/internal/ghrepo"
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUpdateProjectV2Items(t *testing.T) {
@@ -185,6 +186,61 @@ func TestProjectsV2ItemsForPullRequest(t *testing.T) {
 			},
 			expectError: true,
 		},
+		{
+			name: "retrieves project items that have status columns",
+			httpStubs: func(reg *httpmock.Registry) {
+				reg.Register(
+					httpmock.GraphQL(`query PullRequestProjectItems\b`),
+					httpmock.GraphQLQuery(`{
+                        "data": {
+                          "repository": {
+                            "pullRequest": {
+                              "projectItems": {
+                                "nodes": [
+                                  {
+                                    "id": "PVTI_lADOB-vozM4AVk16zgK6U50",
+                                    "project": {
+                                      "id": "PVT_kwDOB-vozM4AVk16",
+                                      "title": "Test Project"
+                                    },
+                                    "status": {
+                                      "optionId": "47fc9ee4",
+                                      "name": "In Progress"
+                                    }
+                                  }
+                                ],
+                                "pageInfo": {
+                                  "hasNextPage": false,
+                                  "endCursor": "MQ"
+                                }
+                              }
+                            }
+                          }
+                        }
+                      }`,
+						func(query string, inputs map[string]interface{}) {
+							require.Equal(t, float64(1), inputs["number"])
+							require.Equal(t, "OWNER", inputs["owner"])
+							require.Equal(t, "REPO", inputs["name"])
+						}),
+				)
+			},
+			expectItems: ProjectItems{
+				Nodes: []*ProjectV2Item{
+					{
+						ID: "PVTI_lADOB-vozM4AVk16zgK6U50",
+						Project: ProjectV2ItemProject{
+							ID:    "PVT_kwDOB-vozM4AVk16",
+							Title: "Test Project",
+						},
+						Status: ProjectV2ItemStatus{
+							OptionID: "47fc9ee4",
+							Name:     "In Progress",
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -199,11 +255,11 @@ func TestProjectsV2ItemsForPullRequest(t *testing.T) {
 			pr := &PullRequest{Number: 1}
 			err := ProjectsV2ItemsForPullRequest(client, repo, pr)
 			if tt.expectError {
-				assert.Error(t, err)
+				require.Error(t, err)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 			}
-			assert.Equal(t, tt.expectItems, pr.ProjectItems)
+			require.Equal(t, tt.expectItems, pr.ProjectItems)
 		})
 	}
 }
diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go
index e816500c2..5bed620c1 100644
--- a/pkg/cmd/issue/list/list_test.go
+++ b/pkg/cmd/issue/list/list_test.go
@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/browser"
 	"github.com/cli/cli/v2/internal/config"
 	"github.com/cli/cli/v2/internal/ghrepo"
@@ -19,6 +20,7 @@ import (
 	"github.com/cli/cli/v2/test"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
@@ -456,3 +458,151 @@ func Test_issueList(t *testing.T) {
 		})
 	}
 }
+
+func TestIssueList_withProjectItems(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+
+	reg.Register(
+		httpmock.GraphQL(`query IssueList\b`),
+		httpmock.GraphQLQuery(`{
+			"data": {
+			  "repository": {
+				"hasIssuesEnabled": true,
+				"issues": {
+				  "totalCount": 1,
+				  "nodes": [
+					{
+					  "projectItems": {
+						"nodes": [
+						  {
+							"id": "PVTI_lAHOAA3WC84AW6WNzgJ8rnQ",
+							"project": {
+							  "id": "PVT_kwHOAA3WC84AW6WN",
+							  "title": "Test Public Project"
+							},
+							"status": {
+							  "optionId": "47fc9ee4",
+							  "name": "In Progress"
+							}
+						  }
+						],
+						"totalCount": 1
+					  }
+					}
+				  ]
+				}
+			  }
+			}
+		  }`, func(_ string, params map[string]interface{}) {
+			require.Equal(t, map[string]interface{}{
+				"owner":  "OWNER",
+				"repo":   "REPO",
+				"limit":  float64(30),
+				"states": []interface{}{"OPEN"},
+			}, params)
+		}))
+
+	client := &http.Client{Transport: reg}
+	issuesAndTotalCount, err := issueList(
+		client,
+		ghrepo.New("OWNER", "REPO"),
+		prShared.FilterOptions{
+			Entity: "issue",
+		},
+		30,
+	)
+
+	require.NoError(t, err)
+	require.Len(t, issuesAndTotalCount.Issues, 1)
+	require.Len(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes, 1)
+
+	require.Equal(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes[0].ID, "PVTI_lAHOAA3WC84AW6WNzgJ8rnQ")
+
+	expectedProject := api.ProjectV2ItemProject{
+		ID:    "PVT_kwHOAA3WC84AW6WN",
+		Title: "Test Public Project",
+	}
+	require.Equal(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes[0].Project, expectedProject)
+
+	expectedStatus := api.ProjectV2ItemStatus{
+		OptionID: "47fc9ee4",
+		Name:     "In Progress",
+	}
+	require.Equal(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes[0].Status, expectedStatus)
+}
+
+func TestIssueList_Search_withProjectItems(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+
+	reg.Register(
+		httpmock.GraphQL(`query IssueSearch\b`),
+		httpmock.GraphQLQuery(`{
+			"data": {
+			  "repository": {
+				"hasIssuesEnabled": true
+			  },
+			  "search": {
+				"issueCount": 1,
+				"nodes": [
+				  {
+					"projectItems": {
+					  "nodes": [
+						{
+						  "id": "PVTI_lAHOAA3WC84AW6WNzgJ8rl0",
+						  "project": {
+							"id": "PVT_kwHOAA3WC84AW6WN",
+							"title": "Test Public Project"
+						  },
+						  "status": {
+							"optionId": "47fc9ee4",
+							"name": "In Progress"
+						  }
+						}
+					  ],
+					  "totalCount": 1
+					}
+				  }
+				]
+			  }
+			}
+		  }`, func(_ string, params map[string]interface{}) {
+			require.Equal(t, map[string]interface{}{
+				"owner": "OWNER",
+				"repo":  "REPO",
+				"type":  "ISSUE",
+				"limit": float64(30),
+				"query": "just used to force the search API branch repo:OWNER/REPO type:issue",
+			}, params)
+		}))
+
+	client := &http.Client{Transport: reg}
+	issuesAndTotalCount, err := issueList(
+		client,
+		ghrepo.New("OWNER", "REPO"),
+		prShared.FilterOptions{
+			Entity: "issue",
+			Search: "just used to force the search API branch",
+		},
+		30,
+	)
+
+	require.NoError(t, err)
+	require.Len(t, issuesAndTotalCount.Issues, 1)
+	require.Len(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes, 1)
+
+	require.Equal(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes[0].ID, "PVTI_lAHOAA3WC84AW6WNzgJ8rl0")
+
+	expectedProject := api.ProjectV2ItemProject{
+		ID:    "PVT_kwHOAA3WC84AW6WN",
+		Title: "Test Public Project",
+	}
+	require.Equal(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes[0].Project, expectedProject)
+
+	expectedStatus := api.ProjectV2ItemStatus{
+		OptionID: "47fc9ee4",
+		Name:     "In Progress",
+	}
+	require.Equal(t, issuesAndTotalCount.Issues[0].ProjectItems.Nodes[0].Status, expectedStatus)
+}
diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go
index 61a8e3698..869db9c38 100644
--- a/pkg/cmd/pr/list/list_test.go
+++ b/pkg/cmd/pr/list/list_test.go
@@ -9,15 +9,18 @@ import (
 	"time"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/browser"
 	"github.com/cli/cli/v2/internal/ghrepo"
 	"github.com/cli/cli/v2/internal/run"
+	prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/test"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
@@ -320,3 +323,146 @@ func TestPRList_web(t *testing.T) {
 		})
 	}
 }
+
+func TestPRList_withProjectItems(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+
+	reg.Register(
+		httpmock.GraphQL(`query PullRequestList\b`),
+		httpmock.GraphQLQuery(`{
+			"data": {
+			  "repository": {
+				"pullRequests": {
+				  "totalCount": 1,
+				  "nodes": [
+					{
+					  "projectItems": {
+						"nodes": [
+						  {
+							"id": "PVTI_lAHOAA3WC84AW6WNzgJ8rnQ",
+							"project": {
+							  "id": "PVT_kwHOAA3WC84AW6WN",
+							  "title": "Test Public Project"
+							},
+							"status": {
+							  "optionId": "47fc9ee4",
+							  "name": "In Progress"
+							}
+						  }
+						],
+						"totalCount": 1
+					  }
+					}
+				  ]
+				}
+			  }
+			}
+		  }`, func(_ string, params map[string]interface{}) {
+			require.Equal(t, map[string]interface{}{
+				"owner": "OWNER",
+				"repo":  "REPO",
+				"limit": float64(30),
+				"state": []interface{}{"OPEN"},
+			}, params)
+		}))
+
+	client := &http.Client{Transport: reg}
+	prsAndTotalCount, err := listPullRequests(
+		client,
+		ghrepo.New("OWNER", "REPO"),
+		prShared.FilterOptions{
+			Entity: "pr",
+			State:  "open",
+		},
+		30,
+	)
+
+	require.NoError(t, err)
+	require.Len(t, prsAndTotalCount.PullRequests, 1)
+	require.Len(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes, 1)
+
+	require.Equal(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes[0].ID, "PVTI_lAHOAA3WC84AW6WNzgJ8rnQ")
+
+	expectedProject := api.ProjectV2ItemProject{
+		ID:    "PVT_kwHOAA3WC84AW6WN",
+		Title: "Test Public Project",
+	}
+	require.Equal(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes[0].Project, expectedProject)
+
+	expectedStatus := api.ProjectV2ItemStatus{
+		OptionID: "47fc9ee4",
+		Name:     "In Progress",
+	}
+	require.Equal(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes[0].Status, expectedStatus)
+}
+
+func TestPRList_Search_withProjectItems(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+
+	reg.Register(
+		httpmock.GraphQL(`query PullRequestSearch\b`),
+		httpmock.GraphQLQuery(`{
+			"data": {
+			  "search": {
+				"issueCount": 1,
+				"nodes": [
+				  {
+					"projectItems": {
+					  "nodes": [
+						{
+						  "id": "PVTI_lAHOAA3WC84AW6WNzgJ8rl0",
+						  "project": {
+							"id": "PVT_kwHOAA3WC84AW6WN",
+							"title": "Test Public Project"
+						  },
+						  "status": {
+							"optionId": "47fc9ee4",
+							"name": "In Progress"
+						  }
+						}
+					  ],
+					  "totalCount": 1
+					}
+				  }
+				]
+			  }
+			}
+		  }`, func(_ string, params map[string]interface{}) {
+			require.Equal(t, map[string]interface{}{
+				"limit": float64(30),
+				"q":     "just used to force the search API branch repo:OWNER/REPO state:open type:pr",
+			}, params)
+		}))
+
+	client := &http.Client{Transport: reg}
+	prsAndTotalCount, err := listPullRequests(
+		client,
+		ghrepo.New("OWNER", "REPO"),
+		prShared.FilterOptions{
+			Entity: "pr",
+			State:  "open",
+			Search: "just used to force the search API branch",
+		},
+		30,
+	)
+
+	require.NoError(t, err)
+	require.Len(t, prsAndTotalCount.PullRequests, 1)
+	require.Len(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes, 1)
+
+	require.Equal(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes[0].ID, "PVTI_lAHOAA3WC84AW6WNzgJ8rl0")
+
+	expectedProject := api.ProjectV2ItemProject{
+		ID:    "PVT_kwHOAA3WC84AW6WN",
+		Title: "Test Public Project",
+	}
+	require.Equal(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes[0].Project, expectedProject)
+
+	expectedStatus := api.ProjectV2ItemStatus{
+		OptionID: "47fc9ee4",
+		Name:     "In Progress",
+	}
+	require.Equal(t, prsAndTotalCount.PullRequests[0].ProjectItems.Nodes[0].Status, expectedStatus)
+}
diff --git a/pkg/cmd/pr/shared/finder_test.go b/pkg/cmd/pr/shared/finder_test.go
index 23138ed2d..6df8d3000 100644
--- a/pkg/cmd/pr/shared/finder_test.go
+++ b/pkg/cmd/pr/shared/finder_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/cli/cli/v2/git"
 	"github.com/cli/cli/v2/internal/ghrepo"
 	"github.com/cli/cli/v2/pkg/httpmock"
+	"github.com/stretchr/testify/require"
 )
 
 func TestFind(t *testing.T) {
@@ -409,6 +410,68 @@ func TestFind(t *testing.T) {
 			wantPR:   13,
 			wantRepo: "https://github.com/OWNER/REPO",
 		},
+		{
+			name: "including project items",
+			args: args{
+				selector: "",
+				fields:   []string{"projectItems"},
+				baseRepoFn: func() (ghrepo.Interface, error) {
+					return ghrepo.FromFullName("OWNER/REPO")
+				},
+				branchFn: func() (string, error) {
+					return "blueberries", nil
+				},
+				branchConfig: func(branch string) (c git.BranchConfig) {
+					c.MergeRef = "refs/pull/13/head"
+					return
+				},
+			},
+			httpStub: func(r *httpmock.Registry) {
+				r.Register(
+					httpmock.GraphQL(`query PullRequestByNumber\b`),
+					httpmock.StringResponse(`{"data":{"repository":{
+						"pullRequest":{"number":13}
+					}}}`))
+
+				r.Register(
+					httpmock.GraphQL(`query PullRequestProjectItems\b`),
+					httpmock.GraphQLQuery(`{
+                        "data": {
+                          "repository": {
+                            "pullRequest": {
+                              "projectItems": {
+                                "nodes": [
+                                  {
+                                    "id": "PVTI_lADOB-vozM4AVk16zgK6U50",
+                                    "project": {
+                                      "id": "PVT_kwDOB-vozM4AVk16",
+                                      "title": "Test Project"
+                                    },
+                                    "status": {
+                                      "optionId": "47fc9ee4",
+                                      "name": "In Progress"
+                                    }
+                                  }
+                                ],
+                                "pageInfo": {
+                                  "hasNextPage": false,
+                                  "endCursor": "MQ"
+                                }
+                              }
+                            }
+                          }
+                        }
+                      }`,
+						func(query string, inputs map[string]interface{}) {
+							require.Equal(t, float64(13), inputs["number"])
+							require.Equal(t, "OWNER", inputs["owner"])
+							require.Equal(t, "REPO", inputs["name"])
+						}),
+				)
+			},
+			wantPR:   13,
+			wantRepo: "https://github.com/OWNER/REPO",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {