Granular webhook events

[jolheiser]

This PR splits out webhook events to give users more control over when to notify via webhooks.

[codecov-io]

Codecov Report

Merging #9626 into master will decrease coverage by 0.04%.
The diff coverage is 27.89%.

@@            Coverage Diff            @@
##           master   #9626      +/-   ##
=========================================
- Coverage   43.65%   43.6%   -0.05%     
=========================================
  Files         587     588       +1     
  Lines       82254   82474     +220     
=========================================
+ Hits        35905   35963      +58     
- Misses      41889   42048     +159     
- Partials     4460    4463       +3

Impacted Files	Coverage Δ
models/migrations/migrations.go	4.16% <ø> (ø)	⬆️
modules/structs/hook.go	6.66% <ø> (ø)	⬆️
modules/auth/repo_form.go	44.44% <ø> (ø)	⬆️
models/migrations/v130.go	0% <0%> (ø)
modules/webhook/discord.go	1.92% <0%> (ø)	⬆️
modules/webhook/dingtalk.go	6.36% <0%> (ø)	⬆️
modules/webhook/telegram.go	3.97% <0%> (ø)	⬆️
routers/api/v1/utils/hook.go	0% <0%> (ø)	⬆️
modules/webhook/msteams.go	0% <0%> (ø)	⬆️
modules/webhook/slack.go	27.31% <0%> (ø)	⬆️
... and 10 more

---

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 80db442...0c37fca. Read the comment docs.

[lunny]

Maybe the bug could be backport to v1.11

[jolheiser]

I will extract the bug and make a new PR for it.
As well, why is this breaking? The migration should update any existing webhooks so that they still fire the equivalent events.

[lunny]

@jolheiser for the CI or any other server will receive the webhook. The types changed, it may affect them. So I think it's a break.

[jolheiser]

Aha, okay that makes sense.

[lafriks]

Maybe also split branch/tag creation/deletion?

[lafriks]

Also it could be better to not make it breaking change by just sending hook type as it is currently so for example: models.HookEventIssueLabel would actually send issues

[jolheiser]

@lafriks
Branch/tag can be done, however the way it's currently set up would need some involved refactoring that might be better in another PR to avoid this growing even more.
Since it would be equally breaking, I'm willing to do it in 1.12 as well so it's just one overall breaking change.

Regarding hook types, they are just string types, so having multiple be the same would not work. The workaround I have found so far is to convert them back into generic types when the final payload is created, but I'm not sure I really like that implementation since it's just another place to check in the future to make sure webhooks don't break.

Or did you have some thoughts on the matter? There's a good chance I'm over thinking it. 😅

[lafriks]

Yes, tag/branch event splitting can be done in other PR, I was just thinking this could be added here if it is easy to add.

Most probably hook types can be changed to enum and than have function that would return hook type string to send based on that enum

[jolheiser]

Yes, tag/branch event splitting can be done in other PR, I was just thinking this could be added here if it is easy to add.

Sounds good, I'll see what the effort looks like after splitting into enums.

Most probably hook types can be changed to enum and than have function that would return hook type string to send based on that enum

That's a great idea. I think that should be doable, will give it a try.

Side-note, this PR can have the kind/bug label removed since the Telegram PR review was moved to a separate PR.

[jolheiser]

I know this has lgtm/done now, but it would be nice to get one more review since a lot has changed since the first approval.

[6543]

the api should be cleanded if v2 ... but for now ok

[lafriks]

please resolve conflicts

[jolheiser]

🙌

[mooror]

Really nice addition @jolheiser, nicely done