Skip to content

feat(billing-platform): add batched ChargeService.list_charges_for_invoice_ids (REVENG-157)#312

Closed
armcknight wants to merge 2 commits into
mainfrom
andrewmcknight/reveng-157-protos-list-refunds-batch
Closed

feat(billing-platform): add batched ChargeService.list_charges_for_invoice_ids (REVENG-157)#312
armcknight wants to merge 2 commits into
mainfrom
andrewmcknight/reveng-157-protos-list-refunds-batch

Conversation

@armcknight

@armcknight armcknight commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

Adds a batched endpoint that returns PlatformCharge (the canonical projection, with refunds embedded via PlatformCharge.refunds from #303) for every charge tied to any of a list of invoices.

  • services/charge/v1/endpoint_list_charges.proto — new ListChargesForInvoiceIdsRequest (repeated uint64 invoice_ids) + ListChargesForInvoiceIdsResponse (flat list of PlatformCharge ordered by (invoice_id, date_added)).

Why

getsentry#20613 hit a Cursor-bot-flagged N+1 in the customer invoices list — paginated rendering was calling ChargeService.list_refunds_by_invoice once per invoice. Earlier review feedback from Noah (getsentry#20611 thread) pointed at the same pattern: derive aggregates from per-row data, batch where possible.

This PR went with charges-with-embedded-refunds rather than a separate list_refunds_by_invoice_ids because:

  1. It leverages existing proto surface. PlatformCharge.refunds already exists (added in feat(billing-platform): add refund types + record_charge_refunds endpoint (REVENG-157) #303); we just need a batch method that populates and returns it.
  2. It sets up admin surfaces. Admin invoice details (deferred follow-up from getsentry#20613) wants full charge + refund detail in one round trip — this is the right shape.
  3. It still solves the customer-list case. Consumers needing only refund totals sum charge.refunds[*].amount_cents grouped by charge.invoice_id.

The singleton ListChargesForInvoiceRequest / slim Charge type in the same file (dormant, defined in #211 but never implemented) is left as-is — deferring its deprecation to a separate proto-policy follow-up.

Test plan

  • Rust + Python bindings regenerated by CI
  • buf breaking passes (verified locally — additive only)
  • Once released + sentry pin bumped, consumed by getsentry#20613's _PlatformInvoiceSource to eliminate the N+1

🤖 Generated with Claude Code

@armcknight armcknight requested a review from a team as a code owner June 15, 2026 22:47
@linear-code

linear-code Bot commented Jun 15, 2026

Copy link
Copy Markdown

REVENG-157

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow ci / buf-checks (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 16, 2026, 5:17 PM

@armcknight armcknight enabled auto-merge (squash) June 15, 2026 22:51
…voice_ids (REVENG-157)

Adds a batched endpoint that returns ``PlatformCharge`` (the canonical
projection, with refunds embedded via ``PlatformCharge.refunds`` from
sentry-protos#303) for every charge tied to any of a list of invoices.
Avoids the N+1 of fetching refunds (or charges) per invoice when
rendering paginated invoice lists.

- **`services/charge/v1/endpoint_list_charges.proto`** — new
  ``ListChargesForInvoiceIdsRequest`` (``repeated uint64 invoice_ids``)
  + ``ListChargesForInvoiceIdsResponse`` (flat list of
  ``PlatformCharge`` ordered by ``(invoice_id, date_added)``).

The singleton ``ListChargesForInvoiceRequest`` / ``Charge`` slim type
in the same file is left as-is for now (currently unused; deferring
deprecation to a separate proto-policy follow-up).

Picked the charges-with-embedded-refunds shape over a separate
``list_refunds_by_invoice_ids`` because it leverages the existing
``PlatformCharge.refunds`` field and sets up admin invoice surfaces
that want full charge + refund detail in one round trip. Consumers
that only need refund totals (e.g. customer invoice list) still get
what they need by summing ``charge.refunds[*].amount_cents`` grouped
by ``charge.invoice_id``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@armcknight armcknight force-pushed the andrewmcknight/reveng-157-protos-list-refunds-batch branch from 9264e07 to 280e77c Compare June 16, 2026 17:15
@armcknight armcknight changed the title feat(billing-platform): add batched ChargeService.list_refunds_by_invoice_ids (REVENG-157) feat(billing-platform): add batched ChargeService.list_charges_for_invoice_ids (REVENG-157) Jun 16, 2026
@armcknight

Copy link
Copy Markdown
Member Author

Closing — switching the batch shape to a dataclass prototype on the getsentry side (per the billing-platform CLAUDE.md prototyping convention). We'll solidify the wire contract here after the design has been validated in production. The current PlatformCharge.refunds field (from #303) is sufficient for the dataclass implementation.

@armcknight armcknight closed this Jun 16, 2026
auto-merge was automatically disabled June 16, 2026 17:44

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant