bugfix: Add check for deleted comments in AzureDevOps event handler

[SSKLCP]

what

Aims to fix bug #2803 which causes Atlantis to ungracefully close a web request when a comment is deleted in a PR.
A simple check to see if the "IsDeleted" property is set to true and return a 200 OK message if so. (ignored)

I've built the project locally with this change and confirm the issue is no longer present when testing in our dev environment.

why

The bug causes Azure DevOps to disable the webhook connector when it sees a web request terminate ungracefully. Effectively bringing down the service until re-enabled.

references

closes #2803

Please let me know if anything else is needed and I'm happy to add - I haven't contributed to any projects this big before 😨

[jamengual]

can you add a test for this? Thanks

[nitrocode]

I would think you'd need to add this if block in the AzureDevops VCS section. It's odd that this function exists in the shared events_controller but this if block makes sense where it is then.

I second @jamengual's suggestion to add a test.

We do not currently have any azure devops tests regarding comments but we do for bitbucket and github. If you can follow those as an example, we'd really appreciate azure devops tests.

atlantis/server/events/event_parser_test.go

Line 1020 in 585ce1e

func TestParseBitbucketServerCommentEvent_EmptyString(t *testing.T) {

Could you add this for azure devops @SSKLCP to validate your change so it doesn't break in the future?

[SSKLCP]

@nitrocode / @jamengual - I've added a test in now.

I copied the makings of this function as it seemed closer to the area I was modifying:

atlantis/server/controllers/events/events_controller_test.go

Line 478 in 585ce1e

func TestPost_AzureDevopsPullRequestIgnoreEvent(t *testing.T) {

I did look at the parser section that you linked however I am not sure that it gets to the ADO parser section in this case, it may be failing before then. (possibly just needs a bit more investigation)

But this test covers the specific case I'm struggling with. Maybe it can be expanded to cover other instances with a blank / null comment once I understand it a bit more.

[nitrocode]

If the tests pass, LGTM!

cc: @jamengual

[nitrocode]

Thank you @SSKLCP ! Please feel free to add more changes to azure devops. It's an area that doesn't get too much love. We appreciate you adding tests too. Please add more if time permits.

[jamengual]

+1