feat(billing-platform): add ChargeService.list_refunds_by_invoice endpoint (REVENG-157)#305
Merged
armcknight merged 19 commits intoJun 15, 2026
Conversation
2 tasks
|
The latest Buf updates on your PR. Results from workflow ci / buf-checks (pull_request).
|
3 tasks
…oint Adds the shared refund types + the charge-service endpoint driven by the `charge.refunded` webhook flow (REVENG-157): - ``common/v1/stripe_charge.proto``: new ``StripeRefund`` message + ``repeated refunds`` field on ``StripeCharge`` so webhook handlers receive per-refund metadata (id, amount, reason) and can record refunds idempotently — the aggregate ``amount_refunded`` on the charge alone isn't enough. - ``services/charge/v1/charge.proto``: new ``PlatformRefund`` canonical projection, paralleling ``PlatformCharge``. The shared response type across all three refund endpoints, so it ships here in the base PR. - ``services/charge/v1/endpoint_record_charge_refunds.proto``: ``ChargeService.record_charge_refunds`` — records per-refund rows from a Stripe webhook payload idempotently (keyed on refund stripe_id) and syncs the aggregate ``amount_refunded`` / ``refunded`` state. The webhook handler in getsentry calls ``ChargeService.record_charge_refunds`` directly. We considered an ``InvoicerService.handle_charge_refunded`` wrapper (mirroring the ``charge.succeeded`` / ``charge.dispute.created`` pattern) but the wrapper would have been a passthrough -- no multi-service orchestration to coordinate, just a single call. ``charge.updated`` set the precedent for skipping the presentation layer when only one data service is involved (getsentry#20559). The other two refund endpoints (``refund_charge``, ``list_refunds_by_invoice``) are split into separate PRs paired with their getsentry consumers. Rust + Python bindings regenerated by CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
armcknight
force-pushed
the
andrewmcknight/reveng-157-protos-record-charge-refunds
branch
from
fc673f0 to
83023b0
Compare
…e_stripe_id Per review feedback on #303: - ``PlatformRefund.charge_stripe_id`` removed (field 2). Nothing consumes this field today; if a future caller needs to identify the parent charge of a standalone ``PlatformRefund`` (e.g. grouping by charge in a UI), we can add it back as a new field number then. - ``PlatformCharge.refunds`` added (field 10): the recorded refund rows against the charge, ordered by ``date_added_st``. Populated by endpoints that have already loaded the refund rows; left empty by endpoints that only need the aggregate ``amount_refunded`` cache. - ``RecordChargeRefundsResponse.refunds`` removed (field 2). Refunds now live under ``response.charge.refunds`` -- the response only needs the single ``charge`` field. Matches the DB relationship (refunds belong to a specific charge) and avoids duplicating the same data at two levels of the response. Remaining ``PlatformRefund`` field numbers compacted (3..6 -> 2..5) since this message hasn't shipped to any consumer yet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…point Adds the proto contract for listing every recorded refund against the charges for a single platform invoice (REVENG-157). Used by the customer presentation layer to render invoice-level refund state (the receipts list page, the invoice detail page, and the receipt PDF) without crossing the charge service boundary. The endpoint walks back from ``PlatformRefund`` to the invoice via the ``PlatformCharge`` FK, so callers don't need to enumerate the (possibly multiple) charges on an invoice themselves. Builds on the ``PlatformRefund`` message added in the previous PR in this proto stack. Sibling to ``refund_charge`` (independent proto file, no cross-references). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make the unit explicit on the canonical refund projection. Matches the unit-explicit naming used on the underlying Django model (``PlatformRefund.amount_cents``) and avoids consumers having to guess whether the value is dollars, cents, or something else. Field number is unchanged (3), so wire-compatible with any in-flight serialized messages from earlier #303 iterations. The sibling ``PlatformCharge.amount`` / ``StripeRefund.amount`` / ``StripeCharge.amount`` aren't renamed -- their values come from Stripe or from the legacy ``Charge`` model, where the unit-implicit naming is already a settled convention. Following up on those would be a broader audit (#REVENG-???) and out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…arge Per review feedback on getsentry#20611: the cached ``amount_refunded`` / ``refunded`` fields on ``PlatformCharge`` are derivable from the embedded ``refunds`` list -- consumers can sum ``refunds[*].amount_cents`` for the aggregate, and check ``len(refunds) > 0`` (or sum == amount) for "is this refunded." Storing them as separate fields creates a drift risk between the cache and the rows (sentry-protos#303 bugbot). Removing fields 7 (``refunded``) and 8 (``amount_refunded``) from the ``PlatformCharge`` proto. Field numbers reserved so they can't be re-used for an incompatible meaning. The corresponding service-side change (drop the cache write in ``record_charge_refunds``, populate ``refunds`` instead of the aggregate fields) lands in getsentry#20611. The DB columns on ``accounts_platformcharge`` are inherited from ``AbstractCharge`` and remain in place since the legacy ``Charge`` model still uses them; splitting the abstract class to fully drop the columns from ``accounts_platformcharge`` is a separate follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…stead of removing The previous attempt (commit 67ce9cf) removed fields 7 (``refunded``) and 8 (``amount_refunded``) and replaced them with ``reserved 7, 8;`` so the field numbers couldn't be re-used. That tripped buf's ``FIELD_NO_DELETE`` rule, which sentry-protos enables deliberately: PR #168 lifted it once to clean up unused prototype protos, and PR #169 immediately restored it as a permanent guardrail. Restoring the fields with ``[deprecated = true]`` so: - buf is happy (no field deletion). - Consumers get a deprecation marker steering them toward ``refunds[*].amount_cents`` as the source of truth. - No drift risk in practice: the getsentry-side producer (``_charge_to_proto``) is already not populating these fields (getsentry#20611 commit ``96fd005``), and a search confirms no getsentry code reads ``PlatformCharge.refunded`` / ``PlatformCharge.amount_refunded`` -- the proto is internal to the billing platform service. This matches the spirit of Noah's review feedback (don't carry a cache that can drift from the rows) while respecting the repo's field- deletion policy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-record-charge-refunds'
… into andrewmcknight/reveng-157-protos-list-refunds-by-invoice
armcknight
force-pushed
the
andrewmcknight/reveng-157-protos-list-refunds-by-invoice
branch
from
57e53a6 to
3124187
Compare
…s-by-invoice # Conflicts: # rust/src/sentry_protos.billing.v1.services.charge.v1.rs
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…-list-refunds-by-invoice'
…formRefund The ``ListRefundsByInvoiceResponse`` returns a flat list of refunds. For multi-charge invoices (charge retries, partial capture), the caller can't tell which ``PlatformCharge`` a refund came from -- a real concern for admin reconciliation surfaces. Mirroring legacy ``Refund.charge`` (FK) and Stripe's own ``refund.charge`` wire field, add a ``stripe_charge_id`` back-reference on ``PlatformRefund`` so the flat list stays useful. This is purely additive and unblocks #305's response shape without restructuring it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-list-refunds-by-invoice'
armcknight
changed the base branch from
andrewmcknight/reveng-157-protos-record-charge-refunds
to
main
armenzg
approved these changes
Jun 15, 2026
4 tasks
armcknight
deleted the
andrewmcknight/reveng-157-protos-list-refunds-by-invoice
branch
Closed
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
ChargeService.list_refunds_by_invoice, the proto contract for both the customer-facing presentation layer and the admin invoice details page. Walks back fromPlatformRefundto the invoice via thePlatformChargeFK so callers don't enumerate per-invoice charges themselves (one invoice can have multiple charges — retries, partial-capture scenarios).services/charge/v1/endpoint_list_refunds.proto(new) —ListRefundsByInvoiceRequest(invoice_id) andListRefundsByInvoiceResponse(repeated PlatformRefund, ordered bydate_added_stascending).Reuses the
PlatformRefundmessage fromcharge.proto(extended by #303).Proto stack
StripeRefund+PlatformRefundshared types,record_charge_refundsendpoint_list_refunds.proto(for customer + admin refund rendering)Stacked on #303 (for
PlatformRefund).Test plan
sentry-protospin is bumped🤖 Generated with Claude Code