Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial refunds causing issues #514

Open
JamesFX2 opened this issue Jun 13, 2022 · 1 comment
Open

Partial refunds causing issues #514

JamesFX2 opened this issue Jun 13, 2022 · 1 comment

Comments

@JamesFX2
Copy link

We are occasionally have issues issusing partial refunds on orders.

What happens is that (1) the refund goes through on checkout.com, (2) the credit memo is generated but when we view the sales_order later, the qty_refunded on the sales_order_item hasn't been updated, with other fields related to total refunded also being 0.

Except they were updated.

Have debugged this and the issue is being caused by the webhook overlapping with the refund process.

So,

  • Process 1: Admin
    • Order is loaded in process that refunds, refund is generated.
  • Process 2: Webhook
    • Order is loaded in webhook callback but the state it loads it in is equivalent to the pre-refunded state.
  • Process 1: Admin
    • Saves the order, sale_order_item qty_refunded is correct.
  • Process 2: Webhook
    • Saves the order again, overwriting changes such as qty_refunded, base_total_refunded, etc. etc.

I'm going to fix this with a plugin that prevents the order from being saved when it doesn't need to be saved, presumably on handleTransaction in TransactionHandlerService, but you know, it would be kind of cool if you could fix this yourselves too.

@edroberts
Copy link

Further feedback from James on this:

_I'd recommend that they don't save the order if no changes are necessary.

If you modify processInvoice, processEmail, processCreditMemo to return a true/false if they actually did any changes to the order, you could use these as a flag.

I've not tested this code but it would be something like this to completely ignore handleTransaction if a credit memo already exists with a matching txnId to the transaction Id._

/**

 * @param TransactionHandlerService$subject
 * @param \Closure$proceed
 * @param OrderInterface$order
 * @param array$webhook
 * @returnvoid
 */
public functionaroundHandleTransaction(\CheckoutCom\Magento2\Model\Service\ TransactionHandlerService$subject ,\ Closure$proceed , OrderInterface$order ,array $webhook) {
    $transaction= $subject->hasTransaction(
        $order,
        $webhook['action_id']
    );

    if(!$transaction ) {
        if(!$this->isWebhookNeeded($order ,$webhook)) {
            // might still need to update sales_order_payment
            // and sales_payment_transaction
            // not sure what the non-webhook sets
           return;
        }
    }
    $proceed($order ,$webhook);
}

/**
 * @param OrderInterface$order

 * @returnbool

 */
protected functionisWebhookNeeded (OrderInterface$order ,array $webhook) {
    $isRefund= \CheckoutCom\Magento2\Model\Service\TransactionHandlerService::TRANSACTION_MAPPER[$webhook['event_type' ]] === TransactionInterface::TYPE_REFUND;
    if( $isRefund&& isset($webhook['action_id' ]) && $txnId= $webhook['action_id' ]) {

       if($this->hasCreditMemoWithTxnId($order ,$txnId)) {

           return false;
        }

    }

   return true;
}

/**
 * @param OrderInterface$order
 * @param$txnId
 * @returnbool
 */
protected functionhasCreditMemoWithTxnId($order ,$txnId) {
    $creditMemos= $order->getCreditmemosCollection();
    if(!empty($creditMemos )) {
        foreach( $creditMemosas $creditMemo) {
            if($creditMemo->getTransactionId () == $txnId) {
                return true;
            }
        }

    }

   return false;
}

_But I haven't had time to attempt to fix this and therefore test the above. I still have question marks over whether I'd need to create a transaction/update the payment or if the refund that's generated without the webhook does that.

You don't need to set order status as much as the original developers thought. A long-standing annoyance with the extension is that it tries to change the order status after a refund or partial refund since Magento handles that itself. Magento will automatically close or cancel an order when certain criteria are met after a refund - see https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Sales/Model/ResourceModel/Order/Handler/State.php - you don't need to set state/status - the only gap is the following criteria._


if(

   $order

   && !$order->isCanceled()
    && !$order->canUnhold()
    && !$order->canInvoice()
) {
    $currentState= $order->getState();
    if(in_array($currentState, [\Magento\Sales\Model\Order::STATE_PROCESSING ,\Magento\Sales\Model\Order::STATE_COMPLETE])

        && !$order->canCreditmemo()

        &&$order->canShip()

    ) {

       $order->setState(\Magento\Sales\Model\Order::STATE_CLOSED);
        $order->setStatus(\Magento\Sales\Model\Order::STATE_CLOSED);

    }

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants