Skip to content

Conversation

lunny
Copy link
Member

@lunny lunny commented Aug 28, 2017

When click the test webhook, only this webhook should be fired but not all the webhooks of the repository.

@lunny lunny added the type/bug label Aug 28, 2017
@lunny lunny added this to the 1.3.0 milestone Aug 28, 2017
@lafriks
Copy link
Member

lafriks commented Aug 28, 2017

LGTM

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 28, 2017
@@ -482,6 +482,57 @@ func UpdateHookTask(t *HookTask) error {
return err
}

// PrepareWebHook adds special webhook to task queue for given payload.
func PrepareWebHook(w *Webhook, repo *Repository, event HookEventType, p api.Payloader) error {
Copy link
Member

Choose a reason for hiding this comment

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

Could we call it PrepareWebhook for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the function named PrepareWebHooks for fire all webhooks of a repository or an orgnization, I named this funtion PrepareWebHook for one webhook is reasonable.

Copy link
Member

Choose a reason for hiding this comment

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

@lunny That function is named PrepareWebhooks, not PrepareWebHooks (https://github.com/go-gitea/gitea/blob/master/models/webhook.go#L486)

@@ -582,7 +590,7 @@ func TestWebhook(ctx *context.Context) {
Pusher: apiUser,
Sender: apiUser,
}
if err := models.PrepareWebhooks(ctx.Repo.Repository, models.HookEventPush, p); err != nil {
if err := models.PrepareWebHook(w, ctx.Repo.Repository, models.HookEventPush, p); err != nil {
ctx.Flash.Error("PrepareWebhooks: " + err.Error())
Copy link
Member

Choose a reason for hiding this comment

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

Need to update error message

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@lunny lunny force-pushed the lunny/fix_webhook_test branch from 4438cef to e4c67e4 Compare August 29, 2017 08:21
@lunny
Copy link
Member Author

lunny commented Aug 29, 2017

@ethantkoenig done and rebased.

@ethantkoenig
Copy link
Member

LGTM

@tboerger tboerger 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 Aug 29, 2017
@lunny lunny merged commit 3e6fed3 into go-gitea:master Aug 29, 2017
@lunny lunny deleted the lunny/fix_webhook_test branch August 29, 2017 14:55
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
project-mirrors-bot-tu bot pushed a commit to project-mirrors/forgejo-as-gitea-fork that referenced this pull request May 14, 2025
Closes go-gitea#2415
Permissions checks are already done by the callee, which also do more correct permission checks.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7835
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Maxim Slipenko <maks1ms@altlinux.org>
Co-committed-by: Maxim Slipenko <maks1ms@altlinux.org>
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. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants