RefundPayment - Payment relation #85
Conversation
Zales0123
added
the
Enhancement
|
Travis says that you forgot to inject an instance of RelatedPaymentIdProviderInterface into RefundPaymentProcessManager. Once you do it, this PR would be merge'able IMO 👍 |
Zales0123
force-pushed
the
refund-payment-payment-relation
branch
from
6823c6a
to
7bbc0a4
Compare
Zales0123
changed the title
| @@ -56,7 +56,8 @@ public function __invoke(UnitsRefunded $event): void | |||
| $event->orderNumber(), | |||
| $event->amount(), | |||
| $event->currencyCode(), | |||
| $event->paymentMethodId() | |||
| $event->paymentMethodId(), | |||
| 1 | |||
There was a problem hiding this comment.
I suppose this is currently hardcoded like this to support single-payment orders?
|
Hey @Zales0123 - I think these changes are great. We can fetch the original payment with this on a successful refund and use its details to talk to other gateways like explained here: Sylius/ShopApiPlugin#310 (comment) I think this is a way to go and be able to use payum payments (even though only offline) with pre-paid r pre-captured payments. Readers be aware: The reason I spawned this kind of PR is because we are using Sylius completely headless and only with ShopAPI - thats why handling Payum gateway-ed payments is a bit difficult and we are having these kinds of workarounds. But this change will not only help our usecase, but everyone who wants to do extra things with refunds. This will attach the original payment information to a refund which is great! :) 🚀 |
Zales0123
changed the title
Zales0123
force-pushed
the
refund-payment-payment-relation
branch
from
7bbc0a4
to
23ea93f
Compare
The necessity of this change is connected with the future possibilities of supporting payment methods with other than offline gateways. Usually, for integration with payment gateways as PayPal or stripe, we need to have reference to an actual Order payment. Of course, there are some problems with this approach, but I think they can be safely ignored (😄) for now:
What if there is more than one order payment? Which should be related?
IMO the fair assumption is, we operate on only one payment for now. As explained in similar issue on
AdminOrderCreationPlugin, even though Sylius gives a possibility to operates on split payments, there is no such feature out-of-the-box. After all, this is Sylius plugin, we need to provide a good and easily extended implementation, rather than think about every possible possibility.What about the partial refunds? Should they all be related to one the same payment?
For now yes, regarding the split payments explanation from the previous question 😄
If (when) we provide other gateways integrations in the future (my dream is to have PayPal and Stripe supported out-of-the-box, as in Sylius), there will be more questions about handling this relation, for sure. Right now we must trust in plugin users' developing skills (however every opinion or contribution would be appreciated :)).
CC @kortwotze as I've talked about this feature a few days ago, I suppose you still need it 🚀
PS. When I wrote this description, I've realized, that supporting only offline gateways is a missing feature 😄 Well, it should be implemented (#33) but something went wrong (https://github.com/Sylius/RefundPlugin/blob/master/src/Action/Admin/OrderRefundsListAction.php#L63). Anyway, it should be implemented (supporting other than offline gateways is not tested at all) and I will change it in separate PR 💣