From ce717bc7129fa0ece8c736b06622bcfb4608be5b Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Sun, 19 Jul 2026 13:15:33 +0000 Subject: [PATCH] appview: add pull request webhook events This adds five new webhook event types alongside the existing `push` and `repository:renamed` events: - `pull_request:created` - `pull_request:resubmitted` - `pull_request:merged` - `pull_request:closed` - `pull_request:reopened` Each event carries a payload with an `action` field, pull request metadata (number, title, state, target branch, source branch/repo/sha), the repository object, and the sender. The patch itself is not embedded; payloads include a `patch_url` pointing at the existing raw round-patch route instead. Merge, close, and reopen reuse the existing `NewPullState` notifier call sites. `Resubmission` previously emitted no notifier event at all, so this adds a `ResubmitPull` method to the `Notifier` interface, fired from both the regular and stacked resubmit paths (stacked resubmits also now fire `NewPull` for pulls newly added to a stack). Stack-pull abandonment intentionally emits no event. The webhook notifier now takes the appview base URL so pull request `html_url`/`patch_url` point at the appview, where pull routes live (`repository.html_url` keeps pointing at the knot, unchanged). Refs #97, #94. --- docs/DOCS.md | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- appview/models/webhook.go | 40 ++++++++++++++++++++++++++++++++++++++-- appview/notify/merged_notifier.go | 4 ++++ appview/notify/notifier.go | 2 ++ appview/pulls/resubmit.go | 30 ++++++++++++++++++++++++++++++ appview/repo/webhooks.go | 42 ++++++++++++++++++++++++++---------------- appview/state/state.go | 2 +- appview/notify/db/db.go | 4 ++++ appview/notify/logging/notifier.go | 5 +++++ appview/notify/webhook/notifier.go | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ appview/notify/webhook/notifier_test.go | 333 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ appview/pages/templates/repo/settings/hooks.html | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 12 file(s) changed, 660 insertion(s)(+), 31 deletion(s)(-) diff --git a/docs/DOCS.md b/docs/DOCS.md --- a/docs/DOCS.md +++ b/docs/DOCS.md @@ -1957,7 +1957,7 @@ ## Overview -Webhooks send HTTP POST requests to URLs you configure whenever specific events happen. Currently, Tangled supports push events, with more event types coming soon. +Webhooks send HTTP POST requests to URLs you configure whenever specific events happen. Currently, Tangled supports push, repository rename, and pull request events, with more event types coming soon. ## Configuring webhooks @@ -1969,7 +1969,7 @@ 4. Configure your webhook: - **Payload URL**: The endpoint that will receive the webhook POST requests - **Secret**: An optional secret key for verifying webhook authenticity (leave blank to send unsigned webhooks) - - **Events**: Select which events trigger the webhook (currently only push events) + - **Events**: Select which events trigger the webhook - **Active**: Toggle whether the webhook is enabled ## Webhook payload @@ -2005,13 +2005,62 @@ } ``` +### Pull request + +Pull request events are sent as separate event types, so you can subscribe to +exactly the transitions you care about: + +- `pull_request:created` — a pull request was opened +- `pull_request:resubmitted` — a new round (revision) was pushed to a pull request +- `pull_request:merged` — a pull request was merged +- `pull_request:closed` — a pull request was closed +- `pull_request:reopened` — a closed pull request was reopened + +All pull request events share the same payload format: + +```json +{ + "action": "created", + "pull_request": { + "number": 4, + "title": "add dark mode", + "body": "implements dark mode as discussed in #2", + "state": "open", + "target_branch": "main", + "source": { + "branch": "dark-mode", + "sha": "7b320e5cbee2734071e4310c1d9ae401d8f6cab5" + }, + "round_number": 0, + "owner": { + "did": "did:plc:hwevmowznbiukdf6uk5dwrrq" + }, + "html_url": "https://tangled.org/did:plc:hwevmowznbiukdf6uk5dwrrq/some-repo/pulls/4", + "patch_url": "https://tangled.org/did:plc:hwevmowznbiukdf6uk5dwrrq/some-repo/pulls/4/round/0.patch", + "created_at": "2025-09-15T08:57:23Z" + }, + "repository": { ... }, + "sender": { + "did": "did:plc:hwevmowznbiukdf6uk5dwrrq" + } +} +``` + +Notes: + +- `action` mirrors the event type suffix (`created`, `resubmitted`, `merged`, `closed`, `reopened`). +- `repository` has the same format as in the push payload. +- The patch itself is not embedded in the payload (patches can be large); fetch it from `patch_url` instead. `round_number` identifies the latest round, and `patch_url` always points at that round's patch. +- `source` is only present for branch-based and fork-based pull requests; it is omitted for patch-based pulls. For fork-based pulls, `source.repo` contains the DID of the source repository. +- `sender` is the user who performed the action. + ## HTTP headers Each webhook request includes the following headers: - `Content-Type: application/json` -- `User-Agent: Tangled-Hook/` — User agent with short SHA of the commit -- `X-Tangled-Event: push` — The event type +- `User-Agent: Tangled-Hook/` — User agent with short SHA of the commit (push events); `Tangled-Hook/pull_request` for pull request events +- `X-Tangled-Event: push` — The full event type (e.g. `push`, `pull_request:merged`) - `X-Tangled-Hook-ID: ` — The webhook ID - `X-Tangled-Delivery: ` — Unique delivery ID - `X-Tangled-Signature-256: sha256=` — HMAC-SHA256 signature (if secret configured) diff --git a/appview/models/webhook.go b/appview/models/webhook.go --- a/appview/models/webhook.go +++ b/appview/models/webhook.go @@ -10,8 +10,13 @@ type WebhookEvent string const ( - WebhookEventPush WebhookEvent = "push" - WebhookEventRepoRenamed WebhookEvent = "repository:renamed" + WebhookEventPush WebhookEvent = "push" + WebhookEventRepoRenamed WebhookEvent = "repository:renamed" + WebhookEventPullRequestCreated WebhookEvent = "pull_request:created" + WebhookEventPullRequestResubmitted WebhookEvent = "pull_request:resubmitted" + WebhookEventPullRequestMerged WebhookEvent = "pull_request:merged" + WebhookEventPullRequestClosed WebhookEvent = "pull_request:closed" + WebhookEventPullRequestReopened WebhookEvent = "pull_request:reopened" ) type Webhook struct { @@ -80,4 +85,35 @@ NewName string `json:"new_name"` Repository WebhookRepository `json:"repository"` Sender WebhookUser `json:"sender"` +} + +// WebhookPullRequestPayload represents the payload for pull_request:* events +type WebhookPullRequestPayload struct { + Action string `json:"action"` + PullRequest WebhookPullRequest `json:"pull_request"` + Repository WebhookRepository `json:"repository"` + Sender WebhookUser `json:"sender"` +} + +// WebhookPullRequest represents pull request information in webhook payload +type WebhookPullRequest struct { + Number int `json:"number"` + Title string `json:"title"` + Body string `json:"body"` + State string `json:"state"` + TargetBranch string `json:"target_branch"` + Source *WebhookPullRequestSource `json:"source,omitempty"` + RoundNumber int `json:"round_number"` + Owner WebhookUser `json:"owner"` + HtmlUrl string `json:"html_url"` + PatchUrl string `json:"patch_url"` + CreatedAt string `json:"created_at"` +} + +// WebhookPullRequestSource represents the source of a branch- or fork-based +// pull request; absent for patch-based pull requests +type WebhookPullRequestSource struct { + Branch string `json:"branch"` + Repo string `json:"repo,omitempty"` + Sha string `json:"sha,omitempty"` } diff --git a/appview/notify/merged_notifier.go b/appview/notify/merged_notifier.go --- a/appview/notify/merged_notifier.go +++ b/appview/notify/merged_notifier.go @@ -90,6 +90,10 @@ m.fanout(func(n Notifier) { n.NewPull(ctx, pull) }) } +func (m *mergedNotifier) ResubmitPull(ctx context.Context, pull *models.Pull) { + m.fanout(func(n Notifier) { n.ResubmitPull(ctx, pull) }) +} + func (m *mergedNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) { m.fanout(func(n Notifier) { n.NewPullState(ctx, actor, pull) }) } diff --git a/appview/notify/notifier.go b/appview/notify/notifier.go --- a/appview/notify/notifier.go +++ b/appview/notify/notifier.go @@ -26,6 +26,7 @@ DeleteFollow(ctx context.Context, follow *models.Follow) NewPull(ctx context.Context, pull *models.Pull) + ResubmitPull(ctx context.Context, pull *models.Pull) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) NewIssueLabelOp(ctx context.Context, actor syntax.DID, issue *models.Issue, ops []models.LabelOp) @@ -72,6 +73,7 @@ func (m *BaseNotifier) DeleteFollow(ctx context.Context, follow *models.Follow) {} func (m *BaseNotifier) NewPull(ctx context.Context, pull *models.Pull) {} +func (m *BaseNotifier) ResubmitPull(ctx context.Context, pull *models.Pull) {} func (m *BaseNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) {} func (m *BaseNotifier) UpdateProfile(ctx context.Context, profile *models.Profile) {} diff --git a/appview/pulls/resubmit.go b/appview/pulls/resubmit.go --- a/appview/pulls/resubmit.go +++ b/appview/pulls/resubmit.go @@ -345,6 +345,16 @@ return } + pull.Submissions = append(pull.Submissions, &models.PullSubmission{ + PullAt: pullAt, + RoundNumber: newRoundNumber, + Patch: newPatch, + Combined: combinedPatch, + SourceRev: newSourceRev, + Created: time.Now(), + }) + s.notifier.ResubmitPull(r.Context(), pull) + ownerSlashRepo := reporesolver.GetBaseRepoPath(r, repo) s.pages.HxLocation(w, fmt.Sprintf("/%s/pulls/%d", ownerSlashRepo, pull.PullId)) } @@ -471,6 +481,9 @@ // pds updates to make var writes []*comatproto.RepoApplyWrites_Input_Writes_Elem + // pulls to notify for after the transaction commits + var resubmitted []*models.Pull + // deleted pulls are marked as deleted in the DB for _, p := range deletions { // do not do delete already merged PRs @@ -580,6 +593,16 @@ Value: knotcompat.Pull(&record), }, }) + + op.Submissions = append(op.Submissions, &models.PullSubmission{ + PullAt: pullAt, + RoundNumber: newRoundNumber, + Patch: newPatch, + Combined: combinedPatch, + SourceRev: newSourceRev, + Created: time.Now(), + }) + resubmitted = append(resubmitted, op) } _, err = comatproto.RepoApplyWrites(r.Context(), client, &comatproto.RepoApplyWrites_Input{ @@ -597,6 +620,13 @@ l.Error("failed to commit resubmit transaction", "err", err) s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") return + } + + for _, p := range additions { + s.notifier.NewPull(r.Context(), p) + } + for _, p := range resubmitted { + s.notifier.ResubmitPull(r.Context(), p) } ownerSlashRepo := reporesolver.GetBaseRepoPath(r, repo) diff --git a/appview/repo/webhooks.go b/appview/repo/webhooks.go --- a/appview/repo/webhooks.go +++ b/appview/repo/webhooks.go @@ -12,6 +12,30 @@ "tangled.org/core/appview/pages" ) +// webhookEventFields maps event checkbox form fields to webhook events +var webhookEventFields = []struct { + field string + event models.WebhookEvent +}{ + {"event_push", models.WebhookEventPush}, + {"event_repo_renamed", models.WebhookEventRepoRenamed}, + {"event_pull_request_created", models.WebhookEventPullRequestCreated}, + {"event_pull_request_resubmitted", models.WebhookEventPullRequestResubmitted}, + {"event_pull_request_merged", models.WebhookEventPullRequestMerged}, + {"event_pull_request_closed", models.WebhookEventPullRequestClosed}, + {"event_pull_request_reopened", models.WebhookEventPullRequestReopened}, +} + +func webhookEventsFromForm(r *http.Request) []string { + events := []string{} + for _, ef := range webhookEventFields { + if r.FormValue(ef.field) == "on" { + events = append(events, string(ef.event)) + } + } + return events +} + // Webhooks displays the webhooks settings page func (rp *Repo) Webhooks(w http.ResponseWriter, r *http.Request) { l := rp.logger.With("handler", "Webhooks") @@ -79,14 +103,7 @@ active := r.FormValue("active") == "on" - events := []string{} - if r.FormValue("event_push") == "on" { - events = append(events, string(models.WebhookEventPush)) - } - if r.FormValue("event_repo_renamed") == "on" { - events = append(events, string(models.WebhookEventRepoRenamed)) - } - + events := webhookEventsFromForm(r) if len(events) == 0 { rp.pages.Notice(w, "webhooks-error", "At least one event must be enabled") return @@ -172,14 +189,7 @@ webhook.Active = r.FormValue("active") == "on" - events := []string{} - if r.FormValue("event_push") == "on" { - events = append(events, string(models.WebhookEventPush)) - } - if r.FormValue("event_repo_renamed") == "on" { - events = append(events, string(models.WebhookEventRepoRenamed)) - } - + events := webhookEventsFromForm(r) if len(events) > 0 { webhook.Events = events } diff --git a/appview/state/state.go b/appview/state/state.go --- a/appview/state/state.go +++ b/appview/state/state.go @@ -176,7 +176,7 @@ } notifiers = append(notifiers, indexer) - notifiers = append(notifiers, whnotify.NewNotifier(d)) + notifiers = append(notifiers, whnotify.NewNotifier(d, config.Core.BaseUrl())) notifier := notify.NewMergedNotifier(notifiers) notifier = lognotify.NewLoggingNotifier(notifier, tlog.SubLogger(logger, "notify")) diff --git a/appview/notify/db/db.go b/appview/notify/db/db.go --- a/appview/notify/db/db.go +++ b/appview/notify/db/db.go @@ -474,6 +474,10 @@ ) } +func (n *databaseNotifier) ResubmitPull(ctx context.Context, pull *models.Pull) { + // no-op for now +} + func (n *databaseNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) { l := log.FromContext(ctx) diff --git a/appview/notify/logging/notifier.go b/appview/notify/logging/notifier.go --- a/appview/notify/logging/notifier.go +++ b/appview/notify/logging/notifier.go @@ -95,6 +95,11 @@ l.inner.NewPull(ctx, pull) } +func (l *loggingNotifier) ResubmitPull(ctx context.Context, pull *models.Pull) { + ctx = tlog.IntoContext(ctx, tlog.SubLogger(l.logger, "ResubmitPull")) + l.inner.ResubmitPull(ctx, pull) +} + func (l *loggingNotifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) { ctx = tlog.IntoContext(ctx, tlog.SubLogger(l.logger, "NewPullState")) l.inner.NewPullState(ctx, actor, pull) diff --git a/appview/notify/webhook/notifier.go b/appview/notify/webhook/notifier.go --- a/appview/notify/webhook/notifier.go +++ b/appview/notify/webhook/notifier.go @@ -20,19 +20,22 @@ "tangled.org/core/appview/models" "tangled.org/core/appview/notify" "tangled.org/core/log" + "tangled.org/core/orm" ) type Notifier struct { notify.BaseNotifier - db *db.DB - logger *slog.Logger - client *http.Client + db *db.DB + baseUrl string + logger *slog.Logger + client *http.Client } -func NewNotifier(database *db.DB) *Notifier { +func NewNotifier(database *db.DB, baseUrl string) *Notifier { return &Notifier{ - db: database, - logger: log.New("webhook-notifier"), + db: database, + baseUrl: baseUrl, + logger: log.New("webhook-notifier"), client: &http.Client{ Timeout: 30 * time.Second, }, @@ -89,6 +92,109 @@ userAgent := "Tangled-Hook/rename" for _, webhook := range webhooks { go w.sendWebhook(ctx, webhook, string(models.WebhookEventRepoRenamed), payload.Repository.FullName, userAgent, payloadBytes) + } +} + +func (w *Notifier) NewPull(ctx context.Context, pull *models.Pull) { + w.pullRequestEvent(ctx, models.WebhookEventPullRequestCreated, "created", pull.OwnerDid, pull) +} + +func (w *Notifier) ResubmitPull(ctx context.Context, pull *models.Pull) { + w.pullRequestEvent(ctx, models.WebhookEventPullRequestResubmitted, "resubmitted", pull.OwnerDid, pull) +} + +func (w *Notifier) NewPullState(ctx context.Context, actor syntax.DID, pull *models.Pull) { + event, action, ok := pullStateEvent(pull.State) + if !ok { + return + } + w.pullRequestEvent(ctx, event, action, actor.String(), pull) +} + +// pullStateEvent maps a pull's state to the webhook event announcing the +// transition into that state +func pullStateEvent(state models.PullState) (models.WebhookEvent, string, bool) { + switch state { + case models.PullMerged: + return models.WebhookEventPullRequestMerged, "merged", true + case models.PullClosed: + return models.WebhookEventPullRequestClosed, "closed", true + case models.PullOpen: + return models.WebhookEventPullRequestReopened, "reopened", true + default: + return "", "", false + } +} + +func (w *Notifier) pullRequestEvent(ctx context.Context, event models.WebhookEvent, action, sender string, pull *models.Pull) { + // pull request events originate from http handlers, whose context is + // canceled as soon as the handler returns; detach so in-flight + // deliveries are not cut short + ctx = context.WithoutCancel(ctx) + + webhooks, err := w.activeWebhooksForEvent(string(pull.RepoDid), event) + if err != nil { + w.logger.Error("failed to get webhooks for repo", "repo_did", pull.RepoDid, "err", err) + return + } + if len(webhooks) == 0 { + return + } + + repo, err := db.GetRepo(w.db, orm.FilterEq("repo_did", string(pull.RepoDid))) + if err != nil { + w.logger.Error("failed to get repo", "repo_did", pull.RepoDid, "err", err) + return + } + + payload := buildPullRequestPayload(action, repo, pull, sender, w.baseUrl) + payloadBytes, err := json.Marshal(payload) + if err != nil { + w.logger.Error("failed to marshal pull request payload", "repo_did", pull.RepoDid, "err", err) + return + } + + userAgent := "Tangled-Hook/pull_request" + for _, webhook := range webhooks { + go w.sendWebhook(ctx, webhook, string(event), payload.Repository.FullName, userAgent, payloadBytes) + } +} + +func buildPullRequestPayload(action string, repo *models.Repo, pull *models.Pull, sender, baseUrl string) *models.WebhookPullRequestPayload { + htmlUrl := fmt.Sprintf("%s/%s/%s/pulls/%d", baseUrl, repo.Did, repo.Slug(), pull.PullId) + + pullRequest := models.WebhookPullRequest{ + Number: pull.PullId, + Title: pull.Title, + Body: pull.Body, + State: pull.State.String(), + TargetBranch: pull.TargetBranch, + Owner: models.WebhookUser{Did: pull.OwnerDid}, + HtmlUrl: htmlUrl, + CreatedAt: pull.Created.Format(time.RFC3339), + } + if len(pull.Submissions) > 0 { + pullRequest.RoundNumber = pull.LastRoundNumber() + pullRequest.PatchUrl = fmt.Sprintf("%s/round/%d.patch", htmlUrl, pull.LastRoundNumber()) + } + if pull.PullSource != nil { + source := &models.WebhookPullRequestSource{ + Branch: pull.PullSource.Branch, + } + if len(pull.Submissions) > 0 { + source.Sha = pull.LatestSha() + } + if pull.IsForkBased() { + source.Repo = pull.PullSource.RepoDid.String() + } + pullRequest.Source = source + } + + return &models.WebhookPullRequestPayload{ + Action: action, + PullRequest: pullRequest, + Repository: buildWebhookRepository(repo), + Sender: models.WebhookUser{Did: sender}, } } diff --git a/appview/notify/webhook/notifier_test.go b/appview/notify/webhook/notifier_test.go new file mode 100644 --- /dev/null +++ b/appview/notify/webhook/notifier_test.go @@ -0,0 +1,333 @@ +package webhook + +import ( + "context" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + "time" + + "github.com/bluesky-social/indigo/atproto/syntax" + "tangled.org/core/appview/db" + "tangled.org/core/appview/models" +) + +func TestPullStateEvent(t *testing.T) { + tests := []struct { + name string + state models.PullState + wantEvent models.WebhookEvent + wantAction string + wantOk bool + }{ + {"merged", models.PullMerged, models.WebhookEventPullRequestMerged, "merged", true}, + {"closed", models.PullClosed, models.WebhookEventPullRequestClosed, "closed", true}, + {"reopened", models.PullOpen, models.WebhookEventPullRequestReopened, "reopened", true}, + {"abandoned", models.PullAbandoned, "", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + event, action, ok := pullStateEvent(tt.state) + if event != tt.wantEvent || action != tt.wantAction || ok != tt.wantOk { + t.Errorf("pullStateEvent(%v) = (%q, %q, %v), want (%q, %q, %v)", + tt.state, event, action, ok, tt.wantEvent, tt.wantAction, tt.wantOk) + } + }) + } +} + +func TestBuildPullRequestPayload(t *testing.T) { + const baseUrl = "https://tangled.org" + + targetDid := syntax.DID("did:plc:target") + forkDid := syntax.DID("did:plc:fork") + + repo := &models.Repo{ + Did: "did:plc:target", + Name: "some-repo", + Knot: "knot.example.com", + Rkey: "some-repo", + Created: time.Date(2025, 9, 15, 8, 57, 23, 0, time.UTC), + } + + basePull := func() models.Pull { + return models.Pull{ + PullId: 4, + RepoDid: targetDid, + OwnerDid: "did:plc:author", + Title: "add dark mode", + Body: "implements dark mode", + TargetBranch: "main", + State: models.PullOpen, + Created: time.Date(2025, 9, 16, 10, 0, 0, 0, time.UTC), + Submissions: []*models.PullSubmission{ + {RoundNumber: 0, SourceRev: "aaaa000"}, + {RoundNumber: 1, SourceRev: "bbbb111"}, + }, + } + } + + t.Run("patch based", func(t *testing.T) { + pull := basePull() + pull.PullSource = nil + + payload := buildPullRequestPayload("created", repo, &pull, "did:plc:author", baseUrl) + + if payload.Action != "created" { + t.Errorf("action = %q, want %q", payload.Action, "created") + } + pr := payload.PullRequest + if pr.Number != 4 { + t.Errorf("number = %d, want 4", pr.Number) + } + if pr.State != "open" { + t.Errorf("state = %q, want %q", pr.State, "open") + } + if pr.Source != nil { + t.Errorf("source = %+v, want nil for patch-based pull", pr.Source) + } + if pr.RoundNumber != 1 { + t.Errorf("round_number = %d, want 1", pr.RoundNumber) + } + wantHtmlUrl := "https://tangled.org/did:plc:target/some-repo/pulls/4" + if pr.HtmlUrl != wantHtmlUrl { + t.Errorf("html_url = %q, want %q", pr.HtmlUrl, wantHtmlUrl) + } + wantPatchUrl := wantHtmlUrl + "/round/1.patch" + if pr.PatchUrl != wantPatchUrl { + t.Errorf("patch_url = %q, want %q", pr.PatchUrl, wantPatchUrl) + } + if pr.Owner.Did != "did:plc:author" { + t.Errorf("owner.did = %q, want %q", pr.Owner.Did, "did:plc:author") + } + if payload.Sender.Did != "did:plc:author" { + t.Errorf("sender.did = %q, want %q", payload.Sender.Did, "did:plc:author") + } + if payload.Repository.FullName != "did:plc:target/some-repo" { + t.Errorf("repository.full_name = %q, want %q", payload.Repository.FullName, "did:plc:target/some-repo") + } + }) + + t.Run("branch based", func(t *testing.T) { + pull := basePull() + pull.PullSource = &models.PullSource{ + Branch: "dark-mode", + RepoDid: &targetDid, + } + + payload := buildPullRequestPayload("merged", repo, &pull, "did:plc:merger", baseUrl) + + pr := payload.PullRequest + if pr.Source == nil { + t.Fatal("source = nil, want non-nil for branch-based pull") + } + if pr.Source.Branch != "dark-mode" { + t.Errorf("source.branch = %q, want %q", pr.Source.Branch, "dark-mode") + } + if pr.Source.Repo != "" { + t.Errorf("source.repo = %q, want empty for branch-based pull", pr.Source.Repo) + } + if pr.Source.Sha != "bbbb111" { + t.Errorf("source.sha = %q, want %q", pr.Source.Sha, "bbbb111") + } + if payload.Sender.Did != "did:plc:merger" { + t.Errorf("sender.did = %q, want %q", payload.Sender.Did, "did:plc:merger") + } + }) + + t.Run("fork based", func(t *testing.T) { + pull := basePull() + pull.PullSource = &models.PullSource{ + Branch: "dark-mode", + RepoDid: &forkDid, + } + + payload := buildPullRequestPayload("created", repo, &pull, "did:plc:author", baseUrl) + + pr := payload.PullRequest + if pr.Source == nil { + t.Fatal("source = nil, want non-nil for fork-based pull") + } + if pr.Source.Repo != "did:plc:fork" { + t.Errorf("source.repo = %q, want %q", pr.Source.Repo, "did:plc:fork") + } + }) + + t.Run("no submissions", func(t *testing.T) { + pull := basePull() + pull.Submissions = nil + + payload := buildPullRequestPayload("created", repo, &pull, "did:plc:author", baseUrl) + + pr := payload.PullRequest + if pr.RoundNumber != 0 { + t.Errorf("round_number = %d, want 0", pr.RoundNumber) + } + if pr.PatchUrl != "" { + t.Errorf("patch_url = %q, want empty when there are no submissions", pr.PatchUrl) + } + }) +} + +type notifierTestEnv struct { + notifier *Notifier + webhook *models.Webhook + db *db.DB + received chan string +} + +// newNotifierTestEnv sets up a real sqlite db with a repo and a webhook +// subscribed to the given events, delivering to a local test server +func newNotifierTestEnv(t *testing.T, events []string) *notifierTestEnv { + t.Helper() + + d, err := db.Make(context.Background(), filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("Make: %v", err) + } + t.Cleanup(func() { d.Close() }) + + tx, err := d.Begin() + if err != nil { + t.Fatalf("Begin: %v", err) + } + if err := db.AddRepo(tx, &models.Repo{ + Did: "did:plc:owner", + Name: "some-repo", + Knot: "knot.example.com", + Rkey: "some-repo", + RepoDid: "did:plc:repo1", + }); err != nil { + t.Fatalf("AddRepo: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("Commit: %v", err) + } + + received := make(chan string, 1) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + received <- r.Header.Get("X-Tangled-Event") + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + webhook := &models.Webhook{ + RepoDid: syntax.DID("did:plc:repo1"), + Url: srv.URL, + Active: true, + Events: events, + } + if err := db.AddWebhook(d, webhook); err != nil { + t.Fatalf("AddWebhook: %v", err) + } + + return ¬ifierTestEnv{ + notifier: NewNotifier(d, "https://tangled.org"), + webhook: webhook, + db: d, + received: received, + } +} + +func (env *notifierTestEnv) awaitDelivery(t *testing.T, wantEvent string) { + t.Helper() + + select { + case event := <-env.received: + if event != wantEvent { + t.Errorf("X-Tangled-Event = %q, want %q", event, wantEvent) + } + case <-time.After(10 * time.Second): + t.Fatalf("webhook %s not delivered", wantEvent) + } + + // wait for the delivery record so the sender goroutine finishes + // before the db is closed + deadline := time.Now().Add(10 * time.Second) + for { + deliveries, err := db.GetWebhookDeliveries(env.db, env.webhook.Id, 10) + if err == nil && len(deliveries) > 0 { + if !deliveries[0].Success { + t.Errorf("delivery recorded as failed, want success") + } + return + } + if time.Now().After(deadline) { + t.Fatal("delivery record not written") + } + time.Sleep(10 * time.Millisecond) + } +} + +func testPull(state models.PullState) *models.Pull { + return &models.Pull{ + PullId: 1, + RepoDid: syntax.DID("did:plc:repo1"), + OwnerDid: "did:plc:author", + Title: "hello", + TargetBranch: "main", + State: state, + Created: time.Now(), + } +} + +// Pull request events fire from http handlers, whose request context is +// canceled as soon as the handler returns. Deliveries run in background +// goroutines and must not be cut short by that cancellation. +func TestPullRequestEventDeliversAfterContextCancel(t *testing.T) { + env := newNotifierTestEnv(t, []string{string(models.WebhookEventPullRequestCreated)}) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + env.notifier.NewPull(ctx, testPull(models.PullOpen)) + env.awaitDelivery(t, "pull_request:created") +} + +func TestNotifierDeliversPullRequestEvents(t *testing.T) { + allEvents := []string{ + string(models.WebhookEventPullRequestCreated), + string(models.WebhookEventPullRequestResubmitted), + string(models.WebhookEventPullRequestMerged), + string(models.WebhookEventPullRequestClosed), + string(models.WebhookEventPullRequestReopened), + } + actor := syntax.DID("did:plc:actor") + + tests := []struct { + name string + notify func(*Notifier, context.Context) + wantEvent string + }{ + { + "resubmitted", + func(n *Notifier, ctx context.Context) { n.ResubmitPull(ctx, testPull(models.PullOpen)) }, + "pull_request:resubmitted", + }, + { + "merged", + func(n *Notifier, ctx context.Context) { n.NewPullState(ctx, actor, testPull(models.PullMerged)) }, + "pull_request:merged", + }, + { + "closed", + func(n *Notifier, ctx context.Context) { n.NewPullState(ctx, actor, testPull(models.PullClosed)) }, + "pull_request:closed", + }, + { + "reopened", + func(n *Notifier, ctx context.Context) { n.NewPullState(ctx, actor, testPull(models.PullOpen)) }, + "pull_request:reopened", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := newNotifierTestEnv(t, allEvents) + tt.notify(env.notifier, context.Background()) + env.awaitDelivery(t, tt.wantEvent) + }) + } +} diff --git a/appview/pages/templates/repo/settings/hooks.html b/appview/pages/templates/repo/settings/hooks.html --- a/appview/pages/templates/repo/settings/hooks.html +++ b/appview/pages/templates/repo/settings/hooks.html @@ -229,8 +229,28 @@ Repository renamed +
+ + Pull request opened +
+
+ + Pull request resubmitted +
+
+ + Pull request merged +
+
+ + Pull request closed +
+
+ + Pull request reopened +

- Additional event types (pull requests, issues) will be available in future updates. + Additional event types (issues) will be available in future updates.

@@ -299,9 +319,19 @@
{{ $hasPush := false }} {{ $hasRepoRenamed := false }} + {{ $hasPullCreated := false }} + {{ $hasPullResubmitted := false }} + {{ $hasPullMerged := false }} + {{ $hasPullClosed := false }} + {{ $hasPullReopened := false }} {{ range $webhook.Events }} {{ if eq . "push" }}{{ $hasPush = true }}{{ end }} {{ if eq . "repository:renamed" }}{{ $hasRepoRenamed = true }}{{ end }} + {{ if eq . "pull_request:created" }}{{ $hasPullCreated = true }}{{ end }} + {{ if eq . "pull_request:resubmitted" }}{{ $hasPullResubmitted = true }}{{ end }} + {{ if eq . "pull_request:merged" }}{{ $hasPullMerged = true }}{{ end }} + {{ if eq . "pull_request:closed" }}{{ $hasPullClosed = true }}{{ end }} + {{ if eq . "pull_request:reopened" }}{{ $hasPullReopened = true }}{{ end }} {{ end }}
@@ -311,8 +341,28 @@ Repository renamed
+
+ + Pull request opened +
+
+ + Pull request resubmitted +
+
+ + Pull request merged +
+
+ + Pull request closed +
+
+ + Pull request reopened +

- Additional event types (pull requests, issues) will be available in future updates. + Additional event types (issues) will be available in future updates.

-- tangled.sh