sync: conform to PullStatusFetcher interface. (#157)

* sync: conform to PullStatusFetcher interface.

* chore: savepoint

* refactor lyft specific mergeability in separate package.

server/controllers/events/events_controller_e2e_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/runatlantis/atlantis/server/events/mocks/matchers"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/vcs"
+	lyft_vcs "github.com/runatlantis/atlantis/server/events/vcs/lyft"
 	vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks"
 	"github.com/runatlantis/atlantis/server/events/webhooks"
 	"github.com/runatlantis/atlantis/server/events/yaml"
@@ -743,10 +744,12 @@ func TestGitHubWorkflowWithPolicyCheck(t *testing.T) {
 
 			// Setup test dependencies.
 			w := httptest.NewRecorder()
-			When(githubClient.PullIsSQMergeable(AnyRepo(), matchers.AnyModelsPullRequest(), AnyStatus())).ThenReturn(true, nil)
 			When(githubClient.PullIsApproved(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn(models.ApprovalStatus{
 				IsApproved: true,
 			}, nil)
+			// TODO: move to separate test, these checks are lyft specific. Should probably be part of a larger refactor
+			When(githubClient.GetRepoStatuses(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn([]*github.RepoStatus{}, nil)
+			When(githubClient.GetRepoChecks(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn([]*github.CheckRun{}, nil)
 			When(githubGetter.GetPullRequest(AnyRepo(), AnyInt())).ThenReturn(GitHubPullRequestParsed(headSHA), nil)
 			When(vcsClient.GetModifiedFiles(AnyRepo(), matchers.AnyModelsPullRequest())).ThenReturn(c.ModifiedFiles, nil)
 
@@ -1001,9 +1004,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 	)
 
 	e2eMockGithubClient := vcsmocks.NewMockIGithubClient()
-	e2ePullReqStatusFetcher := vcs.SQBasedPullStatusFetcher{
-		GithubClient: e2eMockGithubClient,
-	}
+	e2ePullReqStatusFetcher := lyft_vcs.NewSQBasedPullStatusFetcher(e2eMockGithubClient, vcs.NewLyftPullMergeabilityChecker("atlantis"))
 
 	applyCommandRunner := events.NewApplyCommandRunner(
 		e2eVCSClient,
@@ -1019,7 +1020,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 		parallelPoolSize,
 		silenceNoProjects,
 		false,
-		&e2ePullReqStatusFetcher,
+		e2ePullReqStatusFetcher,
 	)
 
 	approvePoliciesCommandRunner := events.NewApprovePoliciesCommandRunner(
@@ -1149,6 +1150,7 @@ func GitHubPullRequestParsed(headSHA string) *github.PullRequest {
 	if headSHA == "" {
 		headSHA = "13940d121be73f656e2132c6d7b4c8e87878ac8d"
 	}
+	cleanstate := "clean"
 	return &github.PullRequest{
 		Number:  github.Int(2),
 		State:   github.String("open"),
@@ -1171,6 +1173,7 @@ func GitHubPullRequestParsed(headSHA string) *github.PullRequest {
 		User: &github.User{
 			Login: github.String("atlantisbot"),
 		},
+		MergeableState: &cleanstate,
 	}
 }
 

server/controllers/github_app_controller.go

@@ -56,7 +56,11 @@ func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Reques
 
 	g.Logger.Debug("Exchanging GitHub app code for app credentials")
 	creds := &vcs.GithubAnonymousCredentials{}
-	client, err := vcs.NewGithubClient(g.GithubHostname, creds, g.Logger, g.GithubStatusName)
+
+	// TODO: unify this in a single inject.go file
+	mergeabilityChecker := vcs.NewLyftPullMergeabilityChecker(g.GithubStatusName)
+	client, err := vcs.NewGithubClient(g.GithubHostname, creds, g.Logger, mergeabilityChecker)
+
 	if err != nil {
 		g.respond(w, logging.Error, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err)
 		return

server/events/apply_requirement_handler.go

@@ -19,7 +19,7 @@ func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx models.
 	for _, req := range ctx.ApplyRequirements {
 		switch req {
 		case raw.ApprovedApplyRequirement:
-			if !ctx.PullReqStatus.Approved.IsApproved {
+			if !ctx.PullReqStatus.ApprovalStatus.IsApproved {
 				return "Pull request must be approved by at least one person other than the author before running apply.", nil
 			}
 		// this should come before mergeability check since mergeability is a superset of this check.

server/events/command_context.go

@@ -38,15 +38,10 @@ type CommandContext struct {
 	// See https://help.github.com/articles/about-pull-request-merges/.
 	HeadRepo models.Repo
 	Pull     models.PullRequest
+	Scope    stats.Scope
 	// User is the user that triggered this command.
-	User  models.User
-	Log   logging.SimpleLogging
-	Scope stats.Scope
-	// PullMergeable is true if Pull is able to be merged. This is available in
-	// the CommandContext because we want to collect this information before we
-	// set our own build statuses which can affect mergeability if users have
-	// required the Atlantis status to be successful prior to merging.
-	PullMergeable bool
+	User models.User
+	Log  logging.SimpleLogging
 
 	// Current PR state
 	PullRequestStatus models.PullReqStatus

server/events/command_runner_test.go

@@ -23,6 +23,7 @@ import (
 	stats "github.com/lyft/gostats"
 	"github.com/runatlantis/atlantis/server/core/db"
 	"github.com/runatlantis/atlantis/server/events/vcs"
+	lyft_vcs "github.com/runatlantis/atlantis/server/events/vcs/lyft"
 	"github.com/runatlantis/atlantis/server/events/yaml/valid"
 	"github.com/runatlantis/atlantis/server/logging"
 
@@ -136,9 +137,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		defaultBoltDB,
 	)
 
-	pullReqStatusFetcher := vcs.SQBasedPullStatusFetcher{
-		GithubClient: githubClient,
-	}
+	pullReqStatusFetcher := lyft_vcs.NewSQBasedPullStatusFetcher(githubClient, vcs.NewLyftPullMergeabilityChecker("atlantis"))
 
 	applyCommandRunner = events.NewApplyCommandRunner(
 		vcsClient,
@@ -154,7 +153,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		parallelPoolSize,
 		SilenceNoProjects,
 		false,
-		&pullReqStatusFetcher,
+		pullReqStatusFetcher,
 	)
 
 	approvePoliciesCommandRunner = events.NewApprovePoliciesCommandRunner(

server/events/models/models.go

@@ -38,6 +38,12 @@ const (
 	LogStreamingClearMsg = "\n-----Starting New Process-----"
 )
 
+type PullReqStatus struct {
+	ApprovalStatus ApprovalStatus
+	Mergeable      bool
+	SQLocked       bool
+}
+
 // Repo is a VCS repository.
 type Repo struct {
 	// FullName is the owner and repo name separated
@@ -145,12 +151,6 @@ func NewRepo(vcsHostType VCSHostType, repoFullName string, cloneURL string, vcsU
 	}, nil
 }
 
-type PullReqStatus struct {
-	Approved  ApprovalStatus
-	Mergeable bool
-	SQLocked  bool
-}
-
 type ApprovalStatus struct {
 	IsApproved bool
 	ApprovedBy string
@@ -385,8 +385,6 @@ type ProjectCommandContext struct {
 	Log logging.SimpleLogging
 	// Scope is the scope for reporting stats setup for this context
 	Scope stats.Scope
-	// PullMergeable is true if the pull request for this project is able to be merged.
-	PullMergeable bool
 	// PullReqStatus holds state about the PR that requires additional computation outside models.PullRequest
 	PullReqStatus PullReqStatus
 	// CurrentProjectPlanStatus is the status of the current project prior to this command.