Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove optionsFieldType #974

Merged
merged 2 commits into from
Oct 12, 2021
Merged

remove optionsFieldType #974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25ba9b4
remove optionsFieldType
oshi97 Oct 12, 2021
04d4737
slight optimization when options order isn't recognized
oshi97 Oct 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 9 additions & 22 deletions static/js/transform-facets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export default function transformFacets(facets, config) {
fieldLabels,
optionsOrder,
optionsOrderList,
optionsFieldType = 'STRING',
...filterOptionsConfig
} = config.fields[facet.fieldId];

Expand All @@ -45,7 +44,7 @@ export default function transformFacets(facets, config) {
if (optionsOrderList) {
options = sortFacetOptionsCustom(options, optionsOrderList);
} else if (optionsOrder) {
options = sortFacetOptions(options, optionsOrder, optionsFieldType, facet.fieldId);
options = sortFacetOptions(options, optionsOrder, facet.fieldId);
}

return {
Expand All @@ -61,37 +60,25 @@ export default function transformFacets(facets, config) {
*
* @param {{ displayName: string }[]} options The facet options to sort.
* @param {'ASC' | 'DESC'} optionsOrder
* @param {'STRING' | 'INT'} optionsFieldType
* @param {string} fieldId
* @returns {{ displayName: string }[]}
*/
function sortFacetOptions(options, optionsOrder, optionsFieldType, fieldId) {
function sortFacetOptions(options, optionsOrder, fieldId) {
const getSortComparator = () => {
if (optionsFieldType === 'STRING') {
return (a, b) => a.displayName.localeCompare(b.displayName);
} else if (optionsFieldType === 'INT') {
return (a, b) => parseInt(a.displayName) - parseInt(b.displayName);
} else {
console.error(`Unknown facet optionsFieldType "${optionsFieldType}" for the "${fieldId}" facet.`);
return undefined;
}
}
const applyDirectionToComparator = (comparator) => {
if (!comparator) {
return undefined;
}

if (optionsOrder === 'ASC') {
return comparator;
return (a, b) => a.displayName.localeCompare(b.displayName);
} else if (optionsOrder === 'DESC') {
return (a, b) => -1 * comparator(a, b)
return (a, b) => b.displayName.localeCompare(a.displayName);
} else {
console.error(`Unknown facet optionsOrder "${optionsOrder}" for the "${fieldId}" facet.`);
return undefined;
}
}

return [...options].sort(applyDirectionToComparator(getSortComparator()))
const comparator = getSortComparator();
if (!comparator) {
return options;
}
return [...options].sort(comparator);
}


Expand Down
60 changes: 3 additions & 57 deletions tests/static/js/transform-facets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe('optionsOrder', () => {
}
];

function createFacetsConfig(optionsOrder, optionsFieldType, fieldLabels) {
function createFacetsConfig(optionsOrder, fieldLabels) {
return {
fields: {
c_mealType: {
Expand All @@ -155,7 +155,6 @@ describe('optionsOrder', () => {
Lunch: 'a lunch',
Dinner: 'duh dinner'
},
optionsFieldType: optionsFieldType || 'STRING',
optionsOrder
}
}
Expand All @@ -167,14 +166,14 @@ describe('optionsOrder', () => {
expect(actualOptions[0].displayName).toEqual('a lunch');
expect(actualOptions[1].displayName).toEqual('duh dinner');
expect(actualOptions[2].displayName).toEqual('ze breakfast');
})
});

it('works for DESC order', () => {
const actualOptions = transformFacets(facets, createFacetsConfig('DESC'))[0].options;
expect(actualOptions[0].displayName).toEqual('ze breakfast');
expect(actualOptions[1].displayName).toEqual('duh dinner');
expect(actualOptions[2].displayName).toEqual('a lunch');
})
});

it('logs an error if you use an unknown optionsOrder', () => {
const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
Expand All @@ -183,59 +182,6 @@ describe('optionsOrder', () => {
expect(consoleError).toHaveBeenCalledWith(
'Unknown facet optionsOrder "PACER" for the "c_mealType" facet.');
consoleError.mockRestore();
})

it('logs an error if you use an unknown optionsFieldType, and does not try to sort', () => {
const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {});
expect(consoleError).toHaveBeenCalledTimes(0);
transformFacets(facets, createFacetsConfig('ASC', 'FAKEINT'))[0].options;
expect(consoleError).toHaveBeenCalledWith(
'Unknown facet optionsFieldType "FAKEINT" for the "c_mealType" facet.');
consoleError.mockRestore();
})

it('works with ASC number display names', () => {
const actualOptions = transformFacets(facets, createFacetsConfig('ASC', 'INT', {
Breakfast: 3,
Lunch: 2,
Dinner: 100
}))[0].options;
expect(actualOptions[0].displayName).toEqual(2);
expect(actualOptions[1].displayName).toEqual(3);
expect(actualOptions[2].displayName).toEqual(100);
})

it('works with DESC number display names', () => {
const actualOptions = transformFacets(facets, createFacetsConfig('DESC', 'INT', {
Breakfast: 2,
Lunch: 3,
Dinner: 100
}))[0].options;
expect(actualOptions[0].displayName).toEqual(100);
expect(actualOptions[1].displayName).toEqual(3);
expect(actualOptions[2].displayName).toEqual(2);
})

it('works with ASC number display names that need to be parsed', () => {
const actualOptions = transformFacets(facets, createFacetsConfig('ASC', 'INT', {
Breakfast: '3',
Lunch: '2',
Dinner: '100'
}))[0].options;
expect(actualOptions[0].displayName).toEqual('2');
expect(actualOptions[1].displayName).toEqual('3');
expect(actualOptions[2].displayName).toEqual('100');
})

it('works with DESC number display names that need to be parsed', () => {
const actualOptions = transformFacets(facets, createFacetsConfig('DESC', 'INT', {
Breakfast: '2',
Lunch: '3',
Dinner: '100'
}))[0].options;
expect(actualOptions[0].displayName).toEqual('100');
expect(actualOptions[1].displayName).toEqual('3');
expect(actualOptions[2].displayName).toEqual('2');
});
});

Expand Down