Skip to content

precision for price overriding by js#14350

Merged
magento-engcom-team merged 1 commit intomagento:2.2-developfrom
cdiacon:cdiacon/fixissue-14249
Mar 29, 2018
Merged

precision for price overriding by js#14350
magento-engcom-team merged 1 commit intomagento:2.2-developfrom
cdiacon:cdiacon/fixissue-14249

Conversation

@cdiacon
Copy link
Copy Markdown

@cdiacon cdiacon commented Mar 24, 2018

When using more then 2 digits ex(9.4880 will be displayed as 9.49) for the price the js will override this value and it will format using the round for 2 digits.

Preconditions

  1. magento 2.2.0
  2. php 7.0
  3. mysql 5.7.21

Steps to reproduce

  1. Create a plugin to use more then 2 digits for the price
  2. The price should be saved in database as 9.4880
  3. category page will display the price based on database value, but when accessing the prodcut page the price will be overrided with value 9.49

Expected result

  1. Price = 9.488

Actual result

  1. Price = 9.49

Going deeer found the issue in price-utils.js on this line

        // replace(/-/, 0) is only for fixing Safari bug which appears
        // when Math.abs(0).toFixed() executed on '0' number.
        // Result is '0.-0' :(

        am = Number(Math.round(Math.abs(amount - i) + 'e+' + precision) + ('e-' + precision));
        r = (j ? i.substr(0, j) + groupSymbol : '') +
            i.substr(j).replace(re, '$1' + groupSymbol) +
            (precision ? decimalSymbol + am.toFixed(2).replace(/-/, 0).slice(2) : '');

am.toFixed(2) <--- is using the hardcoded precision and the price will be overrided with 2 digits value !!!

done a quick check and changing this to

am.toFixed(precision)

seems to do the job.

Fixed Issues (if relevant)

  1. Priduct page price is using the hardcoded digits in js #14249: Priduct page price is using the hardcoded digits in js

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Copy Markdown
Contributor

magento-cicd2 commented Mar 24, 2018

CLA assistant check
All committers have signed the CLA.

r = (j ? i.substr(0, j) + groupSymbol : '') +
i.substr(j).replace(re, '$1' + groupSymbol) +
(precision ? decimalSymbol + am.toFixed(2).replace(/-/, 0).slice(2) : '');
(precision ? decimalSymbol + am.toFixed(precision).replace(/-/, 0).slice(2) : '');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't we slice precision digits as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@orlangur
the slice is applied not on the precision but on the price before digits for ex.

0.4880
the slice will remove the price and it will retun the decimals like this :
4880

in this case it should be always 2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, thanks for explanation as I didn't try to understand this logic in deep :)

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-1084 has been created to process this Pull Request

@magento-engcom-team
Copy link
Copy Markdown
Contributor

@cdiacon thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Event: distributed-cd Distributed Contribution Day Release Line: 2.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants