[RefundPayment] Change order number to Order relation on RefundPayment #307
Conversation
GSadee
added
Enhancement
GSadee
force-pushed
the
relate-refund-payment-with-order
branch
from
46cec9a to
3a8afc2
Compare
| $this->entityManager = $entityManager; | ||
| $this->eventBus = $eventBus; | ||
| } | ||
|
|
||
| public function next(UnitsRefunded $unitsRefunded): void | ||
| { | ||
| /** @var OrderInterface|null $order */ | ||
| $order = $this->orderRepository->findOneByNumber($unitsRefunded->orderNumber()); |
There was a problem hiding this comment.
I wonder if I should find the order here or in RefundPaymentFactory 🤔 Especially that we are already finding the payment method by its id in the factory. Wdyt?
There was a problem hiding this comment.
In my opinion, we should pass to the factory as final data as possible, to allow it to create the desired object and do nothing more 🚀
GSadee
force-pushed
the
relate-refund-payment-with-order
branch
from
3a8afc2 to
66ffcd1
Compare
GSadee
force-pushed
the
relate-refund-payment-with-order
branch
from
66ffcd1 to
781db9d
Compare
GSadee
force-pushed
the
relate-refund-payment-with-order
branch
from
781db9d to
4b25791
Compare
| use Sylius\Bundle\ResourceBundle\Doctrine\ORM\EntityRepository; | ||
| use Sylius\RefundPlugin\Repository\RefundPaymentRepositoryInterface; | ||
|
|
||
| class RefundPaymentRepository extends EntityRepository implements RefundPaymentRepositoryInterface |
There was a problem hiding this comment.
Should we (similar to factories) provide trait that should be implemented in the repository? But I suppose it's not a case for this change but for all the repositories and plugin 💃
There was a problem hiding this comment.
I think so, but this is the plugin, that defines RefundPayment, so creating a repository without traits makes sense here.
| public function findByOrderNumber(string $orderNumber): array | ||
| { | ||
| return $this->createQueryBuilder('o') | ||
| ->innerJoin('o.order', 'ord') |
There was a problem hiding this comment.
ord 😱 Is it because order is a banned name? 😄 maybe then refundPaymentOrder?
There was a problem hiding this comment.
Yup, that is the reason 😃 And to be honest, it is a copy-pasted shortcut from Sylius, but I agree that refundPaymentOrder looks better 😃
|
Thanks, Grzegorz! 🎉 |
This PR was merged into the 1.0-dev branch. Discussion ---------- Similar to Sylius/RefundPlugin#306 and Sylius/RefundPlugin#307. Should be easier to operate on the Invoice with a proper relation rather than just an order number Commits ------- 081b080 Add Order relation on Invoice (and remove order number field) e480edc Use order instead of order number in invoice repository 0ab7fed Final fixes ce949d3 Mention changes in the UPGRADE file 8411c18 Remove unneeded point in UPGRADE file
Partially fixes #193
Similar to #306