Role based product discount calculating discount and price in order administration based on current user role

Interesting. Sounds like a bug I've seen, but I hadn't spotted that the current user's roles were being used.

There's probably a place in the code where the global $user object is being used, and shouldn't be.

I can also confirm this behavior.
A user with a specified role discount is shown the discount at checkout, but the admin page shows no discounts.

I confirm the same. I am giving discount on each product, based on user role. As admin I see orders have a different total, based on the role the admin user. I am going to look for global $user as suggested in #1

The culprit should be the function uc_discount_price_handler_alter which contains a global $user.

I guess a workable solution is adding those lines to the function

if($user->uid != $context['subject']['order']->uid)
$my_user = user_load($context['subject']['order']->uid); // somebody is watching somebody else's order
else
$my_user = $user;

replacing any occurrence (there is actually 1 in my version) of $user in such function with $my_user .

I can confirm that this code will fix the issue. Replace uc_discount_price_handler_alter() with the following code in uc_discount.module. Clear caches and you're on your way.

function uc_discount_price_handler_alter(&$price_info, &$context, &$options) {
  
  global $user;
  static $prices = array();
  
  if($user->uid != $context['subject']['order']->uid)
  $account = user_load($context['subject']['order']->uid); // somebody is watching somebody else's order
  else
  $account = $user;

  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':
           if (isset($context['subject']['node']) && is_object($context['subject']['node'])){
                  $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':
      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, $account);
    $prices[$cache] = $node->sell_price;
  }
  else {
    $node->sell_price = $prices[$cache];
  }

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

Nice work finding the bug, mcarrera!

Does anyone have time to make a patch of that change so it's easier to test it out?

Also, usual practice for a user account that might not be the current user is to call it $account.

@davident: thanks! This needs to be posted as a patch so that it can be reviewed though -- also note Drupal coding standards to do with things like always bracketing if and else blocks, spacing, and indentation.

Hello, the following piece of code is bogus, as it doesn't work for product discounts (compared to order discounts for which it should be fine)

if($user->uid != $context['subject']['order']->uid)
  $account = user_load($context['subject']['order']->uid); // somebody is watching somebody else's order
  else
  $account = $user;

problem is $context['subject']['order'] doesn't exist when you are just watching a product. So, if like in my case, you are giving a discount on products based on user role, no discount will be applied.
A workaround I found is

if($context['subject']['order']->uid)  { // we have an order
        if($context['subject']['order']->uid != $user->uid)  {
	    $account = user_load($context['subject']['order']->uid); // somebody is watching somebody else's order
	  }
	  else  {
 	   $account = user_load($user->uid);
 	  }
   } 
   else {// there is no order
     
      if($context['account']->uid != $user->uid) { 
       
        $account = user_load($context['account']->uid);
      }
      else  {
        $account = user_load($user->uid);
      }
   }

Interesting stuff, and thanks for looking at it.

But could people please post patches too?

Ok, here is a patch for the changes as at #8

Did anybody had a chance to test the above patch? I applied it in a pilot project which never went on production

I tested the patch and it works as advertised.

+++ uc_discount.module	2011-02-09 15:15:19.136546001 +0100
@@ -124,10 +144,12 @@ function uc_discount_price_handler_alter
-      $node = clone $context['subject']['node'];
-      $item = $context['subject']['cart_item'];
-      $cache = 'cart_item:'. $node->nid .':'. $price_info['price'] .':'. serialize($item->data);
-      break;
+           if (isset($context['subject']['node']) && is_object($context['subject']['node'])){
+                  $node = clone $context['subject']['node'];
+                  $item = $context['subject']['cart_item'];
+                  $cache = 'cart_item:'. $node->nid .':'. $price_info['price'] .':'. serialize($item->data);
+           }
+        break;

Is this bit actually related to this bug, or is it a fix for something else? Could someone confirm?

In the meantime, here's a reroll for code style and comments.