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

check.go

@@ -14,7 +14,13 @@ import (
 func Check(request CheckRequest, manager Github) (CheckResponse, error) {
 	var response CheckResponse
 
-	pulls, err := manager.ListOpenPullRequests()
+	// Filter out pull request if it does not have a filtered state
+	filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
+	if len(request.Source.States) > 0 {
+		filterStates = request.Source.States
+	}
+
+	pulls, err := manager.ListPullRequests(filterStates)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get last commits: %s", err)
 	}
@@ -38,23 +44,6 @@ Loop:
 			continue
 		}
 
-		// Filter out pull request if it does not have a filtered state
-		filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
-		if len(request.Source.States) > 0 {
-			filterStates = request.Source.States
-		}
-
-		stateFound := false
-		for _, state := range filterStates {
-			if p.State == state {
-				stateFound = true
-				break
-			}
-		}
-		if !stateFound {
-			continue
-		}
-
 		// Filter out commits that are too old.
 		if !p.UpdatedDate().Time.After(request.Version.CommittedDate) {
 			continue

check_test.go

@@ -267,7 +267,20 @@ func TestCheck(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.description, func(t *testing.T) {
 			github := new(fakes.FakeGithub)
-			github.ListOpenPullRequestsReturns(tc.pullRequests, nil)
+			pullRequests := []*resource.PullRequest{}
+			filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
+			if len(tc.source.States) > 0 {
+				filterStates = tc.source.States
+			}
+			for i := range tc.pullRequests {
+				for j := range filterStates {
+					if filterStates[j] == tc.pullRequests[i].PullRequestObject.State {
+						pullRequests = append(pullRequests, tc.pullRequests[i])
+						break
+					}
+				}
+			}
+			github.ListPullRequestsReturns(pullRequests, nil)
 
 			for i, file := range tc.files {
 				github.ListModifiedFilesReturnsOnCall(i, file, nil)
@@ -279,7 +292,7 @@ func TestCheck(t *testing.T) {
 			if assert.NoError(t, err) {
 				assert.Equal(t, tc.expected, output)
 			}
-			assert.Equal(t, 1, github.ListOpenPullRequestsCallCount())
+			assert.Equal(t, 1, github.ListPullRequestsCallCount())
 		})
 	}
 }

github.go

@@ -20,7 +20,7 @@ import (
 // Github for testing purposes.
 //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o fakes/fake_github.go . Github
 type Github interface {
-	ListOpenPullRequests() ([]*PullRequest, error)
+	ListPullRequests([]githubv4.PullRequestState) ([]*PullRequest, error)
 	ListModifiedFiles(int) ([]string, error)
 	PostComment(string, string) error
 	GetPullRequest(string, string) (*PullRequest, error)
@@ -97,8 +97,8 @@ func NewGithubClient(s *Source) (*GithubClient, error) {
 	}, nil
 }
 
-// ListOpenPullRequests gets the last commit on all open pull requests.
-func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
+// ListPullRequests gets the last commit on all pull requests with the matching state.
+func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([]*PullRequest, error) {
 	var query struct {
 		Repository struct {
 			PullRequests struct {
@@ -136,7 +136,7 @@ func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
 		"repositoryOwner": githubv4.String(m.Owner),
 		"repositoryName":  githubv4.String(m.Repository),
 		"prFirst":         githubv4.Int(100),
-		"prStates":        []githubv4.PullRequestState{githubv4.PullRequestStateOpen, githubv4.PullRequestStateClosed, githubv4.PullRequestStateMerged},
+		"prStates":        prStates,
 		"prCursor":        (*githubv4.String)(nil),
 		"commitsLast":     githubv4.Int(1),
 		"prReviewStates":  []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved},