This is related to #1734502: Tax calculation errors when more than one discount coupon applied.
uc_taxes currently calls the "tax_adjustment" callback for each line item, but doesn't provide information as to which line item is being processed. This makes it difficult for modules which may provide more than one line item to determine which one should be adjusted.
Patch to follow adjusts this behavior so that each callback is invoked only once for all line items which it manages. The alternative would be to pass the whole line-item id to the implementor. That's probably cleaner, but represents an API change. This way, only uc_taxes has to be changed.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 1856596-tax-adjustment.patch | 1.27 KB | wodenx |
Comments
Comment #1
wodenx commentedComment #2
longwave$tax is passed to the adjustment callback, isn't it possible to use this to figure out which of the tax line items is currently being adjusted?No, I see the problem now. But wouldn't it be easier to add $line_item as an extra parameter to the callback? Current users of the callback (if any, other than uc_coupon) will function as they did before, and uc_coupon can use this to figure out which line item is being altered. This seems cleaner than calling this once only, for the arbitrary first line item that the module provides, although I guess it may complicate uc_coupon's calculations?
Comment #3
wodenx commentedYes, I agree it would be cleaner, and yes it would complicate uc_coupons calculations. I suspect there are no other users of the callback, but if there were I suspect they would also be subject to the same bug, and have to be refactored to use the new $line_item parameter. As I said, this way is not as clean, but only requires a change in uc_taxes. Also, adding a third parameter represents an API change (albeit a harmless one).
With all that said, though, if you think it's worth taking the cleaner route, I will update uc_coupon accordingly.
Comment #4
longwaveYes, I would like to take the cleanest route in Ubercart core even if it means complicating uc_coupon; it seems to make sense to call the adjustment for each line item instead of once only.
Comment #5
rakun commentedThis is still issue with ubercart 7.x-3.10 and uc_coupon 7.x-2.1-alpha7. Any progress here?