Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discount problems #209

Closed
it-can opened this issue Oct 11, 2017 · 18 comments
Closed

Discount problems #209

it-can opened this issue Oct 11, 2017 · 18 comments
Labels

Comments

@it-can
Copy link
Contributor

it-can commented Oct 11, 2017

Version: Laravel 5.5.14 + Laracart 1.3.8

Hi,
I have an issue with coupons (both fixed and percentage). I have this:

PERCENTAGE Coupon

  1. Product costs € 121,00 (incl tax) € 100 (ex tax)
  2. Tax percentage is 21%
  3. When adding a discount (5%) = € 5 (subtotal should be € 95)
  4. I get this tax: € 16,00 (should be € 19,95)
  5. Total: € 111,00
LaraCart::add(1, 'Test Product', 1, 100, ['tax' => 0.21]);

$coupon = new LukePOLO\LaraCart\Coupons\Percentage('test', 0.05, [
    'name'        => '5% off',
    'description' => '5% off test',
]);

LaraCart::addCoupon($coupon);

$cart = LaraCart::get();

$cart->subTotal(false) => 100
$cart->totalDiscount(false)  => 5

$cart->taxTotal(false) => 16.00
$cart->total(false) => 111.00

FIXED Coupon

  1. Product costs € 121,00 (incl tax) € 100 (ex tax)
  2. Tax percentage is 21%
  3. When adding a discount € 30 (subtotal should be € 70)
  4. I get this tax: € -9,00 (should be € 14,70)
  5. Total: € 61.00 (should be € 84.70)
LaraCart::add(1, 'Test Product', 1, 100, ['tax' => 0.21]);

$coupon = new LukePOLO\LaraCart\Coupons\Fixed('test', 30, [
    'name'        => '30 off',
    'description' => '30 off test',
]);

LaraCart::addCoupon($coupon);

$cart = LaraCart::get();

$cart->subTotal(false) => 100.00
$cart->totalDiscount(false)  => 30.00

$cart->taxTotal(false) => -9.00
$cart->total(false) => 61.00
@it-can it-can changed the title Discount tax problems Discount problems Oct 11, 2017
@lukepolo
Copy link
Owner

In the confit you have an option to apply taxes before or after totals , look up based on your country which you should use

@lukepolo
Copy link
Owner

Oh nvm that's. bug

@it-can
Copy link
Contributor Author

it-can commented Oct 11, 2017

Also it seems that $cart->total() has an issue with double substraction of the discount, see line https://github.com/lukepolo/laracart/blob/master/src/LaraCart.php#L609

should the second parameter be false so it will not include the discount?

@lukepolo
Copy link
Owner

Probably, looking into this now, writing some tests

@lukepolo
Copy link
Owner

What version are you on? Im getting correct results

@lukepolo
Copy link
Owner

image

@it-can
Copy link
Contributor Author

it-can commented Oct 11, 2017

Version: Laravel 5.5.14 + Laracart 1.3.8

@it-can it-can closed this as completed Oct 11, 2017
@it-can it-can reopened this Oct 11, 2017
@it-can
Copy link
Contributor Author

it-can commented Oct 11, 2017

This is very weird...

this is my config

<?php

return [

    /*
    |--------------------------------------------------------------------------
    | The caching prefix used to lookup the cart
    |--------------------------------------------------------------------------
    |
    */
    'cache_prefix'           => 'laracart',

    /*
    |--------------------------------------------------------------------------
    | Locale is used to convert money into a readable format for the user,
    | please note the UTF-8 , helps to make sure its encoded correctly
    |
    | Common Locales
    |
    | English - United States (en_US): 123,456.00
    | English - UNITED KINGDOM (en_GB) 123,456.00
    | Spanish - Spain (es_ES): 123.456,000
    | Dutch - Netherlands (nl_NL): 123 456,00
    | German - Germany (de_DE): 123.456,00
    | French - France (fr_FR): 123 456,00
    | Italian - Italy (it_IT): 123.456,00
    |
    | This site is pretty useful : http://lh.2xlibre.net/locales/
    |
    |--------------------------------------------------------------------------
    |
    */
    'locale'                 => 'nl_NL.UTF-8',

    /*
    |--------------------------------------------------------------------------
    | If true displays the international format rather thant he national format
    |--------------------------------------------------------------------------
    |
    */
    'international_format'   => false,

    /*
    |--------------------------------------------------------------------------
    | Sets the tax for the cart and items , you can change per item
    | via the object later if  needed
    |--------------------------------------------------------------------------
    |
    */
    'tax'                    => null,

    /*
    |--------------------------------------------------------------------------
    | Calculate tax per item, rather than subtotal
    | https://github.com/lukepolo/laracart/issues/180
    |
    | This will vary , please investigate to follow the rules of your local laws
    | https://money.stackexchange.com/questions/15051/sales-tax-rounded-then-totaled-or-totaled-then-rounded
    |--------------------------------------------------------------------------
    |
    */
    'tax_by_item'            => true,

    /*
    |--------------------------------------------------------------------------
    | Allows you to choose if the discounts applied are taxable
    |--------------------------------------------------------------------------
    |
    */
    'discountTaxable'        => true,

    /*
    |--------------------------------------------------------------------------
    | Allows you to configure if a user can apply multiple coupons
    |--------------------------------------------------------------------------
    |
    */
    'multiple_coupons'       => false,

    /*
    |
    |                     **** DEPRECATED IN 1.3 ****
    |
    |--------------------------------------------------------------------------
    | Applied message when using getMessage on a coupon
    |--------------------------------------------------------------------------
    |
    */
    'coupon_applied_message' => 'Coupon Applied',

    /*
    |--------------------------------------------------------------------------
    | The default item model for your relations
    |--------------------------------------------------------------------------
    |
    */
    'item_model'             => null,

    /*
    |--------------------------------------------------------------------------
    | Binds your data into the correct spots for LaraCart
    |--------------------------------------------------------------------------
    |
    */
    'item_model_bindings'    => [
        \LukePOLO\LaraCart\CartItem::ITEM_ID      => 'id',
        \LukePOLO\LaraCart\CartItem::ITEM_NAME    => 'name',
        \LukePOLO\LaraCart\CartItem::ITEM_PRICE   => 'price',
        \LukePOLO\LaraCart\CartItem::ITEM_TAXABLE => 'taxable',
        \LukePOLO\LaraCart\CartItem::ITEM_OPTIONS => [
            // put columns here for additional options,
            // these will be merged with options that are passed in
            // e.x
            // tax => .07
        ],
    ],


    /*
    |--------------------------------------------------------------------------
    | The default item relations to the item_model
    |--------------------------------------------------------------------------
    |
    */
    'item_model_relations'   => [],

    /*
    |--------------------------------------------------------------------------
    | This allows you to use multiple devices based on your logged in user
    |--------------------------------------------------------------------------
    |
    */
    'cross_devices'          => false,
];

@lukepolo
Copy link
Owner

lukepolo commented Oct 11, 2017

/*
|--------------------------------------------------------------------------
| Calculate tax per item, rather than subtotal
| #180
|
| This will vary , please investigate to follow the rules of your local laws
| https://money.stackexchange.com/questions/15051/sales-tax-rounded-then-totaled-or-totaled-then-rounded
|--------------------------------------------------------------------------
|
*/

@lukepolo
Copy link
Owner

Those are correct because your taxing by item

@lukepolo
Copy link
Owner

If you wish to calculate the tax by subtotal set to false to get what you expect

@it-can
Copy link
Contributor Author

it-can commented Oct 11, 2017

mmm ok, now it seems to work... But is this a bug or not? I like calculating tax per item.

@lukepolo
Copy link
Owner

Its not, cause its calculating correctly , you have to calculate the taxes differently if your doing it by item. Ill verify that in a sec

@lukepolo
Copy link
Owner

Ok so , i've think I had a bug for a while now

#211

Im not sure if this will fix your issue or not , i have to run now but if you want take a look at that PR and see if that fixes your issues. I had the discount flag opposite of what it should have been. Kinda surprised it even worked before

@it-can
Copy link
Contributor Author

it-can commented Oct 11, 2017

Ok will check when I have a bit more time... Thanks

@lukepolo lukepolo added the bug label Oct 11, 2017
@lukepolo
Copy link
Owner

It did not, working on fixing it still

@lukepolo
Copy link
Owner

#212

I believe its fixed! I did your test in the code too (https://github.com/lukepolo/laracart/blob/master/tests/TotalsTest.php#L246) please validate that for me!

I have made a RC for 1.4.1 :

https://github.com/lukepolo/laracart/releases/tag/1.4.1-rc

Once you test ill release it as 1.4.1

@it-can
Copy link
Contributor Author

it-can commented Oct 19, 2017

@lukepolo thanks for the fix...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants