vuestorefront / vue-storefront-api

Vue.js storefront for Magento2 (and not only) - data backend
https://www.vuestorefront.io
MIT License
346 stars 337 forks source link

Bugfix/original_price not calculated correctly due to tax bug #464

Closed didkan closed 4 years ago

didkan commented 4 years ago

Fixes #463

Use normal equality operator instead of strict equality operator to catch both null and undefined values for sourcePriceInclTax. This makes sure we get proper calculation of original_price and its variants.

didkan commented 4 years ago

Exactly. There is similar code in vuestorefront, but that code doesn't even try to fall back on config.tax.sourcePriceIncludesTax. I suppose that means it is mandatory to put the sourcePriceIncludesTax in config.storeViews.xx.tax instead. Not sure if this is by design or a mistake. I suppose I would prefer both vuestorefront and vuestorefront-api to handle that config option in the same way. Here is the code part in vuestorefront: https://github.com/DivanteLtd/vue-storefront/blob/master/core/modules/catalog/store/tax/actions.ts#L52-L61

pkarw commented 4 years ago

@gibkigonzo we need to port it to storefornt-api and the tax.ts in the vue-storefront project as well I guess

gibkigonzo commented 4 years ago

@pkarw yes, we need to port it in storefront-api, but not in vsf @didkan storeView in vsf is prepared before app creation. currentStoreView() doesn't return config but already merged config.storeViews.xx.tax with global config.tax. https://github.com/DivanteLtd/vue-storefront/blob/master/core/lib/multistore.ts#L67 https://github.com/DivanteLtd/vue-storefront/blob/master/core/lib/multistore.ts#L18-L34 So if sourcePriceIncludesTax will not be defined in config.storeViews.xx.tax then it will not override config.tax. I think that we shouldn't build config for multistore in TaxProxy in first place, because it is not TaxProxy responsibility to build config. This require bigger refactor, because there are more places where config is build.