I was trying to find a solution for the bug that shipping method didn't appear in the Invoice. Looking through the relevant code, I got really confused by the use of #key and #method. Because of this confusion some newly made shipcalc partner modules, have errors that cause the shipping method disappear in the Invoice.
Some code snippets to illustrate the confusion:
First I see this.
case 'save':
$method = split('__', $txn->shipping_method);
$txn->ship = array(
'service' => $method[0],
'method' => $method[1],
'cost' => $method[2],
'currency' => $method[3]
);
$txn->screen++;
break;
This shows that the second shipping parameter, separated by underlines, is the shipping method. But then further down I see:
if (sizeof($rates) == 1) {
// Only one shipping method, auto select
$rate = array_pop($rates);
$txn->shipping_method = $rate['#service'] .'__'. $rate['#key'] .'__'. $rate['#cost'] .'__'. $rate['#currency'];
shipping_checkoutapi($txn, 'save');
return;
}
foreach ($rates as $rate) {
$options[$rate['#service'] .'__'. $rate['#key'] .'__'. $rate['#cost'] .'__'. $rate['#currency']] = t('%price %currency - %method', array('%price' => $rate['#cost'], '%currency' => $rate['#currency'], '%method' => $rate['#method']));
}
Here the second parameter is called #key while we saw before that it is the shipping method. This has confused some shipcalc developers into wrongly use use #key.
So I wonder here if we cannot simply get rid of #key here and use only #method, less confusing and says better what it is.
In the last code I see that at the moment #method is nothing else then what shows up in the checkout form shipping choice. This seems an unneeded extra, because this can be composed of #service and #method like shown here:
if (sizeof($rates) == 1) {
// Only one shipping method, auto select
$rate = array_pop($rates);
$txn->shipping_method = $rate['#service'] .'__'. $rate['#method'] .'__'. $rate['#cost'] .'__'. $rate['#currency'];
shipping_checkoutapi($txn, 'save');
return;
}
foreach ($rates as $rate) {
$options[$rate['#service'] .'__'. $rate['#method] .'__'. $rate['#cost'] .'__'. $rate['#currency']] = t('%price %currency - %service %method', array('%price' => $rate['#cost'], '%currency' => $rate['#currency'], '%service' =>$rate['#service'] '%method'=>$rate['#method']));
}
When we use #method for what it means then it looks all more readable avoiding mistakes of developers. If you agree with this change, I can produce patch to 4.7, the version I am working on.
Her some sample shipping partner code to illustrate the improved readability :
$rates[correos] = array( //array key as defined in in correos_shipping_methods
'#service' => 'correos', //this is the shipping service key as defined in correos_shipping_methods
'#method' => 'paquete_azul', //this is the method key as defined in correos_shipping_methods
'#cost' => $cost,
'#currency' => variable_get('payment_symbol', '€')
);
Comments
Comment #1
brmassa commentedRené,
the issue is just too old. if the issue remains relevant, please, reopen it.
regards,
massa