multiple_coupons issue

[aeq-dev]

Hello,
Thanks for this awesome package,
I'm trying to add multi coupons with different types (fixed, percentage),
I'm able to add them to the cart and display them,
The problem is : The cart totals are incorrect, it only takes the last coupon discount in consideration.
Example:
Cart Total : 200
Coupon 1 : Fixed, 50
Coupon 2 : Fixed, 30

The final total : 170, but it should be 200 - 50- 30 = 120
Also if we apply tax, it give wrong totals :/

Thanks

[aeq-dev]

It works if I update this function in LaraCart :

private function updateDiscounts()
    {
        // reset discounted
        foreach ($this->getItems() as $item) {
            $item->discounted = [];
        }
        // go through each item and see if they have a coupon attached
        foreach ($this->getItems() as $item) {
            if ($item->coupon) {
                $item->coupon->discounted = 0;
                for ($qty = 0; $qty < $item->qty; $qty++) {
                    $item->coupon->discounted += $item->discounted[$qty] = $item->coupon->discount($item->subTotalPerItem(false));
                }
            }
        }
        // go through each coupon and apply to items that do not have a coupon attached
        $coupon_item = null;
        foreach ($this->getItems() as $item) {
            if (!$item->coupon) {
                $coupon_item = $item;
                break;
            }
        }
        $i = 0;
        foreach ($this->getCoupons() as $coupon) {
            $coupon->discounted = 0;
            $coupon->discounted += $coupon_item->discounted[$i] = $coupon->discount($this->subTotal(false));
            $i++;

            if (config('laracart.discount_fees', false)) {
                // go through each fee and discount
                foreach ($this->getFees() as $fee) {
                    $coupon->discounted = $fee->discounted = $coupon->discount($fee->amount);
                }
            }
        }
    }

[lukepolo]

can you create a test , and ill look at it here soon

[aeq-dev]

Sorry I'm not able to figure out how this could be done, but I can send PR.
Also, regarding tax calculation, I have this use case :

1. Add product price 500 ==> Cart Total : 500
2. Add coupon 1 :
   Coupon 1 : Fixed 60 ==> Cart Total : 500 - 60 = 440
3. Tax auto calculated : 15 % ==> 440 * 15 / 100 = 66 ==> Cart Total : 440 + 66 = 506
4. Add coupon 2:
   Coupon 2 : Fixed 40 ==> Cart Total : 506 - 40 = 466
5. Add coupon 3 :
   Coupon 3 : Percentage 10 % ==> 500 * 10 / 100 ==> 50 ==> Cart Total : 466 - 50 = 416

Is this correct ? I think the correct totals should be like this :
Cart Total - sum(coupons) + tax
Cart Total - sum(coupons) = 500 - (60+40+50) = 350
Tax = 350 * 15 / 100 = 52.5
Final Total = 350 + 52.5 = 402.5

Am I correct ? thanks

[lukepolo]

im guessing your using the wrong totals to get what your asking for , there are a lot of different totals , also taxes are complicated and theres alot of configurations for it.

Pre taxed coupons vs non pretaxed etc.

ill make it a test and verify what your saying

[aeq-dev]

Thanks for quick reply,
If you don't mind, I can share with you my work (via google meet share screen) and check what I'm doing as use cases

[lukepolo]

yah sure that would be helpful!

[lukepolo]

sorry i didnt read all of that, i cant meet right now, but ill make a test and post it here ~

[lukepolo]

ok i found the issue, multiple coupons arent working as intended , as you pointed above your fix works

[lukepolo]

related #314

[lukepolo]

ive added the test and verified the fix works, i changed the code slightly from what you had

[aeq-dev]

Thanks,
Other thing, what if an item has its own coupon ? I didn't test it yet and I think this piece of code should be fixed too :

foreach ($this->getItems() as $item) {
           if ($item->coupon) {
               $item->coupon->discounted = 0;
               for ($qty = 0; $qty < $item->qty; $qty++) {
                   $item->coupon->discounted += $item->discounted[$qty] = $item->coupon->discount($item->subTotalPerItem(false));
               }
           }
       }

[lukepolo]

i have an issue with taxes , im still working on it , not sure how long it will take

[lukepolo]

my current version of the main branch has broken tests so its making this kinda difficult

[lukepolo]

1) CouponsTest::testSetDiscountOnItem
Failed asserting that 96.3 matches expected 96.30000000000001.

/Users/luke/Code/opensource/laracart/tests/CouponsTest.php:238
phpvfscomposer:///Users/luke/Code/opensource/laracart/vendor/phpunit/phpunit/phpunit:60

2) CouponsTest::testFeeDiscount
Failed asserting that 15.0 matches expected 10.

/Users/luke/Code/opensource/laracart/tests/CouponsTest.php:436
phpvfscomposer:///Users/luke/Code/opensource/laracart/vendor/phpunit/phpunit/phpunit:60

3) ItemsTest::testItemPriceAndQty
Failed asserting that 32.099999999999994 matches expected 32.1.

/Users/luke/Code/opensource/laracart/tests/ItemsTest.php:178
phpvfscomposer:///Users/luke/Code/opensource/laracart/vendor/phpunit/phpunit/phpunit:60

4) SubItemsTest::testTaxSumary
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    '0.01' => 0.1
-    '0.02' => 0.2
+    0 => Array (...)
 )

/Users/luke/Code/opensource/laracart/tests/SubItemsTest.php:322
phpvfscomposer:///Users/luke/Code/opensource/laracart/vendor/phpunit/phpunit/phpunit:60

5) TotalsTest::testTaxationOnCoupons
Failed asserting that 419.02 matches expected 419.12.

/Users/luke/Code/opensource/laracart/tests/TotalsTest.php:342
phpvfscomposer:///Users/luke/Code/opensource/laracart/vendor/phpunit/phpunit/phpunit:60

[aeq-dev]

Thanks, Other thing, what if an item has its own coupon ? I didn't test it yet and I think this piece of code should be fixed too :

foreach ($this->getItems() as $item) {
           if ($item->coupon) {
               $item->coupon->discounted = 0;
               for ($qty = 0; $qty < $item->qty; $qty++) {
                   $item->coupon->discounted += $item->discounted[$qty] = $item->coupon->discount($item->subTotalPerItem(false));
               }
           }
       }

Try to fix this too,

[aeq-dev]

This :
for ($qty = 0; $qty < $item->qty; $qty++) {
Why are you looping on item qty ? what's the difference if I have item1 qty = 1 or qty 5 ?

[lukepolo]

tax can happen per item, if thats the case each qty really means a unique item

[aeq-dev]

Using my changes, it works like a charm,
This is a use case :

1. Add item 500 twice
2. Add coupon 5% discount = 50
3. Add coupon 10% discount = 100
4. Taxes calculation 15% : (500*2 - 50 -100) *0.15 = 127.5
5. Total = 850 + 127.5 = 977.5

Am I wrong ?

[aeq-dev]

tax can happen per item, if thats the case each qty really means a unique item

Why you don't add tax to sum(item->qty) instead of tax on each qty = 1 ?
Tax on Qty=5 === Tax on Qty1 + Tax on Qty1 ....+ Tax on Qty5

[lukepolo]

if you allow taxation on discounts you have to be careful as tax on qty5 1= tax on qty 1 + qty 2 + ..

[lukepolo]

also taxation on fees runs into this same issue~

[lukepolo]

yah the changes you made probably work, but i hvae to fix my tests to make sure i dont break anything else .

This package is also VERY old , im 100% sure i could have written it better 😆

[aeq-dev]

Using my changes, it works like a charm, This is a use case :

1. Add item 500 twice

2. Add coupon 5% discount = 50

3. Add coupon 10% discount = 100

4. Taxes calculation 15% : (500*2 - 50 -100) *0.15 = 127.5

5. Total =  850 + 127.5 = 977.5

Am I wrong ?

Could you please tell if it's correct ? I'm applying tax on cart totals

[lukepolo]

if your in the U.S it will depend on your state , id look up your guidelines for how taxation works etc. (taxation sucks and its more complicated than id liek)

[aeq-dev]

I see it's a headache :/
My use case is a normal tax application, not in US

[lukepolo]

no worries, ill see if i can get to fixing the tests tonight, I ran out of time and have todo the real work that pays the $$$