Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[13.x] Preview upcoming invoice #1146

Merged
merged 12 commits into from
May 17, 2021

Conversation

benjamindoe
Copy link
Contributor

@benjamindoe benjamindoe commented May 13, 2021

Noticed this issue and thought I would suggest a PR since I've done something similar in my current project. Nothing fancy. Just a very basic method that has the same signature as the swap method

$upcomingInvoice = $subscription->previewInvoice('price_to_swap')

Closes #1142

@driesvints
Copy link
Member

Cool stuff, thanks @benjamindoe! I think we can improve this further. Can we re-use the upcomingInvoice method from the billable? I also think we need a separate general upcomingInvoice method on the subscription itself that previewUpcomingInvoice calls.

We can add an argument to upcomingInvoice here so we can pass options and don't need to re-use the stripeOptions call etc.

Also: I believe we need to re-use swapOptions as well, no?

@benjamindoe
Copy link
Contributor Author

benjamindoe commented May 13, 2021

For sure, I kind of just carved this out of my current app without much refactoring.

I'll look into reusing swapOptions. There must have been a reason why I didn't do it. Probably the payload data is slightly different when interacting with invoices directly?

I'll look at factoring out upcomingInvoice and previewUpcomingInvoice

@driesvints driesvints marked this pull request as draft May 13, 2021 14:03
@benjamindoe
Copy link
Contributor Author

So unfortunately, getSwapOptions won't work in its current state with previewing upcoming invoices. The payload is incompatible due to calling Stripe invoices directly instead of via a subscription. A good example of this is subscription_items for invoices vs. items for subscription payloads.

@driesvints
Copy link
Member

Okay. Thanks for working on this @benjamindoe. I'll take it from here.

@driesvints driesvints force-pushed the preview-upcoming-invoice branch 2 times, most recently from 47a0c1d to 7225600 Compare May 17, 2021 15:55
@driesvints driesvints force-pushed the preview-upcoming-invoice branch from 7225600 to 547dbe7 Compare May 17, 2021 15:55
@driesvints driesvints marked this pull request as ready for review May 17, 2021 15:55
@driesvints
Copy link
Member

Thanks @benjamindoe. I've finished this PR. Managed to re-use swapOption in a different way.

@benjamindoe
Copy link
Contributor Author

Nice one... Interesting method

@driesvints driesvints requested a review from taylorotwell May 17, 2021 16:16
@taylorotwell taylorotwell merged commit aa2444b into laravel:master May 17, 2021
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.

3 participants