I notice the discounts were being applied 2x.
Config:
Ubercart 2.2
userpoints_ubercart 6.x-2.x-dev 2010-02-28

Cause:
This module's hook_line_item() defines the discount line item as 'calculated' => TRUE.
According to the Ubercart API (http://www.ubercart.org/docs/api/hook_line_item):

calculated: Whether or not the value of this line item should be added to the order total. (Ex: would be TRUE for a shipping charge line item but FALSE for the subtotal line item since the product prices are already taken into account.)

Then, this module defines hook_order() with the operation 'total' which then subtracts any discounts... but the discounts have already been subtracted with the line item definition of 'calculated'. This causes the discounts to be subtracted 2x.

The attached patch removes the case 'total' from the hook_order(). Also there was a local variable in hook_order $paymethod = strtolower($order->payment_method); that was never used. During my debugging I replaced all strtolower($order->payment_method) with $paymethod.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vood002’s picture

I can confirm the double discounting issue.

Recrit--I believe you forgot to attach the patch you referred to =)

recrit’s picture

i thought it was there... here it is again. We should have a filter like gmail has where it asks you if you wanted to attach something if your email has "attach" in it.

Hope the patch fixes your issue too.

betz’s picture

Hey recrit, thanks for the patch.
I am fully aware that this module is a big mess.
But i will rewrite the whole module from the ground up in the soon future.

Thanks for your contribution Recrit

vood002’s picture

I've tested this patch and it worked for me--no more double discounts.

The patch, however, failed to apply for me (I had to manually apply lines from chunks 2 and 4). Here is the .rej file I received

***************
*** 228,234 ****
  			$ptmaxd = uc_userpoints_discount_max();
  			$ptmaxdPoints = $ptmaxd * $multiplier;
  
- 			if (variable_get(USERPOINTS_DISCOUNT,1) != 0 && strtolower($order->payment_method) != 'points') {
  				if ((!empty($ptamt) || $ptamt != '') && $ptamt < $orderamt && $ptdisc <= min($curUserPoints, $ptmaxdPoints)) {
  					db_query("DELETE FROM {uc_order_line_items} WHERE order_id=%d AND type='ptdiscount'", $arg1->order_id);
  					db_query('INSERT INTO {uc_updiscounts} (uid, oid, ptamt, points) VALUES (%d, %d, \'%f\', %d)', $curUserId, $arg1->order_id, $ptamt, $points);
--- 217,223 ----
  			$ptmaxd = uc_userpoints_discount_max();
  			$ptmaxdPoints = $ptmaxd * $multiplier;
  
+ 			if (variable_get(USERPOINTS_DISCOUNT,1) != 0 && $paymethod != 'points') {
  				if ((!empty($ptamt) || $ptamt != '') && $ptamt < $orderamt && $ptdisc <= min($curUserPoints, $ptmaxdPoints)) {
  					db_query("DELETE FROM {uc_order_line_items} WHERE order_id=%d AND type='ptdiscount'", $arg1->order_id);
  					db_query('INSERT INTO {uc_updiscounts} (uid, oid, ptamt, points) VALUES (%d, %d, \'%f\', %d)', $curUserId, $arg1->order_id, $ptamt, $points);
***************
*** 289,295 ****
  			}
  			break;
  		case 'delete':
- 			if (variable_get(USERPOINTS_DISCOUNT,1) != 0 && strtolower($order->payment_method) != 'points') {
  				global $user;
  				$curUserId	= $user->uid;
  				$oid			= $order->order_id;
--- 278,284 ----
  			}
  			break;
  		case 'delete':
+ 			if (variable_get(USERPOINTS_DISCOUNT,1) != 0 && $paymethod != 'points') {
  				global $user;
  				$curUserId	= $user->uid;
  				$oid			= $order->order_id;

YK85’s picture

Hello betz,

I was wondering if there is a thread where we may follow the progress of the re-write of this module so the community can possibily help out with testing etc that is needed.

Looking forward to following your work!

RoloDMonkey’s picture

yaz085,

To see the Issues for this module, go here:

http://drupal.org/project/issues/userpoints_ubercart

If you want to help with testing Ubercart Userpoints, go to the link below and download one of the versions that end with "-dev".

http://drupal.org/project/userpoints_ubercart

You may also notice that both of these links are in the breadcrumb at the top of the page :)

bsmith451’s picture

Just an FYI of what worked for me.

I manually inserted these 2 rejected chunks on my patch copy and the double discount error is fixed on my install.

gollyg’s picture

Re-rolled the patch - no coding changes

PieterDC’s picture

I've never seen a patch in this format, I usually follow the instructions from http://drupal.org/patch/create

But your patch solves the / my problem. Thanks!

pitxels’s picture

Patch worked for me too

gynekolog’s picture

thank you, worked ;)

bmagistro’s picture

Status: Needs review » Active

I would like to get this issue resolved so that I can maybe clean up a few things or start on a D7 port depending upon what users would like but I cannot reproduce this using the latest dev release. Can someone confirm this is still happening and help me pinpoint where it is occurring?

juankvillegas’s picture

Status: Active » Reviewed & tested by the community

I applied the patch and the problem is fixed.

In a side note... I applied the patch manually because it's in a format that I didn't know and the lines doesn't match... I applied it to the beta1 version

bmagistro’s picture

Hmm... I wish I knew why mine wasn't duplicating that behaviour. I will look at getting this patched and added in this weekend. Thanks for testing.

davipilot’s picture

Status: Reviewed & tested by the community » Needs review

I still believe this not to be fixed.

See http://drupal.org/node/1056886

bmagistro’s picture

Status: Needs review » Fixed

I still haven't found a way to reproduce this but I have applied the patch and commited it to the latest dev release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.