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

fix webhook test #2415

Merged
merged 3 commits into from
Aug 29, 2017
Merged

fix webhook test #2415

merged 3 commits into from
Aug 29, 2017

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
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
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