Fixes #957

server/events/project_locker.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/runatlantis/atlantis/server/events/locking"
 	"github.com/runatlantis/atlantis/server/events/models"
+	"github.com/runatlantis/atlantis/server/events/vcs"
 	"github.com/runatlantis/atlantis/server/logging"
 )
 
@@ -37,7 +38,8 @@ type ProjectLocker interface {
 
 // DefaultProjectLocker implements ProjectLocker.
 type DefaultProjectLocker struct {
-	Locker locking.Locker
+	Locker    locking.Locker
+	VCSClient vcs.Client
 }
 
 // TryLockResponse is the result of trying to lock a project.
@@ -62,10 +64,14 @@ func (p *DefaultProjectLocker) TryLock(log *logging.SimpleLogger, pull models.Pu
 		return nil, err
 	}
 	if !lockAttempt.LockAcquired && lockAttempt.CurrLock.Pull.Num != pull.Num {
+		link, err := p.VCSClient.MarkdownPullLink(lockAttempt.CurrLock.Pull)
+		if err != nil {
+			return nil, err
+		}
 		failureMsg := fmt.Sprintf(
-			"This project is currently locked by an unapplied plan from pull #%d. To continue, delete the lock from #%d or apply that plan and merge the pull request.\n\nOnce the lock is released, comment `atlantis plan` here to re-plan.",
-			lockAttempt.CurrLock.Pull.Num,
-			lockAttempt.CurrLock.Pull.Num)
+			"This project is currently locked by an unapplied plan from pull %s. To continue, delete the lock from %s or apply that plan and merge the pull request.\n\nOnce the lock is released, comment `atlantis plan` here to re-plan.",
+			link,
+			link)
 		return &TryLockResponse{
 			LockAcquired:      false,
 			LockFailureReason: failureMsg,

server/events/project_locker_test.go

@@ -14,21 +14,33 @@
 package events_test
 
 import (
+	"fmt"
 	"testing"
 
 	. "github.com/petergtz/pegomock"
 	"github.com/runatlantis/atlantis/server/events"
 	"github.com/runatlantis/atlantis/server/events/locking"
 	"github.com/runatlantis/atlantis/server/events/locking/mocks"
 	"github.com/runatlantis/atlantis/server/events/models"
+	"github.com/runatlantis/atlantis/server/events/vcs"
+	"github.com/runatlantis/atlantis/server/events/vcs/bitbucketcloud"
+	"github.com/runatlantis/atlantis/server/events/vcs/bitbucketserver"
 	"github.com/runatlantis/atlantis/server/logging"
 	. "github.com/runatlantis/atlantis/testing"
 )
 
 func TestDefaultProjectLocker_TryLockWhenLocked(t *testing.T) {
+	var githubClient *vcs.GithubClient
+	var gitlabClient *vcs.GitlabClient
+	var bitbucketCloudClient *bitbucketcloud.Client
+	var bitbucketServerClient *bitbucketserver.Client
+	var azuredevopsClient *vcs.AzureDevopsClient
+
+	mockClient := vcs.NewClientProxy(githubClient, gitlabClient, bitbucketCloudClient, bitbucketServerClient, azuredevopsClient)
 	mockLocker := mocks.NewMockLocker()
 	locker := events.DefaultProjectLocker{
-		Locker: mockLocker,
+		Locker:    mockLocker,
+		VCSClient: mockClient,
 	}
 	expProject := models.Project{}
 	expWorkspace := "default"
@@ -49,18 +61,27 @@ func TestDefaultProjectLocker_TryLockWhenLocked(t *testing.T) {
 		nil,
 	)
 	res, err := locker.TryLock(logging.NewNoopLogger(), expPull, expUser, expWorkspace, expProject)
+	link, _ := mockClient.MarkdownPullLink(lockingPull)
 	Ok(t, err)
 	Equals(t, &events.TryLockResponse{
 		LockAcquired:      false,
-		LockFailureReason: "This project is currently locked by an unapplied plan from pull #2. To continue, delete the lock from #2 or apply that plan and merge the pull request.\n\nOnce the lock is released, comment `atlantis plan` here to re-plan.",
+		LockFailureReason: fmt.Sprintf("This project is currently locked by an unapplied plan from pull %s. To continue, delete the lock from %s or apply that plan and merge the pull request.\n\nOnce the lock is released, comment `atlantis plan` here to re-plan.", link, link),
 	}, res)
 }
 
 func TestDefaultProjectLocker_TryLockWhenLockedSamePull(t *testing.T) {
 	RegisterMockTestingT(t)
+	var githubClient *vcs.GithubClient
+	var gitlabClient *vcs.GitlabClient
+	var bitbucketCloudClient *bitbucketcloud.Client
+	var bitbucketServerClient *bitbucketserver.Client
+	var azuredevopsClient *vcs.AzureDevopsClient
+
+	mockClient := vcs.NewClientProxy(githubClient, gitlabClient, bitbucketCloudClient, bitbucketServerClient, azuredevopsClient)
 	mockLocker := mocks.NewMockLocker()
 	locker := events.DefaultProjectLocker{
-		Locker: mockLocker,
+		Locker:    mockLocker,
+		VCSClient: mockClient,
 	}
 	expProject := models.Project{}
 	expWorkspace := "default"
@@ -94,9 +115,16 @@ func TestDefaultProjectLocker_TryLockWhenLockedSamePull(t *testing.T) {
 
 func TestDefaultProjectLocker_TryLockUnlocked(t *testing.T) {
 	RegisterMockTestingT(t)
+	var githubClient *vcs.GithubClient
+	var gitlabClient *vcs.GitlabClient
+	var bitbucketCloudClient *bitbucketcloud.Client
+	var bitbucketServerClient *bitbucketserver.Client
+	var azuredevopsClient *vcs.AzureDevopsClient
+	mockClient := vcs.NewClientProxy(githubClient, gitlabClient, bitbucketCloudClient, bitbucketServerClient, azuredevopsClient)
 	mockLocker := mocks.NewMockLocker()
 	locker := events.DefaultProjectLocker{
-		Locker: mockLocker,
+		Locker:    mockLocker,
+		VCSClient: mockClient,
 	}
 	expProject := models.Project{}
 	expWorkspace := "default"

server/events/vcs/azuredevops_client.go

@@ -337,6 +337,11 @@ func (g *AzureDevopsClient) MergePull(pull models.PullRequest) error {
 	return nil
 }
 
+// MarkdownPullLink specifies the string used in a pull request comment to reference another pull request.
+func (g *AzureDevopsClient) MarkdownPullLink(pull models.PullRequest) (string, error) {
+	return fmt.Sprintf("!%d", pull.Num), nil
+}
+
 // SplitAzureDevopsRepoFullName splits a repo full name up into its owner,
 // repo and project name segments. If the repoFullName is malformed, may
 // return empty strings for owner, repo, or project.  Azure DevOps uses

server/events/vcs/azuredevops_client_test.go

@@ -494,6 +494,15 @@ func TestAzureDevopsClient_GetPullRequest(t *testing.T) {
 	})
 }
 
+func TestAzureDevopsClient_MarkdownPullLink(t *testing.T) {
+	client, err := vcs.NewAzureDevopsClient("hostname", "token")
+	Ok(t, err)
+	pull := models.PullRequest{Num: 1}
+	s, _ := client.MarkdownPullLink(pull)
+	exp := "!1"
+	Equals(t, exp, s)
+}
+
 var adMergeSuccess = `{
 	"status": "completed",
 	"mergeStatus": "succeeded",

server/events/vcs/bitbucketcloud/client.go

@@ -200,6 +200,11 @@ func (b *Client) MergePull(pull models.PullRequest) error {
 	return err
 }
 
+// MarkdownPullLink specifies the character used in a pull request comment.
+func (b *Client) MarkdownPullLink(pull models.PullRequest) (string, error) {
+	return fmt.Sprintf("#%d", pull.Num), nil
+}
+
 // prepRequest adds auth and necessary headers.
 func (b *Client) prepRequest(method string, path string, body io.Reader) (*http.Request, error) {
 	req, err := http.NewRequest(method, path, body)

server/events/vcs/bitbucketcloud/client_test.go

@@ -344,3 +344,11 @@ func TestClient_PullIsMergeable(t *testing.T) {
 	}
 
 }
+
+func TestClient_MarkdownPullLink(t *testing.T) {
+	client := bitbucketcloud.NewClient(http.DefaultClient, "user", "pass", "runatlantis.io")
+	pull := models.PullRequest{Num: 1}
+	s, _ := client.MarkdownPullLink(pull)
+	exp := "#1"
+	Equals(t, exp, s)
+}

server/events/vcs/bitbucketserver/client.go

@@ -268,6 +268,11 @@ func (b *Client) MergePull(pull models.PullRequest) error {
 	return err
 }
 
+// MarkdownPullLink specifies the character used in a pull request comment.
+func (b *Client) MarkdownPullLink(pull models.PullRequest) (string, error) {
+	return fmt.Sprintf("#%d", pull.Num), nil
+}
+
 // prepRequest adds auth and necessary headers.
 func (b *Client) prepRequest(method string, path string, body io.Reader) (*http.Request, error) {
 	req, err := http.NewRequest(method, path, body)

server/events/vcs/bitbucketserver/client_test.go

@@ -180,3 +180,12 @@ func TestClient_MergePull(t *testing.T) {
 	})
 	Ok(t, err)
 }
+
+func TestClient_MarkdownPullLink(t *testing.T) {
+	client, err := bitbucketserver.NewClient(nil, "u", "p", "https://base-url", "atlantis-url")
+	Ok(t, err)
+	pull := models.PullRequest{Num: 1}
+	s, _ := client.MarkdownPullLink(pull)
+	exp := "#1"
+	Equals(t, exp, s)
+}

server/events/vcs/client.go

@@ -37,4 +37,5 @@ type Client interface {
 	// about this status.
 	UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error
 	MergePull(pull models.PullRequest) error
+	MarkdownPullLink(pull models.PullRequest) (string, error)
 }

server/events/vcs/github_client.go

@@ -319,3 +319,8 @@ func (g *GithubClient) MergePull(pull models.PullRequest) error {
 	}
 	return nil
 }
+
+// MarkdownPullLink specifies the string used in a pull request comment to reference another pull request.
+func (g *GithubClient) MarkdownPullLink(pull models.PullRequest) (string, error) {
+	return fmt.Sprintf("#%d", pull.Num), nil
+}

server/events/vcs/github_client_test.go

@@ -764,6 +764,15 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 	}
 }
 
+func TestGithubClient_MarkdownPullLink(t *testing.T) {
+	client, err := vcs.NewGithubClient("hostname", "user", "pass")
+	Ok(t, err)
+	pull := models.PullRequest{Num: 1}
+	s, _ := client.MarkdownPullLink(pull)
+	exp := "#1"
+	Equals(t, exp, s)
+}
+
 // disableSSLVerification disables ssl verification for the global http client
 // and returns a function to be called in a defer that will re-enable it.
 func disableSSLVerification() func() {

server/events/vcs/gitlab_client.go

@@ -216,6 +216,11 @@ func (g *GitlabClient) MergePull(pull models.PullRequest) error {
 	return errors.Wrap(err, "unable to merge merge request, it may not be in a mergeable state")
 }
 
+// MarkdownPullLink specifies the string used in a pull request comment to reference another pull request.
+func (g *GitlabClient) MarkdownPullLink(pull models.PullRequest) (string, error) {
+	return fmt.Sprintf("#%d", pull.Num), nil
+}
+
 // GetVersion returns the version of the Gitlab server this client is using.
 func (g *GitlabClient) GetVersion() (*version.Version, error) {
 	req, err := g.Client.NewRequest("GET", "/version", nil, nil)

server/events/vcs/gitlab_client_test.go

@@ -232,4 +232,15 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
 	}
 }
 
+func TestGitlabClient_MarkdownPullLink(t *testing.T) {
+	gitlabClientUnderTest = true
+	defer func() { gitlabClientUnderTest = false }()
+	client, err := NewGitlabClient("gitlab.com", "token", nil)
+	Ok(t, err)
+	pull := models.PullRequest{Num: 1}
+	s, _ := client.MarkdownPullLink(pull)
+	exp := "#1"
+	Equals(t, exp, s)
+}
+
 var mergeSuccess = `{"id":22461274,"iid":13,"project_id":4580910,"title":"Update main.tf","description":"","state":"merged","created_at":"2019-01-15T18:27:29.375Z","updated_at":"2019-01-25T17:28:01.437Z","merged_by":{"id":1755902,"name":"Luke Kysow","username":"lkysow","state":"active","avatar_url":"https://secure.gravatar.com/avatar/25fd57e71590fe28736624ff24d41c5f?s=80\u0026d=identicon","web_url":"https://gitlab.com/lkysow"},"merged_at":"2019-01-25T17:28:01.459Z","closed_by":null,"closed_at":null,"target_branch":"patch-1","source_branch":"patch-1-merger","upvotes":0,"downvotes":0,"author":{"id":1755902,"name":"Luke Kysow","username":"lkysow","state":"active","avatar_url":"https://secure.gravatar.com/avatar/25fd57e71590fe28736624ff24d41c5f?s=80\u0026d=identicon","web_url":"https://gitlab.com/lkysow"},"assignee":null,"source_project_id":4580910,"target_project_id":4580910,"labels":[],"work_in_progress":false,"milestone":null,"merge_when_pipeline_succeeds":false,"merge_status":"can_be_merged","sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","merge_commit_sha":"c9b336f1c71d3e64810b8cfa2abcfab232d6bff6","user_notes_count":0,"discussion_locked":null,"should_remove_source_branch":null,"force_remove_source_branch":false,"web_url":"https://gitlab.com/lkysow/atlantis-example/merge_requests/13","time_stats":{"time_estimate":0,"total_time_spent":0,"human_time_estimate":null,"human_total_time_spent":null},"squash":false,"subscribed":true,"changes_count":"1","latest_build_started_at":null,"latest_build_finished_at":null,"first_deployed_to_production_at":null,"pipeline":null,"diff_refs":{"base_sha":"67cb91d3f6198189f433c045154a885784ba6977","head_sha":"cb86d70f464632bdfbe1bb9bc0f2f9d847a774a0","start_sha":"67cb91d3f6198189f433c045154a885784ba6977"},"merge_error":null,"approvals_before_merge":null}`