From 1362ddd47f76ef96c0f911b3971628dd7f150070 Mon Sep 17 00:00:00 2001 From: daniel-zamora Date: Wed, 8 Jan 2025 09:42:27 -0500 Subject: [PATCH] EDSC-4280 fixes issues with view all facet util functions (#1845) * EDSC-4280 fixes issues with view all facet util functions * EDSC-4280 linter fix --- static/src/js/util/__tests__/facets.test.js | 137 +++++++++++++++++++- static/src/js/util/facets.js | 52 ++++---- 2 files changed, 166 insertions(+), 23 deletions(-) diff --git a/static/src/js/util/__tests__/facets.test.js b/static/src/js/util/__tests__/facets.test.js index dddf99aa55..865045d6b8 100644 --- a/static/src/js/util/__tests__/facets.test.js +++ b/static/src/js/util/__tests__/facets.test.js @@ -1,4 +1,11 @@ -import { changeCmrFacet } from '../facets' +import { + changeCmrFacet, + getNormalizedFirstLetter, + getStartingLetters, + buildOrganizedFacets +} from '../facets' + +import { alphabet } from '../alphabetic-list' beforeEach(() => { jest.resetAllMocks() @@ -53,3 +60,131 @@ describe('changeCmrFacet', () => { }) }) }) + +describe('getNormalizedFirstLetter', () => { + it('returns null for undefined title', () => { + expect(getNormalizedFirstLetter(undefined)).toBeNull() + }) + + it('returns null for empty title', () => { + expect(getNormalizedFirstLetter('')).toBeNull() + }) + + it('converts first letter to uppercase', () => { + expect(getNormalizedFirstLetter('foo')).toBe('F') + expect(getNormalizedFirstLetter('bar')).toBe('B') + }) + + it('returns # for numeric first character', () => { + expect(getNormalizedFirstLetter('123')).toBe('#') + expect(getNormalizedFirstLetter('1foo')).toBe('#') + }) +}) + +describe('getStartingLetters', () => { + it('returns empty array for empty facets', () => { + expect(getStartingLetters([])).toEqual([]) + }) + + it('returns unique starting letters in order of appearance', () => { + const facets = [ + { title: 'foo' }, + { title: 'bar' }, + { title: 'qux' }, + { title: '123' } + ] + expect(getStartingLetters(facets)).toEqual(['F', 'B', 'Q', '#']) + }) + + it('handles facets with undefined titles', () => { + const facets = [ + { title: 'foo' }, + {}, + { title: undefined }, + { title: 'bar' } + ] + expect(getStartingLetters(facets)).toEqual(['F', 'B']) + }) + + it('does not duplicate letters', () => { + const facets = [ + { title: 'foo' }, + { title: 'foobar' }, + { title: 'fizz' } + ] + expect(getStartingLetters(facets)).toEqual(['F']) + }) +}) + +describe('buildOrganizedFacets', () => { + const createFacet = (title, applied = false) => ({ + title, + applied + }) + + it('organizes facets alphabetically when not lifting', () => { + const facets = [ + createFacet('foo'), + createFacet('bar'), + createFacet('123'), + createFacet('baz') + ] + const options = { liftSelectedFacets: false } + + const result = buildOrganizedFacets(facets, options) + + expect(result.facetsToLift).toEqual([]) + expect(result.alphabetizedList['#']).toHaveLength(1) + expect(result.alphabetizedList.F).toHaveLength(1) + expect(result.alphabetizedList.B).toHaveLength(2) + }) + + it('lifts applied facets when liftSelectedFacets is true', () => { + const facets = [ + createFacet('foo', true), + createFacet('bar'), + createFacet('baz', true) + ] + const options = { liftSelectedFacets: true } + + const result = buildOrganizedFacets(facets, options) + + expect(result.facetsToLift).toHaveLength(2) + expect(result.alphabetizedList.B).toHaveLength(1) + expect(result.facetsToLift).toEqual([ + createFacet('foo', true), + createFacet('baz', true) + ]) + }) + + it('handles facets with invalid titles', () => { + const facets = [ + createFacet(''), + createFacet(undefined), + createFacet('foo') + ] + const options = { liftSelectedFacets: false } + + const result = buildOrganizedFacets(facets, options) + + expect(result.alphabetizedList.F).toHaveLength(1) + // Check that invalid facets were skipped + const totalFacets = Object.values(result.alphabetizedList) + .reduce((sum, arr) => sum + arr.length, 0) + expect(totalFacets).toBe(1) + }) + + it('creates empty arrays for all alphabet letters that are not present', () => { + const facets = [createFacet('foo')] + const options = { liftSelectedFacets: false } + + const result = buildOrganizedFacets(facets, options) + + alphabet.forEach((letter) => { + expect(Array.isArray(result.alphabetizedList[letter])).toBe(true) + if (letter !== 'F') { + expect(result.alphabetizedList[letter]).toHaveLength(0) + } + }) + }) +}) diff --git a/static/src/js/util/facets.js b/static/src/js/util/facets.js index be9a3650bf..ec0183f610 100644 --- a/static/src/js/util/facets.js +++ b/static/src/js/util/facets.js @@ -1,7 +1,7 @@ import qs from 'qs' import { camelCase } from 'lodash-es' -import { isNumber } from './isNumber' import { queryParamsFromUrlString } from './url/url' +import { isNumber } from './isNumber' import { alphabet, createEmptyAlphabeticListObj } from './alphabetic-list' /** @@ -25,19 +25,32 @@ export const countSelectedFacets = (groupToCheck, startingValue = 0) => { return startingValue + totalSelectedFacets } +/** + * Returns normalized first letter of given facet title. + * @param {string} title - facet title. + * @return {string} first letter of facet, or if number detected returns #. + */ +export const getNormalizedFirstLetter = (title) => { + if (!title?.[0]) return null + + const firstLetter = title[0].toUpperCase() + + return isNumber(firstLetter) ? '#' : firstLetter +} + /** * Returns an array unique entries for the first letter of each facet's title. If the first letter * is a number, it will return '#'. This function does not count any children facets. - * @param {object} groupToCheck - An object representing the facet to be checked. + * @param {object} facets - An object representing the facet response. * @return {array} An array of the starting letters. */ export const getStartingLetters = (facets) => { - const firstLetters = [] - facets.forEach((facet) => { - let firstLetter = facet.title[0].toUpperCase() - if (isNumber(firstLetter)) firstLetter = '#' - if (!firstLetters.includes(firstLetter)) firstLetters.push(firstLetter) - }) + const firstLetters = facets.reduce((letters, facet) => { + const letter = getNormalizedFirstLetter(facet?.title) + if (!letter) return letters + + return letters.includes(letter) ? letters : [...letters, letter] + }, []) return firstLetters } @@ -208,8 +221,6 @@ export const buildOrganizedFacets = (facets, options) => { let facetsToLift = [] let facetsToSort = [] - // Populate the arrays based on the applied property if liftSelectedFacets is set, - // otherwise put all facets on facetsToSort if (options.liftSelectedFacets) { facetsToLift = facets.filter((facet) => facet.applied) facetsToSort = facets.filter((facet) => !facet.applied) @@ -218,25 +229,22 @@ export const buildOrganizedFacets = (facets, options) => { } let current = '#' - - // Set alphabetizedList to an object where each property is an array for a given letter const alphabetizedList = createEmptyAlphabeticListObj() - // Sort remaining 'non-lifted' facets into their respective arrays based on the first letter facetsToSort.forEach((facet) => { - const firstLetter = facet.title[0].toUpperCase() - const firstisNumber = isNumber(firstLetter) + const firstLetter = getNormalizedFirstLetter(facet.title) + // Skip facets without a valid first letter + if (!firstLetter) return - // If the first letter is not the current letter, set the current letter to the first letter of - // the selected letters facet. This relies on CMR returning the facets in alpabetical order + // If the letter is not the current letter, update current if (firstLetter !== current) { - current = firstisNumber ? '#' : alphabet[alphabet.indexOf(facet.title[0])] + current = firstLetter } - // If the first letter matches the current letter, push it onto the list. We also need to account - // for the first letter being a number, in which case it's added to the '#' list - if (firstLetter === current || (current === '#' && firstisNumber)) { - alphabetizedList[current].push(facet) + // Add to appropriate list if it matches current letter or is a number + // Only add if the letter exists in our alphabet (should always be true due to getNormalizedFirstLetter) + if (alphabet.includes(firstLetter) && (firstLetter === current || (current === '#' && firstLetter === '#'))) { + alphabetizedList[firstLetter].push(facet) } })