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

brmassa’s picture

Status: Active » Closed (fixed)

René,

the issue is just too old. if the issue remains relevant, please, reopen it.

regards,

massa