From dcedacd4f7fbb1579fdf2973f57c326226a3abd9 Mon Sep 17 00:00:00 2001 From: Alisson Santos Date: Thu, 5 Mar 2020 13:55:36 +0100 Subject: [PATCH 1/4] Remove duplicates --- api/queries_pr.go | 16 +++++++- command/pr_test.go | 20 ++++++++++ test/fixtures/prListWithDuplicates.json | 50 +++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/prListWithDuplicates.json diff --git a/api/queries_pr.go b/api/queries_pr.go index 265378b1e..092c7c12b 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -590,7 +590,21 @@ loop: } } - return prs, nil + return removeDuplicates(prs), nil +} + +func removeDuplicates(prs []PullRequest) []PullRequest { + var check = make(map[int]struct{}) + var uniqPRs []PullRequest + + for _, pr := range prs { + if _, ok := check[pr.Number]; !ok { + check[pr.Number] = struct{}{} + uniqPRs = append(uniqPRs, pr) + } + } + + return uniqPRs } func min(a, b int) int { diff --git a/command/pr_test.go b/command/pr_test.go index 51469342a..395dd4097 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -240,6 +240,26 @@ No pull requests match your search eq(t, reqBody.Variables.Labels, []string{"one", "two", "three"}) } +func TestPRList_filteringRemoveDuplicate(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prListWithDuplicates.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(prListCmd, "pr list -l one,two") + if err != nil { + t.Fatal(err) + } + + eq(t, output.String(), `32 New feature feature +29 Fixed bad bug hubot:bug-fix +28 Improve documentation docs +`) +} + func TestPRList_filteringClosed(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() diff --git a/test/fixtures/prListWithDuplicates.json b/test/fixtures/prListWithDuplicates.json new file mode 100644 index 000000000..a9cf19638 --- /dev/null +++ b/test/fixtures/prListWithDuplicates.json @@ -0,0 +1,50 @@ +{ + "data": { + "repository": { + "pullRequests": { + "edges": [ + { + "node": { + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" + } + }, + { + "node": { + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" + } + }, + { + "node": { + "number": 29, + "title": "Fixed bad bug", + "url": "https://github.com/monalisa/hello/pull/29", + "headRefName": "bug-fix", + "isCrossRepository": true, + "headRepositoryOwner": { + "login": "hubot" + } + } + }, + { + "node": { + "number": 28, + "title": "Improve documentation", + "url": "https://github.com/monalisa/hello/pull/28", + "headRefName": "docs" + } + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" + } + } + } + } +} From a3557197fe1190c81f4f41d9bd9dcbd7d575a091 Mon Sep 17 00:00:00 2001 From: Alisson Santos Date: Thu, 5 Mar 2020 14:05:33 +0100 Subject: [PATCH 2/4] 2 space identation for JSON --- test/fixtures/prListWithDuplicates.json | 90 ++++++++++++------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/test/fixtures/prListWithDuplicates.json b/test/fixtures/prListWithDuplicates.json index a9cf19638..a84800f72 100644 --- a/test/fixtures/prListWithDuplicates.json +++ b/test/fixtures/prListWithDuplicates.json @@ -1,50 +1,50 @@ { - "data": { - "repository": { - "pullRequests": { - "edges": [ - { - "node": { - "number": 32, - "title": "New feature", - "url": "https://github.com/monalisa/hello/pull/32", - "headRefName": "feature" - } - }, - { - "node": { - "number": 32, - "title": "New feature", - "url": "https://github.com/monalisa/hello/pull/32", - "headRefName": "feature" - } - }, - { - "node": { - "number": 29, - "title": "Fixed bad bug", - "url": "https://github.com/monalisa/hello/pull/29", - "headRefName": "bug-fix", - "isCrossRepository": true, - "headRepositoryOwner": { - "login": "hubot" - } - } - }, - { - "node": { - "number": 28, - "title": "Improve documentation", - "url": "https://github.com/monalisa/hello/pull/28", - "headRefName": "docs" - } - } - ], - "pageInfo": { - "hasNextPage": false, - "endCursor": "" - } + "data": { + "repository": { + "pullRequests": { + "edges": [ + { + "node": { + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" } + }, + { + "node": { + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" + } + }, + { + "node": { + "number": 29, + "title": "Fixed bad bug", + "url": "https://github.com/monalisa/hello/pull/29", + "headRefName": "bug-fix", + "isCrossRepository": true, + "headRepositoryOwner": { + "login": "hubot" + } + } + }, + { + "node": { + "number": 28, + "title": "Improve documentation", + "url": "https://github.com/monalisa/hello/pull/28", + "headRefName": "docs" + } + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" } + } } + } } From 101d985a16d8d3346debfb26e1b43450dfcf000e Mon Sep 17 00:00:00 2001 From: Alisson Santos Date: Thu, 5 Mar 2020 18:09:09 +0100 Subject: [PATCH 3/4] remove duplicate from pages --- api/queries_pr.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 092c7c12b..dba19a500 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -504,6 +504,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] } }` + var check = make(map[int]struct{}) var prs []PullRequest pageLimit := min(limit, 100) variables := map[string]interface{}{} @@ -576,9 +577,13 @@ loop: } for _, edge := range prData.Edges { - prs = append(prs, edge.Node) - if len(prs) == limit { - break loop + if _, ok := check[edge.Node.Number]; !ok { + check[edge.Node.Number] = struct{}{} + prs = append(prs, edge.Node) + + if len(prs) == limit { + break loop + } } } @@ -590,21 +595,7 @@ loop: } } - return removeDuplicates(prs), nil -} - -func removeDuplicates(prs []PullRequest) []PullRequest { - var check = make(map[int]struct{}) - var uniqPRs []PullRequest - - for _, pr := range prs { - if _, ok := check[pr.Number]; !ok { - check[pr.Number] = struct{}{} - uniqPRs = append(uniqPRs, pr) - } - } - - return uniqPRs + return prs, nil } func min(a, b int) int { From 7a0a6658d516b9017f22655c04de32e3eca9e815 Mon Sep 17 00:00:00 2001 From: Alisson Santos Date: Thu, 5 Mar 2020 18:52:27 +0100 Subject: [PATCH 4/4] avoid nested ifs --- api/queries_pr.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index dba19a500..34884d16f 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -577,13 +577,14 @@ loop: } for _, edge := range prData.Edges { - if _, ok := check[edge.Node.Number]; !ok { - check[edge.Node.Number] = struct{}{} - prs = append(prs, edge.Node) + if _, exists := check[edge.Node.Number]; exists { + continue + } - if len(prs) == limit { - break loop - } + prs = append(prs, edge.Node) + check[edge.Node.Number] = struct{}{} + if len(prs) == limit { + break loop } }