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

Fix widget amount #16

Merged
merged 3 commits into from
Apr 8, 2024
Merged

Fix widget amount #16

merged 3 commits into from
Apr 8, 2024

Conversation

MarijaIv
Copy link
Collaborator

What is the goal?

Fix reported issue with widget amount when product price has more than 2 decimals.

How is it being implemented?

When product price is converted to minor unit, it is multiplied by 100, rounded to 0 decimals and cast to integer.

Does it affect (changes or update) any sensitive data?

No.

How is it tested?

Manual tests.

In Magento 2 shop, enable taxes and set a Tax rule with a percentage that has several decimals. On storefront, check the widgets on product pages.

How is it going to be deployed?

Standard deployment

$product->getPriceInfo()->getPrice('regular_price')->getMinimalPrice()->getValue() :
$product->getFinalPrice());

if ($product->getTypeId() === 'bundle' || !$this->isTaxEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bundled products will have taxes too, no?
Shouldn't we always take the price with taxes included? Even when taxes are not enabled it will be the same prices as the price without taxes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For bundle products, regular price is always equal to the price displayed in catalog pages - if tax is enabled, regular price will contain the tax, if tax is disabled, regular price will be without tax.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is why I'm commenting it because we will always charge the prices with tax included.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the fix should always use the price with tax included with no exceptions.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @m1k3lm Let's keep the communication about the issue in the ticket. Our team will respond in the ticket itself -> https://sequra.atlassian.net/browse/LIS-13

@m1k3lm m1k3lm merged commit 42defff into master Apr 8, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants