Skip to content

Commit 9ec47e2

Browse files
authored
fix: filter pull requests on the server, not client (#240)
* fix: filter pull requests on the server, not client Ensure that the GraphQL query requests PRs matching the desired states, rather than requesting all PRs and filtering on the client. Fixes #233 * fix: ensure function name reflects implementation * fix: add missing fake
1 parent a98122f commit 9ec47e2

File tree

4 files changed

+78
-61
lines changed

4 files changed

+78
-61
lines changed

check.go

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ import (
1414
func Check(request CheckRequest, manager Github) (CheckResponse, error) {
1515
var response CheckResponse
1616

17-
pulls, err := manager.ListOpenPullRequests()
17+
// Filter out pull request if it does not have a filtered state
18+
filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
19+
if len(request.Source.States) > 0 {
20+
filterStates = request.Source.States
21+
}
22+
23+
pulls, err := manager.ListPullRequests(filterStates)
1824
if err != nil {
1925
return nil, fmt.Errorf("failed to get last commits: %s", err)
2026
}
@@ -38,23 +44,6 @@ Loop:
3844
continue
3945
}
4046

41-
// Filter out pull request if it does not have a filtered state
42-
filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
43-
if len(request.Source.States) > 0 {
44-
filterStates = request.Source.States
45-
}
46-
47-
stateFound := false
48-
for _, state := range filterStates {
49-
if p.State == state {
50-
stateFound = true
51-
break
52-
}
53-
}
54-
if !stateFound {
55-
continue
56-
}
57-
5847
// Filter out commits that are too old.
5948
if !p.UpdatedDate().Time.After(request.Version.CommittedDate) {
6049
continue

check_test.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,20 @@ func TestCheck(t *testing.T) {
267267
for _, tc := range tests {
268268
t.Run(tc.description, func(t *testing.T) {
269269
github := new(fakes.FakeGithub)
270-
github.ListOpenPullRequestsReturns(tc.pullRequests, nil)
270+
pullRequests := []*resource.PullRequest{}
271+
filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
272+
if len(tc.source.States) > 0 {
273+
filterStates = tc.source.States
274+
}
275+
for i := range tc.pullRequests {
276+
for j := range filterStates {
277+
if filterStates[j] == tc.pullRequests[i].PullRequestObject.State {
278+
pullRequests = append(pullRequests, tc.pullRequests[i])
279+
break
280+
}
281+
}
282+
}
283+
github.ListPullRequestsReturns(pullRequests, nil)
271284

272285
for i, file := range tc.files {
273286
github.ListModifiedFilesReturnsOnCall(i, file, nil)
@@ -279,7 +292,7 @@ func TestCheck(t *testing.T) {
279292
if assert.NoError(t, err) {
280293
assert.Equal(t, tc.expected, output)
281294
}
282-
assert.Equal(t, 1, github.ListOpenPullRequestsCallCount())
295+
assert.Equal(t, 1, github.ListPullRequestsCallCount())
283296
})
284297
}
285298
}

fakes/fake_github.go

Lines changed: 52 additions & 37 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

github.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
// Github for testing purposes.
2121
//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o fakes/fake_github.go . Github
2222
type Github interface {
23-
ListOpenPullRequests() ([]*PullRequest, error)
23+
ListPullRequests([]githubv4.PullRequestState) ([]*PullRequest, error)
2424
ListModifiedFiles(int) ([]string, error)
2525
PostComment(string, string) error
2626
GetPullRequest(string, string) (*PullRequest, error)
@@ -97,8 +97,8 @@ func NewGithubClient(s *Source) (*GithubClient, error) {
9797
}, nil
9898
}
9999

100-
// ListOpenPullRequests gets the last commit on all open pull requests.
101-
func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
100+
// ListPullRequests gets the last commit on all pull requests with the matching state.
101+
func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([]*PullRequest, error) {
102102
var query struct {
103103
Repository struct {
104104
PullRequests struct {
@@ -136,7 +136,7 @@ func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
136136
"repositoryOwner": githubv4.String(m.Owner),
137137
"repositoryName": githubv4.String(m.Repository),
138138
"prFirst": githubv4.Int(100),
139-
"prStates": []githubv4.PullRequestState{githubv4.PullRequestStateOpen, githubv4.PullRequestStateClosed, githubv4.PullRequestStateMerged},
139+
"prStates": prStates,
140140
"prCursor": (*githubv4.String)(nil),
141141
"commitsLast": githubv4.Int(1),
142142
"prReviewStates": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved},

0 commit comments

Comments
 (0)