This error arises when viewing the checkout page:

Notice: Trying to get property of non-object in uc_line_item_tax_subtotal() (line 246 of uc_taxes.module).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna’s picture

Status: Active » Needs review
FileSize
709 bytes

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

TR’s picture

Status: Needs review » Postponed (maintainer needs more info)

How can you have an order with no products?

This to me seems like a classic case of covering up the symptoms rather than finding the cause. $order->products should *always* have something in it on the checkout page. If it doesn't, there's a more fundamental problem - perhaps with some contributed module. Those same conditionals are in Ubercart 7.x-3.x, but I've never seen this PHP notice in either version, nor has it been reported previously in the issue queue.

DamienMcKenna’s picture

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

DamienMcKenna’s picture

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.

DamienMcKenna’s picture

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

DamienMcKenna’s picture

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.

DamienMcKenna’s picture

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.

DamienMcKenna’s picture

Title: Notice: Trying to get property of non-object in uc_line_item_tax_subtotal() (line 246 of uc_taxes.module) » Incorrect argument handling in uc_line_item_tax_subtotal() leads to notices
Version: 6.x-2.x-dev » 7.x-3.x-dev
Status: Postponed (maintainer needs more info) » Active

Updating the title.

DamienMcKenna’s picture

DamienMcKenna’s picture

Version: 7.x-3.x-dev » 6.x-2.x-dev

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.

DamienMcKenna’s picture

Status: Active » Needs review
FileSize
2.23 KB

Food for thought.

DamienMcKenna’s picture

FileSize
2.71 KB

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.

longwave’s picture

In the current code, the cart-preview case is not used as $different is always FALSE, and therefore seems like it can be safely removed, which would simplify this even further.

#644840: subtotal excluding tax line item logic is wrong is also related to this function.