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

precision should honor 0 if it is passed in #1479

Closed
brianxieseattle opened this issue Aug 10, 2017 · 3 comments · Fixed by #1591
Closed

precision should honor 0 if it is passed in #1479

brianxieseattle opened this issue Aug 10, 2017 · 3 comments · Fixed by #1591
Assignees
Labels

Comments

@brianxieseattle
Copy link

Type of issue

Bug

Description

If I give a bucket with precision to 0 and call getPriceBucketString. getPriceBucketString will treat 0 to the default 2 precision.
'buckets': [
{
'precision': 0,
'min': 3,
'max': 18,
'increment': 0.05,
}

Steps to reproduce

let cpm = 16.50908;
let customConfig = {
  'buckets': [
  {
    'precision': 0,
    'min': 3,
    'max': 18,
    'increment': 0.05,
  }
  ]
};

let output = getPriceBucketString(cpm, customConfig);

Expected results

cpm.custom should be 17 as I set precision to 0

Actual results

cpm.custom is emit as 16.51.

Other information

Root cause seems to be around in cpmBucketManager
function getCpmTarget(cpm, increment, precision) {
if (!precision) {
precision = _defaultPrecision;
}

!precision will evaluate 0 as true and defaultPrecision is used.

Shall we consider
(!precision && precision != 0)

@mkendall07
Copy link
Member

Nice catch! We accept PRs if you want or we will fix next sprint. Thanks

@brianxieseattle
Copy link
Author

Great.. I seem to have some issues in pushing the changes remotely. You can feel free to include the fix in the next sprint.. BTW, when is the estimate time for the next sprint?

@brianxieseattle
Copy link
Author

BTW, here are the two unit tests you can consider adding to cpmBucketManager_spec.js when you fix this issue.

it('gets custom bucket strings and it should honor 0', () => {
let cpm = 16.50908;
let customConfig = {
'buckets': [
{
'precision': 0,
'min': 3,
'max': 18,
'increment': 0.05,
}
]
};
let expected = '{"low":"5.00","med":"16.50","high":"16.50","auto":"16.50","dense":"16.50","custom":"17"}';
let output = getPriceBucketString(cpm, customConfig);
expect(JSON.stringify(output)).to.deep.equal(expected);
});

it('gets the custom bucket strings without passing precision and it should honor the default precision', () => {
let cpm = 16.50908;
let customConfig = {
'buckets': [
{
'min': 3,
'max': 18,
'increment': 0.05,
}
]
};
let expected = '{"low":"5.00","med":"16.50","high":"16.50","auto":"16.50","dense":"16.50","custom":"16.50"}';
let output = getPriceBucketString(cpm, customConfig);
expect(JSON.stringify(output)).to.deep.equal(expected);
});

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

Successfully merging a pull request may close this issue.

3 participants