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] Refactor payment exceptions #1095

Merged
merged 1 commit into from
Mar 15, 2021
Merged

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Mar 12, 2021

This simplifies the payment exceptions within Cashier. Atm we were still rolling separate exceptions for the different statuses of a payment. But these types of statuses can easily be derived from the encapsulated payment intent object. In general, it's unlikely that someone implementing Cashier will make abundant use of these different types of payment exceptions. We also only document the use of the parent IncompletePayment exception in our docs: https://laravel.com/docs/8.x/billing#handling-failed-payments

The sub types are currently documented but probably that's not needed as you'd usually just make use of the parent exception. If you really need to differentiate between them, then the Payment object has helper methods to do just that.

This PR also makes ->validate() on the Payment object throw the exception for payments with the status of requires_confirmation.

@taylorotwell
Copy link
Member

So what if they did need to know the specific type of problem? What would the user do then?

@driesvints
Copy link
Member Author

driesvints commented Mar 15, 2021

@taylorotwell the type is contained within the payment object:

$exception->payment->requiresPaymentMethod();

If you really need to differentiate between them, then the Payment object has helper methods to do just that.

@taylorotwell taylorotwell merged commit fae8258 into master Mar 15, 2021
@taylorotwell taylorotwell deleted the refactor-payment-exceptions branch March 15, 2021 14:10
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.

2 participants