feat: added price with tax without unit multiplier #121
Conversation
|
Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖 Please select which version do you want to release:
And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.
|
mihainutiu-vtex
changed the title
| @@ -79,6 +80,9 @@ function SellingPrice({ | |||
|
|
|||
| const taxValue = commercialOffer.Tax | |||
|
|
|||
| const sellingPriceWithTaxWithoutUnit = | |||
| sellingPriceValue + (sellingPriceValue * taxPercentage / unitMultiplier) | |||
There was a problem hiding this comment.
This is the correct sellingPriceWithTax calculation, so it seems like there's no need to create another property for it. I suggest altering the sellingPriceWithTax value to what would be sellingPriceWithTaxWithoutUnit as it would properly represent the selling price of the product with taxes.
There was a problem hiding this comment.
Hi @icazevedo
The issue is that the commercial offer Tax that is used in the sellingPriceWithTax will return the tax for the entire multiplied value of the product.
If there is a product that costs 100$ and has a 10% tax and the product has a 5.0 unit multiplier then the commercialOffer.Tax will return the tax for all 5 units.
10% of 100 = 10
commercialOffer.Tax = 50
So when displaying the price of a product without the unit multiplier but with the tax, that value will be wrong (150$ instead of 110$)
Please check the description of this PR, and also the video recording I attached.
You can test on this workspace to see the variable.
on the master workspace you will see the sellingPriceWithTax variable.
Screen.Recording.2023-07-13.at.13.56.37.mov
There was a problem hiding this comment.
Regarding modifying the sellingPriceWithTax it would mean modifying the commercialOffer.Tax and I think that would impact existing implementations. This way it will not be a breaking change. Maybe there are some use cases where the original sellingPriceWithTax calculation is used.
| @@ -78,6 +79,8 @@ function ListPrice({ | |||
| const hasMeasurementUnit = measurementUnit && measurementUnit !== 'un' | |||
| const hasUnitMultiplier = unitMultiplier !== 1 | |||
|
|
|||
| const listPriceWithTaxWithoutUnit = listPriceValue + (listPriceValue * taxPercentage / unitMultiplier) | |||
There was a problem hiding this comment.
Hi @icazevedo
Like I mentioned in the other comment, these variables will return a different value. The regular listPriceWithTax will return the tax for all units in a sku with unit multiplier, so it is not enough. We need to be able to display the price of a single unit with taxes only for that unit, not the price for 1 unit but the tax for all units in the SKU.
|
I'm closing this PR since it is 6 month old. If this is still relevant, feel free to reopen it. |
|
Hi @marcelovicentegc |
|
Hi @leo-prange-vtex Please no not close this PR... We have a client waiting for it, they need the new price values that were added in this update. |
mihainutiu-vtex
requested review from
marcelovicentegc
and removed request for
icazevedo
What problem is this solving?
I've added a new handle for
sellingPriceWithTaxWithoutUnitandlistPriceWithTaxWithoutUnitthat allows the display of a product that has a unit multiplier and also taxes. The defaultsellingPriceWithTaxcalculates the taxes for the entire quantity, not for 1 unit.How to test it?
You can install
[email protected]and change the price block configuration on a PDP. The product you are viewing must have a unit multiplier different from 1 and also active taxes.You can check out this product that has a 5,0 unit multiplier, 100 ron price and a 10% tax.
You can notice the calculation using the default {sellingPriceWithTax} will display the price for 1 unit but the tax for all 5 units
https://vtexromania.myvtex.com/rama-foto-argintata-jardin-d-eden-christofle/p
Checking the new sellingPriceWithTaxWithoutUnit you will see the correct calculation for 1 unit + 1 unit tax
https://mihaitest--vtexromania.myvtex.com/rama-foto-argintata-jardin-d-eden-christofle/p