Conversation
✅ Deploy Preview for hoppdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis pull request adds a billing email management system for invoices. It introduces API endpoints (GET/PUT) for billing settings, stores billing email addresses on the Team model, sends invoice payment emails to configured billing addresses, generates an HTML invoice email template, and adds UI for managing billing email in the subscription settings page. Changes
Sequence DiagramsequenceDiagram
participant Stripe as Stripe<br/>(External)
participant Handler as BillingHandler
participant DB as Database
participant Email as EmailClient
Stripe->>Handler: POST /webhook<br/>invoice.payment_succeeded
Handler->>Handler: Validate event signature
Handler->>Handler: Extract customer ID
Handler->>DB: Query Subscription<br/>by customer
DB-->>Handler: Subscription with Team
Handler->>DB: Get Team details<br/>(with BillingEmail)
DB-->>Handler: Team data
alt BillingEmail configured
Handler->>Email: SendInvoiceEmail()<br/>(toEmail, data)
Email->>Email: Load template
Email->>Email: Replace placeholders
Email->>Stripe: Send async
Stripe-->>Email: Queued
else BillingEmail not set
Handler->>Handler: Skip email
end
Handler-->>Stripe: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/api-files/openapi.yaml`:
- Around line 159-175: Update the OpenAPI schemas so the API permits disabling
invoice emails via an empty value: add nullable: true to
BillingSettingsRequest.properties.billing_email and to
BillingSettingsResponse.properties.billing_email and update their descriptions
to state that an empty string (or null) disables invoice emails; keep the
BillingSettingsResponse.required list if you want the key present but allow its
value to be empty/null. This ensures the schema matches the backend
validate:"omitempty,email" behavior and the UI that sends "".
In `@backend/internal/handlers/billing.go`:
- Around line 542-549: The log statement currently emits PII by printing the
full billing email; update the logging in the invoice send block (around
bh.EmailClient, SendInvoiceEmail, team.BillingEmail and c.Logger().Infof) to
avoid logging the raw email — either redact the email (e.g., mask local part) or
remove it entirely and instead log non-PII identifiers such as team.ID and
invoice.Number (e.g., "Sent invoice email for %s to team %s"). Ensure the
SendInvoiceEmail call remains unchanged but replace the c.Logger().Infof call to
only include invoice.Number and team.ID or a redacted email string.
- Around line 504-549: Add idempotency to handleInvoicePaymentSucceeded by
recording processed Stripe event IDs in the DB and skipping duplicates: create a
simple model/table (e.g., ProcessedWebhook with EventID string unique) and in
handleInvoicePaymentSucceeded check bh.DB for the event.ID at the start; if
found return nil to acknowledge the webhook; otherwise insert a new
ProcessedWebhook row in the same transactional flow (or attempt insert and treat
a unique-constraint error as “already processed”) before calling
bh.EmailClient.SendInvoiceEmail; ensure the check/insert is done atomically or
handle unique-constraint errors to avoid race conditions so duplicate stripe
event retries do not send duplicate emails.
In `@web-app/src/pages/Subscription.tsx`:
- Around line 141-172: The invalidateQueries call in handleSaveBillingEmail and
handleDeleteBillingEmail is using ["get", "/api/auth/billing/settings"] which
doesn't match the query's custom queryHash ('billing-settings-${authToken}'), so
cache invalidation silently fails; update the invalidation to use the same
queryHash (e.g., queryClient.invalidateQueries({ queryKey:
['billing-settings-${authToken}'] })) or call the refetch callback returned by
the billing settings query hook instead; change the invalidateQueries calls in
handleSaveBillingEmail and handleDeleteBillingEmail (and keep
updateBillingSettingsMutation usage) to target the matching queryHash or use the
existing refetch function like other components (Teammates.tsx).
🧹 Nitpick comments (1)
web-app/src/pages/Subscription.tsx (1)
338-385: Disable the input while saving to avoid mid-save edits.This prevents users from editing during an in-flight save/delete and makes the UI feel more deterministic.
Proposed tweak
- disabled={billingSettingsLoading} + disabled={billingSettingsLoading || billingEmailSaving}
| BillingSettingsRequest: | ||
| type: object | ||
| properties: | ||
| billing_email: | ||
| type: string | ||
| format: email | ||
| description: Email address where invoices will be sent (leave empty to disable invoice emails) | ||
|
|
||
| BillingSettingsResponse: | ||
| type: object | ||
| required: | ||
| - billing_email | ||
| properties: | ||
| billing_email: | ||
| type: string | ||
| description: Email address where invoices are sent (empty string if not configured) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the exact content of the openapi.yaml file at lines 159-175
sed -n '159,175p' backend/api-files/openapi.yamlRepository: gethopp/hopp
Length of output: 543
🏁 Script executed:
# Search for all references to billing_email to understand how it's handled
rg -n -C5 'billing_email'Repository: gethopp/hopp
Length of output: 10942
The OpenAPI schema doesn't accurately reflect that empty strings are valid.
The BillingSettingsRequest schema specifies format: email, which forbids empty strings per OpenAPI spec. However, the backend uses validate:"omitempty,email" (which allows empty) and the UI sends "" to disable invoice emails. Either add nullable: true to the schema to clarify empty is permitted, or document that the field can be omitted entirely instead of sent as empty string.
🤖 Prompt for AI Agents
In `@backend/api-files/openapi.yaml` around lines 159 - 175, Update the OpenAPI
schemas so the API permits disabling invoice emails via an empty value: add
nullable: true to BillingSettingsRequest.properties.billing_email and to
BillingSettingsResponse.properties.billing_email and update their descriptions
to state that an empty string (or null) disables invoice emails; keep the
BillingSettingsResponse.required list if you want the key present but allow its
value to be empty/null. This ensures the schema matches the backend
validate:"omitempty,email" behavior and the UI that sends "".
| // handleInvoicePaymentSucceeded handles the invoice.payment_succeeded webhook event | ||
| // It sends an invoice email to the team's billing email if configured | ||
| func (bh *BillingHandler) handleInvoicePaymentSucceeded(c echo.Context, event stripe.Event) error { | ||
| var invoice stripe.Invoice | ||
| if err := json.Unmarshal(event.Data.Raw, &invoice); err != nil { | ||
| c.Logger().Errorf("Failed to unmarshal invoice: %v", err) | ||
| return err | ||
| } | ||
|
|
||
| // Skip if invoice has no customer | ||
| if invoice.Customer == nil || invoice.Customer.ID == "" { | ||
| c.Logger().Info("Invoice has no customer, skipping invoice email") | ||
| return nil | ||
| } | ||
|
|
||
| // Look up subscription by Stripe customer ID | ||
| var subscription models.Subscription | ||
| if err := bh.DB.Preload("Team").Where("stripe_customer_id = ?", invoice.Customer.ID).First(&subscription).Error; err != nil { | ||
| c.Logger().Infof("Could not find subscription for customer %s: %v", invoice.Customer.ID, err) | ||
| // Return nil to acknowledge the webhook as subscription might not exist yet | ||
| return nil | ||
| } | ||
|
|
||
| team := subscription.Team | ||
|
|
||
| // Only send if team has billing | ||
| // email set else skip | ||
| if team.BillingEmail == nil || *team.BillingEmail == "" { | ||
| c.Logger().Infof("Team %d has no billing email set, skipping invoice email", team.ID) | ||
| return nil | ||
| } | ||
|
|
||
| // Format period from timestamps (period_start, period_end) | ||
| periodStart := time.Unix(invoice.PeriodStart, 0) | ||
| periodEnd := time.Unix(invoice.PeriodEnd, 0) | ||
| period := fmt.Sprintf("%s - %s", periodStart.Format("Jan 2"), periodEnd.Format("Jan 2, 2006")) | ||
|
|
||
| // Send invoice email | ||
| if bh.EmailClient != nil { | ||
| bh.EmailClient.SendInvoiceEmail(*team.BillingEmail, email.InvoiceEmailData{ | ||
| InvoiceNumber: invoice.Number, | ||
| Period: period, | ||
| HostedInvoiceURL: invoice.HostedInvoiceURL, | ||
| InvoicePDFURL: invoice.InvoicePDF, | ||
| }) | ||
| c.Logger().Infof("Sent invoice email for %s to %s", invoice.Number, *team.BillingEmail) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for Stripe webhook event handling and any idempotency mechanisms
rg -n "stripe.*[Ee]vent|event_id|invoice_id|webhook.*dedup|idempotent|processed.*event|stripe\\.Event" backend --type go -A 3 -B 3Repository: gethopp/hopp
Length of output: 2908
🏁 Script executed:
# Check the billing handler file for webhook processing context
fd "billing" backend --type go | head -20Repository: gethopp/hopp
Length of output: 225
🏁 Script executed:
# Search for webhook route handling
rg -n "webhook|Webhook" backend --type go -A 2 | head -50Repository: gethopp/hopp
Length of output: 2952
🏁 Script executed:
# Check for any database tracking of processed events
rg -n "processed_event|event_tracking|webhook_event" backend --type goRepository: gethopp/hopp
Length of output: 38
🌐 Web query:
Stripe webhook delivery guarantee at-least-once retry policy invoice.payment_succeeded
💡 Result:
Stripe webhooks (including invoice.payment_succeeded) are at-least-once delivery: your endpoint can receive the same event more than once, so you must make processing idempotent (for example, store and de-dupe by event.id). [1]
If Stripe doesn’t get a successful 2xx response (including timeouts), it will automatically retry delivery:
- Live mode: retries for up to 3 days using exponential backoff. [1]
- Sandbox/test: retries 3 times over a few hours. [1]
Stripe also doesn’t guarantee event ordering, so don’t assume invoice.payment_succeeded arrives in sequence relative to other invoice/subscription events. [1]
Practical takeaway: treat invoice.payment_succeeded as “delivered at least once, possibly duplicated, possibly out of order,” and always reconcile against the API object state if needed. [1]
Sources:
[1] Stripe Docs — Webhooks: Event delivery behaviors (automatic retries, ordering) + best practice “Handle duplicate events” https://docs.stripe.com/webhooks
Implement idempotency to prevent duplicate invoice emails on webhook retries.
Stripe webhooks guarantee at-least-once delivery with automatic retries for up to 3 days. Without deduplication, duplicate invoice.payment_succeeded events will trigger duplicate invoice emails. Add idempotency by either: (1) tracking processed event.ID with a unique constraint in the database, or (2) recording last_invoice_emailed_at per invoice and skipping if already sent.
🤖 Prompt for AI Agents
In `@backend/internal/handlers/billing.go` around lines 504 - 549, Add idempotency
to handleInvoicePaymentSucceeded by recording processed Stripe event IDs in the
DB and skipping duplicates: create a simple model/table (e.g., ProcessedWebhook
with EventID string unique) and in handleInvoicePaymentSucceeded check bh.DB for
the event.ID at the start; if found return nil to acknowledge the webhook;
otherwise insert a new ProcessedWebhook row in the same transactional flow (or
attempt insert and treat a unique-constraint error as “already processed”)
before calling bh.EmailClient.SendInvoiceEmail; ensure the check/insert is done
atomically or handle unique-constraint errors to avoid race conditions so
duplicate stripe event retries do not send duplicate emails.
| if bh.EmailClient != nil { | ||
| bh.EmailClient.SendInvoiceEmail(*team.BillingEmail, email.InvoiceEmailData{ | ||
| InvoiceNumber: invoice.Number, | ||
| Period: period, | ||
| HostedInvoiceURL: invoice.HostedInvoiceURL, | ||
| InvoicePDFURL: invoice.InvoicePDF, | ||
| }) | ||
| c.Logger().Infof("Sent invoice email for %s to %s", invoice.Number, *team.BillingEmail) |
There was a problem hiding this comment.
Avoid logging full billing email addresses (PII).
Logging full emails can violate privacy/compliance expectations. Prefer redaction or log team ID + invoice number.
Suggested change
- c.Logger().Infof("Sent invoice email for %s to %s", invoice.Number, *team.BillingEmail)
+ c.Logger().Infof("Sent invoice email for %s to team %d", invoice.Number, team.ID)🤖 Prompt for AI Agents
In `@backend/internal/handlers/billing.go` around lines 542 - 549, The log
statement currently emits PII by printing the full billing email; update the
logging in the invoice send block (around bh.EmailClient, SendInvoiceEmail,
team.BillingEmail and c.Logger().Infof) to avoid logging the raw email — either
redact the email (e.g., mask local part) or remove it entirely and instead log
non-PII identifiers such as team.ID and invoice.Number (e.g., "Sent invoice
email for %s to team %s"). Ensure the SendInvoiceEmail call remains unchanged
but replace the c.Logger().Infof call to only include invoice.Number and team.ID
or a redacted email string.
| const handleSaveBillingEmail = async () => { | ||
| setBillingEmailSaving(true); | ||
| try { | ||
| await updateBillingSettingsMutation.mutateAsync({ | ||
| body: { | ||
| billing_email: billingEmail, | ||
| }, | ||
| }); | ||
| // Invalidate the billing settings query to refetch | ||
| queryClient.invalidateQueries({ queryKey: ["get", "/api/auth/billing/settings"] }); | ||
| toast.success("Billing email saved"); | ||
| } catch (error: unknown) { | ||
| console.error("Error saving billing email:", error); | ||
| const errorMessage = error instanceof Error ? error.message : "Failed to save billing email"; | ||
| toast.error(errorMessage); | ||
| } finally { | ||
| setBillingEmailSaving(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleDeleteBillingEmail = async () => { | ||
| setBillingEmailSaving(true); | ||
| try { | ||
| await updateBillingSettingsMutation.mutateAsync({ | ||
| body: { | ||
| billing_email: "", | ||
| }, | ||
| }); | ||
| setBillingEmail(""); | ||
| // Invalidate the billing settings query to refetch | ||
| queryClient.invalidateQueries({ queryKey: ["get", "/api/auth/billing/settings"] }); | ||
| toast.success("Billing email removed"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find useAPI implementation and related hook files
fd -e ts -e tsx 'useAPI|useQuery' web-app/src/hooksRepository: gethopp/hopp
Length of output: 93
🏁 Script executed:
# Check the actual Subscription.tsx file to see full context
sed -n '130,180p' web-app/src/pages/Subscription.tsxRepository: gethopp/hopp
Length of output: 1814
🏁 Script executed:
# Search for useAPI hook implementation and key construction
rg -n 'useAPI|queryKey.*billing|queryHash' web-app/src/hooks -A 5 -B 2Repository: gethopp/hopp
Length of output: 617
🏁 Script executed:
# Search for billing settings query/hook usage
rg -n 'billing.*settings|getBillingSettings|useBillingSettings' web-app/src -A 5 -B 2Repository: gethopp/hopp
Length of output: 6075
🏁 Script executed:
# Look at the full useQueryClients.tsx to understand query key construction
cat -n web-app/src/hooks/useQueryClients.tsxRepository: gethopp/hopp
Length of output: 3092
🏁 Script executed:
# Search for where queryKey is constructed, especially with "get" prefix
rg -n 'queryKey.*\["get"' web-app/src -A 2 -B 2Repository: gethopp/hopp
Length of output: 933
🏁 Script executed:
# Check how queryHash is used in openapi-react-query and if there are other invalidation patterns in the codebase
rg -n 'queryHash|invalidateQueries' web-app/src -B 3 -A 3Repository: gethopp/hopp
Length of output: 6565
🏁 Script executed:
# Look for package.json to understand openapi-react-query version/behavior
cat web-app/package.json | grep -A 2 -B 2 'openapi'Repository: gethopp/hopp
Length of output: 38
🏁 Script executed:
# Find and display package.json
fd -name package.json web-app -x catRepository: gethopp/hopp
Length of output: 286
🏁 Script executed:
# Search for how other mutations handle invalidateQueries to find the pattern
rg -n 'useMutation.*invalidateQueries|invalidateQueries.*queryHash' web-app/src -B 5 -A 5Repository: gethopp/hopp
Length of output: 38
🏁 Script executed:
# Find package.json properly
find web-app -name package.json -exec cat {} \;Repository: gethopp/hopp
Length of output: 2154
🏁 Script executed:
# Search for other mutation examples in the codebase
rg 'useMutation' web-app/src/pages -A 10 | head -100Repository: gethopp/hopp
Length of output: 6713
🏁 Script executed:
# Look for how other queries with queryHash handle invalidation
rg 'invalidateQueries' web-app/src -B 5 -A 2Repository: gethopp/hopp
Length of output: 2404
Fix cache invalidation key to match the query's queryHash.
The billing settings query uses a custom queryHash: 'billing-settings-${authToken}' (line 68), but invalidateQueries targets ["get", "/api/auth/billing/settings"]. These keys don't match—openapi-react-query caches under the queryHash value, so the invalidation silently fails and stale data persists. Either pass the matching queryHash to invalidateQueries or use the refetch callback from the query hook, as done in other components like Teammates.tsx.
🤖 Prompt for AI Agents
In `@web-app/src/pages/Subscription.tsx` around lines 141 - 172, The
invalidateQueries call in handleSaveBillingEmail and handleDeleteBillingEmail is
using ["get", "/api/auth/billing/settings"] which doesn't match the query's
custom queryHash ('billing-settings-${authToken}'), so cache invalidation
silently fails; update the invalidation to use the same queryHash (e.g.,
queryClient.invalidateQueries({ queryKey: ['billing-settings-${authToken}'] }))
or call the refetch callback returned by the billing settings query hook
instead; change the invalidateQueries calls in handleSaveBillingEmail and
handleDeleteBillingEmail (and keep updateBillingSettingsMutation usage) to
target the matching queryHash or use the existing refetch function like other
components (Teammates.tsx).
1 participant
Closes #252
This PR adds explicit functionality to send invoice email. It relies on Stripe's
invoice.payment_succeededevent.How email looks like:
How UI looks like:
To test, first ensure you have a in Stripe Sandbox a current subscription. Then you can use the following to start forwarding webhook calls locally to the server:
Create a webhook event:
Summary by CodeRabbit