Fixes sort-by's handling of multiple keys (#375)

- Prior to this change, passing multiple keys to `sort-by` resulted in the target array being sorted by the first key and then the whole thing being resorted by each subsequent key. The ultimate result was determined solely by the last key. This is not the intended behavior.
- The desired behavior (and the behavior of this addon prior to 4.1.0) is for the second and subsequent sort keys only to be applied in the case where the first sort key does not yield a clear ordering.
- The test for this case uses the very common example of sorting people by last name then first name, as is shown in the README
- This commit also consolidates the code path taken when a sort function is passed in place of a sort key

addon/helpers/sort-by.js

@@ -55,11 +55,10 @@ class SortBy {
     }
 
     this.array = [...array];
-    this.callbacks = null;
   }
 
   comparator(key) {
-    return this.callback ? this.callback : this.defaultSort(key);
+    return (typeof key === 'function') ? key : this.defaultSort(key);
   }
 
   defaultSort(sortKey) {
@@ -70,10 +69,6 @@ class SortBy {
 
     return (a, b) => func(sortKey.replace(/:desc|:asc/, ''), a, b);
   }
-
-  addCallback(callback) {
-    this.callback = callback;
-  }
 }
 
 /**
@@ -85,10 +80,20 @@ class SortBy {
  * @extends SortBy
  */
 class BubbleSort extends SortBy {
-  perform(key) {
+  perform(keys = []) {
     let swapped = false;
 
-    let compFunc = this.comparator(key);
+    let compFuncs = keys.map(key => this.comparator(key));
+    let compFunc = (a, b) => {
+      for (let i = 0; i < compFuncs.length; i += 1) {
+        let result = compFuncs[i](a,b);
+        if (result === 0) {
+          continue;
+        }
+        return result;
+      }
+      return 0;
+    };
     for (let i = 1; i < this.array.length; i += 1) {
       for (let j = 0; j < this.array.length - i; j += 1) {
         let shouldSwap = normalizeToBoolean(compFunc(this.array[j+1], this.array[j]));
@@ -122,17 +127,7 @@ export function sortBy(params) {
   }
 
   const sortKlass = new BubbleSort(array);
-
-  if (typeof sortKeys[0] === 'function') { // || isEmberArray(firstSortProp)) {
-    sortKlass.addCallback(sortKeys[0]);
-    sortKlass.perform();
-  } else {
-    for (let key of sortKeys) {
-      sortKlass.perform(key);
-    }
-  }
-
-
+  sortKlass.perform(sortKeys);
   return sortKlass.array;
 }
 

tests/integration/helpers/sort-by-test.js

@@ -184,7 +184,7 @@ module('Integration | Helper | {{sort-by}}', function(hooks) {
     assert.equal(find('*').textContent.trim(), 'abcd', 'list is still sorted after addition');
   });
 
-  test('It also accepts an array of sort properties', async function(assert) {
+  test('It accepts an array of sort properties (one prop)', async function(assert) {
     this.set('array', emberArray([
       { name: 'c' },
       { name: 'a' },
@@ -202,6 +202,44 @@ module('Integration | Helper | {{sort-by}}', function(hooks) {
     assert.equal(find('*').textContent.trim(), 'abc', 'cab is sorted to abc');
   });
 
+  test('It accepts an array of sort properties (more than one prop)', async function(assert) {
+    this.set('array', emberArray([
+      { firstName: 'Adam', lastName: 'Coda' },
+      { firstName: 'Billy', lastName: 'Jones' },
+      { firstName: 'William', lastName: 'Abrams' },
+      { firstName: 'Sam', lastName: 'Jones' },
+      { firstName: 'Donnie', lastName: 'Brady' }
+    ]));
+
+    this.set('sortBy', ['lastName', 'firstName']);
+
+    await render(hbs`
+      {{~#each (sort-by sortBy array) as |user|~}}
+        {{~user.lastName~}},{{~user.firstName~}};
+      {{~/each~}}
+    `);
+
+    assert.equal(find('*').textContent.trim(), 'Abrams,William;Brady,Donnie;Coda,Adam;Jones,Billy;Jones,Sam;', 'Names are sorted alphabetically by last name then first name');
+  });
+
+  test('It accepts multiple sort properties as helper params', async function(assert) {
+    this.set('array', emberArray([
+      { firstName: 'Adam', lastName: 'Coda' },
+      { firstName: 'Billy', lastName: 'Jones' },
+      { firstName: 'William', lastName: 'Abrams' },
+      { firstName: 'Sam', lastName: 'Jones' },
+      { firstName: 'Donnie', lastName: 'Brady' }
+    ]));
+
+    await render(hbs`
+      {{~#each (sort-by "lastName" "firstName" array) as |user|~}}
+        {{~user.lastName~}},{{~user.firstName~}};
+      {{~/each~}}
+    `);
+
+    assert.equal(find('*').textContent.trim(), 'Abrams,William;Brady,Donnie;Coda,Adam;Jones,Billy;Jones,Sam;', 'Names are sorted alphabetically by last name then first name');
+  });
+
   test('It accepts a function sort property', async function(assert) {
     this.set('array', emberArray([
       { name: 'c' },