From dd4d243f867edc89df88fd6cd30bcdae34934f61 Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Wed, 7 May 2025 13:10:07 +0500 Subject: [PATCH 1/3] [gh issue view] Expose `closedByPullRequestsReferences` JSON fields --- api/export_pr.go | 19 ++++++++- api/export_pr_test.go | 73 ++++++++++++++++++++++++++++++++- api/queries_issue.go | 22 ++++++++++ api/query_builder.go | 22 ++++++++++ pkg/cmd/issue/view/http.go | 39 ++++++++++++++++++ pkg/cmd/issue/view/view.go | 27 +++++++----- pkg/cmd/issue/view/view_test.go | 1 + 7 files changed, 191 insertions(+), 12 deletions(-) diff --git a/api/export_pr.go b/api/export_pr.go index 7ae1a4ff4e9..9b030c39ed7 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -28,6 +28,24 @@ func (issue *Issue) ExportData(fields []string) map[string]interface{} { }) } data[f] = items + case "closedByPullRequestsReferences": + items := make([]map[string]interface{}, 0, len(issue.ClosedByPullRequestsReferences.Nodes)) + for _, n := range issue.ClosedByPullRequestsReferences.Nodes { + items = append(items, map[string]interface{}{ + "id": n.ID, + "number": n.Number, + "url": n.URL, + "repository": map[string]interface{}{ + "id": n.Repository.ID, + "name": n.Repository.Name, + "owner": map[string]interface{}{ + "id": n.Repository.Owner.ID, + "login": n.Repository.Owner.Login, + }, + }, + }) + } + data[f] = items default: sf := fieldByName(v, f) data[f] = sf.Interface() @@ -143,7 +161,6 @@ func (pr *PullRequest) ExportData(fields []string) map[string]interface{} { items := make([]map[string]interface{}, 0, len(pr.ClosingIssuesReferences.Nodes)) for _, n := range pr.ClosingIssuesReferences.Nodes { items = append(items, map[string]interface{}{ - "id": n.ID, "number": n.Number, "url": n.URL, diff --git a/api/export_pr_test.go b/api/export_pr_test.go index 09a1dffe870..1f310693e68 100644 --- a/api/export_pr_test.go +++ b/api/export_pr_test.go @@ -107,6 +107,70 @@ func TestIssue_ExportData(t *testing.T) { } `), }, + { + name: "linked pull requests", + fields: []string{"closedByPullRequestsReferences"}, + inputJSON: heredoc.Doc(` + { "closedByPullRequestsReferences": { "nodes": [ + { + "id": "I_123", + "number": 123, + "url": "https://github.com/cli/cli/pull/123", + "repository": { + "id": "R_123", + "name": "cli", + "owner": { + "id": "O_123", + "login": "cli" + } + } + }, + { + "id": "I_456", + "number": 456, + "url": "https://github.com/cli/cli/pull/456", + "repository": { + "id": "R_456", + "name": "cli", + "owner": { + "id": "O_456", + "login": "cli" + } + } + } + ] } } + `), + outputJSON: heredoc.Doc(` + { "closedByPullRequestsReferences": [ + { + "id": "I_123", + "number": 123, + "repository": { + "id": "R_123", + "name": "cli", + "owner": { + "id": "O_123", + "login": "cli" + } + }, + "url": "https://github.com/cli/cli/pull/123" + }, + { + "id": "I_456", + "number": 456, + "repository": { + "id": "R_456", + "name": "cli", + "owner": { + "id": "O_456", + "login": "cli" + } + }, + "url": "https://github.com/cli/cli/pull/456" + } + ] } + `), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -120,7 +184,14 @@ func TestIssue_ExportData(t *testing.T) { enc := json.NewEncoder(&buf) enc.SetIndent("", "\t") require.NoError(t, enc.Encode(exported)) - assert.Equal(t, tt.outputJSON, buf.String()) + + var gotData interface{} + dec = json.NewDecoder(&buf) + require.NoError(t, dec.Decode(&gotData)) + var expectData interface{} + require.NoError(t, json.Unmarshal([]byte(tt.outputJSON), &expectData)) + + assert.Equal(t, expectData, gotData) }) } } diff --git a/api/queries_issue.go b/api/queries_issue.go index 094b6b198c7..f093601522a 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -44,6 +44,28 @@ type Issue struct { Milestone *Milestone ReactionGroups ReactionGroups IsPinned bool + + ClosedByPullRequestsReferences ClosedByPullRequestsReferences +} + +type ClosedByPullRequestsReferences struct { + Nodes []struct { + ID string + Number int + URL string + Repository struct { + ID string + Name string + Owner struct { + ID string + Login string + } + } + } + PageInfo struct { + HasNextPage bool + EndCursor string + } } // return values for Issue.Typename diff --git a/api/query_builder.go b/api/query_builder.go index 4c45da3c1b2..47fb4c2253b 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -56,6 +56,25 @@ var issueCommentLast = shortenQuery(` } `) +var issueClosedByPullRequestsReferences = shortenQuery(` + closedByPullRequestsReferences(first: 100) { + nodes { + id, + number, + url, + repository { + id, + name, + owner { + id, + login + } + } + } + pageInfo{hasNextPage,endCursor} + } +`) + var prReviewRequests = shortenQuery(` reviewRequests(first: 100) { nodes { @@ -296,6 +315,7 @@ var sharedIssuePRFields = []string{ var issueOnlyFields = []string{ "isPinned", "stateReason", + "closedByPullRequestsReferences", } var IssueFields = append(sharedIssuePRFields, issueOnlyFields...) @@ -388,6 +408,8 @@ func IssueGraphQL(fields []string) string { q = append(q, StatusCheckRollupGraphQLWithCountByState()) case "closingIssuesReferences": q = append(q, prClosingIssuesReferences) + case "closedByPullRequestsReferences": + q = append(q, issueClosedByPullRequestsReferences) default: q = append(q, field) } diff --git a/pkg/cmd/issue/view/http.go b/pkg/cmd/issue/view/http.go index e4f756436c6..4adc71802dc 100644 --- a/pkg/cmd/issue/view/http.go +++ b/pkg/cmd/issue/view/http.go @@ -53,3 +53,42 @@ func preloadIssueComments(client *http.Client, repo ghrepo.Interface, issue *api issue.Comments.PageInfo.HasNextPage = false return nil } + +func preloadClosedByPullRequestsReferences(client *http.Client, repo ghrepo.Interface, issue *api.Issue) error { + if !issue.ClosedByPullRequestsReferences.PageInfo.HasNextPage { + return nil + } + + type response struct { + Node struct { + Issue struct { + ClosedByPullRequestsReferences api.ClosedByPullRequestsReferences `graphql:"closedByPullRequestsReferences(first: 100, after: $endCursor)"` + } `graphql:"...on Issue"` + } `graphql:"node(id: $id)"` + } + + variables := map[string]interface{}{ + "id": githubv4.ID(issue.ID), + "endCursor": githubv4.String(issue.ClosedByPullRequestsReferences.PageInfo.EndCursor), + } + + gql := api.NewClientFromHTTP(client) + + for { + var query response + err := gql.Query(repo.RepoHost(), "closedByPullRequestsReferences", &query, variables) + if err != nil { + return err + } + + issue.ClosedByPullRequestsReferences.Nodes = append(issue.ClosedByPullRequestsReferences.Nodes, query.Node.Issue.ClosedByPullRequestsReferences.Nodes...) + + if !query.Node.Issue.ClosedByPullRequestsReferences.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Node.Issue.ClosedByPullRequestsReferences.PageInfo.EndCursor) + } + + issue.ClosedByPullRequestsReferences.PageInfo.HasNextPage = false + return nil +} diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index a9e25513bc9..cf2fa2897e5 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -106,16 +106,16 @@ func viewRun(opts *ViewOptions) error { return err } - lookupFields := set.NewStringSet() + fields := set.NewStringSet() if opts.Exporter != nil { - lookupFields.AddValues(opts.Exporter.Fields()) + fields.AddValues(opts.Exporter.Fields()) } else if opts.WebMode { - lookupFields.Add("url") + fields.Add("url") } else { - lookupFields.AddValues(defaultFields) + fields.AddValues(defaultFields) if opts.Comments { - lookupFields.Add("comments") - lookupFields.Remove("lastComment") + fields.Add("comments") + fields.Remove("lastComment") } // TODO projectsV1Deprecation @@ -127,25 +127,32 @@ func viewRun(opts *ViewOptions) error { projectsV1Support := opts.Detector.ProjectsV1() if projectsV1Support == gh.ProjectsV1Supported { - lookupFields.Add("projectCards") + fields.Add("projectCards") } } opts.IO.DetectTerminalTheme() opts.IO.StartProgressIndicator() - lookupFields.Add("id") + defer opts.IO.StopProgressIndicator() - issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, lookupFields.ToSlice()) + fields.Add("id") + + issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, fields.ToSlice()) if err != nil { return err } - if lookupFields.Contains("comments") { + if fields.Contains("comments") { // FIXME: this re-fetches the comments connection even though the initial set of 100 were // fetched in the previous request. err = preloadIssueComments(httpClient, baseRepo, issue) } + + if fields.Contains("closedByPullRequestsReferences") { + err = preloadClosedByPullRequestsReferences(httpClient, baseRepo, issue) + } + opts.IO.StopProgressIndicator() if err != nil { var loadErr *issueShared.PartialLoadError diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 391a288fb21..71b0884a1d8 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -31,6 +31,7 @@ func TestJSONFields(t *testing.T) { "body", "closed", "comments", + "closedByPullRequestsReferences", "createdAt", "closedAt", "id", From 62940b732460314533b4edd8a063c88583d0b855 Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Wed, 7 May 2025 14:37:33 +0500 Subject: [PATCH 2/3] Incorporate GitHub Copilot review suggestions --- pkg/cmd/issue/view/view.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index cf2fa2897e5..5b37da17fa5 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -140,29 +140,32 @@ func viewRun(opts *ViewOptions) error { issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, fields.ToSlice()) if err != nil { - return err + var partialLoadErr *issueShared.PartialLoadError + if opts.Exporter == nil && errors.As(err, &partialLoadErr) { + fmt.Fprintf(opts.IO.ErrOut, "warning: %s\n", partialLoadErr.Error()) + } else { + return err + } } if fields.Contains("comments") { // FIXME: this re-fetches the comments connection even though the initial set of 100 were // fetched in the previous request. - err = preloadIssueComments(httpClient, baseRepo, issue) + err := preloadIssueComments(httpClient, baseRepo, issue) + if err != nil { + return err + } } if fields.Contains("closedByPullRequestsReferences") { - err = preloadClosedByPullRequestsReferences(httpClient, baseRepo, issue) - } - - opts.IO.StopProgressIndicator() - if err != nil { - var loadErr *issueShared.PartialLoadError - if opts.Exporter == nil && errors.As(err, &loadErr) { - fmt.Fprintf(opts.IO.ErrOut, "warning: %s\n", loadErr.Error()) - } else { + err := preloadClosedByPullRequestsReferences(httpClient, baseRepo, issue) + if err != nil { return err } } + opts.IO.StopProgressIndicator() + if opts.WebMode { openURL := issue.URL if opts.IO.IsStdoutTTY() { From 1e129698bc291f443cd73bf792fba7f62f871b87 Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Wed, 7 May 2025 16:39:25 +0500 Subject: [PATCH 3/3] Incorporate review changes --- pkg/cmd/issue/view/view.go | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 5b37da17fa5..3b02a3f2dc7 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -1,7 +1,6 @@ package view import ( - "errors" "fmt" "io" "net/http" @@ -106,16 +105,16 @@ func viewRun(opts *ViewOptions) error { return err } - fields := set.NewStringSet() + lookupFields := set.NewStringSet() if opts.Exporter != nil { - fields.AddValues(opts.Exporter.Fields()) + lookupFields.AddValues(opts.Exporter.Fields()) } else if opts.WebMode { - fields.Add("url") + lookupFields.Add("url") } else { - fields.AddValues(defaultFields) + lookupFields.AddValues(defaultFields) if opts.Comments { - fields.Add("comments") - fields.Remove("lastComment") + lookupFields.Add("comments") + lookupFields.Remove("lastComment") } // TODO projectsV1Deprecation @@ -127,7 +126,7 @@ func viewRun(opts *ViewOptions) error { projectsV1Support := opts.Detector.ProjectsV1() if projectsV1Support == gh.ProjectsV1Supported { - fields.Add("projectCards") + lookupFields.Add("projectCards") } } @@ -136,19 +135,14 @@ func viewRun(opts *ViewOptions) error { opts.IO.StartProgressIndicator() defer opts.IO.StopProgressIndicator() - fields.Add("id") + lookupFields.Add("id") - issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, fields.ToSlice()) + issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, lookupFields.ToSlice()) if err != nil { - var partialLoadErr *issueShared.PartialLoadError - if opts.Exporter == nil && errors.As(err, &partialLoadErr) { - fmt.Fprintf(opts.IO.ErrOut, "warning: %s\n", partialLoadErr.Error()) - } else { - return err - } + return err } - if fields.Contains("comments") { + if lookupFields.Contains("comments") { // FIXME: this re-fetches the comments connection even though the initial set of 100 were // fetched in the previous request. err := preloadIssueComments(httpClient, baseRepo, issue) @@ -157,7 +151,7 @@ func viewRun(opts *ViewOptions) error { } } - if fields.Contains("closedByPullRequestsReferences") { + if lookupFields.Contains("closedByPullRequestsReferences") { err := preloadClosedByPullRequestsReferences(httpClient, baseRepo, issue) if err != nil { return err