Use closedAt and mergedAt when comparing version timestamps

When we're considering PRs in the closed or merged state, we need to use
their closedAt or mergedAt timestamps to check how recent the version
is. This is because PRs will not necessarily be closed or merged in the
same order their commits have been made.

check.go

@@ -6,6 +6,8 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+
+	"github.com/shurcooL/githubv4"
 )
 
 // Check (business logic)
@@ -25,33 +27,37 @@ Loop:
 		if !disableSkipCI && ContainsSkipCI(p.Title) {
 			continue
 		}
+
 		// [ci skip]/[skip ci] in Commit message
 		if !disableSkipCI && ContainsSkipCI(p.Tip.Message) {
 			continue
 		}
+
 		// Filter pull request if the BaseBranch does not match the one specified in source
 		if request.Source.BaseBranch != "" && p.PullRequestObject.BaseRefName != request.Source.BaseBranch {
 			continue
 		}
-		// Filter out commits that are too old.
-		if !p.Tip.CommittedDate.Time.After(request.Version.CommittedDate) {
-			continue
-		}
 
 		// Filter out pull request if it does not have a filtered state
+		filterStates := []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
 		if len(request.Source.States) > 0 {
-			stateFound := false
+			filterStates = request.Source.States
+		}
 
-			for _, state := range request.Source.States {
-				if p.State == state {
-					stateFound = true
-					break
-				}
+		stateFound := false
+		for _, state := range filterStates {
+			if p.State == state {
+				stateFound = true
+				break
 			}
+		}
+		if !stateFound {
+			continue
+		}
 
-			if !stateFound {
-				continue
-			}
+		// Filter out commits that are too old.
+		if !p.UpdatedDate().Time.After(request.Version.CommittedDate) {
+			continue
 		}
 
 		// Filter out pull request if it does not contain at least one of the desired labels

check_test.go

@@ -229,8 +229,8 @@ func TestCheck(t *testing.T) {
 			pullRequests: testPullRequests,
 			files:        [][]string{},
 			expected: resource.CheckResponse{
-				resource.NewVersion(testPullRequests[10]),
 				resource.NewVersion(testPullRequests[9]),
+				resource.NewVersion(testPullRequests[10]),
 			},
 		},
 	}

in_test.go

@@ -357,6 +357,8 @@ func createTestPR(
 			},
 			IsCrossRepository: isCrossRepo,
 			State:             state,
+			ClosedAt:          githubv4.DateTime{Time: time.Now()},
+			MergedAt:          githubv4.DateTime{Time: time.Now()},
 		},
 		Tip: resource.CommitObject{
 			ID:            fmt.Sprintf("commit%s", n),

models.go

@@ -51,9 +51,6 @@ func (s *Source) Validate() error {
 			return errors.New(fmt.Sprintf("states value \"%s\" must be one of: OPEN, MERGED, CLOSED", state))
 		}
 	}
-	if len(s.States) == 0 {
-		s.States = []githubv4.PullRequestState{githubv4.PullRequestStateOpen}
-	}
 	return nil
 }
 
@@ -85,7 +82,7 @@ func NewVersion(p *PullRequest) Version {
 	return Version{
 		PR:                  strconv.Itoa(p.Number),
 		Commit:              p.Tip.OID,
-		CommittedDate:       p.Tip.CommittedDate.Time,
+		CommittedDate:       p.UpdatedDate().Time,
 		ApprovedReviewCount: strconv.Itoa(p.ApprovedReviewCount),
 		State:               p.State,
 	}
@@ -113,6 +110,21 @@ type PullRequestObject struct {
 	}
 	IsCrossRepository bool
 	State             githubv4.PullRequestState
+	ClosedAt          githubv4.DateTime
+	MergedAt          githubv4.DateTime
+}
+
+// UpdatedDate returns the last time a PR was updated, either by commit
+// or being closed/merged.
+func (p *PullRequest) UpdatedDate() githubv4.DateTime {
+	date := p.Tip.CommittedDate
+	switch p.State {
+	case githubv4.PullRequestStateClosed:
+		date = p.ClosedAt
+	case githubv4.PullRequestStateMerged:
+		date = p.MergedAt
+	}
+	return date
 }
 
 // CommitObject represents the GraphQL commit node.