From 8d82a96349ef2068c9a47cab13a1d335146e09c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 23 Dec 2022 17:14:06 +0100 Subject: [PATCH] Fix fetching issue/PR comments The "Author" struct was too overloaded in different types of queries that treat struct fields in incompatible ways. This change defines a simpler CommentAuthor struct for use in comments until we can figure out how to query `... on User` information for comments too. --- api/export_pr.go | 24 ------------------- api/queries_comments.go | 2 +- api/queries_issue.go | 33 +++++++++++++++++++++++---- api/query_builder.go | 4 ++-- pkg/cmd/issue/comment/comment_test.go | 4 ++-- pkg/cmd/issue/view/view.go | 2 ++ pkg/cmd/pr/comment/comment_test.go | 4 ++-- 7 files changed, 37 insertions(+), 36 deletions(-) diff --git a/api/export_pr.go b/api/export_pr.go index ef463b8a260..e6f0f72217e 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -11,18 +11,6 @@ func (issue *Issue) ExportData(fields []string) map[string]interface{} { for _, f := range fields { switch f { - case "author": - author := map[string]interface{}{ - "is_bot": issue.Author.IsBot(), - } - if issue.Author.IsBot() { - author["login"] = "app/" + issue.Author.Login - } else { - author["login"] = issue.Author.Login - author["name"] = issue.Author.Name - author["id"] = issue.Author.ID - } - data[f] = author case "comments": data[f] = issue.Comments.Nodes case "assignees": @@ -46,18 +34,6 @@ func (pr *PullRequest) ExportData(fields []string) map[string]interface{} { for _, f := range fields { switch f { - case "author": - author := map[string]interface{}{ - "is_bot": pr.Author.IsBot(), - } - if pr.Author.IsBot() { - author["login"] = "app/" + pr.Author.Login - } else { - author["login"] = pr.Author.Login - author["name"] = pr.Author.Name - author["id"] = pr.Author.ID - } - data[f] = author case "headRepository": data[f] = pr.HeadRepository case "statusCheckRollup": diff --git a/api/queries_comments.go b/api/queries_comments.go index 0e62351b18a..5cc84a3e43f 100644 --- a/api/queries_comments.go +++ b/api/queries_comments.go @@ -27,7 +27,7 @@ func (cs Comments) CurrentUserComments() []Comment { type Comment struct { ID string `json:"id"` - Author Author `json:"author"` + Author CommentAuthor `json:"author"` AuthorAssociation string `json:"authorAssociation"` Body string `json:"body"` CreatedAt time.Time `json:"createdAt"` diff --git a/api/queries_issue.go b/api/queries_issue.go index 1fd1b6ac62d..1fceac39e30 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -1,6 +1,7 @@ package api import ( + "encoding/json" "fmt" "time" @@ -120,13 +121,35 @@ type Owner struct { } type Author struct { - ID string `json:"id,omitempty"` - Name string `json:"name,omitempty"` - Login string `json:"login"` + ID string + Name string + Login string } -func (author *Author) IsBot() bool { - return author.ID == "" +func (author Author) MarshalJSON() ([]byte, error) { + if author.ID == "" { + return json.Marshal(map[string]interface{}{ + "is_bot": true, + "login": "app/" + author.Login, + }) + } + return json.Marshal(map[string]interface{}{ + "is_bot": false, + "login": author.Login, + "id": author.ID, + "name": author.Name, + }) +} + +type CommentAuthor struct { + Login string `json:"login"` + // Unfortunately, there is no easy way to add "id" and "name" fields to this struct because it's being + // used in both shurcool-graphql type queries and string-based queries where the response gets parsed + // by an ordinary JSON decoder that doesn't understand "graphql" directives via struct tags. + // User *struct { + // ID string + // Name string + // } `graphql:"... on User"` } // IssueCreate creates an issue in a GitHub repository diff --git a/api/query_builder.go b/api/query_builder.go index 7155552f203..ce7b1791b94 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -24,7 +24,7 @@ var issueComments = shortenQuery(` comments(first: 100) { nodes { id, - author{login}, + author{login,...on User{id,name}}, authorAssociation, body, createdAt, @@ -43,7 +43,7 @@ var issueComments = shortenQuery(` var issueCommentLast = shortenQuery(` comments(last: 1) { nodes { - author{login}, + author{login,...on User{id,name}}, authorAssociation, body, createdAt, diff --git a/pkg/cmd/issue/comment/comment_test.go b/pkg/cmd/issue/comment/comment_test.go index 4f747d882c5..6ab8df050d4 100644 --- a/pkg/cmd/issue/comment/comment_test.go +++ b/pkg/cmd/issue/comment/comment_test.go @@ -324,8 +324,8 @@ func Test_commentRun(t *testing.T) { ID: "ISSUE-ID", URL: "https://github.com/OWNER/REPO/issues/123", Comments: api.Comments{Nodes: []api.Comment{ - {ID: "id1", Author: api.Author{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-111", ViewerDidAuthor: true}, - {ID: "id2", Author: api.Author{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-222"}, + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-111", ViewerDidAuthor: true}, + {ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-222"}, }}, }, ghrepo.New("OWNER", "REPO"), nil } diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 7e1982c18a4..b721451fae6 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -153,6 +153,8 @@ func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), } if fieldSet.Contains("comments") { + // FIXME: this re-fetches the comments connection even though the initial set of 100 were + // fetched in the previous request. err = preloadIssueComments(client, repo, issue) } return issue, err diff --git a/pkg/cmd/pr/comment/comment_test.go b/pkg/cmd/pr/comment/comment_test.go index f72701859d0..8de1be94089 100644 --- a/pkg/cmd/pr/comment/comment_test.go +++ b/pkg/cmd/pr/comment/comment_test.go @@ -344,8 +344,8 @@ func Test_commentRun(t *testing.T) { Number: 123, URL: "https://github.com/OWNER/REPO/pull/123", Comments: api.Comments{Nodes: []api.Comment{ - {ID: "id1", Author: api.Author{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true}, - {ID: "id2", Author: api.Author{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-222"}, + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true}, + {ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-222"}, }}, }, ghrepo.New("OWNER", "REPO"), nil }