leverage templates for webhook messages

[lieut-data]

This change replaces the fmt.Sprintf strategy for rendering webhook messages with Go's text/template package, extracting sub-templates for common components. A few minor changes were made along the way:

• "Issue ... closed by" now links the repo vs just wrapping it with brackets
• (force-)pushed now pluralizes "commits" only when necessary

In addition, whenever rendering a GitHub username, if there is a linked Mattermost user, the rendered message replaces this with an @username mention for the corresponding Mattermost user. In the future, I hope to be able to expose this mapping for other plugins to do the same.

I need some feedback on how to properly test this -- there are unit tests for all the templates, but of course I have to populate sample data, and I really want to see the markdown rendering on a real Mattermost server. Do we have a shared testing OAuth app that can be used for localhost testing as such?

[jwilander]

Much cleaner, really nice 👍 Translating usernames is awesome too

[lieut-data]

@crspeller, I've been able to test the issue/pull request open/close/comment flows via a fork of the mattermost-server (and I fixed an escape issue with brackets), but it's going to be hard to test the review flow without another test user on GitHub. Wondering if it made sense for me to register my webhook on the real mattermost-server instead and consume all the real events there to vet everything?

[crspeller]

@lieut-data I don't quite understand the ask. What exactly are you looking to capture?

[lieut-data]

@crspeller, essentially I'm looking for a low-friction way of forcing the plugin to render all the various messages "for real". Right now, the testing amounts to simulating all the events by hand on a repository I control, but I'm now at the point of doing multi-actor testing (e.g. a reviewer and an author), and I'm hoping not to jump through those hoops. Hooking up a repository like mattermost-server temporarily to my ngrok-exposed GitHub plugin would probably give me the coverage I need.

Ideally, we would capture these webhooks, store them and replay for future testing. Maybe something we can build into the GitHub plugin itself to stream all events to disk.

[crspeller]

Chatted offline with @lieut-data setup the webhook for him and more tests are going to be added before this is merged.

[lieut-data]

@jwilander & @crspeller, I was able to verify a number of other events, but didn't end up getting a full set to provide 100% coverage of these changes with "real data". (The new unit tests cover all the new code, except that the event data is of course faked.)

With 5.12 having been cut, what are your thoughts on merging this now and letting it soak in master over the next two months while we iterate separately on a test framework that would give us the desired coverage here?

[jwilander]

I'm completely fine with that 👍

[crspeller]

Sounds good. Going to hold off merging for a couple days though, because I am probably patching github again for this release.

[jasonblais]

@crspeller / @lieut-data Is this one ready to merge?

[crspeller]

@jasonblais Thanks for the reminder!