Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: filter pull requests on the server, not client #240

Merged
merged 3 commits into from
Feb 6, 2021
Merged

fix: filter pull requests on the server, not client #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f119d22
fix: filter pull requests on the server, not client
jhosteny Nov 2, 2020
64fb5db
fix: ensure function name reflects implementation
jhosteny Nov 2, 2020
b587863
fix: add missing fake
jhosteny Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 7 additions & 18 deletions check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand Down
17 changes: 15 additions & 2 deletions check_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
})
}
}
Expand Down
89 changes: 52 additions & 37 deletions fakes/fake_github.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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},
Expand Down