From 9d7e223205e0ef57c31e63e14bce9d377eb94a96 Mon Sep 17 00:00:00 2001 From: Yusaku ONO Date: Tue, 3 Apr 2018 22:35:52 +0900 Subject: [PATCH 1/4] Use event's Issue/PullRequest when user doesn't have permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue/PR にアクセス権を持たない場合は event が持つ Issue/PR を使う --- lib/format.go | 92 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 31 deletions(-) diff --git a/lib/format.go b/lib/format.go index 71c06b5..c90c63b 100644 --- a/lib/format.go +++ b/lib/format.go @@ -47,40 +47,72 @@ func (f *Format) Line(event *github.Event, i int) Line { e := payload.(*github.IssuesEvent) issue := getIssue(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - if issue.PullRequestLinks == nil { - line = Line{ - title: *issue.Title, - repoName: *event.Repo.Name, - url: *issue.HTMLURL, - user: *issue.User.Login, - status: getIssueStatus(issue), + if issue != nil { + if issue.PullRequestLinks == nil { + line = Line{ + title: *issue.Title, + repoName: *event.Repo.Name, + url: *issue.HTMLURL, + user: *issue.User.Login, + status: getIssueStatus(issue), + } + } else { + pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) + + line = Line{ + title: *pr.Title, + repoName: *event.Repo.Name, + url: *pr.HTMLURL, + user: *pr.User.Login, + status: getPullRequestStatus(pr), + } } } else { - pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - line = Line{ - title: *pr.Title, + title: *e.Issue.Title, repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), + url: *e.Issue.HTMLURL, + user: *e.Issue.User.Login, + status: getIssueStatus(e.Issue), } } case "IssueCommentEvent": e := payload.(*github.IssueCommentEvent) issue := getIssue(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - if issue.PullRequestLinks == nil { + if issue != nil { + if issue.PullRequestLinks == nil { + line = Line{ + title: *issue.Title, + repoName: *event.Repo.Name, + url: *issue.HTMLURL, + user: *issue.User.Login, + status: getIssueStatus(issue), + } + } else { + pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) + + line = Line{ + title: *pr.Title, + repoName: *event.Repo.Name, + url: *pr.HTMLURL, + user: *pr.User.Login, + status: getPullRequestStatus(pr), + } + } + } else { line = Line{ - title: *issue.Title, + title: *e.Issue.Title, repoName: *event.Repo.Name, - url: *issue.HTMLURL, - user: *issue.User.Login, - status: getIssueStatus(issue), + url: *e.Issue.HTMLURL, + user: *e.Issue.User.Login, + status: getIssueStatus(e.Issue), } - } else { - pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - + } + case "PullRequestEvent": + e := payload.(*github.PullRequestEvent) + pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, e.GetNumber()) + if pr != nil { line = Line{ title: *pr.Title, repoName: *event.Repo.Name, @@ -88,16 +120,14 @@ func (f *Format) Line(event *github.Event, i int) Line { user: *pr.User.Login, status: getPullRequestStatus(pr), } - } - case "PullRequestEvent": - e := payload.(*github.PullRequestEvent) - pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, e.GetNumber()) - line = Line{ - title: *pr.Title, - repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), + } else { + line = Line{ + title: *e.PullRequest.Title, + repoName: *event.Repo.Name, + url: *e.PullRequest.HTMLURL, + user: *e.PullRequest.User.Login, + status: getPullRequestStatus(e.PullRequest), + } } case "PullRequestReviewCommentEvent": e := payload.(*github.PullRequestReviewCommentEvent) From c14623c218595860263d8fa737edfd4433d97b47 Mon Sep 17 00:00:00 2001 From: Takashi Masuda Date: Sat, 14 Apr 2018 17:55:42 +0900 Subject: [PATCH 2/4] :bug: Avoid panic in the case of PullRequestReviewCommentEvent --- lib/format.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/format.go b/lib/format.go index c90c63b..eae1961 100644 --- a/lib/format.go +++ b/lib/format.go @@ -132,12 +132,22 @@ func (f *Format) Line(event *github.Event, i int) Line { case "PullRequestReviewCommentEvent": e := payload.(*github.PullRequestReviewCommentEvent) pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.PullRequest.Number) - line = Line{ - title: *pr.Title, - repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), + if pr != nil { + line = Line{ + title: *pr.Title, + repoName: *event.Repo.Name, + url: *pr.HTMLURL, + user: *pr.User.Login, + status: getPullRequestStatus(pr), + } + } else { + line = Line{ + title: *e.PullRequest.Title, + repoName: *event.Repo.Name, + url: *e.PullRequest.HTMLURL, + user: *e.PullRequest.User.Login, + status: getPullRequestStatus(e.PullRequest), + } } } From 0319016b25eb75ad42bc5d135f47245c9d89e18f Mon Sep 17 00:00:00 2001 From: Takashi Masuda Date: Sat, 14 Apr 2018 18:18:13 +0900 Subject: [PATCH 3/4] :recycle: Refactor --- lib/format.go | 104 ++++++++++++++++---------------------------------- 1 file changed, 32 insertions(+), 72 deletions(-) diff --git a/lib/format.go b/lib/format.go index eae1961..704fd60 100644 --- a/lib/format.go +++ b/lib/format.go @@ -33,6 +33,28 @@ type Line struct { status string } +// NewLineByIssue is an initializer by Issue +func NewLineByIssue(repoName string, issue *github.Issue) Line { + return Line{ + title: *issue.Title, + repoName: repoName, + url: *issue.HTMLURL, + user: *issue.User.Login, + status: getIssueStatus(issue), + } +} + +// NewLineByPullRequest is an initializer by PR +func NewLineByPullRequest(repoName string, pr *github.PullRequest) Line { + return Line{ + title: *pr.Title, + repoName: repoName, + url: *pr.HTMLURL, + user: *pr.User.Login, + status: getPullRequestStatus(pr), + } +} + // Line returns Issue/PR info retrieving from GitHub func (f *Format) Line(event *github.Event, i int) Line { if f.debug { @@ -49,32 +71,13 @@ func (f *Format) Line(event *github.Event, i int) Line { if issue != nil { if issue.PullRequestLinks == nil { - line = Line{ - title: *issue.Title, - repoName: *event.Repo.Name, - url: *issue.HTMLURL, - user: *issue.User.Login, - status: getIssueStatus(issue), - } + line = NewLineByIssue(*event.Repo.Name, issue) } else { pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - - line = Line{ - title: *pr.Title, - repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), - } + line = NewLineByPullRequest(*event.Repo.Name, pr) } } else { - line = Line{ - title: *e.Issue.Title, - repoName: *event.Repo.Name, - url: *e.Issue.HTMLURL, - user: *e.Issue.User.Login, - status: getIssueStatus(e.Issue), - } + line = NewLineByIssue(*event.Repo.Name, e.Issue) } case "IssueCommentEvent": e := payload.(*github.IssueCommentEvent) @@ -82,72 +85,29 @@ func (f *Format) Line(event *github.Event, i int) Line { if issue != nil { if issue.PullRequestLinks == nil { - line = Line{ - title: *issue.Title, - repoName: *event.Repo.Name, - url: *issue.HTMLURL, - user: *issue.User.Login, - status: getIssueStatus(issue), - } + line = NewLineByIssue(*event.Repo.Name, issue) } else { pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - - line = Line{ - title: *pr.Title, - repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), - } + line = NewLineByPullRequest(*event.Repo.Name, pr) } } else { - line = Line{ - title: *e.Issue.Title, - repoName: *event.Repo.Name, - url: *e.Issue.HTMLURL, - user: *e.Issue.User.Login, - status: getIssueStatus(e.Issue), - } + line = NewLineByIssue(*event.Repo.Name, e.Issue) } case "PullRequestEvent": e := payload.(*github.PullRequestEvent) pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, e.GetNumber()) if pr != nil { - line = Line{ - title: *pr.Title, - repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), - } + line = NewLineByPullRequest(*event.Repo.Name, pr) } else { - line = Line{ - title: *e.PullRequest.Title, - repoName: *event.Repo.Name, - url: *e.PullRequest.HTMLURL, - user: *e.PullRequest.User.Login, - status: getPullRequestStatus(e.PullRequest), - } + line = NewLineByPullRequest(*event.Repo.Name, e.PullRequest) } case "PullRequestReviewCommentEvent": e := payload.(*github.PullRequestReviewCommentEvent) pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.PullRequest.Number) if pr != nil { - line = Line{ - title: *pr.Title, - repoName: *event.Repo.Name, - url: *pr.HTMLURL, - user: *pr.User.Login, - status: getPullRequestStatus(pr), - } + line = NewLineByPullRequest(*event.Repo.Name, pr) } else { - line = Line{ - title: *e.PullRequest.Title, - repoName: *event.Repo.Name, - url: *e.PullRequest.HTMLURL, - user: *e.PullRequest.User.Login, - status: getPullRequestStatus(e.PullRequest), - } + line = NewLineByPullRequest(*event.Repo.Name, e.PullRequest) } } From 179cb2f922599624f14cb75cd2ec5de9e2d980ab Mon Sep 17 00:00:00 2001 From: Takashi Masuda Date: Sat, 14 Apr 2018 18:22:44 +0900 Subject: [PATCH 4/4] :recycle: More refactor --- lib/format.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/format.go b/lib/format.go index 704fd60..2c249d0 100644 --- a/lib/format.go +++ b/lib/format.go @@ -34,7 +34,7 @@ type Line struct { } // NewLineByIssue is an initializer by Issue -func NewLineByIssue(repoName string, issue *github.Issue) Line { +func NewLineByIssue(repoName string, issue github.Issue) Line { return Line{ title: *issue.Title, repoName: repoName, @@ -45,7 +45,7 @@ func NewLineByIssue(repoName string, issue *github.Issue) Line { } // NewLineByPullRequest is an initializer by PR -func NewLineByPullRequest(repoName string, pr *github.PullRequest) Line { +func NewLineByPullRequest(repoName string, pr github.PullRequest) Line { return Line{ title: *pr.Title, repoName: repoName, @@ -71,13 +71,13 @@ func (f *Format) Line(event *github.Event, i int) Line { if issue != nil { if issue.PullRequestLinks == nil { - line = NewLineByIssue(*event.Repo.Name, issue) + line = NewLineByIssue(*event.Repo.Name, *issue) } else { pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - line = NewLineByPullRequest(*event.Repo.Name, pr) + line = NewLineByPullRequest(*event.Repo.Name, *pr) } } else { - line = NewLineByIssue(*event.Repo.Name, e.Issue) + line = NewLineByIssue(*event.Repo.Name, *e.Issue) } case "IssueCommentEvent": e := payload.(*github.IssueCommentEvent) @@ -85,29 +85,29 @@ func (f *Format) Line(event *github.Event, i int) Line { if issue != nil { if issue.PullRequestLinks == nil { - line = NewLineByIssue(*event.Repo.Name, issue) + line = NewLineByIssue(*event.Repo.Name, *issue) } else { pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.Issue.Number) - line = NewLineByPullRequest(*event.Repo.Name, pr) + line = NewLineByPullRequest(*event.Repo.Name, *pr) } } else { - line = NewLineByIssue(*event.Repo.Name, e.Issue) + line = NewLineByIssue(*event.Repo.Name, *e.Issue) } case "PullRequestEvent": e := payload.(*github.PullRequestEvent) pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, e.GetNumber()) if pr != nil { - line = NewLineByPullRequest(*event.Repo.Name, pr) + line = NewLineByPullRequest(*event.Repo.Name, *pr) } else { - line = NewLineByPullRequest(*event.Repo.Name, e.PullRequest) + line = NewLineByPullRequest(*event.Repo.Name, *e.PullRequest) } case "PullRequestReviewCommentEvent": e := payload.(*github.PullRequestReviewCommentEvent) pr := getPullRequest(f.ctx, f.client, *event.Repo.Name, *e.PullRequest.Number) if pr != nil { - line = NewLineByPullRequest(*event.Repo.Name, pr) + line = NewLineByPullRequest(*event.Repo.Name, *pr) } else { - line = NewLineByPullRequest(*event.Repo.Name, e.PullRequest) + line = NewLineByPullRequest(*event.Repo.Name, *e.PullRequest) } } @@ -126,7 +126,7 @@ func getPullRequest(ctx context.Context, client *github.Client, repoFullName str return pr } -func getIssueStatus(issue *github.Issue) string { +func getIssueStatus(issue github.Issue) string { result := "" if *issue.State == "closed" { result = "closed" @@ -134,7 +134,7 @@ func getIssueStatus(issue *github.Issue) string { return result } -func getPullRequestStatus(pr *github.PullRequest) string { +func getPullRequestStatus(pr github.PullRequest) string { result := "" if *pr.Merged { result = "merged"