From e53e65a945a4df5cbd736575ac971c46df2a30ff Mon Sep 17 00:00:00 2001 From: Neil Drumm Date: Mon, 25 Apr 2011 12:46:45 -0700 Subject: [PATCH 1/4] #1072500 Code cleanup for uc_ogone_ok() and uc_ogone_return_ok(). --- uc_ogone_payment.module | 18 ++++-------------- 1 files changed, 4 insertions(+), 14 deletions(-) diff --git a/uc_ogone_payment.module b/uc_ogone_payment.module index f007e14..f0b67ed 100644 --- a/uc_ogone_payment.module +++ b/uc_ogone_payment.module @@ -446,18 +446,11 @@ function uc_ogone_call(&$arg1, $arg2) { function uc_ogone_ok() { $result = db_query("SELECT sha1_test_succes, order_status FROM {uc_payment_ogone} WHERE order_id = %d", $_SESSION['ogone_order_id']); $ogone_order = db_fetch_object($result); - //Debug - //print_r($ogone_order); - //exit; - //Define valid statusses $valid_stat = array(5, 9); - if ($ogone_order->sha1_test_succes == 'OK' && in_array($ogone_order->order_status, $valid_stat)){ + if ($ogone_order->sha1_test_succes == 'OK' && in_array($ogone_order->order_status, $valid_stat)) { uc_order_update_status($_SESSION['ogone_order_id'], uc_order_state_default('payment_received')); - //Debug - //drupal_set_message('cartid:'.uc_cart_get_id().' ogone order id:'.$_SESSION['ogone_order_id']); - // Ensure the cart we're looking at is the one that payment was attempted for. $_SESSION['cart_order'] = $_SESSION['ogone_order_id']; unset($_SESSION['ogone_order_id']); @@ -466,7 +459,8 @@ function uc_ogone_ok() { $_SESSION['do_complete'] = TRUE; drupal_goto('cart/checkout/complete'); - }else{ + } + else { drupal_goto('cart/ogone_false'); } } @@ -530,7 +524,7 @@ function uc_ogone_return_ok($auto = FALSE) { 'PM' => trim($_GET['PM']), 'STATUS' => trim($_GET['STATUS']), 'TRXDATE' => trim($_GET['TRXDATE']), - 'VC' => trim($_GET['VC']) + 'VC' => trim($_GET['VC']), ); $sha1_return_key = strtoupper($_GET['SHASIGN']); @@ -552,10 +546,6 @@ function uc_ogone_return_ok($auto = FALSE) { $sha1_key = strtoupper(sha1($sha1_key)); - //Uncomment for manual SHA comparison for debug purposes - // echo $sha1_return_key.'
'.$sha1_key; - // exit; - if (($sha1_key == $sha1_return_key) && in_array($arguments['STATUS'], $valid_stat)) { $payment = array( 'order_id' => $arguments['ORDERID'], -- 1.7.1 From b408f83d03d64ceddb530c22bd3e04972c5df8e0 Mon Sep 17 00:00:00 2001 From: Neil Drumm Date: Mon, 25 Apr 2011 12:48:53 -0700 Subject: [PATCH 2/4] #1072500 Simplify calling of uc_ogone_return_ok(); --- uc_ogone_payment.module | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/uc_ogone_payment.module b/uc_ogone_payment.module index f0b67ed..7f48763 100644 --- a/uc_ogone_payment.module +++ b/uc_ogone_payment.module @@ -79,9 +79,12 @@ function uc_ogone_payment_menu() { 'type' => MENU_CALLBACK, ); + // Can be used optionally to let Ogone return the payment status + // automatically/unmanaged. $items['cart/ogone_return_ok_callback'] = array( 'title' => 'Ogone Payment return status', - 'callback' => 'uc_ogone_return_ok_callback', + 'callback' => 'uc_ogone_return_ok', + 'callback arguments' => array(TRUE), 'access arguments' => array('access content'), 'type' => MENU_CALLBACK, ); @@ -566,8 +569,3 @@ function uc_ogone_return_ok($auto = FALSE) { } } } - -//Can be used optionally to let Ogone return the payment status automatically/unmanaged -function uc_ogone_return_ok_callback() { - uc_ogone_return_ok(TRUE); -} -- 1.7.1 From 34cf534174131221542d0ea63909a699a3830bf9 Mon Sep 17 00:00:00 2001 From: Neil Drumm Date: Mon, 25 Apr 2011 13:00:17 -0700 Subject: [PATCH 3/4] #1072500 Reduce the number of redirects on payment, faster and less chances for bugs. --- uc_ogone_payment.module | 58 +++++++++------------------------------------- 1 files changed, 12 insertions(+), 46 deletions(-) diff --git a/uc_ogone_payment.module b/uc_ogone_payment.module index 7f48763..a335ddd 100644 --- a/uc_ogone_payment.module +++ b/uc_ogone_payment.module @@ -51,20 +51,6 @@ function uc_ogone_payment_menu() { 'type' => MENU_CALLBACK, ); - $items['cart/ogone_ok'] = array( - 'title' => 'Ogone Payment OK', - 'page callback' => 'uc_ogone_ok', - 'access arguments' => array('access content'), - 'type' => MENU_CALLBACK, - ); - - $items['cart/ogone_false'] = array( - 'title' => 'Ogone Payment Error', - 'page callback' => 'uc_ogone_false', - 'access arguments' => array('access content'), - 'type' => MENU_CALLBACK, - ); - $items['cart/checkout/ogone_cancel'] = array( 'title' => 'Ogone Payment cancel', 'page callback' => 'uc_ogone_return_cancel', @@ -409,7 +395,6 @@ function uc_ogone_call(&$arg1, $arg2) { $sha1_signature = $orderid . $amount . $currency . $pspid . $sha_suffix; } - $_SESSION['ogone_order_id'] = $arg1->order_id; $_SESSION['ogone_form']='
'.$redirect_message1.' @@ -445,35 +430,6 @@ function uc_ogone_call(&$arg1, $arg2) { drupal_goto('cart/checkout/ogone_redirect'); } - -function uc_ogone_ok() { - $result = db_query("SELECT sha1_test_succes, order_status FROM {uc_payment_ogone} WHERE order_id = %d", $_SESSION['ogone_order_id']); - $ogone_order = db_fetch_object($result); - $valid_stat = array(5, 9); - - if ($ogone_order->sha1_test_succes == 'OK' && in_array($ogone_order->order_status, $valid_stat)) { - uc_order_update_status($_SESSION['ogone_order_id'], uc_order_state_default('payment_received')); - - // Ensure the cart we're looking at is the one that payment was attempted for. - $_SESSION['cart_order'] = $_SESSION['ogone_order_id']; - unset($_SESSION['ogone_order_id']); - - // This lets us know it's a legitimate access of the complete page. - $_SESSION['do_complete'] = TRUE; - - drupal_goto('cart/checkout/complete'); - } - else { - drupal_goto('cart/ogone_false'); - } -} - -function uc_ogone_false() { - drupal_set_message(t('Ogone returned an error for your payment. Your order is not completed. Please try again or contact us.'), 'ERROR'); - watchdog('uc_ogone_payment', 'Ogone returned an error for this payment.', NULL, WATCHDOG_ERROR); - drupal_goto('cart/checkout'); -} - function uc_ogone_template() { $template = ''.filter_xss(variable_get('ogone_layout_title', TRUE)).''; $template .= '
'; @@ -550,6 +506,7 @@ function uc_ogone_return_ok($auto = FALSE) { $sha1_key = strtoupper(sha1($sha1_key)); if (($sha1_key == $sha1_return_key) && in_array($arguments['STATUS'], $valid_stat)) { + // Record the successful payment. $payment = array( 'order_id' => $arguments['ORDERID'], 'pay_id' => $arguments['PAYID'], @@ -559,13 +516,22 @@ function uc_ogone_return_ok($auto = FALSE) { 'sha1_test_succes' => 'OK', ); drupal_write_record('uc_payment_ogone', $payment); + + uc_order_update_status($payment->order_id, uc_order_state_default('payment_received')); + // This lets us know it's a legitimate access of the complete page. + $_SESSION['cart_order'] = $payment->order_id; + $_SESSION['do_complete'] = TRUE; + if (!$auto) { - drupal_goto('cart/ogone_ok'); + drupal_goto('cart/checkout/complete'); } } else { + // Payment was not successful. + watchdog('uc_ogone_payment', 'Ogone returned an error for this payment.', NULL, WATCHDOG_ERROR); if (!$auto) { - drupal_goto('cart/ogone_false'); + drupal_set_message(t('Ogone returned an error for your payment. Your order is not completed. Please try again or contact us.'), 'ERROR'); + drupal_goto('cart/checkout'); } } } -- 1.7.1 From 07086395499897eb5a18dd0344a84eaf21ba40d2 Mon Sep 17 00:00:00 2001 From: Neil Drumm Date: Mon, 25 Apr 2011 17:11:23 -0700 Subject: [PATCH 4/4] #1072500 Improve handling of duplicates. --- uc_ogone_payment.module | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/uc_ogone_payment.module b/uc_ogone_payment.module index a335ddd..7bdcb19 100644 --- a/uc_ogone_payment.module +++ b/uc_ogone_payment.module @@ -507,7 +507,7 @@ function uc_ogone_return_ok($auto = FALSE) { if (($sha1_key == $sha1_return_key) && in_array($arguments['STATUS'], $valid_stat)) { // Record the successful payment. - $payment = array( + $payment = (object) array( 'order_id' => $arguments['ORDERID'], 'pay_id' => $arguments['PAYID'], 'description' => 'Back-office', @@ -515,9 +515,21 @@ function uc_ogone_return_ok($auto = FALSE) { 'order_status' => $arguments['STATUS'], 'sha1_test_succes' => 'OK', ); - drupal_write_record('uc_payment_ogone', $payment); + // Duplicates happen, check if we have one. + if (db_result(db_query_range("SELECT 1 FROM {uc_payment_ogone} WHERE pay_id = %d", $payment->pay_id, 0, 1))) { + watchdog('uc_ogone_payment', 'Duplicate Ogone Pay ID @pay_id recieved.', array('@pay_id' => $payment->pay_id)); + // TODO The drupal_goto('cart/checkout/complete') below will redo some + // order operations, like sending invoices. However, the logic for + // messages and redirects in uc_cart_checkout_complete() and + // uc_cart_complete_sale() is complex enough that we do not want to + // replicate it here. http://drupal.org/node/644538 has a thorough + // discussion about this situation. + } + else { + drupal_write_record('uc_payment_ogone', $payment); + uc_order_update_status($payment->order_id, uc_order_state_default('payment_received')); + } - uc_order_update_status($payment->order_id, uc_order_state_default('payment_received')); // This lets us know it's a legitimate access of the complete page. $_SESSION['cart_order'] = $payment->order_id; $_SESSION['do_complete'] = TRUE; -- 1.7.1