Skip to content

Commit

Permalink
Fix sort-by with null values (#377)
Browse files Browse the repository at this point in the history
* sort-by should sort null values last as well

`aValue.toLowerCase` throws an error if `null` is not explicitly handled:
`Uncaught TypeError: Cannot read property 'toLowerCase' of null`

* Improve sort-by undefined values test

Test that undefined values are sorted, not ignored
  • Loading branch information
wongpeiyi authored Jul 5, 2020
1 parent 105e1a7 commit 6732d65
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
8 changes: 4 additions & 4 deletions addon/helpers/sort-by.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ function sortDesc(key, a, b) {
const aValue = get(a, key);
const bValue = get(b, key);

if (typeof bValue == 'undefined') {
if (typeof bValue == 'undefined' || bValue === null) {
// keep bValue last
return -1;
}
if (typeof aValue == 'undefined') {
if (typeof aValue == 'undefined' || aValue === null) {
// put aValue last
return 1;
}
Expand All @@ -47,11 +47,11 @@ function sortAsc(key, a, b) {
const aValue = get(a, key);
const bValue = get(b, key);

if (typeof bValue == 'undefined') {
if (typeof bValue == 'undefined' || bValue === null) {
// keep bValue last
return -1;
}
if (typeof aValue == 'undefined') {
if (typeof aValue == 'undefined' || aValue === null) {
// put aValue last
return 1;
}
Expand Down
33 changes: 25 additions & 8 deletions tests/integration/helpers/sort-by-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,21 +301,38 @@ module('Integration | Helper | {{sort-by}}', function(hooks) {
assert.equal(find('*').textContent.trim(), 'abc', 'cab is sorted to abc');
});

test('ignores undefined values sorting', async function (assert) {
test('it sorts undefined values last', async function(assert) {
this.set('array', [
{ name: 'c' },
{ name: 'a' },
{ name: undefined },
{ name: 'b' },
{ id: 1, name: 'c' },
{ id: 2, name: 'a' },
{ id: 3, name: undefined },
{ id: 4, name: 'b' },
]);

await render(hbs`
{{~#each (sort-by 'name' array) as |pet|~}}
{{~pet.name~}}
{{~#each (sort-by 'name' array) as |user|~}}
{{~user.id~}}
{{~/each~}}
`);

assert.equal(find('*').textContent.trim(), '2413');
});

test('it sorts null values last', async function(assert) {
this.set('array', [
{ id: 1, name: 'c' },
{ id: 2, name: 'a' },
{ id: 3, name: null },
{ id: 4, name: 'b' },
]);

await render(hbs`
{{~#each (sort-by 'name' array) as |user|~}}
{{~user.id~}}
{{~/each~}}
`);

assert.equal(find('*').textContent.trim(), 'abc');
assert.equal(find('*').textContent.trim(), '2413');
});

test('It maintains order when values are the same', async function(assert) {
Expand Down

0 comments on commit 6732d65

Please sign in to comment.