From 0c6b4a5a231a71100ce5d3a3b0d18fdb66f77578 Mon Sep 17 00:00:00 2001
From: Henry <akhenry@gmail.com>
Date: Tue, 5 Apr 2016 12:05:11 -0700
Subject: [PATCH] [Tables] Fix to correct sorting in realtime tables

---
 .../src/controllers/MCTTableController.js     | 29 ++++++++----
 .../controllers/MCTTableControllerSpec.js     | 46 ++++++++++++++++++-
 2 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/platform/features/table/src/controllers/MCTTableController.js b/platform/features/table/src/controllers/MCTTableController.js
index 7b4c748c7b..2fe7fc1ba6 100644
--- a/platform/features/table/src/controllers/MCTTableController.js
+++ b/platform/features/table/src/controllers/MCTTableController.js
@@ -295,6 +295,20 @@ define(
             }
         };
 
+        /**
+         * Given a value, if it can be coerced to a number, then return a
+         * number representation. It's a little more robust than using just
+         * Number() or parseFloat, or isNaN in isolation, all of which are
+         * fairly inconsistent in their results.
+         * @param value The value to cast (if possible)
+         * @returns {*} The value cast to a Number, or the original value if
+         * a Number representation is not possible.
+         */
+        MCTTableController.prototype.toNumber = function (value){
+            var val = !isNaN(Number(value)) && !isNaN(parseFloat(value)) ? Number(value) : value;
+            return val;
+        };
+
         /**
          * @private
          */
@@ -311,12 +325,8 @@ define(
                     return min; // Element is not in array, min gives direction
                 }
 
-                valA = isNaN(searchElement[sortKey].text) ?
-                    searchElement[sortKey].text :
-                    parseFloat(searchElement[sortKey].text);
-                valB = isNaN(searchArray[sampleAt][sortKey].text) ?
-                    searchArray[sampleAt][sortKey].text :
-                    parseFloat(searchArray[sampleAt][sortKey].text);
+                valA = self.toNumber(searchElement[sortKey].text);
+                valB = self.toNumber(searchArray[sampleAt][sortKey].text);
 
                 switch(self.sortComparator(valA, valB)) {
                     case -1:
@@ -397,10 +407,9 @@ define(
                 //If the values to compare can be compared as
                 // numbers, do so. String comparison of number
                 // values can cause inconsistencies
-                var valA = isNaN(a[sortKey].text) ? a[sortKey].text :
-                        parseFloat(a[sortKey].text),
-                    valB = isNaN(b[sortKey].text) ? b[sortKey].text :
-                        parseFloat(b[sortKey].text);
+
+                var valA = self.toNumber(a[sortKey].text),
+                    valB = self.toNumber(b[sortKey].text);
 
                 return self.sortComparator(valA, valB);
             });
diff --git a/platform/features/table/test/controllers/MCTTableControllerSpec.js b/platform/features/table/test/controllers/MCTTableControllerSpec.js
index 7bca5c65b4..436b53bb16 100644
--- a/platform/features/table/test/controllers/MCTTableControllerSpec.js
+++ b/platform/features/table/test/controllers/MCTTableControllerSpec.js
@@ -180,6 +180,50 @@ define(
                         expect(sortedRows[2].col2.text).toEqual('abc');
                     });
 
+                    it('converts number strings to numbers', function () {
+                        var val1 = "",
+                            val2 = "1",
+                            val3 = "2016-04-05 18:41:30.713Z",
+                            val4 = "1.1",
+                            val5 = "8.945520958175627e-13";
+
+                        expect(controller.toNumber(val1)).toEqual("");
+                        expect(controller.toNumber(val2)).toEqual(1);
+                        expect(controller.toNumber(val3)).toEqual("2016-04-05 18:41:30.713Z");
+                        expect(controller.toNumber(val4)).toEqual(1.1);
+                        expect(controller.toNumber(val5)).toEqual(8.945520958175627e-13);
+                    });
+
+                    it('correctly sorts rows of differing types', function () {
+                        mockScope.sortColumn = 'col2';
+                        mockScope.sortDirection = 'desc';
+
+                        testRows.push({
+                            'col1': {'text': 'row4 col1'},
+                            'col2': {'text': '123'},
+                            'col3': {'text': 'row4 col3'}
+                        });
+                        testRows.push({
+                            'col1': {'text': 'row5 col1'},
+                            'col2': {'text': '456'},
+                            'col3': {'text': 'row5 col3'}
+                        });
+                        testRows.push({
+                            'col1': {'text': 'row5 col1'},
+                            'col2': {'text': ''},
+                            'col3': {'text': 'row5 col3'}
+                        });
+
+                        sortedRows = controller.sortRows(testRows);
+                        expect(sortedRows[0].col2.text).toEqual('ghi');
+                        expect(sortedRows[1].col2.text).toEqual('def');
+                        expect(sortedRows[2].col2.text).toEqual('abc');
+
+                        expect(sortedRows[sortedRows.length-3].col2.text).toEqual('456');
+                        expect(sortedRows[sortedRows.length-2].col2.text).toEqual('123');
+                        expect(sortedRows[sortedRows.length-1].col2.text).toEqual('');
+                    });
+
                     describe('Adding new rows', function() {
                         var row4,
                             row5,
@@ -251,7 +295,7 @@ define(
                         });
 
                         it('Adds new rows at the correct sort position when' +
-                            ' not sorted ', function() {
+                            ' not sorted ', function () {
                             mockScope.sortColumn = undefined;
                             mockScope.sortDirection = undefined;
                             mockScope.filters = {};