fix: set ctx.PullRequestStatus in import_command_runner for import_re…

…quirements (runatlantis#2936)

* refactor: pull_status_fetcher contains vcsStatusName in struct / reduce FetchPullStatus parameter

* import_command_runner need to set ctx.PullRequestStatus for import_requirements

server/controllers/events/events_controller_e2e_test.go

@@ -1195,7 +1195,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 		discardApprovalOnPlan,
 	)
 
-	e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient)
+	e2ePullReqStatusFetcher := vcs.NewPullReqStatusFetcher(e2eVCSClient, "atlantis-test")
 
 	applyCommandRunner := events.NewApplyCommandRunner(
 		e2eVCSClient,
@@ -1211,7 +1211,6 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 		parallelPoolSize,
 		silenceNoProjects,
 		false,
-		"atlantis-test",
 		e2ePullReqStatusFetcher,
 	)
 
@@ -1241,6 +1240,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 
 	importCommandRunner := events.NewImportCommandRunner(
 		pullUpdater,
+		e2ePullReqStatusFetcher,
 		projectCommandBuilder,
 		projectCommandRunner,
 	)

server/events/apply_command_runner.go

@@ -21,7 +21,6 @@ func NewApplyCommandRunner(
 	parallelPoolSize int,
 	SilenceNoProjects bool,
 	silenceVCSStatusNoProjects bool,
-	VCSStatusName string,
 	pullReqStatusFetcher vcs.PullReqStatusFetcher,
 ) *ApplyCommandRunner {
 	return &ApplyCommandRunner{
@@ -38,7 +37,6 @@ func NewApplyCommandRunner(
 		parallelPoolSize:           parallelPoolSize,
 		SilenceNoProjects:          SilenceNoProjects,
 		silenceVCSStatusNoProjects: silenceVCSStatusNoProjects,
-		VCSStatusName:              VCSStatusName,
 		pullReqStatusFetcher:       pullReqStatusFetcher,
 	}
 }
@@ -61,7 +59,6 @@ type ApplyCommandRunner struct {
 	SilenceNoProjects bool
 	// SilenceVCSStatusNoPlans is whether any plan should set commit status if no projects
 	// are found
-	VCSStatusName              string
 	silenceVCSStatusNoProjects bool
 }
 
@@ -105,7 +102,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
 	// required the Atlantis status checks to pass, then we've now changed
 	// the mergeability status of the pull request.
 	// This sets the approved, mergeable, and sqlocked status in the context.
-	ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(baseRepo, pull, a.VCSStatusName)
+	ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(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,

server/events/command_runner_test.go

@@ -23,7 +23,6 @@ import (
 	"github.com/runatlantis/atlantis/server/core/config/valid"
 	"github.com/runatlantis/atlantis/server/core/db"
 	"github.com/runatlantis/atlantis/server/events/command"
-	"github.com/runatlantis/atlantis/server/events/vcs"
 	"github.com/runatlantis/atlantis/server/logging"
 	"github.com/runatlantis/atlantis/server/metrics"
 
@@ -52,6 +51,7 @@ var pendingPlanFinder *mocks.MockPendingPlanFinder
 var drainer *events.Drainer
 var deleteLockCommand *mocks.MockDeleteLockCommand
 var commitUpdater *mocks.MockCommitStatusUpdater
+var pullReqStatusFetcher *vcsmocks.MockPullReqStatusFetcher
 
 // TODO: refactor these into their own unit tests.
 // these were all split out from default command runner in an effort to improve
@@ -102,6 +102,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock
 	workingDir = mocks.NewMockWorkingDir()
 	pendingPlanFinder = mocks.NewMockPendingPlanFinder()
 	commitUpdater = mocks.NewMockCommitStatusUpdater()
+	pullReqStatusFetcher = vcsmocks.NewMockPullReqStatusFetcher()
 	tmp := t.TempDir()
 	defaultBoltDB, err := db.New(tmp)
 	Ok(t, err)
@@ -156,8 +157,6 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock
 		testConfig.discardApprovalOnPlan,
 	)
 
-	pullReqStatusFetcher := vcs.NewPullReqStatusFetcher(vcsClient)
-
 	applyCommandRunner = events.NewApplyCommandRunner(
 		vcsClient,
 		false,
@@ -172,7 +171,6 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock
 		testConfig.parallelPoolSize,
 		testConfig.SilenceNoProjects,
 		false,
-		testConfig.StatusName,
 		pullReqStatusFetcher,
 	)
 
@@ -202,6 +200,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock
 
 	importCommandRunner = events.NewImportCommandRunner(
 		pullUpdater,
+		pullReqStatusFetcher,
 		projectCommandBuilder,
 		projectCommandRunner,
 	)

server/events/import_command_runner.go

@@ -2,28 +2,46 @@ package events
 
 import (
 	"github.com/runatlantis/atlantis/server/events/command"
+	"github.com/runatlantis/atlantis/server/events/vcs"
 )
 
 func NewImportCommandRunner(
 	pullUpdater *PullUpdater,
+	pullReqStatusFetcher vcs.PullReqStatusFetcher,
 	prjCmdBuilder ProjectImportCommandBuilder,
 	prjCmdRunner ProjectImportCommandRunner,
 ) *ImportCommandRunner {
 	return &ImportCommandRunner{
-		pullUpdater:   pullUpdater,
-		prjCmdBuilder: prjCmdBuilder,
-		prjCmdRunner:  prjCmdRunner,
+		pullUpdater:          pullUpdater,
+		pullReqStatusFetcher: pullReqStatusFetcher,
+		prjCmdBuilder:        prjCmdBuilder,
+		prjCmdRunner:         prjCmdRunner,
 	}
 }
 
 type ImportCommandRunner struct {
-	pullUpdater   *PullUpdater
-	prjCmdBuilder ProjectImportCommandBuilder
-	prjCmdRunner  ProjectImportCommandRunner
+	pullUpdater          *PullUpdater
+	pullReqStatusFetcher vcs.PullReqStatusFetcher
+	prjCmdBuilder        ProjectImportCommandBuilder
+	prjCmdRunner         ProjectImportCommandRunner
 }
 
 func (v *ImportCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
 	var err error
+	// Get the mergeable status before we set any build statuses of our own.
+	// 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.
+	// This sets the approved, mergeable, and sqlocked status in the context.
+	ctx.PullRequestStatus, err = v.pullReqStatusFetcher.FetchPullStatus(ctx.Pull)
+	if err != nil {
+		// On error we continue the request with mergeable assumed false.
+		// We want to continue because not all import will need this status,
+		// only if they rely on the mergeability requirement.
+		// 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)
+	}
+
 	var projectCmds []command.ProjectContext
 	projectCmds, err = v.prjCmdBuilder.BuildImportCommands(ctx, cmd)
 	if err != nil {

server/events/import_command_runner_test.go

@@ -0,0 +1,69 @@
+package events_test
+
+import (
+	"testing"
+
+	. "github.com/petergtz/pegomock"
+	"github.com/runatlantis/atlantis/server/events"
+	"github.com/runatlantis/atlantis/server/events/command"
+	"github.com/runatlantis/atlantis/server/events/models"
+	"github.com/runatlantis/atlantis/server/events/models/fixtures"
+	"github.com/runatlantis/atlantis/server/logging"
+	"github.com/runatlantis/atlantis/server/metrics"
+	. "github.com/runatlantis/atlantis/testing"
+)
+
+func TestImportCommandRunner_Run(t *testing.T) {
+	RegisterMockTestingT(t)
+
+	tests := []struct {
+		name          string
+		pullReqStatus models.PullReqStatus
+		projectCmds   []command.ProjectContext
+		expComment    string
+	}{
+		{
+			name: "success with zero projects",
+			pullReqStatus: models.PullReqStatus{
+				ApprovalStatus: models.ApprovalStatus{IsApproved: true},
+				Mergeable:      true,
+			},
+			projectCmds: []command.ProjectContext{},
+			expComment:  "Ran Import for 0 projects:\n\n\n\n",
+		},
+		{
+			name: "failure with multiple projects",
+			pullReqStatus: models.PullReqStatus{
+				ApprovalStatus: models.ApprovalStatus{IsApproved: true},
+				Mergeable:      true,
+			},
+			projectCmds: []command.ProjectContext{{}, {}},
+			expComment:  "**Import Failed**: import cannot run on multiple projects. please specify one project.\n",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			vcsClient := setup(t)
+
+			scopeNull, _, _ := metrics.NewLoggingScope(logger, "atlantis")
+			modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, State: models.OpenPullState, Num: fixtures.Pull.Num}
+			ctx := &command.Context{
+				User:     fixtures.User,
+				Log:      logging.NewNoopLogger(t),
+				Scope:    scopeNull,
+				Pull:     modelPull,
+				HeadRepo: fixtures.GithubRepo,
+				Trigger:  command.CommentTrigger,
+			}
+			cmd := &events.CommentCommand{Name: command.Import}
+
+			When(pullReqStatusFetcher.FetchPullStatus(modelPull)).ThenReturn(tt.pullReqStatus, nil)
+			When(projectCommandBuilder.BuildImportCommands(ctx, cmd)).ThenReturn(tt.projectCmds, nil)
+
+			importCommandRunner.Run(ctx, cmd)
+
+			Assert(t, ctx.PullRequestStatus.Mergeable == true, "PullRequestStatus must be set for import_requirements")
+			vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, modelPull.Num, tt.expComment, "import")
+		})
+	}
+}