Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible fix the webhook API creation #13960

Merged
merged 3 commits into from
Dec 12, 2020
Merged

Conversation

lunny
Copy link
Member

@lunny lunny commented Dec 12, 2020

Maybe fix #13907

@bobemoe
Copy link
Contributor

bobemoe commented Dec 12, 2020

Sorry to report, still seeing the error:

curl -X POST "http://localhost:3000/api/v1/repos/test/test/hooks?access_token=da0c14c710e2e000fcd80d7d7b4a0b4eba9c485d" -H  "accept: application/json" -H  "Content-Type: application/json" -d "{  \"active\": true,  \"branch_filter\": \"master\",  \"config\": {      \"url\":\"http://test.local\",     \"content_type\":\"json\"  },  \"events\": [    \"push\"  ],  \"type\": \"gitea\"}"

{"message":"Invalid hook type: gitea","url":"http://localhost:3000/api/swagger"}

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 12, 2020
@bobemoe
Copy link
Contributor

bobemoe commented Dec 12, 2020

Is it because "Gitea" is not in the webhooks array:

https://github.com/go-gitea/gitea/blob/master/services/webhook/webhook.go#L28-L59

Previously there was a default case:

https://github.com/go-gitea/gitea/blob/v1.13.0/modules/webhook/webhook.go#L99-L138

@lunny
Copy link
Member Author

lunny commented Dec 12, 2020

@bobemoe done.

@6543 6543 added the type/bug label Dec 12, 2020
@6543 6543 added this to the 1.14.0 milestone Dec 12, 2020
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Dec 12, 2020
@bobemoe
Copy link
Contributor

bobemoe commented Dec 12, 2020

@lunny, seems to be working at fist test using curl :) I will see if Drone is working next...

@codecov-io
Copy link

codecov-io commented Dec 12, 2020

Codecov Report

Merging #13960 (6c98bfe) into master (6074e13) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13960      +/-   ##
==========================================
- Coverage   42.22%   42.08%   -0.15%     
==========================================
  Files         710      710              
  Lines       77228    77230       +2     
==========================================
- Hits        32612    32503     -109     
- Misses      39252    39370     +118     
+ Partials     5364     5357       -7     
Impacted Files Coverage Δ
routers/api/v1/utils/hook.go 0.00% <0.00%> (ø)
services/webhook/webhook.go 52.42% <0.00%> (-1.04%) ⬇️
modules/util/timer.go 42.85% <0.00%> (-42.86%) ⬇️
modules/notification/ui/ui.go 84.72% <0.00%> (-11.12%) ⬇️
models/issue_comment.go 43.50% <0.00%> (-9.22%) ⬇️
modules/queue/unique_queue_channel.go 58.06% <0.00%> (-6.46%) ⬇️
modules/notification/mail/mail.go 33.33% <0.00%> (-5.75%) ⬇️
modules/git/commit.go 49.67% <0.00%> (-3.95%) ⬇️
services/pull/check.go 48.90% <0.00%> (-2.92%) ⬇️
modules/notification/base/null.go 74.28% <0.00%> (-2.86%) ⬇️
... and 7 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 6074e13...6c98bfe. Read the comment docs.

@bobemoe
Copy link
Contributor

bobemoe commented Dec 12, 2020

I can confirm that Drone is now able to use the API to add a webhook to a Gitea repo.

And that from Gitea I am able to successfully test delivery of the Drone webhook :)

@@ -60,12 +60,15 @@ var (

// RegisterWebhook registers a webhook
func RegisterWebhook(name string, webhook *webhook) {
webhooks[models.HookTaskType(name)] = webhook
webhooks[models.HookTaskType(strings.TrimSpace(name))] = webhook
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will keep that since it's harmless.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 12, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 12, 2020
@6543 6543 merged commit 9f100a4 into go-gitea:master Dec 12, 2020
@lunny lunny deleted the lunny/fix_webhook_api branch December 12, 2020 15:43
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Webhooks api stopped working
6 participants