enhance(scm/repo): mirror allowed events with sent events

[ecrupper]

Closes go-vela/community#539

Now that we store the webhook_id in the hooks table, we're able to edit the webhook whenever a user updates their repo settings. With this functionality, we will consume far less webhook payloads from the SCM and users won't have to deal with error messages for events they do not want enabled.

A thing to note: I added an initialize webhook whenever a repo is enabled. This ensures that when a user adjusts settings right after enabling their repo, we have record of the webhook_id to edit.

Would love to hear thoughts!

[codecov]

Codecov Report

Merging #679 (e6f3fa8) into main (b42e80f) will increase coverage by 0.10%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   56.89%   56.99%   +0.10%     
==========================================
  Files         243      243              
  Lines       15939    15997      +58     
==========================================
+ Hits         9069     9118      +49     
- Misses       6468     6474       +6     
- Partials      402      405       +3     

Impacted Files	Coverage Δ
scm/github/github.go	100.00% <ø> (ø)
scm/github/repo.go	76.36% <85.71%> (+1.36%)	⬆️

[cognifloyd]

Nice! From the issue description, we really need this.

I don't understand what the issue with push vs tag is. If you have time, could you post a few links to where the code needs to be disambiguated? I can also go looking next time I have some time to look.

[ecrupper]

I don't understand what the issue with push vs tag is. If you have time, could you post a few links to where the code needs to be disambiguated? I can also go looking next time I have some time to look.

For sure! The way GitHub is set up, they send tag events as a sub-action of a push (Docs). We distinguish this in the code in scm/webhook.go. So if a user chose to unsubscribe to push events but not tag events, then we still need to handle all push events. I think a cleaner way would be to do some sort of if r.GetAllowPush() || r.GetAllowTag(), which I think I'll change now.

[cognifloyd]

Thanks for the links. That makes more sense.

Maybe update the PR description for future reviewers since that question has been resolved. 😄

[jbrockopp]

LGTM

[wass3r]

in my testing against github.com this does not seem to work. when enabling a repo i get

Status 500 (unable to create initialization webhook for wass3r/vela-test: empty webhook repo_id provided)

we will need to set repo id or otherwise solve for this. the error comes from: https://github.com/go-vela/types/blob/ef95941428d9195503c3db067a9f27447b4dcc16/database/hook.go#L162

[wass3r]

little bit of whack-a-mole.. now getting this error:

Status 500 (unable to create initialization webhook for wass3r/vela-test: empty webhook source_id provided)

😭

[ecrupper]

Sorry about that! All required hook fields have been set now. I also included some extra testing in the scm enable function

[wass3r]

thanks for the fixes. tested on github.com and works great. nice work.

too bad we can't force update existing hooks in github.

[jbrockopp]

LGTM