diff --git a/internal/embeds/templates/partials/operator_billing.html b/internal/embeds/templates/partials/operator_billing.html index f49f7ea0..b001d69d 100644 --- a/internal/embeds/templates/partials/operator_billing.html +++ b/internal/embeds/templates/partials/operator_billing.html @@ -15,7 +15,7 @@ already names Billing / Invoices (maintainer 2026-08-31). .ActiveSection drives both the active pill and the page's IAPosition (operator_pages.go). The recency stamp renders only when - provider events exist; a quiet stream says nothing here (maintainer, + Stripe events exist; a quiet stream says nothing here (maintainer, 2026-08-30: the zero-events banner conflated the provider stream with the records below it; stream health belongs to the Stripe integration settings page). Delegates to .InnerTemplate / .InnerData via diff --git a/internal/integrations/discourse/linkage/linkage.go b/internal/integrations/discourse/linkage/linkage.go index a68e5ba6..04674e3d 100644 --- a/internal/integrations/discourse/linkage/linkage.go +++ b/internal/integrations/discourse/linkage/linkage.go @@ -259,7 +259,7 @@ func (l *Linker) confirmCreated(ctx context.Context, ident dcmod.GetPersonIdenti return nil, fmt.Errorf("confirm created forum user: %w", err) } if u == nil { - return nil, fmt.Errorf("forum reported user created for %q but post-create lookup found nothing (duplicate email swallowed by enumeration protection?)", ident.PrimaryEmail) + return nil, fmt.Errorf("forum reported user created for person %s but post-create lookup found nothing (duplicate email swallowed by enumeration protection?)", ident.PersonID) } return u, nil } diff --git a/internal/integrations/discourse/linkage/linkage_test.go b/internal/integrations/discourse/linkage/linkage_test.go index 92d2f4c2..3c00d706 100644 --- a/internal/integrations/discourse/linkage/linkage_test.go +++ b/internal/integrations/discourse/linkage/linkage_test.go @@ -9,6 +9,7 @@ import ( "errors" "log/slog" "os" + "strings" "testing" "git.coopcloud.tech/wiki-cafe/member-console/internal/identity" @@ -196,6 +197,36 @@ func TestAutoCreateOIDC(t *testing.T) { } } +// Scenario: auto-create in oidc mode where the forum swallows the create as +// a duplicate email → the confirmation finds nobody, and its error names the +// person by id, never by email address (the error reaches logs and Temporal). +func TestAutoCreateUnconfirmedErrorNamesPersonByID(t *testing.T) { + f, personID := newFixture(t) + // No external id, and a username other than the seed usernameBase derives + // from the email, so the lookup by subject finds nobody and the create is + // swallowed as a duplicate email. + f.fake.AddUser(discoursetest.FakeUser{ + Username: "someone-else", Email: f.email, Active: true, + }) + + outcome, err := f.linker(linkage.ModeOIDC, true).EnsureLink(f.ctx, f.q, personID) + if err == nil { + t.Fatalf("EnsureLink = (%q, nil), want the confirmation error", outcome) + } + if !strings.Contains(err.Error(), personID) { + t.Errorf("error %q does not contain the person's id %q", err, personID) + } + if !strings.Contains(err.Error(), "post-create lookup found nothing") { + t.Errorf("error %q is not the confirmation path's error", err) + } + if strings.Contains(err.Error(), f.email) { + t.Errorf("error %q contains the person's email address", err) + } + if _, err := f.q.GetUserLinkByPersonID(f.ctx, personID); !errors.Is(err, sql.ErrNoRows) { + t.Errorf("expected no link, got err=%v", err) + } +} + // Scenario: the resolved forum user is already linked to a different person // → existing link marked conflict, no reassignment, this person unlinked. func TestConflictNeverReassigns(t *testing.T) { diff --git a/internal/server/operator_billing.go b/internal/server/operator_billing.go index 7865de2f..71649997 100644 --- a/internal/server/operator_billing.go +++ b/internal/server/operator_billing.go @@ -246,20 +246,27 @@ type BillingAccountsData struct { Nav ListNav } -// latestProcessedWebhookEventAt returns the most recent time any provider's +// rowQuerier is the one-row read both *sql.DB and *sql.Tx provide. +type rowQuerier interface { + QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row +} + +// latestProcessedStripeEventAt returns the most recent time a Stripe // webhook event finished processing (processed_at IS NOT NULL): the four -// operator billing views are projections from these events, so this is the -// recency stamp the honest-surfaces delta requires (design.md Decision 3; -// spec: operator-billing-views) — "data as of ..." when at least one event -// has ever finished processing, ok=false when none has (a fresh deployment, -// or Stripe never configured), so the caller can state that plainly instead -// of rendering an empty or stale-looking table as if it were current. -func (h *OperatorPartialsHandler) latestProcessedWebhookEventAt(ctx context.Context) (time.Time, bool) { +// operator billing views are projections from Stripe's events, so this is +// the recency stamp the honest-surfaces delta requires (design.md Decision 3; +// spec: operator-billing-views) — "data as of ..." when at least one such +// event has ever finished processing, ok=false when none has (a fresh +// deployment, or Stripe never configured), so the caller can state that +// plainly instead of rendering an empty or stale-looking table as if it were +// current. Other providers' events share the table and project nothing into +// the billing views, so they do not move the stamp. +func latestProcessedStripeEventAt(ctx context.Context, db rowQuerier, logger *slog.Logger) (time.Time, bool) { var latest sql.NullTime - if err := h.Database.QueryRowContext(ctx, - `SELECT MAX(processed_at) FROM core.webhook_events WHERE processed_at IS NOT NULL`, + if err := db.QueryRowContext(ctx, + `SELECT MAX(processed_at) FROM core.webhook_events WHERE provider = 'stripe' AND processed_at IS NOT NULL`, ).Scan(&latest); err != nil { - h.Logger.Error("failed to load latest processed webhook event", "error", err) + logger.Error("failed to load latest processed Stripe webhook event", "error", err) return time.Time{}, false } if !latest.Valid { diff --git a/internal/server/operator_billing_render_test.go b/internal/server/operator_billing_render_test.go index 0ffbed47..972c0b5a 100644 --- a/internal/server/operator_billing_render_test.go +++ b/internal/server/operator_billing_render_test.go @@ -232,3 +232,45 @@ func TestOperatorBillingRecencyStamp(t *testing.T) { } }) } + +// TestLatestProcessedStripeEventAt covers the recency stamp's provider scope +// (operator-billing-views, "Billing views disclose projection recency"): a +// processed Discourse event after the last Stripe event does not move the +// stamp, and with no processed Stripe event there is no stamp. The shared +// table may hold committed rows from other tests, so each subtest clears the +// Stripe rows inside its own rolled-back transaction. +func TestLatestProcessedStripeEventAt(t *testing.T) { + ctx := context.Background() + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + database := dbtest.Open(t) + + fixture := func(t *testing.T) (*dbtest.Q, func(provider, processedAt string)) { + t.Helper() + q := dbtest.Tx(t, database) + q.Exec(`DELETE FROM core.webhook_events WHERE provider = 'stripe'`) + return q, func(provider, processedAt string) { + t.Helper() + q.Exec(`INSERT INTO core.webhook_events (provider, provider_event_id, event_type, status, processed_at) + VALUES ($1, $2, 'x', 'completed', $3::timestamptz)`, + provider, dbtest.Name("evt"), processedAt) + } + } + + t.Run("a Discourse event alone gives no stamp", func(t *testing.T) { + q, insert := fixture(t) + insert("discourse", "2026-03-05 10:00:00+00") + if got, ok := latestProcessedStripeEventAt(ctx, q.Tx(), logger); ok { + t.Errorf("only a processed Discourse event: stamp = %s, want none", got) + } + }) + + t.Run("a later Discourse event does not move the Stripe stamp", func(t *testing.T) { + q, insert := fixture(t) + insert("stripe", "2026-03-01 10:00:00+00") + insert("discourse", "2026-03-05 10:00:00+00") + got, ok := latestProcessedStripeEventAt(ctx, q.Tx(), logger) + if want := "2026-03-01 10:00:00"; !ok || got.UTC().Format("2006-01-02 15:04:05") != want { + t.Errorf("stamp = (%s, %v), want the Stripe event's time %s", got, ok, want) + } + }) +} diff --git a/internal/server/operator_pages.go b/internal/server/operator_pages.go index f264ccc4..df2e9ee4 100644 --- a/internal/server/operator_pages.go +++ b/internal/server/operator_pages.go @@ -450,7 +450,7 @@ type OperatorBillingWrapperData struct { // DataAsOf carries the webhook-projection recency stamp (design.md // Decision 3; spec: operator-billing-views) shared by all four billing // sub-pages: renderBillingPage computes it once here. Empty when no - // events have been processed, in which case the wrapper says nothing + // Stripe events have been processed, in which case the wrapper says nothing // (maintainer, 2026-08-30: the zero-events banner conflated the // provider stream with the records below it). DataAsOf string @@ -500,7 +500,7 @@ func (h *OperatorPartialsHandler) renderBillingPage(w http.ResponseWriter, r *ht InnerData: innerData, TestMode: h.StripeMode == "test", } - if processedAt, ok := h.latestProcessedWebhookEventAt(r.Context()); ok { + if processedAt, ok := latestProcessedStripeEventAt(r.Context(), h.Database, h.Logger); ok { wrapper.DataAsOf = processedAt.Format("Jan 2, 2006 15:04 MST") } wrapper.Header = PageHeader{Title: "Billing"} diff --git a/openspec/changes/discourse-events-keep-forum-ids/tasks.md b/openspec/changes/discourse-events-keep-forum-ids/tasks.md index 760d11b6..85a6ba34 100644 --- a/openspec/changes/discourse-events-keep-forum-ids/tasks.md +++ b/openspec/changes/discourse-events-keep-forum-ids/tasks.md @@ -31,14 +31,14 @@ Sections 1 to 3 are one commit each, and their items are that commit's parts; se ## 2. Ride-alongs: the confirmation error names the person by id, and the billing stamp reads Stripe's events -- [ ] 2.1 In `confirmCreated` (`linkage/linkage.go:262`), the error names the person by id: `for person %s` with `ident.PersonID` in place of `for %q` with `ident.PrimaryEmail`; the rest of the text stays. The error at `:241` stays as it is (design D9). -- [ ] 2.2 Test in `linkage/linkage_test.go`, `TestAutoCreateUnconfirmedErrorNamesPersonByID`: `oidc` mode with auto-create on; the fake holds a forum user whose email is the person's and who has no external id, and whose username differs from the seed `usernameBase` derives, so the lookup by subject finds nobody, the fake swallows the create as a duplicate email, and the confirmation finds nothing. `EnsureLink` returns an error that contains the person's id and does not contain the person's email address; no link is recorded. -- [ ] 2.3 Breaks: +- [x] 2.1 In `confirmCreated` (`linkage/linkage.go:262`), the error names the person by id: `for person %s` with `ident.PersonID` in place of `for %q` with `ident.PrimaryEmail`; the rest of the text stays. The error at `:241` stays as it is (design D9). +- [x] 2.2 Test in `linkage/linkage_test.go`, `TestAutoCreateUnconfirmedErrorNamesPersonByID`: `oidc` mode with auto-create on; the fake holds a forum user whose email is the person's and who has no external id, and whose username differs from the seed `usernameBase` derives, so the lookup by subject finds nobody, the fake swallows the create as a duplicate email, and the confirmation finds nothing. `EnsureLink` returns an error that contains the person's id and does not contain the person's email address; no link is recorded. +- [x] 2.3 Breaks: - the error carries the email again (`ident.PrimaryEmail` added beside the id): caught by the no-email assertion; - the error drops the id (`ident.PersonID` replaced by a fixed word): caught by the person-id assertion. -- [ ] 2.4 The billing views' recency stamp (`latestProcessedWebhookEventAt`, `internal/server/operator_billing.go`) reads only Stripe's events: its query gains `provider = 'stripe'`, and its doc comment says that the views are projected from Stripe's events and that other providers' events, which share the table, project nothing into them (spec `operator-billing-views`, "Billing views disclose projection recency"). -- [ ] 2.5 Test in `internal/server`, beside the existing tests of the billing views' recency line: with a processed Stripe event and a later processed Discourse event, the stamp is the Stripe event's processing time; with only a processed Discourse event, no recency line renders. Break, run with section 2's list: the `provider = 'stripe'` condition removed, caught by both cases. -- [ ] 2.6 No golden moves. +- [x] 2.4 The billing views' recency stamp (`latestProcessedWebhookEventAt`, `internal/server/operator_billing.go`) reads only Stripe's events: its query gains `provider = 'stripe'`, and its doc comment says that the views are projected from Stripe's events and that other providers' events, which share the table, project nothing into them (spec `operator-billing-views`, "Billing views disclose projection recency"). +- [x] 2.5 Test in `internal/server`, beside the existing tests of the billing views' recency line: with a processed Stripe event and a later processed Discourse event, the stamp is the Stripe event's processing time; with only a processed Discourse event, no recency line renders. Break, run with section 2's list: the `provider = 'stripe'` condition removed, caught by both cases. +- [x] 2.6 No golden moves. ## 3. The data model's contradiction, recorded for upstream