mollie / laravel-cashier-mollie

Official Mollie integration for Laravel Cashier
https://www.cashiermollie.com/
MIT License
137 stars 44 forks source link

Payment type inconsistent for FirstPaymentPaid and OrderPaymentPaid #211

Closed michelterstege81 closed 3 months ago

michelterstege81 commented 1 year ago

I have an Administration service in my application that uses the $payment object in the event when a FirstPaymentPaid event is received and when an OrderPaymentPaid is received. After handling a succesfull first payment I now noticed an exception in my app when I handled the OrderPaymentPaid event. The cause is that in FirstPaymentPaid \Mollie\Api\Resources\Payment is used for the $payment object and in OrderPaymentPaid \Laravel\Cashier\Payment.

I think it would be nice to keep the types consistent.

Naoray commented 1 year ago

Hi @michelterstege81,

can you please paste your error log here?

michelterstege81 commented 1 year ago

Below you find the relevant part of the error log. I call my service setInvoicePaid() from FirstPaymentPaid and OrderPaymentPaid. From the FirstPaymentPaid all went well, but then I discovered it goes wrong when passing the $payment object when I call it when handling the OrderPaymentPaid event. There Cashier uses a different type. You don't really need my error log for it. Just look up the 2 events in Cashier and you can see that the type used for the $payment object is not consistent.

[2023-10-18 00:41:55] production.ERROR: App\Services\Moneybird::setInvoicePaid(): Argument #2 ($payment) must be of type Mollie\Api\Resources\Payment, Laravel\Cashier\Payment given, called in /var/www/inpromas/app/Listeners/CashierEventSubscriber.php on line 217 {"exception":"[object] (TypeError(code: 0): App\\Services\\Moneybird::setInvoicePaid(): Argument #2 ($payment) must be of type Mollie\\Api\\Resources\\Payment, Laravel\\Cashier\\Payment given, called in /var/www/inpromas/app/Listeners/CashierEventSubscriber.php on line 217 at /var/www/inpromas/app/Services/Moneybird.php:377)
[stacktrace]
#0 /var/www/inpromas/app/Listeners/CashierEventSubscriber.php(217): App\\Services\\Moneybird->setInvoicePaid()
#1 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(441): App\\Listeners\\CashierEventSubscriber->handleOrderPaymentPaid()
#2 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(249): Illuminate\\Events\\Dispatcher->Illuminate\\Events\\{closure}()
#3 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(353): Illuminate\\Events\\Dispatcher->dispatch()
#4 /var/www/inpromas/vendor/mollie/laravel-cashier-mollie/src/Order/Order.php(543): Illuminate\\Support\\Facades\\Facade::__callStatic()
#5 /var/www/inpromas/vendor/mollie/laravel-cashier-mollie/src/Http/Controllers/WebhookController.php(30): Laravel\\Cashier\\Order\\Order->handlePaymentPaid()
#6 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(46): Laravel\\Cashier\\Http\\Controllers\\WebhookController->handleWebhook()
#7 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Route.php(260): Illuminate\\Routing\\ControllerDispatcher->dispatch()
#8 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Route.php(205): Illuminate\\Routing\\Route->runController()
#9 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Router.php(799): Illuminate\\Routing\\Route->run()
#10 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(141): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}()
#11 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(116): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#12 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Router.php(800): Illuminate\\Pipeline\\Pipeline->then()
#13 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Router.php(777): Illuminate\\Routing\\Router->runRouteWithinStack()
#14 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Router.php(741): Illuminate\\Routing\\Router->runRoute()
#15 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Routing/Router.php(730): Illuminate\\Routing\\Router->dispatchToRoute()
#16 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(200): Illuminate\\Routing\\Router->dispatch()
#17 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(141): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}()
#18 /var/www/inpromas/vendor/livewire/livewire/src/DisableBrowserCache.php(19): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#19 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Livewire\\DisableBrowserCache->handle()
#20 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#21 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php(31): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle()
#22 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\ConvertEmptyStringsToNull->handle()
#23 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#24 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TrimStrings.php(40): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle()
#25 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\TrimStrings->handle()
#26 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#27 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle()
#28 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php(89): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#29 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\PreventRequestsDuringMaintenance->handle()
#30 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Http/Middleware/HandleCors.php(49): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#31 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Http\\Middleware\\HandleCors->handle()
#32 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Http/Middleware/TrustProxies.php(39): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#33 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Http\\Middleware\\TrustProxies->handle()
#34 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(116): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#35 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(175): Illuminate\\Pipeline\\Pipeline->then()
#36 /var/www/inpromas/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(144): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter()
#37 /var/www/inpromas/public/index.php(52): Illuminate\\Foundation\\Http\\Kernel->handle()
Naoray commented 1 year ago

Sorry @michelterstege81, I didn’t get exactly what you were reporting from your initial description, which is why I requested the error log.

To summarise the issue: Both events, FirstPaymentPaid as well as OrderPaymentPaid, use an $order and a $payment property, but the $payment property in FirstPaymentPaid Event is an object from the underlying Mollie API PHP package, while all other properties in both events are dedicated Cashier objects.

I will note this down and include the change in the next major release.

Thank you for reporting the inconsistency 🙏!

michelterstege81 commented 1 year ago

Indeed good to put this in the next major release, since this can break existing applications ;)

Naoray commented 3 months ago

noted down on our roadmap. closing for now