In case of a refund notification by paypal IPN no status change occurs.

Add a state "PayPal refunded" and add transition to it in case of a refund IPN.
It is important to update the status. Remaining on e.g. "Payment received" is no more corresponding the current account balance.

Comments

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker commented

File uc_paypal.pages.inc @119 

function uc_paypal_ipn($order_id = 0) {
...
     // You, the merchant, refunded the payment.
      case 'Refunded':
        $comment = t('PayPal transaction ID: @txn_id', array('@txn_id' => $txn_id));
        uc_payment_enter($order_id, 'paypal_wps', $payment_amount, $order->uid, NULL, $comment);
        break;

missing something like

uc_order_comment_save($order_id, 0, t('Payment is refunded at PayPal.'), 'order', 'paypal_refunded');

plus the status creation in .install.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

This part is pretty much site-specific.

One easy way of doing that is to add an conditional action that will do the transition you need when the order balance transitions from 0 to non-zero.

univate’s picture

univate CreditAttribution: univate commented

If we cleaned up the paypal IPN code then developers could manually hook into this and add addition requirements ourselves: #781720: Refactor the Paypal WPS IPN code

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker commented

Interesting..
Damien - logging the payment is one thing. But it is absolutely mission critical to have a status indicator that an order is no more representing the balance 0. This requirement is not site specific. In ubercart one problem is, that we only have one single status (because a payment status may differ to a delivery status and a final abstract order/process status...)

We hit the situation since we're implementing 2 further cc gateways (previously unavailable) - using Paypal as a reference source.

I see Paypal might be improved, hookified, but more general - if a system uses multiple payment solutions - we need abstraction and abstract states not specific ro a cc provider.
Since this would be a huge change to ubercart (not possible currently) i strongly vote for the per-payment-provider hook.

univate - So i'll support your paypal ipn hook approach and i'll implement a custom hooked module to add our transactions needed. Note my comments overthere.

Additionally i see no way to differ between a payment settlement and an authorization only on the payment status level. It would also be very important to make uc understand on a payment status level whether one or the other occured. (Every cc provider i know makes this difference.)

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

@miro_dietiker: what I meant exactly is that: what to do with the order once it is in production (ie. got to or is past to "payment_received"), but the payment gets cancelled is definitely site specific. There is no "correct" way to handle that.

In that case, the uc_paypal should not simply change the status of the order because it is generally not the correct thing to do.

You can handle this case very easily today, you probably don't need any modification to Ubercart itself: just create a predicate triggered by 'uc_payment_entered', with conditions: the order is in 'payment_received' (or past that, depending on your status configuration), the balance is not zero. On the action part, you can do whatever fits your needs.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

In ubercart one problem is, that we only have one single status (because a payment status may differ to a delivery status and a final abstract order/process status...)

Not exactly true: you have the order status (which is the status of production of the order in your workflow), and the payment *balance*. As soon as it is non zero, there is something to do with the payment.

The real problem, as you noted is that some payment implementation actually modify the order status itself, or even create new order status! That's definitely wrong. Everything related to the completion of the payment should be handled by conditional actions, it's not the decision of the payment method to make.

TR’s picture

TR CreditAttribution: TR commented

Well, I think the real problem is that support for refunds/cancellations was never really fully implemented in Ubercart. It's going to require more than just changes to the PayPal module to do this properly. We need to also do other things that have nothing to do with payment when an order is refunded/cancelled, such as increase stock levels. I think an order status to indicate a refunded/cancelled purchase is probably a good idea, to distinguish that state from an order which is pending payment. However, it shouldn't be a PayPal specific order status.

@Damien: While the transition between order states is definitely site-specific to an extent, Ubercart really needs to define all the states necessary to describe a normal order lifecycle. If there were a new status for refunded/cancelled, then each site could make site-specific decisions with CA how to handle that workflow. But if there is no trigger (state change) for when an order gets refunded/cancelled, the site has no "hook" into the refunding process.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker commented

Damien,
Paypal currently adds "Paypal pending". Do you already object this?
As i told, i'd expect to see a further state representing "Payment Authorized" instead of Payment received.
In business cases where you settle payments after e.g. delivery, the state "Payment received" would never be reached - if it's a pre-delivery state.
Note that authorization of a full payment currently sets the balance to 0 on ubercart side - while it is unchanged on the bank side - since authorization only is no balance change.
So payment and order state are no strict ans simple state machine with one active state.

univate’s picture

univate CreditAttribution: univate commented

Even if ubercart doesn't support refunds, it doesn't mean people can't implement their own support. All they need is access the the IPN messages, patch ready for review in:
#781720: Refactor the Paypal WPS IPN code

TR’s picture

TR CreditAttribution: TR commented
Version: 6.x-2.x-dev » 8.x-4.x-dev
Issue summary: View changes

We will be implementing new features in 8.x-4.x first at this point, with a backport to 7.x-3.x if there is interest. It's highly unlikely that this will ever make it into 6.x-2.x, given that D6 is near its end-of-life and given that D6 uses CA instead of Rules so it would need an entirely different implementation.