Incorrect argument handling in uc_line_item_tax_subtotal() leads to notices

This resolves the problem by first checking if the object property exists, and fixes a similar problem in line 253.

I cannot deny that there may be a problem elsewhere, I'll dig into it further and see what I can find.

Digging into it a little, I've found that when uc_line_item_tax_subtotal() is executed with $op='load' that the $order has turned into a StdClass instead of a UcOrder.

Ah. When $op == 'cart-preview' it seems to be passed the $order->products array rather than the $order object itself.

When the checkout page loads it executes _line_item_list() and then searches the line items for any elements with a callback. With my site's current setup there are five callbacks: uc_line_item_subtotal, uc_line_item_generic, uc_line_item_tax_subtotal, uc_line_item_tax and uc_line_item_total; of these, the uc_line_item_generic function doesn't exist. This array is ran through the following:

  foreach ($list as $line_item) {
    if (isset($line_item['callback']) && function_exists($line_item['callback'])) {
      $line_item['callback']('cart-preview', $items);
    }
  }

This is what uc_line_item_total() looks like:

function uc_line_item_total($op, $arg1) {
  switch ($op) {
    case 'display':
      $lines[] = array(
        'id' => 'total',
        'title' => t('Total'),
        'amount' => uc_order_get_total($arg1),
      );
      return $lines;
  }
}

For comparison, here's uc_line_item_tax():

function uc_line_item_tax($op, $order) {
  switch ($op) {
    case 'load':
      $lines = array();
      $taxes = uc_taxes_calculate($order);
      foreach ($taxes as $tax) {
        $lines[] = array(
          'id' => ($tax->summed ? 'tax' : 'tax_included'),
          'title' => $tax->name,
          'amount' => $tax->amount,
          'weight' => variable_get('uc_li_tax_weight', 9) + $tax->weight / 10,
          'data' => $tax->data,
        );
      }
      return $lines;
  }
}

Here's the function prototype for uc_line_item_tax_subtotal():

function uc_line_item_tax_subtotal($op, $order) {

You'll notice that uc_line_item_total() calls the second argument $arg1, whereas uc_line_item_tax() and uc_line_item_tax_subtotal() call it $order. I think this is the key problem, the second argument should be called $arg1 and handled differently depending on $op because clearly the value passed to it is different based on $op.

Reading into it further, I think this may be a symptom of the problem identified in #1281346: tax_subtotal line item no longer alterable -- breaks compatibility with uc_coupon.

Updating the title.

Related: #1740944: Use error_level=2 for all local development

I reviewed the code in 7.x-3.x and it seems the API is quite different than in 6.x-2.x, so I'm dragging this back to the world of "a legacy D6 problem".

Just to reiterate the core problem - Ubercart's API was changed but uc_line_item_tax() and uc_line_item_tax_subtotal() were not properly updated to match.

Food for thought.

An alternative patch that bundles all of the logic into the switch() statement - I'm not familiar enough with the codebase to know which is the correct path.