Command to purge expired pending orders. #1217
Conversation
| * | ||
| * @author Ka-Yue Yeung <kayuey@gmail.com> | ||
| */ | ||
| interface PurgerInterface |
There was a problem hiding this comment.
You could reuse the interface from CartBundle actually, couldn't you?
|
@winzou Updated, please have a look. |
|
I'm a bit confused, who is @foopang? :D |
|
@winzou By release inventory, did you mean those on-hold? It should be done in another command, because it happens a lot earlier. We release inventory after 15 minutes, and we purge order after 3 hours. |
|
@kayue yes I meant it. Whatever the other command, we need to release inventory units here before the deletion (and if onhold=0 then it will do nothing) to ensure that we don't delete onhold units. Can you try to add the release command in this PR? Should be quite similar. You release inventory after 15 minutes only at the end? You told me before that you might have late payments in your solution and couldn't release after only 15min. What has changed since? |
|
So we will have two commands:
PayPal could clear payment after 2 weeks, and there is nothing I can do about it. And you just remind me something, I think we cannot delete the order here.. We have to change order state to |
|
@winzou Should we combine Less state is less confusing... |
|
@winzou Actually order is soft delete, do we still want to delete them at all? or just change the state is enough? |
|
Indeed we should delete nothing here. Let's just change the state to And how do you ensure you have enough inventory if you release them after 15min and receive payment after 2 weeks? |
|
@winzou We can't ensure. It will result in backordered. |
|
|
||
| $queryBuilder | ||
| ->andWhere($queryBuilder->expr()->lt($this->getAlias().'.updatedAt', ':expiresAt')) | ||
| ->andWhere($queryBuilder->expr()->eq($this->getAlias().'.state', OrderInterface::STATE_PENDING)) |
There was a problem hiding this comment.
Or
->andWhere($this->getAlias().'.state = :state')
->setParameter('state', OrderInterface::STATE_PENDING)
| $orders = $this->repository->findExpiredPendingOrders($expiresAt); | ||
| foreach ($orders as $order) { | ||
| $this->dispatcher->dispatch(SyliusOrderEvents::PRE_RELEASE, new GenericEvent($order)); | ||
| $this->dispatcher->dispatch(SyliusOrderEvents::POST_RELEASE, new GenericEvent($order)); |
There was a problem hiding this comment.
You still can't dispatch this event before the flush.
edit: can not
There was a problem hiding this comment.
Can we put the flush inside the foreach loop?
There was a problem hiding this comment.
Best way is to loop over $orders twice I think, with the flush between the 2 loops.
|
Nice work @foopang! I think it would be good idea to move the logic of Release command to a separate service, so that it can be used outside of the CLI. What do you think? It should be quite easy! |
| /** | ||
| * Get the order from event and void payment. | ||
| * | ||
| * @param GenericEvent $event |
| protected function execute(InputInterface $input, OutputInterface $output) | ||
| { | ||
| $holdingDuration = $this->getContainer()->getParameter('sylius.inventory.holding.duration'); | ||
| $expiresAt = new \DateTime(sprintf('-%s', $holdingDuration)); |
There was a problem hiding this comment.
Those two lines should go into else after that if ($input->isInteractive()) {} call.
|
@pjedrzejewski I've moved the logic of Release command to a separate service. |
Command to purge expired pending orders.
No description provided.