From 1dcdcb5c31e18c44ca7beeb223ea3bbce5f545f8 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Thu, 12 Sep 2019 15:17:34 +0200 Subject: [PATCH] FIX: Cast all numerical values in reports (#8087) * FIX: Cast all numerical values in reports The backend can return some numerical values in report as strings. That results in unexpected order of values when sorting report tables. * Create `toNumber()` helper The `typeof` and `parseFloat` seem to be the fastest path: https://jsperf.com/number-vs-typeof-vs-parsefloat#results --- app/assets/javascripts/admin/models/report.js.es6 | 14 +++++++++----- .../javascripts/discourse/lib/utilities.js.es6 | 6 +++++- test/javascripts/fixtures/reports_bulk.js.es6 | 4 ++-- test/javascripts/models/report-test.js.es6 | 4 ++-- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/admin/models/report.js.es6 b/app/assets/javascripts/admin/models/report.js.es6 index 831fe8e7657..c7967cc6d54 100644 --- a/app/assets/javascripts/admin/models/report.js.es6 +++ b/app/assets/javascripts/admin/models/report.js.es6 @@ -1,7 +1,11 @@ import { escapeExpression } from "discourse/lib/utilities"; import { ajax } from "discourse/lib/ajax"; import round from "discourse/lib/round"; -import { fillMissingDates, formatUsername } from "discourse/lib/utilities"; +import { + fillMissingDates, + formatUsername, + toNumber +} from "discourse/lib/utilities"; import computed from "ember-addons/ember-computed-decorators"; import { number, durationTiny } from "discourse/lib/formatter"; import { renderAvatar } from "discourse/helpers/user-avatar"; @@ -374,14 +378,14 @@ const Report = Discourse.Model.extend({ _secondsLabel(value) { return { - value, + value: toNumber(value), formatedValue: durationTiny(value) }; }, _percentLabel(value) { return { - value, + value: toNumber(value), formatedValue: value ? `${value}%` : "—" }; }, @@ -394,14 +398,14 @@ const Report = Discourse.Model.extend({ const formatedValue = () => (formatNumbers ? number(value) : value); return { - value, + value: toNumber(value), formatedValue: value ? formatedValue() : "—" }; }, _bytesLabel(value) { return { - value, + value: toNumber(value), formatedValue: I18n.toHumanSize(value) }; }, diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index 3f2be558721..5bc038e227e 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/app/assets/javascripts/discourse/lib/utilities.js.es6 @@ -599,8 +599,12 @@ export function clipboardData(e, canUpload) { return { clipboard, types, canUpload, canPasteHtml }; } +export function toNumber(input) { + return typeof input === "number" ? input : parseFloat(input); +} + export function isNumeric(input) { - return !isNaN(parseFloat(input)) && isFinite(input); + return !isNaN(toNumber(input)) && isFinite(input); } export function fillMissingDates(data, startDate, endDate) { diff --git a/test/javascripts/fixtures/reports_bulk.js.es6 b/test/javascripts/fixtures/reports_bulk.js.es6 index f993978f8d8..c315989170d 100644 --- a/test/javascripts/fixtures/reports_bulk.js.es6 +++ b/test/javascripts/fixtures/reports_bulk.js.es6 @@ -7,7 +7,7 @@ let signups = { yaxis: "Number of signups", description: "New account registrations for this period", data: [ - { x: "2018-06-16", y: 12 }, + { x: "2018-06-16", y: "12" }, { x: "2018-06-17", y: 16 }, { x: "2018-06-18", y: 42 }, { x: "2018-06-19", y: 38 }, @@ -18,7 +18,7 @@ let signups = { { x: "2018-06-24", y: 17 }, { x: "2018-06-25", y: 27 }, { x: "2018-06-26", y: 32 }, - { x: "2018-06-27", y: 7 } + { x: "2018-06-27", y: "7" } ], start_date: "2018-06-16T00:00:00Z", end_date: "2018-07-16T23:59:59Z", diff --git a/test/javascripts/models/report-test.js.es6 b/test/javascripts/models/report-test.js.es6 index 3ba6a5198b0..1fa622a1d3a 100644 --- a/test/javascripts/models/report-test.js.es6 +++ b/test/javascripts/models/report-test.js.es6 @@ -398,7 +398,7 @@ QUnit.test("computed labels", assert => { username: "joffrey", user_id: 1, user_avatar: "/", - flag_count: 1876, + flag_count: "1876", time_read: 287362, note: "This is a long note", topic_id: 2, @@ -470,7 +470,7 @@ QUnit.test("computed labels", assert => { assert.equal(flagCountLabel.type, "number"); let computedFlagCountLabel = flagCountLabel.compute(row); assert.equal(computedFlagCountLabel.formatedValue, "1.9k"); - assert.equal(computedFlagCountLabel.value, 1876); + assert.strictEqual(computedFlagCountLabel.value, 1876); computedFlagCountLabel = flagCountLabel.compute(row, { formatNumbers: false });