diff --git a/src/course-home/courseware-search/CoursewareResultsFilter.jsx b/src/course-home/courseware-search/CoursewareResultsFilter.jsx index 7f4217b731..6ff443ce32 100644 --- a/src/course-home/courseware-search/CoursewareResultsFilter.jsx +++ b/src/course-home/courseware-search/CoursewareResultsFilter.jsx @@ -8,15 +8,10 @@ import messages from './messages'; import { useCoursewareSearchParams } from './hooks'; import { useModel } from '../../generic/model-store'; -const allFilterKey = 'all'; -const otherFilterKey = 'other'; -const allowedFilterKeys = { - [allFilterKey]: true, - text: true, - video: true, - sequence: true, - [otherFilterKey]: true, -}; +const filterAll = 'all'; +const filterTypes = ['text', 'video', 'sequence']; +const filterOther = 'other'; +const validFilters = [filterAll, ...filterTypes, filterOther]; export const CoursewareSearchResultsFilter = ({ intl }) => { const { courseId } = useParams(); @@ -27,23 +22,38 @@ export const CoursewareSearchResultsFilter = ({ intl }) => { const { results: data = [] } = lastSearch; - const results = useMemo(() => data.reduce((acc, { type, ...rest }) => { - acc[allFilterKey] = [...(acc[allFilterKey] || []), { type: allFilterKey, ...rest }]; - if (type === allFilterKey) { return acc; } + // If there's no data, we show an empty result. + if (!data.length) { return ; } + + const results = useMemo(() => { + // This reducer distributes the data into different groups to make it easy to + // use on the filters. + // All results are added to the "all" key and then to its proper group key as well. + const grouped = data.reduce((acc, { type, ...rest }) => { + const resultType = filterTypes.includes(type) ? type : filterOther; + acc[filterAll].push({ type: resultType, ...rest }); + acc[resultType] = [...(acc[resultType] || []), { type: resultType, ...rest }]; + return acc; + }, { [filterAll]: [] }); + + // This is just to format the output object with the expected tab order. + const output = {}; + validFilters.forEach(key => { if (grouped[key]) { output[key] = grouped[key]; } }); + + return output; + }, [lastSearch]); - let targetKey = otherFilterKey; - if (allowedFilterKeys[type]) { targetKey = type; } - acc[targetKey] = [...(acc[targetKey] || []), { type: targetKey, ...rest }]; - return acc; - }, {}), [data]); + const tabKeys = Object.keys(results); + // Filter has no use if it has only 2 tabs (The "all" tab and another one with the same items). + if (tabKeys.length < 3) { return ; } - const filters = useMemo(() => Object.keys(allowedFilterKeys).map((key) => ({ + const filters = useMemo(() => tabKeys.map((key) => ({ key, label: intl.formatMessage(messages[`filter:${key}`]), - count: results[key]?.length || 0, + count: results[key].length, })), [results]); - const activeKey = allowedFilterKeys[filterKeyword] ? filterKeyword : allFilterKey; + const activeKey = validFilters.includes(filterKeyword) ? filterKeyword : filterAll; return ( { activeKey={activeKey} onSelect={setFilter} > - {filters.map(({ key, label }) => ( + {filters.filter(({ count }) => (count > 0)).map(({ key, label }) => ( diff --git a/src/course-home/courseware-search/CoursewareResultsFilter.test.jsx b/src/course-home/courseware-search/CoursewareResultsFilter.test.jsx index 28251e1139..785167003c 100644 --- a/src/course-home/courseware-search/CoursewareResultsFilter.test.jsx +++ b/src/course-home/courseware-search/CoursewareResultsFilter.test.jsx @@ -52,49 +52,75 @@ function renderComponent(props = {}) { describe('CoursewareSearchResultsFilter', () => { beforeAll(initializeMockApp); - describe('', () => { - beforeEach(() => { - useCoursewareSearchParams.mockReturnValue(coursewareSearch); - }); + beforeEach(() => { + useCoursewareSearchParams.mockReturnValue(coursewareSearch); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); - afterEach(() => { - jest.clearAllMocks(); + describe('when returning full results', () => { + beforeEach(async () => { + useModel.mockReturnValue(searchResultsFactory()); + await renderComponent(); }); it('should render without errors', async () => { - useModel.mockReturnValue(searchResultsFactory()); + await waitFor(() => { + expect(useCoursewareSearchParams).toBeCalled(); + }); + expect(screen.queryByTestId('courseware-search-results-tabs')).toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-results-tabs-all')).toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-results-tabs-text')).toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-results-tabs-video')).toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-results-tabs-sequence')).toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-results-tabs-other')).toBeInTheDocument(); + }); + }); + + describe('when returning only one result type', () => { + beforeEach(async () => { + // Get results for only videos + const data = searchResultsFactory(); + const onlyVideos = data.results.filter(({ type }) => type === 'video'); + const filteredResults = { + ...data, + results: onlyVideos, + }; + + useModel.mockReturnValue(filteredResults); await renderComponent(); + }); + it('should not render', async () => { await waitFor(() => { - expect(screen.queryByTestId('courseware-search-results-tabs-all')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-text')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-video')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-sequence')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-other')).toBeInTheDocument(); + expect(useCoursewareSearchParams).toBeCalled(); }); + + expect(screen.queryByTestId('courseware-search-results-tabs')).not.toBeInTheDocument(); + }); + }); + + describe('when there are not results', () => { + beforeEach(async () => { + useModel.mockReturnValue(searchResultsFactory('blah', { + results: [], + filters: [], + total: 0, + maxScore: null, + ms: 5, + })); + await renderComponent(); }); - describe('when there are not results', () => { - it('should render without errors', async () => { - useModel.mockReturnValue(searchResultsFactory('blah', { - results: [], - filters: [], - total: 0, - maxScore: null, - ms: 5, - })); - - await renderComponent(); - - await waitFor(() => { - expect(screen.queryByTestId('courseware-search-results-tabs-all')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-text')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-video')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-sequence')).toBeInTheDocument(); - expect(screen.queryByTestId('courseware-search-results-tabs-other')).toBeInTheDocument(); - }); + it('should not render', async () => { + await waitFor(() => { + expect(useCoursewareSearchParams).toBeCalled(); }); + + expect(screen.queryByTestId('courseware-search-results-tabs')).not.toBeInTheDocument(); }); }); }); diff --git a/src/course-home/courseware-search/CoursewareSearch.jsx b/src/course-home/courseware-search/CoursewareSearch.jsx index 6bdd53a0c9..5fb8c9742c 100644 --- a/src/course-home/courseware-search/CoursewareSearch.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.jsx @@ -122,16 +122,16 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => { )} {status === 'results' ? ( <> -
{total > 0 - ? intl.formatMessage(messages.searchResultsLabel, { total, keyword: lastSearchKeyword }) - : intl.formatMessage(messages.searchResultsNone)} -
+ {total > 0 ? ( +
{intl.formatMessage(messages.searchResultsLabel, { total, keyword: lastSearchKeyword })} +
+ ) : null} ) : null} diff --git a/src/course-home/courseware-search/CoursewareSearch.test.jsx b/src/course-home/courseware-search/CoursewareSearch.test.jsx index 125125a728..38353cd54e 100644 --- a/src/course-home/courseware-search/CoursewareSearch.test.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.test.jsx @@ -236,14 +236,14 @@ describe('CoursewareSearch', () => { expect(screen.queryByTestId('courseware-search-error')).toBeInTheDocument(); }); - it('should show "No results found." if results is empty', () => { + it('should not show a summary if there are no results', () => { mockModels({ searchKeyword: 'test', total: 0, }); renderComponent(); - expect(screen.queryByTestId('courseware-search-summary').textContent).toBe('No results found.'); + expect(screen.queryByTestId('courseware-search-summary')).not.toBeInTheDocument(); }); it('should show a summary for the results', () => { diff --git a/src/course-home/courseware-search/CoursewareSearchResults.jsx b/src/course-home/courseware-search/CoursewareSearchResults.jsx index 04f9344b77..f8bc910890 100644 --- a/src/course-home/courseware-search/CoursewareSearchResults.jsx +++ b/src/course-home/courseware-search/CoursewareSearchResults.jsx @@ -8,9 +8,10 @@ import PropTypes from 'prop-types'; import CoursewareSearchEmpty from './CoursewareSearchEmpty'; const iconTypeMapping = { - document: Folder, text: TextFields, video: VideoCamera, + sequence: Folder, + other: Article, }; const defaultIcon = Article; @@ -34,9 +35,7 @@ const CoursewareSearchResults = ({ results = [] }) => { }) => { const key = type.toLowerCase(); const icon = iconTypeMapping[key] || defaultIcon; - const isExternal = !url.startsWith('/'); - const linkProps = isExternal ? { href: url, target: '_blank', diff --git a/src/course-home/courseware-search/__snapshots__/CoursewareSearchResults.test.jsx.snap b/src/course-home/courseware-search/__snapshots__/CoursewareSearchResults.test.jsx.snap index c38958db29..12178a62ed 100644 --- a/src/course-home/courseware-search/__snapshots__/CoursewareSearchResults.test.jsx.snap +++ b/src/course-home/courseware-search/__snapshots__/CoursewareSearchResults.test.jsx.snap @@ -26,7 +26,7 @@ exports[`CoursewareSearchResults when list of results is provided should match t xmlns="http://www.w3.org/2000/svg" > @@ -140,7 +140,7 @@ exports[`CoursewareSearchResults when list of results is provided should match t xmlns="http://www.w3.org/2000/svg" > diff --git a/src/course-home/courseware-search/courseware-search.scss b/src/course-home/courseware-search/courseware-search.scss index 3acfafb740..aa96b5b47c 100644 --- a/src/course-home/courseware-search/courseware-search.scss +++ b/src/course-home/courseware-search/courseware-search.scss @@ -51,6 +51,8 @@ &__empty { color: $gray-500; + padding: 6rem 0; + text-align: center; } &__item { diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js index a7e64a4c3d..57222d41f8 100644 --- a/src/course-home/courseware-search/hooks.js +++ b/src/course-home/courseware-search/hooks.js @@ -70,12 +70,13 @@ export function useLockScroll() { }, []); } +const initSearchParams = { q: '', f: '' }; export function useCoursewareSearchParams() { - const [searchParams, setSearchParams] = useSearchParams(); - const clearSearchParams = () => setSearchParams({ q: '', f: '' }); + const [searchParams, setSearchParams] = useSearchParams(initSearchParams); + const clearSearchParams = () => setSearchParams(initSearchParams); const query = searchParams.get('q'); - const filter = searchParams.get('f'); + const filter = searchParams.get('f')?.toLowerCase(); const setQuery = (q) => setSearchParams((params) => ({ q, f: params.get('f') })); const setFilter = (f) => setSearchParams((params) => ({ q: params.get('q'), f })); diff --git a/src/course-home/courseware-search/hooks.test.jsx b/src/course-home/courseware-search/hooks.test.jsx index cc6b67554a..bbf7016251 100644 --- a/src/course-home/courseware-search/hooks.test.jsx +++ b/src/course-home/courseware-search/hooks.test.jsx @@ -1,9 +1,13 @@ import { renderHook, act } from '@testing-library/react-hooks'; -import { useParams } from 'react-router-dom'; +import { useParams, useSearchParams } from 'react-router-dom'; import { useSelector } from 'react-redux'; import { fetchCoursewareSearchSettings } from '../data/thunks'; import { - useCoursewareSearchFeatureFlag, useCoursewareSearchState, useElementBoundingBox, useLockScroll, + useCoursewareSearchFeatureFlag, + useCoursewareSearchParams, + useCoursewareSearchState, + useElementBoundingBox, + useLockScroll, } from './hooks'; jest.mock('react-redux'); @@ -184,4 +188,45 @@ describe('CoursewareSearch Hooks', () => { expect(removeBodyClassSpy).toHaveBeenCalledWith('_search-no-scroll'); }); }); + + describe('useSearchParams', () => { + const initSearch = { q: '', f: '' }; + const q = { value: '' }; + const f = { value: '' }; + const mockedQuery = { q, f }; + const searchParams = { get: (prop) => mockedQuery[prop].value }; + const setSearchParams = jest.fn(); + + beforeEach(() => { + useSearchParams.mockImplementation(() => [searchParams, setSearchParams]); + }); + + it('should init the search params properly', () => { + const { + query, filter, setQuery, setFilter, clearSearchParams, + } = useCoursewareSearchParams(); + + expect(useSearchParams).toBeCalledWith(initSearch); + expect(query).toBe(''); + expect(filter).toBe(''); + + setQuery('setQuery'); + expect(setSearchParams).toBeCalledWith(expect.any(Function)); + + setFilter('setFilter'); + expect(setSearchParams).toBeCalledWith(expect.any(Function)); + + clearSearchParams(); + expect(setSearchParams).toBeCalledWith(initSearch); + }); + + it('should return the query and lowercase filter if any', () => { + q.value = '42'; + f.value = 'LOWERCASE'; + const { query, filter } = useCoursewareSearchParams(); + + expect(query).toBe('42'); + expect(filter).toBe('lowercase'); + }); + }); });