From 93298e0a52230c94e7cccd2c16702c2f41615a3e Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Fri, 10 May 2019 17:20:23 +0200 Subject: [PATCH] fix(sortBy): compare whole prefix instead of first character (#702) * fix(sortBy): compare whole prefix instead of first character This was noticed in https://github.com/algolia/algoliasearch-helper-js/pull/690#discussion_r282467917 by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO) * test(formatSort): add UT * refactor: change conditions to lower nesting * clearer flow (slightly slower, but clearer) --- src/functions/formatSort.js | 32 +++++++++++++++--------- test/spec/functions/formatSort.js | 41 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 11 deletions(-) create mode 100644 test/spec/functions/formatSort.js diff --git a/src/functions/formatSort.js b/src/functions/formatSort.js index 68cfe78c8..52bb49a60 100644 --- a/src/functions/formatSort.js +++ b/src/functions/formatSort.js @@ -5,22 +5,32 @@ var find = require('./find'); /** * Transform sort format from user friendly notation to lodash format * @param {string[]} sortBy array of predicate of the form "attribute:order" + * @param {string[]} [defaults] array of predicate of the form "attribute:order" * @return {array.} array containing 2 elements : attributes, orders */ module.exports = function formatSort(sortBy, defaults) { + var defaultInstructions = (defaults || []).map(function(sort) { + return sort.split(':'); + }); + return sortBy.reduce( - function preparePredicate(out, sortInstruction) { - var sortInstructions = sortInstruction.split(':'); - if (defaults && sortInstructions.length === 1) { - var similarDefault = find(defaults, function(predicate) { - return predicate[0] === sortInstruction[0]; - }); - if (similarDefault) { - sortInstructions = similarDefault.split(':'); - } + function preparePredicate(out, sort) { + var sortInstruction = sort.split(':'); + + var matchingDefault = find(defaultInstructions, function( + defaultInstruction + ) { + return defaultInstruction[0] === sortInstruction[0]; + }); + + if (sortInstruction.length > 1 || !matchingDefault) { + out[0].push(sortInstruction[0]); + out[1].push(sortInstruction[1]); + return out; } - out[0].push(sortInstructions[0]); - out[1].push(sortInstructions[1]); + + out[0].push(matchingDefault[0]); + out[1].push(matchingDefault[1]); return out; }, [[], []] diff --git a/test/spec/functions/formatSort.js b/test/spec/functions/formatSort.js new file mode 100644 index 000000000..00189e4d9 --- /dev/null +++ b/test/spec/functions/formatSort.js @@ -0,0 +1,41 @@ +'use strict'; + +var formatSort = require('../../../src/functions/formatSort'); + +it('splits into attribute & direction', function() { + expect(formatSort(['isRefined:desc', 'isNotRefined:desc'])).toEqual([ + ['isRefined', 'isNotRefined'], + ['desc', 'desc'] + ]); +}); + +it('leaves direction empty if no direction was given', function() { + expect(formatSort(['isRefined:desc', 'isNotRefined'])).toEqual([ + ['isRefined', 'isNotRefined'], + ['desc', undefined] + ]); +}); + +it('takes from defaults if no direction was given', function() { + expect( + formatSort( + ['isRefined:desc', 'isNotRefined'], + ['books:asc', 'isRefined:desc', 'isNotRefined:asc'] + ) + ).toEqual([ + ['isRefined', 'isNotRefined'], + ['desc', 'asc'] + ]); +}); + +it('leaves direction empty if no direction was given & no default matches', function() { + expect( + formatSort( + ['isRefined:desc', 'isNotRefined'], + ['books:asc', 'isRefined:desc'] + ) + ).toEqual([ + ['isRefined', 'isNotRefined'], + ['desc', undefined] + ]); +});