ref(billing): PreviewNextInvoiceRequest gains cancel + partner hints#367
ref(billing): PreviewNextInvoiceRequest gains cancel + partner hints#367armcknight wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow ci / buf-checks (pull_request).
|
brendanhsentry
left a comment
There was a problem hiding this comment.
I don't think we need this.
Isn't the better long term solution to find a way to represent these concepts in the Contract? Even after we add these parameters to the request, we'd need to the derive their values from the legacy Subscription model right?
|
@brendanhsentry It does sound like that's the long term plan, but I deemed that out of scope for the current work. We could do this now to keep it working, and move the rest over afterwards, right? Even if we derive these values from Contract, wouldn't it still be able to be transmitted as part of this message? |
I don't think so. If a contract should be canceled at the end of the period, then we should (depends on change contract implementation) have a PlatformPendingChange record that will generate the correct subscription invoice items. Similarly for self serve partners, the tax calculation function should know if a contract is eligible to be taxes. But yeah I guess it's ok to pass for now and leave a TODO. |
noahsmartin
left a comment
There was a problem hiding this comment.
It's also not clear to me why we'd need this. The invoicer should be able to read this from the contract_id. What extra information would be necessary from these two fields. Aren't they just derived from the contract_id?
I think for this change it would be good to see the getsentry version (how you plan to use these fields) before we merge the protos version
Some bot comments pointed out that the original changes in https://github.com/getsentry/getsentry/pull/20791 didn't account for these data. Local validation showed that twiddling the cancel bit changed the value in the next bill box. Claude is telling me there's no good way to test the partner data, although I could probably mock it but haven't tested it yet; maybe it can be removed from here.
There are some pieces of data used in the legacy next bill preview that weren't yet transmitted in the RPC message we're using, and these still aren't represented in platform models so need to currently be derived from Subscriptions.
It looks like maybe the renew data would be done as part of https://linear.app/getsentry/issue/REVENG-266/add-platform-plan-change-compute-service-preview-apply and the self serve partner data from https://linear.app/getsentry/issue/REVENG-228/expand-billing-platform-supportsondemand-to-cover-self-serve-partners.