Fixes #957

[mcdafydd]

How does this look for a solution to this issue?

[jpreese]

Definitely gets the job done! Looking forward to this one getting released.

I will say it does feel a bit cleaner to have each VCS client define their own pull request link format. Thinking ahead, I would have concerns about littering checks/tests/etc that are specific to a single VCS.

In this case it's probably overkill to let a VCS define their own ProjectLocker, because the only difference is the PR link. Maybe its something we add into the VCSHost struct as it should be global for the entire VCS, and then the DefaultProjectLocker just uses that.

[mcdafydd]

Good point. I'll look into something a bit more general.

[lkysow]

Yeah this was an issue waiting to happen because it's coupled the error message with the markdown linking method of the VCS.

Let's make vcs.Client a field of ProjectLocker and add a new method to the interface: MarkdownPullLink(pullNum int) string that is implemented by all the vcs clients.

This can then be used by the project locker:

		link := p.VCSClient.MarkdownPullLink(lockAttempt.CurrLock.Pull.Num)
		failureMsg := 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)

[lkysow]

Looks good. You'll probably need to rebase off master to get the tests passing.

[mcdafydd]

Sorry for the messy PR if there's a better way to rebase in this case.

I'm getting some local errors trying to regen the mock client. Hopefully, I will sort this out quickly and push a fix.

[jpreese]

No worries, I always lean on my good friend squash merge. 😄

[mcdafydd]

Sorry for the delay on this. I should be able to wrap it up this weekend.

[codecov]

Codecov Report

Merging #983 into master will decrease coverage by 0.01%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##           master     #983      +/-   ##
==========================================
- Coverage   71.98%   71.96%   -0.02%     
==========================================
  Files          65       65              
  Lines        5411     5429      +18     
==========================================
+ Hits         3895     3907      +12     
- Misses       1210     1215       +5     
- Partials      306      307       +1     

Impacted Files	Coverage Δ
server/events/vcs/not_configured_vcs_client.go	0.00% <0.00%> (ø)
server/events/vcs/proxy.go	0.00% <0.00%> (ø)
server/events/project_locker.go	82.60% <66.66%> (-7.40%)	⬇️
server/events/vcs/azuredevops_client.go	66.82% <100.00%> (+0.32%)	⬆️
server/events/vcs/bitbucketcloud/client.go	47.40% <100.00%> (+0.79%)	⬆️
server/events/vcs/bitbucketserver/client.go	35.63% <100.00%> (+0.74%)	⬆️
server/events/vcs/github_client.go	80.70% <100.00%> (+0.22%)	⬆️
server/events/vcs/gitlab_client.go	39.66% <100.00%> (+1.01%)	⬆️
server/server.go	63.77% <100.00%> (+0.10%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fea1891...86cf4d3. Read the comment docs.

[lkysow]

Almost there

[mcdafydd]

Sounds good, thanks!