Cancelled payment results in a "confirmed" email, error cancel trigger

[OpenGlobal]

I can't work out how to move my original post to this category so I'll just link to it:

http://forum.virtuemart.net/index.php?topic=99396.0

Cancelling a payment, for example with the PayPal plugin, cancels the order, but the confirmation email still gets sent to the vendor and the customer. The customer's email shows the order status as "Cancelled" in the body of the email, but the email subject still says "Confirmed". The vendor's email says "Confirmed" in the title and this is not clarified anywhere in the email with the actual status.

The problem seems to be that components/com_virtuemart/views/invoice/view.html.php is using the subject COM_VIRTUEMART_VENDOR_NEW_ORDER_CONFIRMED regardless of the actual status of the order.

There are two possible options:

1) Create a series of email subjects: COM_VIRTUEMART_VENDOR_NEW_ORDER_CANCELLED, COM_VIRTUEMART_VENDOR_NEW_ORDER_PENDING, etc. Or even better, a generic email subject COM_VIRTUEMART_VENDOR_NEW_ORDER_STATUS="[%3$s], %4$s order by %1$s, total %2$s" where %4$s is the order status (Confirmed, Pending, Cancelled, etc)

2) Do not send the emails at all when a payment is cancelled by the customer.

OpenGlobal

[OpenGlobal]

Of course, all references above to COM_VIRTUEMART_VENDOR_NEW_ORDER_CONFIRMED also refer to COM_VIRTUEMART_SHOPPER_NEW_ORDER_CONFIRMED.

( Just for the pedantic out there :-) )

OpenGlobal

[OpenGlobal]

Is it right that upgrading to 203H should remove Virtuemart from the Components menu completely?

I can now only get to it by typing option=com_virtuemart into the address bar.

OpenGlobal

[Milbo]

Hello openglobal, I am sorry, I missed your post. Please write your used version, I think it is solved in 2.0.3H.

[OpenGlobal]

I can tell just by looking at the code that this will not work (plugins/vmpayment/paypal.php):

Code Select Expand


    function plgVmOnUserPaymentCancel() {

if (!class_exists('VirtueMartModelOrders'))
    require( JPATH_VM_ADMINISTRATOR . DS . 'models' . DS . 'orders.php' );

$order_number = JRequest::getString('on', '');
$virtuemart_paymentmethod_id = JRequest::getInt('pm', '');
if (empty($order_number) or empty($virtuemart_paymentmethod_id) or !$this->selectedThisByMethodId($virtuemart_paymentmethod_id)) {
    return null;
}

if (!($paymentTable = $this->getDataByOrderId($virtuemart_order_id))) {
    return null;
}

VmInfo(Jtext::_('VMPAYMENT_PAYPAL_PAYMENT_CANCELLED'));
$session = JFactory::getSession();
$return_context = $session->getId();
if (strcmp($paymentTable->paypal_custom, $return_context) === 0) {
    $this->handlePaymentUserCancel($result->virtuemart_order_id);
}
return true;
    }


$virtuemart_order_id The section of code which obtained this variable has been deleted so this is no longer set.
$result->virtuemart_order_id is left over from a previous version of this code and no longer exists.

Who the hell is writing this stuff? And worse still, who is testing it?

OpenGlobal

[OpenGlobal]

In administrator/components/com_virtuemart/plugins/vmplugin.php => storePluginInternalData(), the following line NEVER returns the database errors:

$errors = $this->_vmpItable->getErrors();

The only way I can find out what the actual database errors are is to add the line:

print_r($this->_vmpItable);

I haven't looked into the reason behind the getErrors() failure, I leave that up to you :-D

OpenGlobal

[OpenGlobal]

I can tell just by looking at the code that this will not work (plugins/vmpayment/paypal.php):

Code Select Expand


    function plgVmOnUserPaymentCancel() {

if (!class_exists('VirtueMartModelOrders'))
    require( JPATH_VM_ADMINISTRATOR . DS . 'models' . DS . 'orders.php' );

$order_number = JRequest::getString('on', '');
$virtuemart_paymentmethod_id = JRequest::getInt('pm', '');
if (empty($order_number) or empty($virtuemart_paymentmethod_id) or !$this->selectedThisByMethodId($virtuemart_paymentmethod_id)) {
    return null;
}

if (!($paymentTable = $this->getDataByOrderId($virtuemart_order_id))) {
    return null;
}

VmInfo(Jtext::_('VMPAYMENT_PAYPAL_PAYMENT_CANCELLED'));
$session = JFactory::getSession();
$return_context = $session->getId();
if (strcmp($paymentTable->paypal_custom, $return_context) === 0) {
    $this->handlePaymentUserCancel($result->virtuemart_order_id);
}
return true;
    }


$virtuemart_order_id The section of code which obtained this variable has been deleted so this is no longer set.
$result->virtuemart_order_id is left over from a previous version of this code and no longer exists.

Who the hell is writing this stuff? And worse still, who is testing it?

OpenGlobal

I suggest that the code for this function is changed to the following (or similar) in order to make it stand any chance of working:

Code Select Expand


    function plgVmOnUserPaymentCancel() {

if (!class_exists('VirtueMartModelOrders'))
    require( JPATH_VM_ADMINISTRATOR . DS . 'models' . DS . 'orders.php' );

$order_number = JRequest::getString('on', '');
$virtuemart_paymentmethod_id = JRequest::getInt('pm', '');
if (empty($order_number) or empty($virtuemart_paymentmethod_id) or !$this->selectedThisByMethodId($virtuemart_paymentmethod_id)) {
return null;
}
if (!($virtuemart_order_id = VirtueMartModelOrders::getOrderIdByOrderNumber($order_number))) {
return null;
}
if (!($paymentTable = $this->getDataByOrderId($virtuemart_order_id))) {
    return null;
}

VmInfo(Jtext::_('VMPAYMENT_PAYPAL_PAYMENT_CANCELLED'));
$session = JFactory::getSession();
$return_context = $session->getId();
if (strcmp($paymentTable->paypal_custom, $return_context) === 0) {
    $this->handlePaymentUserCancel($virtuemart_order_id);
}
return true;
    }


OpenGlobal

[OpenGlobal]

A couple of other points regarding payment cancellation.

203H seems to have fixed the problem of emailing a "confirmation" email when the payment is cancelled. However, the process now seems to be that the order is completely deleted rather than just cancelled. Is this expected behaviour?

One other minor point is that another bug has arisen when the order is deleted.

The order history shows the following:

Friday, 30 March 2012 16:27    No   Pending   
Friday, 30 March 2012 16:28    No   Cancelled   
Friday, 30 March 2012 17:28    No      Order deleted

In fact, the times were 17:27, 17:28, 17:28. The first two entries are using GMT time, the third entry is using local time (BST). Of course the last two entries happened at EXACTLY the same time because the order was cancelled and then deleted instantly. But it looks like it happened an hour apart.

What can possibly be causing an inconsistency timestamp timezone. Surely this is handled by the same function for every history entry.

Surely...

Surely?

That's just "How to Program 101" right? :-)

OpenGlobal

[OpenGlobal]

Hi Milbo,

Yes, I've been updating the 2.0.3H thread now. It will be fixed in the future version that implements my changes to the plgVmOnUserPaymentCancel(). Hopefully 2.0.3i??? 2.0.3H doesn't handle payment cancellations at all.

OpenGlobal

[alatak]

Hi,


plgVmOnUserPaymentCancel () fixed.