From 49ac292ca080c2ad7d430cf03bfd45993c82e1ff Mon Sep 17 00:00:00 2001 From: Gianluca Zuccarelli Date: Tue, 2 Nov 2021 15:46:20 +0000 Subject: [PATCH] CreateImageWizard: sort packages by relevance Sort the package results in the CreateImageWizard first by exact matches and then by pacakge results that start with the same letters as the search term, otherwise sort alphabetically. Fixes #256 --- .../formComponents/Packages.js | 55 ++++++-- .../CreateImageWizard.test.js | 119 +++++++++++++++--- 2 files changed, 149 insertions(+), 25 deletions(-) diff --git a/src/Components/CreateImageWizard/formComponents/Packages.js b/src/Components/CreateImageWizard/formComponents/Packages.js index 22f42e01..90f5281f 100644 --- a/src/Components/CreateImageWizard/formComponents/Packages.js +++ b/src/Components/CreateImageWizard/formComponents/Packages.js @@ -1,4 +1,4 @@ -import React, { useState, useRef, useEffect, } from 'react'; +import React, { useState, useRef, useEffect, useCallback } from 'react'; import useFormApi from '@data-driven-forms/react-form-renderer/use-form-api'; import useFieldApi from '@data-driven-forms/react-form-renderer/use-field-api'; import api from '../../../api'; @@ -31,6 +31,45 @@ const Packages = ({ defaultArch, ...props }) => { const [ filterChosen, setFilterChosen ] = useState(''); const [ focus, setFocus ] = useState(''); + const searchResultsComparator = useCallback((searchTerm) => { + return (a, b) => { + // check exact match first + if (a.name === searchTerm) { + return -1; + } + + if (b.name === searchTerm) { + return 1; + } + + // check for packages that start with the search term + if (a.name.startsWith(searchTerm) && !b.name.startsWith(searchTerm)) { + return -1; + } + + if (b.name.startsWith(searchTerm) && !a.name.startsWith(searchTerm)) { + return 1; + } + + // if both (or neither) start with the search term + // sort alphabetically + if (a.name < b.name) { + return -1; + } + + if (b.name < a.name) { + return 1; + } + + return 0; + }; + }); + + const sortPackages = useCallback((packageList) => { + const sortResults = packageList.sort(searchResultsComparator(packagesSearchName.current)); + setPackagesAvailable(sortResults); + }); + // call api to list available packages const handlePackagesAvailableSearch = async () => { const { data } = await api.getPackages( @@ -38,7 +77,7 @@ const Packages = ({ defaultArch, ...props }) => { getState()?.values?.architecture || defaultArch, packagesSearchName.current ); - setPackagesAvailable(data); + sortPackages(data); }; // filter displayed selected packages @@ -91,11 +130,11 @@ const Packages = ({ defaultArch, ...props }) => { }); if (fromAvailable) { - setPackagesAvailable(updatedSourcePackages); + sortPackages(updatedSourcePackages); setPackagesChosen([ ...destinationPackages ]); } else { setPackagesChosen(updatedSourcePackages); - setPackagesAvailable([ ...destinationPackages ]); + sortPackages([ ...destinationPackages ]); } // set the steps field to the current chosen packages list @@ -106,9 +145,9 @@ const Packages = ({ defaultArch, ...props }) => { const moveAll = (fromAvailable) => { if (fromAvailable) { setPackagesChosen([ ...packagesAvailable.filter(pack => !pack.isHidden), ...packagesChosen ]); - setPackagesAvailable([ ...packagesAvailable.filter(pack => pack.isHidden) ]); + sortPackages([ ...packagesAvailable.filter(pack => pack.isHidden) ]); } else { - setPackagesAvailable([ ...packagesChosen.filter(pack => !pack.isHidden), ...packagesAvailable ]); + sortPackages([ ...packagesChosen.filter(pack => !pack.isHidden), ...packagesAvailable ]); setPackagesChosen([ ...packagesChosen.filter(pack => pack.isHidden) ]); } @@ -124,7 +163,7 @@ const Packages = ({ defaultArch, ...props }) => { } else { const newAvailable = [ ...packagesAvailable ]; newAvailable[index].selected = !packagesAvailable[index].selected; - setPackagesAvailable(newAvailable); + sortPackages(newAvailable); } }; @@ -150,7 +189,7 @@ const Packages = ({ defaultArch, ...props }) => { Search ] }> - + {packagesAvailable.map((pack, index) => { return !pack.isHidden ? ( { + userEvent.type(searchbox, searchTerm); + await act(async() => { + screen.getByTestId('search-available-pkgs-button').click(); + }); +}; + // mock the insights dependency beforeAll(() => { global.insights = { @@ -431,6 +461,75 @@ describe('Step Packages', () => { name: 'Search button for available packages' }); }); + + test('search results should be sorted with most relevant results first', async () => { + const searchbox = screen.getAllByRole('textbox')[0]; // searching by id doesn't update the input ref + + searchbox.click(); + + const getPackages = jest + .spyOn(api, 'getPackages') + .mockImplementation(() => Promise.resolve(mockPkgResult)); + + await searchForPackages(searchbox, 'test'); + expect(getPackages).toHaveBeenCalledTimes(1); + + const availablePackages = screen.getByTestId('available-pkgs-list'); + expect(availablePackages.children.length).toEqual(3); + const [ firstItem, secondItem, thirdItem ] = availablePackages.children; + expect(firstItem).toHaveTextContent('testsummary for test package'); + expect(secondItem).toHaveTextContent('testPkgtest package summary'); + expect(thirdItem).toHaveTextContent('lib-testlib-test package summary'); + }); + + test('search results should be sorted after selecting them and then deselecting them', async () => { + const searchbox = screen.getAllByRole('textbox')[0]; // searching by id doesn't update the input ref + + searchbox.click(); + + const getPackages = jest + .spyOn(api, 'getPackages') + .mockImplementation(() => Promise.resolve(mockPkgResult)); + + await searchForPackages(searchbox, 'test'); + expect(getPackages).toHaveBeenCalledTimes(1); + + screen.getByRole('option', { name: /testPkg test package summary/ }).click(); + screen.getByRole('button', { name: /Add selected/ }).click(); + + screen.getByRole('option', { name: /testPkg test package summary/ }).click(); + screen.getByRole('button', { name: /Remove selected/ }).click(); + + const availablePackages = screen.getByTestId('available-pkgs-list'); + expect(availablePackages.children.length).toEqual(3); + const [ firstItem, secondItem, thirdItem ] = availablePackages.children; + expect(firstItem).toHaveTextContent('testsummary for test package'); + expect(secondItem).toHaveTextContent('testPkgtest package summary'); + expect(thirdItem).toHaveTextContent('lib-testlib-test package summary'); + }); + + test('search results should be sorted after adding and then removing all packages', async () => { + const searchbox = screen.getAllByRole('textbox')[0]; // searching by id doesn't update the input ref + + searchbox.click(); + + const getPackages = jest + .spyOn(api, 'getPackages') + .mockImplementation(() => Promise.resolve(mockPkgResult)); + + await searchForPackages(searchbox, 'test'); + expect(getPackages).toHaveBeenCalledTimes(1); + + screen.getByRole('button', { name: /Add all/ }).click(); + screen.getByRole('button', { name: /Remove all/ }).click(); + + const availablePackages = screen.getByTestId('available-pkgs-list'); + expect(availablePackages.children.length).toEqual(3); + const [ firstItem, secondItem, thirdItem ] = availablePackages.children; + expect(firstItem).toHaveTextContent('testsummary for test package'); + expect(secondItem).toHaveTextContent('testPkgtest package summary'); + expect(thirdItem).toHaveTextContent('lib-testlib-test package summary'); + }); }); describe('Step Review', () => { @@ -527,24 +626,11 @@ describe('Click through all steps', () => { // packages const getPackages = jest .spyOn(api, 'getPackages') - .mockImplementation(() => { - return Promise.resolve({ - meta: { count: 100 }, - links: { first: '', last: '' }, - data: [ - { - name: 'testPkg', - summary: 'test package summary', - version: '1.0', - } - ], - }); - }); + .mockImplementation(() => Promise.resolve(mockPkgResult)); screen.getByText('Add optional additional packages to your image by searching available packages.'); - userEvent.type(screen.getByTestId('search-available-pkgs-input'), 'test'); - screen.getByTestId('search-available-pkgs-button').click(); - await expect(getPackages).toHaveBeenCalledTimes(1); + await searchForPackages(screen.getByTestId('search-available-pkgs-input'), 'test'); + expect(getPackages).toHaveBeenCalledTimes(1); screen.getByRole('option', { name: /testPkg test package summary/ }).click(); screen.getByRole('button', { name: /Add selected/ }).click(); next.click(); @@ -561,7 +647,6 @@ describe('Click through all steps', () => { const composeImage = jest .spyOn(api, 'composeImage') .mockImplementation(body => { - console.log(body, 'huuuh!'); let id; if (body.image_requests[0].upload_request.type === 'aws') { expect(body).toEqual({