For details of this potential bug please read: http://www.ubercart.org/forum/support/16932/addon_payments_charging_double
It appears that the $amount variable is ignored in hook_charge. I don't have a payflow pro account, so I'm a little wary to create a patch when I can't test it. I think all you need to do is pass $amount on to _uc_payflowpro_invoice_xml, and replace $order->order_total with $amount, but I'm not sure about what to do with the <TaxAmt> on line 1064
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | uc_payflowpro.module.patch | 2.56 KB | tstackhouse |
Comments
Comment #1
kwinters commentedWhat specific steps are resulting in the order object's amount not being equal to the $amount passed in to the function? It seems like you're editing an order, but I need specifics.
I'm marking this feature request, since it's not something that is explicitly supported at this time and is unlikely to be a trivial change (particularly considering negative amounts).
Comment #2
tstackhouse commentedkwinters,
The following steps lead to this condition:
* Create an order and checkout as normal
* Using the store manager backend, edit the order to add one or more additional items, creating a balance due
* Attempt to process a charge for the balance using the virtual terminal
In this condition, the $order->order_total represents the new total of the order, not the charge amount being requested, $amount.
This could also affect the use of the virtual terminal to process partial payments.
Comment #3
Andy_Lowe commented$amount is designed to handle partial and additional charges, not refunds, so $amount should never be negative. I tested it, and even if the order total is negative, the $amount in the virtual terminal is set to $0. Still, some crazy admin could manually enter a negative number. If you want to, just check and fail if $amount is negative. Tstackhouse hopefully provided enough info, so I'm setting back to active.
Comment #4
tstackhouse commentedI've done some research on the XMLPay API, and it looks like everything beyond
<TotalAmt>is optional, informational data, and I can't find any reference that that data is used in a validation context. I'm going to try rolling a patch similar to what andy_lowe sugdested and I'll submit it back here.Comment #5
kwinters commentedDon't worry about a patch. I'm working on 2.0 already so I'll take care of it there, and I don't plan on making major changes to 1.x in the future.
Comment #6
tstackhouse commentedOkay. I won't bother to submit the fix I make than, however I'm still going to fix this on my own because I need to have this up and running ASAP for a client project. At any rate, thanks for all your work on this that made it possible for this project to even get off the ground!
Comment #7
tstackhouse commentedI was able to take Andy's suggestion and ran through a test scenario that replicated the issue, and these changes fixed the problem. I'm submitting a patch anyway, hopefully they can be of help while working on 2.0.
Comment #8
kwinters commentedIt's pretty close to what I was going to do anyway, but maybe someone else can make use of it until 2.x is production-worthy.
The only thing I'm not sure about is the special conditioning handling. I've had other requests for converting a $0 charge into a $1 authorization (for later charging) and similar things, so it's not a safe assumption that $0 = fail. It's probably fine for your case though.
Comment #9
tstackhouse commentedI see. It may be useful to make that behaviour a configurable option, than tweak the code accordingly.
That's precisely why I submitted the patch, hopefully someone else will find it useful.
Comment #10
kwinters commentedI wouldn't spend a lot of time on that. The edge cases can be very different depending on what kind of site you're trying to run (subscriptions, physical products, etc.) so I plan on just making the system more hookable. See #821754: Allow for authoriztion of card if order total is zero for the original discussion.