Stock transaction should be more flexible through Rules

Yes, uc_order has rules as dependencies and we could use rules_invoke_all() and this way uc_order will implement the hooks and events and stock module will implement actions and default rules using those events witch is better approach I think...

I'll start working on patches latter today...

Unfortunately I didn't use tests since now but I'll try to read some documentation...

uc_order module changelog:

In uc_order.admin.inc:

function uc_order_edit_form_submit($form, &$form_state) {
  ...
  $order->products = array();
  if (isset($form_state['values']['products']) && is_array($form_state['values']['products'])) {
    foreach ($form_state['values']['products'] as $product) {
      $product['data'] = unserialize($product['data']);
      $product = (object)$product;
      $product->order_id = $order->order_id;
      if (!isset($product['remove']) && intval($product['qty']) > 0) {
        $order->products[] = $product;
        $temp = $product->qty;
        $product->qty = $product->qty - $qtys[$product->order_product_id];
        rules_invoke_all('uc_order_product_update', $product);
        $product->qty = $temp;
        }
      }
      else {
        $product->qty = -$product->qty;
        rules_invoke_all('uc_order_product_update', $product);
        $log['remove_' . $product['nid']] = $product['title'] . ' removed from order.';
      }
    }
  }
  ...
}

in uc_order.rules.inc:

/**
 * Implements hook_rules_event_info().
 */
function uc_order_rules_event_info() {
  ...
  
  $events['uc_order_product_update'] = array(
    'label' => t('An order product is being updated'),
    'group' => t('Order'),
    'variables' => array(
      'product' => array(
        'type' => 'uc_order_product',
        'label' => t('Product'),
      ),
    ),
  );

  return $events;
}

Maybe the best approach is to pass 2 args to event/hook uc_order_product_update $product and $updated_product and do the logic increase/decrease in hook like the one of stock?! I'm waiting for suggestions...

Changes for uc_order module...

I think we should pass to this new events also the $order object as 3rd param in order to let user manipulate stock action based on order status/state, total, etc....

Patch with $order object as event param.

I'm totally opposed to this approach unless you can demonstrate that it's able to properly deal with concurrency.

What you mean by this? The stock operation I think should be handled by Rules because user will have flexibility on how the system react based on any conditions...
I don't see where the problem is with concurency??? This approach, at least by now is only for orders edited by administrators and the checkout remain the same old way?!

Practical example:
For instance an operator could have some products that are not in stock anymore and when he finds out, basically when he tries to fulfill the order, he puts the order in a custom status like "Incomplete". Then he call the customer and tell him that the product is not available anymore, the customer wants other products from his order so the operator delete the out of stock product and change the order status to "Ready for shipping". The out of stock product is displayed on site that is out of stock untill operator delete it from order, then product is back in stock but in fact it is not. Should operator manually edit the stock of product?! I think not. What if you have an module that notifies customer by email of back in stock products on the out of stock product? The client will receive notification that the product has been restocked but in fact it is not...

This is the final unpolished approach to prove the concepts... It's working on every product add/edit/delete from admin. Take a look and tell me what you think?

The concurrency problem should be fixed at rules_invoke_all() call level so the uc_order will have to deal with when the event is called or not or at uc_stock_adjust() level for better results so I don't see why rules can't be used for conditions?!?!

Using rules we can solve the following issues:
#1034044: Add checkboxes to enable/disable stock level changes when editing orders (the admin can put the order in a status like "Returned Stockable" and "Returned Nonstockable" for example and select if stock adjust action is applied to order by order status, order date, order role, order shipping method, peyment method and so on. More flexible than checkbox where the user input is required)
#588746: After deleting/cancelling an order, stock is not incremented again (The user can put uc_stock_action_adjust on Order delete event)
#751994: Possible to decrement stock on add to cart instead of checkout? (Stock level can decrement on add item to cart event and increment when order status is "Abandoned")
#1234446: Publish a rules event when stock level changes for a product (Like longwave said in #3)

That's exactly my point - it seems like you're jumping into this without awareness of the need for concurrent programming when dealing with stock, and perhaps you're not familiar with the concept of concurrency in computer science either. There's a four year history of issues on ubercart.org and here which talk about all the problems that arise because concurrency has not been properly taken into account by the current uc_stock. Replacing or augmenting uc_stock with Rules like you suggest will make it much more likely that sites encounter these problems. Stock operations need to be transactional and journaled in order to meet even the most basic accounting standards. So yes, in your example if there's a discrepancy between your physical stock count with your electronic stock count there needs to be the equivalent of a ledger entry reconciling the two, because this reconciliation has tax implications among other things.

Fist of all Ubercart is not an accountancy software.
I have the following scenario at the store I manage:
I have one supplier (but this could be extended to unlimited level of suppliers) and our stock (wich is composed of products that client don't want anymore + returned products + products that we stock for faster shipping).
The supplier sends us his stock once per day in xls format. When we receive the supplier's stock, we upload it to the site as document custom node type with taxonomy term "Supplier stock" wich is child of "Stocks" term. On form submit, if tid has parrent "Stocks", a batch runs that on first operation read the stock file and save it to db in a table with the following format:
[TID][SKU] [PRODUCT NAME] [QTY] [PRICE - supplier list price]
A second operation sum the stock level using group by SKU and sum(QTY) and we get the real stock of the shop. If qty in uc_stock != summed qty then the uc_stock is set to sum(QTY) so the site display our stocks and our supplier(s) stock cumulative.
Our stock is exported from accountancy softer in xls format and uploaded to the site in same manner. The same batch is ran and the stock is updated on every stock document update.

Because of these scenario and because we don't have access to supplier's live stock there might be some cases when a product of the supplier's stock is out of stock without letting us know about this.

Some of our clients buy one of these products and other products that can be delivered. So we order to supplier all the products in orders in custom status, like "Confirmed", without knowing that one of those products are out of stock. When we receive the goods we find out that we didn't recieve one or more products that were ordered so we cannot fulfill orders with those products, so we put those orders in a custom state like "Incomplete" and we'll call customer latter to see if they want only products that we received without the ones out of stock.
If customer wants the order without the out of stock product(s) we manually remove them from orders, recalculate shipping and change the order status to "Processing" in order to be delivered. Obvious products from orders that are "Incomplete" should not be putted back in stock.

I don't see the imperative need of uc_stock to be journaled. Uc stock is a basic stock module, if users have hooks and the stock is not hardcoded they can make their own modules or their own set of rules. Almost any store has an accountancy software from which admins could export the real stock of the store and, using a custom module like ours, update the shop stock.

If you want to build an accountancy software the hole ubercart must be rewritten with accountancy standards in mind. Remember that accountancy laws in every country are different so no universal software could work worldwide.

By providing a rule based uc_stock you let user implement his own module that could accomplish accountancy standards or even using rules interface to calculate his stock, taxes or even journal his transactions, because uc provides the hooks and events for rules. Accountants could make their own rules.

Maybe a journaled stock module can be written as an optional module but not all users need it.

By the way my store is at: http://www.magazinulcuscule.ro