From 262860c865ee2d3fa012d0f37225ba42335f8f97 Mon Sep 17 00:00:00 2001 From: Mike Lay Date: Wed, 14 Dec 2016 09:39:33 -0500 Subject: [PATCH] Fix math in InventoryPurchaseItem.costPerUnit and add math util for rounding (#864) * Add unit model test InventoryPurchaseItem - Add unit model test inv-purchase InventoryPurchaseItem * Fix incorrect rounding add math util - Fix incorrect calculation `InventoryPurchaseItem.costPerUnit` to round property to the nearest 100th - Add test for demostrating issue in `inv-purchase-test.js` "costPerUnit properly round" - Add math util with function `round100` - Add tests for `round100` function * Fix rounding in mixin number-format and unit test - Fix rounding in `_numberFormat` for number-format mixin - Add mixin unit tests for number-format mixin * Fix incorrect directory for number-format-test * Use NumberFormat for InventoryPurchaseItem - Use `NumberFormat` mixin for `InventoryPurchaseItem` and use in `costPerUnit` method - Remove math util as no longer needed * Remove duplicate number-format-test * Prefer _numberFormat to _round100 --- app/mixins/number-format.js | 7 ++- app/models/inv-purchase.js | 5 ++- tests/unit/mixins/number-format-test.js | 59 +++++++++++++++++++++++++ tests/unit/models/inv-purchase-test.js | 55 +++++++++++++++++++++++ 4 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 tests/unit/mixins/number-format-test.js create mode 100644 tests/unit/models/inv-purchase-test.js diff --git a/app/mixins/number-format.js b/app/mixins/number-format.js index 581d968ee0..de26c25887 100644 --- a/app/mixins/number-format.js +++ b/app/mixins/number-format.js @@ -49,7 +49,7 @@ export default Ember.Mixin.create({ if (Math.round(value) === value) { returnValue = Number(value).toString(); } else { - returnValue = Number(value).toFixed(2); + returnValue = this._round100(value).toFixed(2); } if (returnAsNumber) { return Number(returnValue); @@ -61,6 +61,11 @@ export default Ember.Mixin.create({ _validNumber(number) { return (!Ember.isEmpty(number) && !isNaN(number) && number > 0); + }, + + _round100(number) { + let tempNumber = 100 * number; + return Math.round(tempNumber) / 100; } }); diff --git a/app/models/inv-purchase.js b/app/models/inv-purchase.js index 10bd51a7f9..e2a0d72e22 100644 --- a/app/models/inv-purchase.js +++ b/app/models/inv-purchase.js @@ -2,6 +2,7 @@ import AbstractModel from 'hospitalrun/models/abstract'; import DS from 'ember-data'; import Ember from 'ember'; import LocationName from 'hospitalrun/mixins/location-name'; +import NumberFormat from 'hospitalrun/mixins/number-format'; function defaultQuantityGroups() { return []; @@ -13,7 +14,7 @@ function defaultQuantityGroups() { * items to be shown as inventory items since the pouchdb adapter does a * retrieve for keys starting with 'inventory' to fetch inventory items. */ -let InventoryPurchaseItem = AbstractModel.extend(LocationName, { +let InventoryPurchaseItem = AbstractModel.extend(LocationName, NumberFormat, { purchaseCost: DS.attr('number'), lotNumber: DS.attr('string'), dateReceived: DS.attr('date'), @@ -23,7 +24,7 @@ let InventoryPurchaseItem = AbstractModel.extend(LocationName, { if (Ember.isEmpty(purchaseCost) || Ember.isEmpty(quantity) || purchaseCost === 0 || quantity === 0) { return 0; } - return Number((purchaseCost / quantity).toFixed(2)); + return this._numberFormat(purchaseCost / quantity, true); }.property('purchaseCost', 'originalQuantity'), originalQuantity: DS.attr('number'), currentQuantity: DS.attr('number'), diff --git a/tests/unit/mixins/number-format-test.js b/tests/unit/mixins/number-format-test.js new file mode 100644 index 0000000000..2ec518500e --- /dev/null +++ b/tests/unit/mixins/number-format-test.js @@ -0,0 +1,59 @@ +import NumberFormat from 'hospitalrun/mixins/number-format'; +import { moduleFor, test } from 'ember-qunit'; +import Ember from 'ember'; + +moduleFor('mixin:number-format', 'Unit | Mixin | number-format'); + +test('_calculateTotal', function(assert) { + let records = [5, 5, 6].map((id) => Ember.Object.create({ id })); + let numberFormat = Ember.Object.extend(NumberFormat).create({ records }); + + assert.strictEqual(numberFormat._calculateTotal('records', 'id'), 16, 'Should add property array'); +}); + +test('_calculateTotal property name', function(assert) { + let records = [5, 2, 3].map((id) => Ember.Object.create({ id })); + let numberFormat = Ember.Object.extend(NumberFormat).create(); + + assert.strictEqual(numberFormat._calculateTotal(records, 'id'), 10, 'Should add passed in array'); +}); + +test('_calculateTotal invalid number', function(assert) { + let records = [5, 'test', 3].map((id) => Ember.Object.create({ id })); + let numberFormat = Ember.Object.extend(NumberFormat).create({ records }); + + assert.strictEqual(numberFormat._calculateTotal('records', 'id'), 8, 'Should treat invalid number as 0'); +}); + +test('_numberFormat', function(assert) { + let numberFormat = Ember.Object.extend(NumberFormat).create(); + + assert.strictEqual(numberFormat._numberFormat(), undefined, 'Should return undefined for no argument'); + assert.strictEqual(numberFormat._numberFormat('test'), undefined, 'Should return undefined for no number'); + assert.strictEqual(numberFormat._numberFormat(12), '12', 'Should return basic int as string'); + assert.strictEqual(numberFormat._numberFormat(12, true), 12, 'Should return basic int as number'); + assert.strictEqual(numberFormat._numberFormat(12.2, true), 12.2, 'Should round tenths properly'); + assert.strictEqual(numberFormat._numberFormat(12.2), '12.20', 'Should pad decial to two places'); + assert.strictEqual(numberFormat._numberFormat(35.555, true), 35.56, 'Should round 35.555 to 35.56'); + assert.strictEqual(numberFormat._numberFormat(35.555), '35.56', 'Should return 35.555 as string "35.56"'); +}); + +test('_getValidNumber', function(assert) { + let numberFormat = Ember.Object.extend(NumberFormat).create(); + + assert.strictEqual(numberFormat._getValidNumber(), 0, 'Should return 0 for no argument'); + assert.strictEqual(numberFormat._getValidNumber('test'), 0, 'Should return 0 for invalid number'); + assert.strictEqual(numberFormat._getValidNumber(NaN), 0, 'Should return 0 for NaN'); + assert.strictEqual(numberFormat._getValidNumber('12.2'), 12.2, 'Should convert string to number'); + assert.strictEqual(numberFormat._getValidNumber(1), 1, 'Should return basic int'); +}); + +test('_validNumber', function(assert) { + let numberFormat = Ember.Object.extend(NumberFormat).create(); + + assert.strictEqual(numberFormat._validNumber(1), true, 'Should return true for basic int'); + assert.strictEqual(numberFormat._validNumber(1.5), true, 'Should return true for float'); + assert.strictEqual(numberFormat._validNumber('1'), true, 'Should return true for numeric string'); + assert.strictEqual(numberFormat._validNumber(-1), false, 'Should return false for negative'); + assert.strictEqual(numberFormat._validNumber('test'), false, 'Should return false for non numeric string'); +}); diff --git a/tests/unit/models/inv-purchase-test.js b/tests/unit/models/inv-purchase-test.js new file mode 100644 index 0000000000..330aee78c5 --- /dev/null +++ b/tests/unit/models/inv-purchase-test.js @@ -0,0 +1,55 @@ +import { moduleForModel, test } from 'ember-qunit'; + +import { testValidPropertyValues, testInvalidPropertyValues } from '../../helpers/validate-properties'; + +moduleForModel('inv-purchase', 'Unit | Model | inv-purchase', { + needs: [ + 'ember-validations@validator:local/numericality', + 'ember-validations@validator:local/presence' + ] +}); + +test('costPerUnit', function(assert) { + let inventoryPurchaseItem = this.subject({ + purchaseCost: 12.50, + originalQuantity: 5 + }); + + assert.strictEqual(inventoryPurchaseItem.get('costPerUnit'), 2.5); +}); + +test('costPerUnit properly round', function(assert) { + let inventoryPurchaseItem = this.subject({ + purchaseCost: 71.11, + originalQuantity: 2 + }); + + assert.strictEqual(inventoryPurchaseItem.get('costPerUnit'), 35.56); +}); + +test('costPerUnit invalid input', function(assert) { + let inventoryPurchaseItem = this.subject({ + purchaseCost: 0, + originalQuantity: 5 + }); + + assert.strictEqual(inventoryPurchaseItem.get('costPerUnit'), 0); +}); + +test('costPerUnit 0 input', function(assert) { + let inventoryPurchaseItem = this.subject({ + purchaseCost: 12.50, + originalQuantity: 0 + }); + + assert.strictEqual(inventoryPurchaseItem.get('costPerUnit'), 0); +}); + +testValidPropertyValues('purchaseCost', [123, 123.0, '123']); +testInvalidPropertyValues('purchaseCost', ['test', undefined]); + +testValidPropertyValues('originalQuantity', [0, 123, '0']); +testInvalidPropertyValues('originalQuantity', [-1, '-1', undefined]); + +testValidPropertyValues('vendor', ['test']); +testInvalidPropertyValues('vendor', [undefined]);