[RFC] It's not possible to distinguish between refund requests; Incorrect DB design?

[diimpp]

Hi,

RefundPlugin doesn't not track refund requests, only result. One may think, that Entity\Refund will contain multiple Entity\RefundUnit and therefore I will always know which OrderItemUnits were refunded and when, but it's not the case though, Entity\Refund is mapped to refunded unit

         $this->eventBus->dispatch(new UnitsRefunded(
             $orderNumber,
             $command->units(),
             $command->shipments(),
             $command->paymentMethodId(),
             $refundedTotal,
             $order->getCurrencyCode(),
             $command->comment()
         ));

I expect to have some kind of Entity\RefundGroup/Entity\RefundRequest with issuedAt, which will tie together Refund, RefundPayment and possibly CreditMemo.

It's also strange to me, that RefundPayment and Refund don't have doctrine relationship (Unidirectional at least) to Order and use order_number to manually track the relation, while CreditMemo have the doctrine relationship.
I would expected this to be the other hand around, like InvoicingPlugin\Invoice only tracks orderNumber

Another point is that CreditMemo should be generated only after RefundPayment is confirmed. I would guess there are various financial flows, where CreditMemo can be generated before the refund executed, but for simplicity sake it should the same as for Invoice, which is generated only after order is paid.

And usage of messenger for CreditMemo and not CreditMemoPdf is not called for: it will never be executed async and adds complexity of messenger message wrapping, like critical error will be lowered to error level without execution flow stop, therefore overall refund state will be broken without no way of solving it.

[Zales0123]

Hello Dmitri! First of all, thank you for your engagement in this plugin. It's great to see how much goodwill you can get from people in the open-source world :)
Indeed, the plugin still suffers from some not well-considered decisions from the past. I hope we will be able to solve most of them in the nearest future 🖖

1. Refund entity is indeed something like RefundUnit - that's why it has refundedUnitId property (which refers to shipment or order item unit). Indeed, the naming could be better. The idea was not to save refunds as the whole object, but each unit separately. That's also the reason why there is no such concept as RefundRequest.
2. We also wanted to lower the number of relations, but it seems senseless from the point of the time. Then I would be good with changing orderNumber on Refund and especially RefundPayment to the relation
3. Again, the idea was to separate CreditMemo and RefundPayment which seemed to be a good option, but the reality says - it can be probably better. The issue about it is already on the repo for a while Is Process Manager a suitable solution when refunding order units? #91 (but you've already seen it :)). I totally agree that this plugin should as close to the InvoicingPlugin as possible 👍
4. Regarding messenger - I agree, less complexity is always better 👌

I see that you feel this plugin well, so if you would like to open some fixes that will be more than welcomed 🎉 I hope we would find some more energy to put in this (and other) plugins in the nearest future.

Thanks a lot! 🚀