Invoice prices on old orders changed with uc_discount enabled

Ran into this issue. Product discounts keep on recalculating after products are added to the order.

I created a fix for this issue, by saving the calculated discount directly to the price for the product order lines:

in the uc_discount.module (Will supply a patch when time permits)

function uc_discount_price_handler_alter(&$price_info, &$context, &$options) {
  global $user;
  static $prices = array();
  switch ($context['type']) {
    case 'product':
      if (isset($context['field']) && $context['field'] != 'sell_price') {
        // Don't discount list_price or cost.
        return;
      }
      $node = clone $context['subject']['node'];
      $cache = 'node:'. $node->nid .':'. $price_info['price'];
      break;
    case 'cart_item':
      $node = clone $context['subject']['node'];
      $item = $context['subject']['cart_item'];
      $cache = 'cart_item:'. $node->nid .':'. $price_info['price'] .':'. serialize($item->data);
      break;
    case 'order_product':
      // Added by TØH: 2010-07-11
      if (isset($context['subject']['order']->order_id) && ($context['subject']['order']->order_id > 0)) {
        // The order is saved so the modified price was saved with the order
        return;
      }
      // End TØH: 2010-07-11
      if (isset($context['subject']['node'])){
        $node = clone $context['subject']['node'];
        $item = $context['subject']['product'];
        $cache = 'order_product:'. $node->nid .':'. $price_info['price'] .':'. serialize($item->data);
      }
      else if (isset($context['subject']['order_product'])){
        $order_product = clone $context['subject']['order_product'];
        $cache = 'order_product:'. $order_product->nid .':'. $price_info['price'] .':'. serialize($order_product->data);
      }
      break;
    default:
      // Nothing to do.
      return;
  }

  if (!isset($prices[$cache])) {
    // The discount action modifies the node's sell_price field.
    $node->sell_price = $price_info['price'];
    ca_pull_trigger('calculate_product_discounts', $node, $user);
    $prices[$cache] = $node->sell_price;
  }
  else {
    $node->sell_price = $prices[$cache];
  }

  // Alter the price given in place.
  $price_info['price'] = $node->sell_price;
}

// Added by TØH: 2010-07-11
function uc_discount_order_product_alter(&$product, $order) {
  // We modify the price before saving the order.
  // TODO: Logic to handle and recalculate product discounts added directly to order
  if (!isset($product->data['uc_discount']['calculated'])) {
    $node = node_load($product->nid);
    $user = user_load($order->uid);
    $product->data['uc_discount']['original'] = $product->price;
    $node->sell_price = $product->price;
    ca_pull_trigger('calculate_product_discounts', $node, $user);
    $product->data['uc_discount']['calculated'] = $node->sell_price;
    $product->price = $node->sell_price;
  }
}
// End by TØH: 2010-07-11

I installed the UC Discount Framework on two websites running the same 25% off promotion. One one site had "Calculate order discounts", to give 25% off at the checkout. The other site had "Calculate Product Discounts", which showed the price of each product with a 25% discount. I did this because the first site was having trouble with calculating VAT as there are incompatibilities with the UC_VAT module.

For the second site, I received an email from the client saying that ALL of the previous orders in the system had their prices showing at 25% less than they were sold for. This was a problem for customers who could look at their past purchases. The company also allows customers to pay by invoice, but now all of the past invoices in Ubercart show the lower price, making it hard to charge the customers.

I tried to change the condition "Check the current date" to be after the date that the promotion started, but it still affected the product prices previous to this date. Even when I disabled this conditional action, it still showed the discounted price on all orders, however when I ran update.php it updated the prices to their original values. In the end, I had to apply the "Calculate order discounts" predicate on this site as well, and go into each order which was eligible for the discount and edit each order to include the "25% Off Promotion" line.

So it seems as though there is an issue with the predicate "Calculate Product Discounts", where it goes in and changes the price of products on old orders when you go to view them. Unless there's a solution I haven't tried yet?

Also using the newest dev version from 25th February 2011 and ran into this problem, so it's definitely NOT fixed.
The above modifications DO work.
The module without the patch above is just plain DANGEROUS. Depending on how often you add/remove discounts, you could very quickly have a complete accounting mess on your hands.

Be aware that if you make product changes to an existing order AFTER the patch is applied, your currently enabled discounts will be applied to the order!
If that order has already has been charged, then it's basically a record-keeping problem, but if the order hasn't been processed yet, you'll end up discounting an order incorrectly.

I had to fix about 30 orders today that were in a pending state (auth, no capture). The first bit of the patch would have eliminated the discount and the second bit would have added the 'calculated' amount at full price. I needed to comment out the top block to get the discount, update the orders (at which point the discount appeared to be applied twice), then re-enable the first block to get the proper amounts back.

Adding my voice to this:

The module as it stands now, with regard to the "calculate product discounts" feature, is broken. As the @millenniumtree states, this could be extremely dangerous from an accounting standpoint.

Without this patch, the full "sell price" of a product is saved to the database table {uc_order_products}. The reason is because, in uc_order_save() (in uc_order.module), there is a call to drupal_alter():

 if (isset($order->products) && is_array($order->products)) {
    foreach ($order->products as $product) {
      drupal_alter('order_product', $product, $order);
      uc_order_product_save($order->order_id, $product);
    }
  }

drupal_alter() looks for a function by this pattern: [module]_[type]_alter($data). So using the parameters above, the function uc_order_save looks for is hook_order_product_alter. In uc_discount's case, that function should be called uc_discount_order_product_alter(), which does not exist in the current module. (It is included in the code block in comment #2.)

You can verify this by creating a new Conditional Action which includes the Trigger "calculate product discounts" and the Action to "Apply a discount to a product". Create a new order, and then examine it. In the Orders interface, the correct discounted price is calculated. However if you view the {uc_order_products} rows for that order_id, all of the "price" column values are the original sell price. This would essentially skew our numbers, with no real (easy) way of getting the correct amounts in there; essentially accounting would "think" more money came in than actually did, if they were to run product reports from the raw database information.

I've attached a patch based on the code in #2, which I've tested in production and seems to work great. The price value in {uc_order_products} is correctly altered to be the discounted price, as it is during the checkout and product pages.

Setting to needs review to hopefully get maintainers' eyes on it.

Forgot to reset the status. Also setting it to critical, because I really hope there aren't a lot of other shops out there using the module this way, not knowing that discounted product amounts are being calculated but not saved to the database.