Skip to content

Commit f30afce

Browse files
mislavmislav
authored
Merge pull request cli#3547 from cli/pr-lookup-refactor
Eliminate API overfetching in `pr` commands
2 parents 068ad31 + 1440fd8 commit f30afce

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+2080
-2900
lines changed

‎api/export_pr.go‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,6 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} {
1111

1212
for _, f := range fields {
1313
switch f {
14-
case "milestone":
15-
if issue.Milestone.Title != "" {
16-
data[f] = map[string]string{"title": issue.Milestone.Title}
17-
} else {
18-
data[f] = nil
19-
}
2014
case "comments":
2115
data[f] = issue.Comments.Nodes
2216
case "assignees":
@@ -41,19 +35,36 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} {
4135
for _, f := range fields {
4236
switch f {
4337
case "headRepository":
44-
data[f] = map[string]string{"name": pr.HeadRepository.Name}
45-
case "milestone":
46-
if pr.Milestone.Title != "" {
47-
data[f] = map[string]string{"title": pr.Milestone.Title}
48-
} else {
49-
data[f] = nil
50-
}
38+
data[f] = pr.HeadRepository
5139
case "statusCheckRollup":
52-
if n := pr.Commits.Nodes; len(n) > 0 {
40+
if n := pr.StatusCheckRollup.Nodes; len(n) > 0 {
5341
data[f] = n[0].Commit.StatusCheckRollup.Contexts.Nodes
5442
} else {
5543
data[f] = nil
5644
}
45+
case "commits":
46+
commits := make([]interface{}, 0, len(pr.Commits.Nodes))
47+
for _, c := range pr.Commits.Nodes {
48+
commit := c.Commit
49+
authors := make([]interface{}, 0, len(commit.Authors.Nodes))
50+
for _, author := range commit.Authors.Nodes {
51+
authors = append(authors, map[string]interface{}{
52+
"name": author.Name,
53+
"email": author.Email,
54+
"id": author.User.ID,
55+
"login": author.User.Login,
56+
})
57+
}
58+
commits = append(commits, map[string]interface{}{
59+
"oid": commit.OID,
60+
"messageHeadline": commit.MessageHeadline,
61+
"messageBody": commit.MessageBody,
62+
"committedDate": commit.CommittedDate,
63+
"authoredDate": commit.AuthoredDate,
64+
"authors": authors,
65+
})
66+
}
67+
data[f] = commits
5768
case "comments":
5869
data[f] = pr.Comments.Nodes
5970
case "assignees":

‎api/export_pr_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ func TestIssue_ExportData(t *testing.T) {
4040
outputJSON: heredoc.Doc(`
4141
{
4242
"milestone": {
43-
"title": "The next big thing"
43+
"number": 0,
44+
"title": "The next big thing",
45+
"description": "",
46+
"dueOn": null
4447
},
4548
"number": 2345
4649
}
@@ -119,7 +122,10 @@ func TestPullRequest_ExportData(t *testing.T) {
119122
outputJSON: heredoc.Doc(`
120123
{
121124
"milestone": {
122-
"title": "The next big thing"
125+
"number": 0,
126+
"title": "The next big thing",
127+
"description": "",
128+
"dueOn": null
123129
},
124130
"number": 2345
125131
}
@@ -129,7 +135,7 @@ func TestPullRequest_ExportData(t *testing.T) {
129135
name: "status checks",
130136
fields: []string{"statusCheckRollup"},
131137
inputJSON: heredoc.Doc(`
132-
{ "commits": { "nodes": [
138+
{ "statusCheckRollup": { "nodes": [
133139
{ "commit": { "statusCheckRollup": { "contexts": { "nodes": [
134140
{
135141
"__typename": "CheckRun",

‎api/pull_request_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
func TestPullRequest_ChecksStatus(t *testing.T) {
1111
pr := PullRequest{}
1212
payload := `
13-
{ "commits": { "nodes": [{ "commit": {
13+
{ "statusCheckRollup": { "nodes": [{ "commit": {
1414
"statusCheckRollup": {
1515
"contexts": {
1616
"nodes": [

‎api/queries_comments.go‎

Lines changed: 4 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@ import (
44
"context"
55
"time"
66

7-
"github.com/cli/cli/internal/ghrepo"
87
"github.com/shurcooL/githubv4"
98
"github.com/shurcooL/graphql"
109
)
1110

1211
type Comments struct {
1312
Nodes []Comment
1413
TotalCount int
15-
PageInfo PageInfo
14+
PageInfo struct {
15+
HasNextPage bool
16+
EndCursor string
17+
}
1618
}
1719

1820
type Comment struct {
@@ -26,83 +28,6 @@ type Comment struct {
2628
ReactionGroups ReactionGroups `json:"reactionGroups"`
2729
}
2830

29-
type PageInfo struct {
30-
HasNextPage bool
31-
EndCursor string
32-
}
33-
34-
func CommentsForIssue(client *Client, repo ghrepo.Interface, issue *Issue) (*Comments, error) {
35-
type response struct {
36-
Repository struct {
37-
Issue struct {
38-
Comments Comments `graphql:"comments(first: 100, after: $endCursor)"`
39-
} `graphql:"issue(number: $number)"`
40-
} `graphql:"repository(owner: $owner, name: $repo)"`
41-
}
42-
43-
variables := map[string]interface{}{
44-
"owner": githubv4.String(repo.RepoOwner()),
45-
"repo": githubv4.String(repo.RepoName()),
46-
"number": githubv4.Int(issue.Number),
47-
"endCursor": (*githubv4.String)(nil),
48-
}
49-
50-
gql := graphQLClient(client.http, repo.RepoHost())
51-
52-
var comments []Comment
53-
for {
54-
var query response
55-
err := gql.QueryNamed(context.Background(), "CommentsForIssue", &query, variables)
56-
if err != nil {
57-
return nil, err
58-
}
59-
60-
comments = append(comments, query.Repository.Issue.Comments.Nodes...)
61-
if !query.Repository.Issue.Comments.PageInfo.HasNextPage {
62-
break
63-
}
64-
variables["endCursor"] = githubv4.String(query.Repository.Issue.Comments.PageInfo.EndCursor)
65-
}
66-
67-
return &Comments{Nodes: comments, TotalCount: len(comments)}, nil
68-
}
69-
70-
func CommentsForPullRequest(client *Client, repo ghrepo.Interface, pr *PullRequest) (*Comments, error) {
71-
type response struct {
72-
Repository struct {
73-
PullRequest struct {
74-
Comments Comments `graphql:"comments(first: 100, after: $endCursor)"`
75-
} `graphql:"pullRequest(number: $number)"`
76-
} `graphql:"repository(owner: $owner, name: $repo)"`
77-
}
78-
79-
variables := map[string]interface{}{
80-
"owner": githubv4.String(repo.RepoOwner()),
81-
"repo": githubv4.String(repo.RepoName()),
82-
"number": githubv4.Int(pr.Number),
83-
"endCursor": (*githubv4.String)(nil),
84-
}
85-
86-
gql := graphQLClient(client.http, repo.RepoHost())
87-
88-
var comments []Comment
89-
for {
90-
var query response
91-
err := gql.QueryNamed(context.Background(), "CommentsForPullRequest", &query, variables)
92-
if err != nil {
93-
return nil, err
94-
}
95-
96-
comments = append(comments, query.Repository.PullRequest.Comments.Nodes...)
97-
if !query.Repository.PullRequest.Comments.PageInfo.HasNextPage {
98-
break
99-
}
100-
variables["endCursor"] = githubv4.String(query.Repository.PullRequest.Comments.PageInfo.EndCursor)
101-
}
102-
103-
return &Comments{Nodes: comments, TotalCount: len(comments)}, nil
104-
}
105-
10631
type CommentCreateInput struct {
10732
Body string
10833
SubjectId string
@@ -135,24 +60,6 @@ func CommentCreate(client *Client, repoHost string, params CommentCreateInput) (
13560
return mutation.AddComment.CommentEdge.Node.URL, nil
13661
}
13762

138-
func commentsFragment() string {
139-
return `comments(last: 1) {
140-
nodes {
141-
author {
142-
login
143-
}
144-
authorAssociation
145-
body
146-
createdAt
147-
includesCreatedEdit
148-
isMinimized
149-
minimizedReason
150-
` + reactionGroupsFragment() + `
151-
}
152-
totalCount
153-
}`
154-
}
155-
15663
func (c Comment) AuthorLogin() string {
15764
return c.Author.Login
15865
}

‎api/queries_issue.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,12 @@ type Issue struct {
3636
Assignees Assignees
3737
Labels Labels
3838
ProjectCards ProjectCards
39-
Milestone Milestone
39+
Milestone *Milestone
4040
ReactionGroups ReactionGroups
4141
}
4242

4343
type Assignees struct {
44-
Nodes []struct {
45-
Login string `json:"login"`
46-
}
44+
Nodes []GitHubUser
4745
TotalCount int
4846
}
4947

@@ -56,9 +54,7 @@ func (a Assignees) Logins() []string {
5654
}
5755

5856
type Labels struct {
59-
Nodes []struct {
60-
Name string `json:"name"`
61-
}
57+
Nodes []IssueLabel
6258
TotalCount int
6359
}
6460

@@ -101,7 +97,16 @@ type IssuesDisabledError struct {
10197
error
10298
}
10399

100+
type Owner struct {
101+
ID string `json:"id,omitempty"`
102+
Name string `json:"name,omitempty"`
103+
Login string `json:"login"`
104+
}
105+
104106
type Author struct {
107+
// adding these breaks generated GraphQL requests
108+
//ID string `json:"id,omitempty"`
109+
//Name string `json:"name,omitempty"`
105110
Login string `json:"login"`
106111
}
107112

@@ -269,13 +274,18 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e
269274
createdAt
270275
assignees(first: 100) {
271276
nodes {
277+
id
278+
name
272279
login
273280
}
274281
totalCount
275282
}
276283
labels(first: 100) {
277284
nodes {
285+
id
278286
name
287+
description
288+
color
279289
}
280290
totalCount
281291
}
@@ -291,7 +301,10 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e
291301
totalCount
292302
}
293303
milestone {
304+
number
294305
title
306+
description
307+
dueOn
295308
}
296309
reactionGroups {
297310
content

0 commit comments

Comments
 (0)