From dcd43bead1686f92e51fd902920e509cb71bfc56 Mon Sep 17 00:00:00 2001 From: Sam Park Date: Fri, 24 Jan 2020 21:15:28 -0800 Subject: [PATCH] Add pagination --- server/events/vcs/github_client.go | 25 +++--- server/events/vcs/github_client_test.go | 100 +++++++++++++++++++++++- 2 files changed, 115 insertions(+), 10 deletions(-) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 9629e4e0b..ea2b0b029 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -140,17 +140,24 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri } func (g *GithubClient) HideOldComments(repo models.Repo, pullNum int) error { - // TODO: Paginate issues. - comments, _, err := g.client.Issues.ListComments(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueListCommentsOptions{ - Sort: "created", - Direction: "asc", - ListOptions: github.ListOptions{}, - }) - if err != nil { - return err + var allComments []*github.IssueComment + for page := 0; ; { + comments, resp, err := g.client.Issues.ListComments(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueListCommentsOptions{ + Sort: "created", + Direction: "asc", + ListOptions: github.ListOptions{Page: page}, + }) + if err != nil { + return err + } + allComments = append(allComments, comments...) + if resp.NextPage == 0 { + break + } + page = resp.NextPage } - for _, comment := range comments { + for _, comment := range allComments { if comment.User != nil && comment.User.GetLogin() != g.user { continue } diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index 1ae64d409..8f3f03563 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -135,6 +135,105 @@ func TestGithubClient_GetModifiedFilesMovedFile(t *testing.T) { Equals(t, []string{"new/filename.txt", "previous/filename.txt"}, files) } +func TestGithubClient_PaginatesComments(t *testing.T) { + calls := 0 + issueResps := []string{ + `[ + {"node_id": "1", "body": "asd\nplan\nasd", "user": {"login": "someone-else"}}, + {"node_id": "2", "body": "asd plan\nasd", "user": {"login": "user"}} +]`, + `[ + {"node_id": "3", "body": "asd", "user": {"login": "someone-else"}}, + {"node_id": "4", "body": "asdasd", "user": {"login": "someone-else"}} +]`, + `[ + {"node_id": "5", "body": "asd plan", "user": {"login": "someone-else"}}, + {"node_id": "6", "body": "asd\nplan", "user": {"login": "user"}} +]`, + `[ + {"node_id": "7", "body": "asd", "user": {"login": "user"}}, + {"node_id": "8", "body": "asd plan \n asd", "user": {"login": "user"}} +]`, + } + minimizeResp := "{}" + type graphQLCall struct { + Variables struct { + Input githubv4.MinimizeCommentInput `json:"input"` + } `json:"variables"` + } + gotMinimizeCalls := make([]graphQLCall, 0, 2) + testServer := httptest.NewTLSServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.RequestURI { + case "POST /graphql": + defer r.Body.Close() // nolint: errcheck + body, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Errorf("read body error: %v", err) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + call := graphQLCall{} + err = json.Unmarshal(body, &call) + if err != nil { + t.Errorf("parse body error: %v", err) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + gotMinimizeCalls = append(gotMinimizeCalls, call) + w.Write([]byte(minimizeResp)) // nolint: errcheck + return + default: + if r.Method != "GET" || !strings.HasPrefix(r.RequestURI, "/api/v3/repos/owner/repo/issues/123/comments") { + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + if (calls + 1) < len(issueResps) { + w.Header().Add( + "Link", + fmt.Sprintf( + `; rel="next"`, + r.Host, + calls+1, + ), + ) + } + w.Write([]byte(issueResps[calls])) // nolint: errcheck + calls += 1 + } + }), + ) + + testServerURL, err := url.Parse(testServer.URL) + Ok(t, err) + + client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass") + Ok(t, err) + defer disableSSLVerification()() + + err = client.HideOldComments( + models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Hostname: "github.com", + Type: models.Github, + }, + }, + 123, + ) + Ok(t, err) + Equals(t, 2, len(gotMinimizeCalls)) + Equals(t, "2", gotMinimizeCalls[0].Variables.Input.SubjectID) + Equals(t, "8", gotMinimizeCalls[1].Variables.Input.SubjectID) + Equals(t, githubv4.ReportedContentClassifiersOutdated, gotMinimizeCalls[0].Variables.Input.Classifier) + Equals(t, githubv4.ReportedContentClassifiersOutdated, gotMinimizeCalls[1].Variables.Input.Classifier) +} + func TestGithubClient_HideOldComments(t *testing.T) { // Only comment 6 should be minimized, because it's by the same Atlantis bot user // and it has "plan" in the first line of the comment body. @@ -174,7 +273,6 @@ func TestGithubClient_HideOldComments(t *testing.T) { http.Error(w, "server error", http.StatusInternalServerError) return } - t.Log(string(body)) call := graphQLCall{} err = json.Unmarshal(body, &call) if err != nil {