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.

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

CoursewareSearch - Show filters if needed only #1287

Merged
merged 4 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 31 additions & 21 deletions src/course-home/courseware-search/CoursewareResultsFilter.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 <CoursewareSearchResults />; }

const results = useMemo(() => {
rijuma marked this conversation as resolved.
Show resolved Hide resolved
// 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 <CoursewareSearchResults results={results[filterAll]} />; }

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 (
<Tabs
Expand All @@ -54,7 +64,7 @@ export const CoursewareSearchResultsFilter = ({ intl }) => {
activeKey={activeKey}
onSelect={setFilter}
>
{filters.map(({ key, label }) => (
{filters.filter(({ count }) => (count > 0)).map(({ key, label }) => (
<Tab key={key} eventKey={key} title={label} data-testid={`courseware-search-results-tabs-${key}`}>
<CoursewareSearchResults results={results[key]} />
</Tab>
Expand Down
88 changes: 57 additions & 31 deletions src/course-home/courseware-search/CoursewareResultsFilter.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,49 +52,75 @@ function renderComponent(props = {}) {
describe('CoursewareSearchResultsFilter', () => {
beforeAll(initializeMockApp);

describe('</CoursewareSearchResultsFilter />', () => {
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();
});
});
});
20 changes: 10 additions & 10 deletions src/course-home/courseware-search/CoursewareSearch.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,16 @@ const CoursewareSearch = ({ intl, ...sectionProps }) => {
)}
{status === 'results' ? (
<>
<div
className="courseware-search__results-summary"
aria-live="polite"
aria-relevant="all"
aria-atomic="true"
data-testid="courseware-search-summary"
>{total > 0
? intl.formatMessage(messages.searchResultsLabel, { total, keyword: lastSearchKeyword })
: intl.formatMessage(messages.searchResultsNone)}
</div>
{total > 0 ? (
<div
className="courseware-search__results-summary"
aria-live="polite"
aria-relevant="all"
aria-atomic="true"
data-testid="courseware-search-summary"
>{intl.formatMessage(messages.searchResultsLabel, { total, keyword: lastSearchKeyword })}
</div>
) : null}
<CoursewareSearchResultsFilterContainer />
</>
) : null}
Expand Down
4 changes: 2 additions & 2 deletions src/course-home/courseware-search/CoursewareSearch.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
5 changes: 2 additions & 3 deletions src/course-home/courseware-search/CoursewareSearchResults.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ exports[`CoursewareSearchResults when list of results is provided should match t
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M3 3v18h18V3H3Zm11 14H7v-2h7v2Zm3-4H7v-2h10v2Zm0-4H7V7h10v2Z"
d="M10 4H2v16h20V6H12l-2-2z"
fill="currentColor"
/>
</svg>
Expand Down Expand Up @@ -140,7 +140,7 @@ exports[`CoursewareSearchResults when list of results is provided should match t
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M3 3v18h18V3H3Zm11 14H7v-2h7v2Zm3-4H7v-2h10v2Zm0-4H7V7h10v2Z"
d="M10 4H2v16h20V6H12l-2-2z"
fill="currentColor"
/>
</svg>
Expand Down
2 changes: 2 additions & 0 deletions src/course-home/courseware-search/courseware-search.scss
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@

&__empty {
color: $gray-500;
padding: 6rem 0;
text-align: center;
}

&__item {
Expand Down
7 changes: 4 additions & 3 deletions src/course-home/courseware-search/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
Expand Down
49 changes: 47 additions & 2 deletions src/course-home/courseware-search/hooks.test.jsx
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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');
});
});
});
Loading