please find attached a patch for HEAD to fix initial payment status assignment (pending) when using a translated ecommerce.

this patch for HEAD also fix misc translation issues already fixed for 4.6, see http://drupal.org/node/31936

patch modifies cart.module, payment.module, product.module, and store.module.

thanks

CommentFileSizeAuthor
#13 ec-head-payment-status-transaction-workflow-fix.patch1.6 KBPierreM-1
#12 ec-head-payment-status-fix_3.patch777 bytesPierreM-1
#9 ec-head-payment-status-fix_2.patch789 bytessyllance
#2 ec-head-payment-status-fix_1.patch788 bytesñull
#1 ec-head-payment-status-fix_0.patch791 bytessyllance
ec-head-translation-fix_0.patch7.33 KBsyllance

Comments

syllance’s picture

syllance commented
StatusFileSize
new ec-head-payment-status-fix_0.patch791 bytes

here's a new patch that only fix the payment status problem.

function payment_build_status() returns translated strings, so t() calls were removed in payment_get_status() and payment_get_status_id(), fixing the initial payment status bug when using translated ecommerce.

fix for translation issues was moved to a translation related thread, here : http://drupal.org/node/34096

ñull’s picture

ñull commented
StatusFileSize
new ec-head-payment-status-fix_1.patch788 bytes

In your payment status patch I discovered something that imho looks incorrect to me:

function payment_get_status_id($name) {
-  return array_search(strtolower(t('%payment-status', array('%payment-status' => $name))), payment_build_status());
+  return array_search(strtolower(t($name)), payment_build_status());
 }

should be:

function payment_get_status_id($name) {
-  return array_search(strtolower(t('%payment-status', array('%payment-status' => $name))), payment_build_status());
+  return array_search(strtolower($name), payment_build_status());
 }
syllance’s picture

syllance commented

after a quick look, i don't think the t() should be removed.

payment_get_status_id() is called using untranslated status strings, such as : payment_get_status_id('pending') or ('failed'). the function uses array_search to find the index of its parameter (untranslated status string) in the payment status array returned by payment_build_status. this array contains translated strings, so i think the array_search should be done with both translated strings. that's why the t() is imho needed (and was there in the original code).

my last comment is wrong (writtent quickly ... sorry), but my patches should be correct.

are you facing any issue due to this particular t() call ?

ñull’s picture

ñull commented

It is my suspicion that a problem I see with Paypal transactions have to do with it. I use Spanish localized version and your patches drew my attention.

With Paypal sandbox transactions I saw a blank payment status and did not I see them updated through IPN. I am still fiddling around with it and will report if that was the problem or not.

ñull’s picture

ñull commented

Paypal sandbox was down today. I could not continue my tests :-(
I will continue after the weekend.

syllance’s picture

syllance commented

while playing with the latest 4.6 e-commerce, which now contains backported HEAD code, i faced again the issue of payment status not being set, and the _0 patch fixes it.

the patch for head now also applies to 4.6.

syllance’s picture

syllance commented

there's a similar issue with workflow status, and i've sent a similar patch (http://drupal.org/node/36284). my setups still works fine with the _0 patch of this issue, although i did not yet played very much with paypal.

com2, did you find anything new on this ? is the paypal problem happening after applying my _0 patch ?

ñull’s picture

ñull commented

Yes, sorry for the late response. I found another error that caused my problem, in the Paypal module. See here for more details: http://drupal.org/node/36083 . If you have access to CVS, could you please include it after verifying?

syllance’s picture

syllance commented
StatusFileSize
new ec-head-payment-status-fix_2.patch789 bytes

ok. thanks for the update com2. i'm posting a new patch that applies to latest HEAD.

it does the same job than my first patch, it was just updated to follow HEAD changes.

If you look at the issues postings, you'll see that there are a lot patches that would be added to CVS if i had an account :-)

syllance’s picture

syllance commented

the patch also applies to 4.6.

ñull’s picture

ñull commented

When I apply this patch I still get incorrectly zero totals on the store overview page, only when I set the user to another language then English. Then when I add t( ) around the status strings in the get...id functions: 

/**
 * Default view of the admin store link.
 *
 * Generates statistical data of the store with links to view and the details
 * of transactions.
 */
function store_admin_overview() {

  $num_pending    = db_result(db_query("SELECT COUNT(txnid) FROM {ec_transaction} WHERE payment_status = '%d' AND (workflow = '%d' OR workflow = '%d' OR workflow = '%d' OR workflow = '%d')", payment_get_status_id(t('pending')), transaction_get_workflow_id(t("transaction received")), transaction_get_workflow_id(t("invoiced")), transaction_get_workflow_id(t("shipped")), transaction_get_workflow_id(t("awaiting customer response"))));
  $num_processing = db_result(db_query("SELECT COUNT(txnid) FROM {ec_transaction} WHERE payment_status = '%d' AND (workflow = '%d' OR workflow = '%d' OR workflow = '%d' OR workflow = '%d')", payment_get_status_id(t('completed')), transaction_get_workflow_id(t("transaction received")), transaction_get_workflow_id(t("invoiced")), transaction_get_workflow_id(t("shipped")), transaction_get_workflow_id(t("awaiting customer response"))));
  $num_completed  = db_result(db_query("SELECT COUNT(txnid) FROM {ec_transaction} WHERE payment_status = '%d' and workflow = '%d'", payment_get_status_id(t('completed')), transaction_get_workflow_id(t("completed"))));

only then I see the totals correctly in the other language. This, however, is difficult to understand when you see that both functions (transaction_get_workflow_id and payment_get_status_id) translate the parameter already:

function transaction_get_workflow_id($name) {
  return array_search(strtolower(t('%transaction-status', array('%transaction-status' => $name))), store_transaction_workflow());
}

function payment_get_status_id($name) {
  return array_search(strtolower(t($name)), payment_build_status());}

Can you explain why there would be a need to translate twice, once outside and once inside these functions? And when you understand it, can it be done better? I did not want to upload a patch until I understand what I am doing.

PierreM-1’s picture

PierreM-1 commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new ec-head-payment-status-fix_3.patch777 bytes

The latest patch (ec-head-payment-status-fix_2.patch) works for me and fixes a bug with cod.module not changing the payment status to completed (when the option is selected in cod settings), as the bug makes payment_get_status_id returning nothing.

I am not sure if the "strtolower" call in the patch is necessary since payment_build_status returns a case-untouched array. I am correcting this in ec-head-payment-status-fix_3.patch.

I think this patch can be commited.

PierreM-1’s picture

PierreM-1 commented
Title: initial payment status not set when using translated ecommerce » transaction workflow and payment status functions not working when using translated ecommerce
StatusFileSize
new ec-head-payment-status-transaction-workflow-fix.patch1.6 KB

There is a similar issue with transaction_get_workflow functions. It breaks the store admin overview.

Here is a patch against HEAD that corrects both transaction workflow and payment status functions.

sime’s picture

sime
any/any
Location Melbourne
commented
Component: payment.module » payment

bumping - a lot of work went into this it seems

brmassa’s picture

brmassa commented
Status: Reviewed & tested by the community » Closed (fixed)

Its simply too old. anyone that wants to continue the issue, reopen it.