refactor: Add PullStatusFetcher interface (#1904)

server/controllers/events/events_controller_e2e_test.go

@@ -726,6 +726,7 @@ func TestGitHubWorkflowWithPolicyCheck(t *testing.T) {
 			userConfig.EnablePolicyChecksFlag = true
 
 			ctrl, vcsClient, githubGetter, atlantisWorkspace := setupE2E(t, c.RepoDir)
+
 			// Set the repo to be cloned through the testing backdoor.
 			repoDir, headSHA, cleanup := initializeRepo(t, c.RepoDir)
 			defer cleanup()
@@ -937,8 +938,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 		Webhooks:         &mockWebhookSender{},
 		WorkingDirLocker: locker,
 		AggregateApplyRequirements: &events.AggregateApplyRequirements{
-			PullApprovedChecker: e2eVCSClient,
-			WorkingDir:          workingDir,
+			WorkingDir: workingDir,
 		},
 	}
 
@@ -984,6 +984,8 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 		boltdb,
 	)
 
+	e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient)
+
 	applyCommandRunner := events.NewApplyCommandRunner(
 		e2eVCSClient,
 		false,
@@ -998,6 +1000,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl
 		parallelPoolSize,
 		silenceNoProjects,
 		false,
+		e2ePullReqStatusFetcher,
 	)
 
 	approvePoliciesCommandRunner := events.NewApprovePoliciesCommandRunner(

server/events/apply_command_runner.go

@@ -21,6 +21,7 @@ func NewApplyCommandRunner(
 	parallelPoolSize int,
 	SilenceNoProjects bool,
 	silenceVCSStatusNoProjects bool,
+	pullReqStatusFetcher vcs.PullReqStatusFetcher,
 ) *ApplyCommandRunner {
 	return &ApplyCommandRunner{
 		vcsClient:                  vcsClient,
@@ -36,21 +37,23 @@ func NewApplyCommandRunner(
 		parallelPoolSize:           parallelPoolSize,
 		SilenceNoProjects:          SilenceNoProjects,
 		silenceVCSStatusNoProjects: silenceVCSStatusNoProjects,
+		pullReqStatusFetcher:       pullReqStatusFetcher,
 	}
 }
 
 type ApplyCommandRunner struct {
-	DisableApplyAll     bool
-	DB                  *db.BoltDB
-	locker              locking.ApplyLockChecker
-	vcsClient           vcs.Client
-	commitStatusUpdater CommitStatusUpdater
-	prjCmdBuilder       ProjectApplyCommandBuilder
-	prjCmdRunner        ProjectApplyCommandRunner
-	autoMerger          *AutoMerger
-	pullUpdater         *PullUpdater
-	dbUpdater           *DBUpdater
-	parallelPoolSize    int
+	DisableApplyAll      bool
+	DB                   *db.BoltDB
+	locker               locking.ApplyLockChecker
+	vcsClient            vcs.Client
+	commitStatusUpdater  CommitStatusUpdater
+	prjCmdBuilder        ProjectApplyCommandBuilder
+	prjCmdRunner         ProjectApplyCommandRunner
+	autoMerger           *AutoMerger
+	pullUpdater          *PullUpdater
+	dbUpdater            *DBUpdater
+	parallelPoolSize     int
+	pullReqStatusFetcher vcs.PullReqStatusFetcher
 	// SilenceNoProjects is whether Atlantis should respond to PRs if no projects
 	// are found
 	SilenceNoProjects bool
@@ -98,17 +101,16 @@ func (a *ApplyCommandRunner) Run(ctx *CommandContext, cmd *CommentCommand) {
 	// We do this here because when we set a "Pending" status, if users have
 	// required the Atlantis status checks to pass, then we've now changed
 	// the mergeability status of the pull request.
-	ctx.PullMergeable, err = a.vcsClient.PullIsMergeable(baseRepo, pull)
+	// This sets the approved, mergeable, and sqlocked status in the context.
+	ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(baseRepo, pull)
 	if err != nil {
 		// On error we continue the request with mergeable assumed false.
 		// We want to continue because not all apply's will need this status,
 		// only if they rely on the mergeability requirement.
-		ctx.PullMergeable = false
-		ctx.Log.Warn("unable to get mergeable status: %s. Continuing with mergeable assumed false", err)
+		// All PullRequestStatus fields are set to false by default when error.
+		ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err)
 	}
 
-	ctx.Log.Info("pull request mergeable status: %t", ctx.PullMergeable)
-
 	var projectCmds []models.ProjectCommandContext
 	projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd)
 

server/events/apply_requirement_handler.go

@@ -1,8 +1,6 @@
 package events
 
 import (
-	"github.com/pkg/errors"
-	"github.com/runatlantis/atlantis/server/core/runtime"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/yaml/raw"
 	"github.com/runatlantis/atlantis/server/events/yaml/valid"
@@ -14,20 +12,14 @@ type ApplyRequirement interface {
 }
 
 type AggregateApplyRequirements struct {
-	PullApprovedChecker runtime.PullApprovedChecker
-	WorkingDir          WorkingDir
+	WorkingDir WorkingDir
 }
 
 func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx models.ProjectCommandContext) (failure string, err error) {
-
 	for _, req := range ctx.ApplyRequirements {
 		switch req {
 		case raw.ApprovedApplyRequirement:
-			approvalStatus, err := a.PullApprovedChecker.PullIsApproved(ctx.Pull.BaseRepo, ctx.Pull) // nolint: vetshadow
-			if err != nil {
-				return "", errors.Wrap(err, "checking if pull request was approved")
-			}
-			if !approvalStatus.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.
@@ -36,7 +28,7 @@ func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx models.
 				return "All policies must pass for project before running apply", nil
 			}
 		case raw.MergeableApplyRequirement:
-			if !ctx.PullMergeable {
+			if !ctx.PullReqStatus.Mergeable {
 				return "Pull request must be mergeable before running apply.", nil
 			}
 		case raw.UnDivergedApplyRequirement:

server/events/command_context.go

@@ -40,11 +40,6 @@ type CommandContext struct {
 	// User is the user that triggered this command.
 	User models.User
 	Log  logging.SimpleLogging
-	// 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
 
 	// Current PR state
 	PullRequestStatus models.PullReqStatus

server/events/command_runner_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/runatlantis/atlantis/server/core/db"
+	"github.com/runatlantis/atlantis/server/events/vcs"
 	"github.com/runatlantis/atlantis/server/events/yaml/valid"
 	"github.com/runatlantis/atlantis/server/logging"
 
@@ -131,6 +132,8 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		defaultBoltDB,
 	)
 
+	pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient)
+
 	applyCommandRunner = events.NewApplyCommandRunner(
 		vcsClient,
 		false,
@@ -145,6 +148,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		parallelPoolSize,
 		SilenceNoProjects,
 		false,
+		pullReqStatusFetcher,
 	)
 
 	approvePoliciesCommandRunner = events.NewApprovePoliciesCommandRunner(

server/events/models/models.go

@@ -35,6 +35,11 @@ const (
 	planfileSlashReplace = "::"
 )
 
+type PullReqStatus struct {
+	ApprovalStatus ApprovalStatus
+	Mergeable      bool
+}
+
 // Repo is a VCS repository.
 type Repo struct {
 	// FullName is the owner and repo name separated
@@ -142,10 +147,6 @@ func NewRepo(vcsHostType VCSHostType, repoFullName string, cloneURL string, vcsU
 	}, nil
 }
 
-type PullReqStatus struct {
-	Approved ApprovalStatus
-}
-
 type ApprovalStatus struct {
 	IsApproved bool
 	ApprovedBy string
@@ -374,9 +375,7 @@ type ProjectCommandContext struct {
 	HeadRepo Repo
 	// Log is a logger that's been set up for this context.
 	Log logging.SimpleLogging
-	// PullMergeable is true if the pull request for this project is able to be merged.
-	PullMergeable bool
-	// CurrentProjectPlanStatus is the status of the current project prior to this command.
+	// 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.
 	ProjectPlanStatus ProjectPlanStatus