diff --git a/src/IsaacAppTypes.tsx b/src/IsaacAppTypes.tsx index 94a3dad922..3a2996738e 100644 --- a/src/IsaacAppTypes.tsx +++ b/src/IsaacAppTypes.tsx @@ -142,9 +142,6 @@ export type Action = | {type: ACTION_TYPE.TOPIC_RESPONSE_SUCCESS; topic: ApiTypes.IsaacTopicSummaryPageDTO} | {type: ACTION_TYPE.TOPIC_RESPONSE_FAILURE} - | {type: ACTION_TYPE.SEARCH_REQUEST; query: string; types: string | undefined} - | {type: ACTION_TYPE.SEARCH_RESPONSE_SUCCESS; searchResults: ApiTypes.ResultsWrapper} - | {type: ACTION_TYPE.TOASTS_SHOW; toast: Toast} | {type: ACTION_TYPE.TOASTS_HIDE; toastId: string} | {type: ACTION_TYPE.TOASTS_REMOVE; toastId: string} diff --git a/src/app/components/elements/SearchInputs.tsx b/src/app/components/elements/SearchInputs.tsx index 6b0e1804d7..622270d6de 100644 --- a/src/app/components/elements/SearchInputs.tsx +++ b/src/app/components/elements/SearchInputs.tsx @@ -1,8 +1,7 @@ -import React, {ChangeEvent, FormEvent, useEffect, useRef, useState} from "react"; +import React, {ChangeEvent, useCallback, useEffect, useRef, useState} from "react"; import {Button, Form, Input, InputGroup, InputProps, Label} from "reactstrap"; -import {ifKeyIsEnter, pushSearchToHistory, SEARCH_CHAR_LENGTH_LIMIT, siteSpecific} from "../../services"; +import {ifKeyIsEnter, SEARCH_CHAR_LENGTH_LIMIT, siteSpecific} from "../../services"; import classNames from "classnames"; -import { useHistory, useLocation } from "react-router"; interface SearchInputProps { setSearchText: (s: string) => void; @@ -23,22 +22,29 @@ function withSearch(Component: React.FC) { const [searchText, setSearchText] = useState(initialValue ?? ""); const searchInputRef = useRef(null); - const history = useHistory(); - function doSearch(e: FormEvent) { - e.preventDefault(); - if (searchText === "") { + const doSearch = useCallback((text: string) => { + if (text === "") { if (searchInputRef.current) searchInputRef.current.focus(); } else { - onSearch?.(searchText); - pushSearchToHistory(history, searchText, []); + onSearch?.(text); } - } + }, [onSearch]); - // Clear this search field on location (i.e. search query) change - user should use the main search bar - const location = useLocation(); - useEffect(() => { if (location.pathname === "/search") { setSearchText(initialValue ?? ""); }}, [location]); + useEffect(() => { + // If the initial value changes, update the search text - allows the search input to reflect URL changes + if (initialValue !== undefined) { + setSearchText(initialValue); + } + }, [initialValue]); - return
+ return { + e.preventDefault(); + doSearch(searchText); + }} + >
{cardBody} ; diff --git a/src/app/components/pages/Search.tsx b/src/app/components/pages/Search.tsx index 9b70b5de45..545130a728 100644 --- a/src/app/components/pages/Search.tsx +++ b/src/app/components/pages/Search.tsx @@ -1,6 +1,6 @@ -import React, {FormEvent, MutableRefObject, useEffect, useRef, useState} from "react"; +import React, {useEffect, useMemo, useState} from "react"; import {RouteComponentProps, withRouter} from "react-router-dom"; -import {AppState, fetchSearch, selectors, useAppDispatch, useAppSelector} from "../../state"; +import {selectors, useAppSelector, useSearchRequestQuery} from "../../state"; import { Card, CardBody, @@ -10,11 +10,11 @@ import { Form, Container, } from "reactstrap"; -import {ShowLoading} from "../handlers/ShowLoading"; import { DOCUMENT_TYPE, documentDescription, isAda, + isDefined, parseLocationSearch, pushSearchToHistory, SEARCH_RESULT_TYPE, @@ -28,11 +28,15 @@ import {TitleAndBreadcrumb} from "../elements/TitleAndBreadcrumb"; import {ShortcutResponse} from "../../../IsaacAppTypes"; import {UserContextPicker} from "../elements/inputs/UserContextPicker"; import {CSSObjectWithLabel, GroupBase, StylesConfig} from "react-select"; -import {IsaacSpinner} from "../handlers/IsaacSpinner"; import classNames from "classnames"; import {SearchPageSearch} from "../elements/SearchInputs"; import {StyledSelect} from "../elements/inputs/StyledSelect"; import { ListView } from "../elements/list-groups/ListView"; +import { ContentSummaryDTO } from "../../../IsaacApiTypes"; +import { ShowLoadingQuery } from "../handlers/ShowLoadingQuery"; +import { History } from "history"; +import { debounce } from "lodash"; +import { skipToken } from "@reduxjs/toolkit/query"; interface Item { value: T; @@ -57,70 +61,61 @@ const selectStyle: StylesConfig, true, GroupBase ({ ...base, zIndex: 19 }) }; +function updateSearchUrl(history: History, queryState: Nullable, filtersState: Item[]) { + pushSearchToHistory(history, queryState || "", filtersState.map(deitemise)); +} + // Interacting with the page's filters change the query parameters. // Whenever the query parameters change we send a search request to the API. export const Search = withRouter((props: RouteComponentProps) => { const {location, history} = props; - const dispatch = useAppDispatch(); - const searchResults = useAppSelector((state: AppState) => state?.search?.searchResults || null); const user = useAppSelector(selectors.user.orNull); const [urlQuery, urlFilters] = parseLocationSearch(location.search); - const [queryState, setQueryState] = useState(urlQuery); - + let initialFilters = urlFilters; if (isAda && urlFilters.length === 0) { initialFilters = [DOCUMENT_TYPE.CONCEPT, DOCUMENT_TYPE.TOPIC_SUMMARY, DOCUMENT_TYPE.GENERIC] as SearchableDocumentType[]; } const [filtersState, setFiltersState] = useState[]>(initialFilters.map(itemise)); + const [queryState, setQueryState] = useState(urlQuery); - useEffect(function triggerSearchAndUpdateLocalStateOnUrlChange() { - dispatch(fetchSearch(urlQuery ?? "", initialFilters.length ? initialFilters.join(",") : undefined)); - setQueryState(urlQuery); - setFiltersState(initialFilters.map(itemise)); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [dispatch, location.search]); + // searchQuery is really just {queryState, filtersState}, but updating it triggers a request; we wish to debounce this, so the state is kept separate + const [searchQuery, setSearchQuery] = useState<{query: string; types: string} | typeof skipToken>(skipToken); + const searchResult = useSearchRequestQuery(searchQuery); - function updateSearchUrl(e?: FormEvent) { - if (e) {e.preventDefault();} - pushSearchToHistory(history, queryState || "", filtersState.map(deitemise)); - } + // Trigger update to query on state change + const onUpdate = useMemo(() => { + return debounce((query: Nullable, filters: Item[]) => { + setSearchQuery(query ? {query, types: filters.map(deitemise).join(",")} : skipToken); + updateSearchUrl(history, query, filters); + }, 500, {leading: true, trailing: true}); + }, [history]); - // Trigger update to search url on query or filter change - const timer: MutableRefObject = useRef(); useEffect(() => { - if (queryState !== urlQuery) { - timer.current = window.setTimeout(() => {updateSearchUrl();}, 800); - return () => {clearTimeout(timer.current);}; - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [queryState]); + onUpdate(queryState, filtersState); + }, [queryState, filtersState, onUpdate]); - useEffect(() => { - const filtersStateMatchesQueryParamFilters = - filtersState.length === initialFilters.length && - filtersState.map(deitemise).every(f => initialFilters.includes(f)); - if (!filtersStateMatchesQueryParamFilters) { - updateSearchUrl(); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [filtersState]); + useEffect(function triggerSearchOnUrlChange() { + setQueryState(urlQuery); + }, [urlQuery]); // Process results and add shortcut responses - const filteredSearchResults = searchResults?.results && searchResults.results - .filter(result => searchResultIsPublic(result, user)); - const shortcutResponses = (queryState ? shortcuts(queryState) : []) as ShortcutResponse[]; - const shortcutAndFilteredSearchResults = (shortcutResponses || []).concat(filteredSearchResults || []); - const gotResults = shortcutAndFilteredSearchResults && shortcutAndFilteredSearchResults.length > 0; + + const shortcutAndFilterResults = (results?: ContentSummaryDTO[]) => { + const filteredSearchResults = results && results.filter(result => searchResultIsPublic(result, user)); + const shortcutResponses = (queryState ? shortcuts(queryState) : []) as ShortcutResponse[]; + return (shortcutResponses || []).concat(filteredSearchResults || []); + }; return ( - +

- Search Results {urlQuery != "" ? shortcutAndFilteredSearchResults ? {shortcutAndFilteredSearchResults.length} : : null} + Search Results {urlQuery != "" && isDefined(searchResult?.currentData?.results) ? {searchResult?.currentData?.results.length} : null}

@@ -147,12 +142,17 @@ export const Search = withRouter((props: RouteComponentProps) => {
- {urlQuery != "" && - - {gotResults - ? - : No results found} - + {urlQuery !== "" && + { + const shortcutAndFilteredSearchResults = shortcutAndFilterResults(results); + return shortcutAndFilteredSearchResults.length > 0 + ? + : No results found; + }} + /> }
diff --git a/src/app/components/site/phy/NavigationMenuPhy.tsx b/src/app/components/site/phy/NavigationMenuPhy.tsx index d9471a694b..f68006cdf8 100644 --- a/src/app/components/site/phy/NavigationMenuPhy.tsx +++ b/src/app/components/site/phy/NavigationMenuPhy.tsx @@ -7,7 +7,7 @@ import { HUMAN_STAGES, HUMAN_SUBJECTS, LearningStage, PATHS, PHY_NAV_STAGES, PHY import { selectors, useAppSelector } from "../../../state"; import { LoginLogoutButton } from "./HeaderPhy"; import { useAssignmentsCount } from "../../navigation/NavigationBar"; -import { Link } from "react-router-dom"; +import { Link, useHistory } from "react-router-dom"; import { HoverableNavigationContext, PageContextState } from "../../../../IsaacAppTypes"; import max from "lodash/max"; @@ -424,6 +424,7 @@ export const NavigationMenuPhy = ({toggleMenu}: {toggleMenu: () => void}) => { // while moving the mouse between two hoverables, preventing the second dropdown from opening. const deviceSize = useDeviceSize(); + const history = useHistory(); const stageCategories : NavigationCategory[] = Object.entries(PHY_NAV_STAGES).map(([stage, subjects]) => { const humanStage = HUMAN_STAGES[stage]; @@ -461,7 +462,10 @@ export const NavigationMenuPhy = ({toggleMenu}: {toggleMenu: () => void}) => { return {below["sm"](deviceSize) &&
- + { + history.push(`/search?query=${encodeURIComponent(s)}`); + toggleMenu(); + }}/>
} @@ -471,7 +475,7 @@ export const NavigationMenuPhy = ({toggleMenu}: {toggleMenu: () => void}) => { {above["md"](deviceSize) && <>
- + history.push(`/search?query=${encodeURIComponent(s)}`)} />
}
; diff --git a/src/app/services/constants.ts b/src/app/services/constants.ts index c8d263f940..979fe3e6bb 100644 --- a/src/app/services/constants.ts +++ b/src/app/services/constants.ts @@ -214,9 +214,6 @@ export enum ACTION_TYPE { TOPIC_RESPONSE_SUCCESS = "TOPIC_RESPONSE_SUCCESS", TOPIC_RESPONSE_FAILURE = "TOPIC_RESPONSE_FAILURE", - SEARCH_REQUEST = "SEARCH_REQUEST", - SEARCH_RESPONSE_SUCCESS = "SEARCH_RESPONSE_SUCCESS", - TOASTS_SHOW = "TOASTS_SHOW", TOASTS_HIDE = "TOASTS_HIDE", TOASTS_REMOVE = "TOASTS_REMOVE", diff --git a/src/app/state/actions/index.tsx b/src/app/state/actions/index.tsx index d8ed976436..9cd727622b 100644 --- a/src/app/state/actions/index.tsx +++ b/src/app/state/actions/index.tsx @@ -583,20 +583,6 @@ export const testQuestion = (questionChoices: FreeTextRule[], testCases: TestCas } }; -// Search -export const fetchSearch = (query: string, types: string | undefined) => async (dispatch: Dispatch) => { - dispatch({type: ACTION_TYPE.SEARCH_REQUEST, query, types}); - try { - if (query === "") { - return; - } - const searchResponse = await api.search.get(query, types); - dispatch({type: ACTION_TYPE.SEARCH_RESPONSE_SUCCESS, searchResults: searchResponse.data}); - } catch (e) { - dispatch(showAxiosErrorToastIfNeeded("Search failed", e)); - } -}; - // Admin export const resetMemberPassword = (member: AppGroupMembership) => async (dispatch: Dispatch) => { diff --git a/src/app/state/index.ts b/src/app/state/index.ts index 9936e9e923..5f2b1c21a2 100644 --- a/src/app/state/index.ts +++ b/src/app/state/index.ts @@ -16,7 +16,6 @@ export * from "./slices/doc"; export * from "./slices/linkableSetting"; export * from "./actions/logging"; export * from "./reducers/staticState"; -export * from "./reducers/searchState"; export * from "./middleware/userConsistencyCheckerCurrentUser"; export * from "./middleware/hidePreviousQuestionAttempt"; export * from "./reducers/quizState"; @@ -46,6 +45,7 @@ export * from "./slices/api/questionsApi"; export * from "./slices/api/quizApi"; export * from "./slices/api/miscApi"; export * from "./slices/api/topicsApi"; +export * from "./slices/api/searchApi"; export * from "./slices/gameboards"; export * from "./reducers/userState"; export * from "./actions/popups"; diff --git a/src/app/state/reducers/index.ts b/src/app/state/reducers/index.ts index 6a32e87998..03f6557d1f 100644 --- a/src/app/state/reducers/index.ts +++ b/src/app/state/reducers/index.ts @@ -21,7 +21,6 @@ import { quizAttempt, groupMemberships, questionSearchResult, - search, isaacApi, gameboardsSlice, adminUserSearchSlice, @@ -83,9 +82,6 @@ export const rootReducer = combineReducers({ // Gameboards boards: gameboardsSlice.reducer, questionSearchResult, - - // Search - search, // Quizzes quizAttempt, diff --git a/src/app/state/reducers/searchState.ts b/src/app/state/reducers/searchState.ts deleted file mode 100644 index 8180a63dd6..0000000000 --- a/src/app/state/reducers/searchState.ts +++ /dev/null @@ -1,15 +0,0 @@ -import {ContentSummaryDTO, ResultsWrapper} from "../../../IsaacApiTypes"; -import {Action} from "../../../IsaacAppTypes"; -import {ACTION_TYPE} from "../../services"; - -type SearchState = {searchResults: ResultsWrapper | null} | null; -export const search = (search: SearchState = null, action: Action) => { - switch (action.type) { - case ACTION_TYPE.SEARCH_REQUEST: - return {...search, searchResults: null}; - case ACTION_TYPE.SEARCH_RESPONSE_SUCCESS: - return {...search, searchResults: action.searchResults}; - default: - return search; - } -}; diff --git a/src/app/state/slices/api/searchApi.ts b/src/app/state/slices/api/searchApi.ts new file mode 100644 index 0000000000..01cbfc31cc --- /dev/null +++ b/src/app/state/slices/api/searchApi.ts @@ -0,0 +1,22 @@ +import { ContentSummaryDTO, ResultsWrapper } from "../../../../IsaacApiTypes"; +import { isaacApi } from "./baseApi"; +import { onQueryLifecycleEvents } from "./utils"; + +const searchApi = isaacApi.injectEndpoints({ + endpoints: (build) => ({ + searchRequest: build.query | null, {query: string, types?: string}>({ + query: ({query, types}) => ({ + url: `/search`, + params: {query, types}, + }), + + onQueryStarted: onQueryLifecycleEvents({ + errorTitle: "Unable to perform search", + }), + }) + }) +}); + +export const { + useSearchRequestQuery, +} = searchApi; diff --git a/src/test/pages/SiteSearch.test.tsx b/src/test/pages/SiteSearch.test.tsx new file mode 100644 index 0000000000..d9b3f76c21 --- /dev/null +++ b/src/test/pages/SiteSearch.test.tsx @@ -0,0 +1,74 @@ +import { http } from "msw"; +import { API_PATH } from "../../app/services"; +import { expectH1, renderTestEnvironment, setUrl } from "../testUtils"; +import { fireEvent, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +describe("Site search", () => { + const searchHandler = jest.fn(); + + const renderSiteSearch = async (params?: `?${string}`) => { + renderTestEnvironment({ + extraEndpoints: [ + http.get(API_PATH + "/search", searchHandler), + ] + }); + await setUrl({pathname: "/search", search: params}); + expectH1("Search"); + }; + + afterEach(() => { + searchHandler.mockReset(); + }); + + it("should display no results on load with an empty query", async () => { + await renderSiteSearch(); + expect(screen.queryAllByTestId("list-view-item")).toHaveLength(0); + }); + + it("typing a query should trigger a search", async () => { + await renderSiteSearch(); + + await userEvent.type( + within(screen.getByRole("main")).getByRole("searchbox"), + "energy" + ); + fireEvent.submit(within(screen.getByRole("main")).getByTestId("search-form")); + + await waitFor(() => { + expect(searchHandler).toHaveBeenCalledTimes(1); + expect(searchHandler).toHaveBeenRequestedWith(async (req) => { + return req.request.url.includes("query=energy"); + }); + }); + }); + + it.skip("changing filters should trigger a search", async () => { + await renderSiteSearch(); + + // TODO how do you interact with styled-select in a test? + await userEvent.selectOptions( + within(screen.getByRole("main")).getAllByRole("combobox")[0], + "Concepts" + ); + fireEvent.submit(within(screen.getByRole("main")).getByTestId("search-form")); + + await waitFor(() => { + expect(searchHandler).toHaveBeenCalledTimes(1); + expect(searchHandler).toHaveBeenRequestedWith(async (req) => { + return req.request.url.includes("types=isaacConceptPage"); + }); + }); + }); + + it("loading a URL with query parameters should trigger a search", async () => { + await renderSiteSearch("?query=energy&types=isaacQuestionPage,isaacConceptPage"); + + await waitFor(() => { + expect(searchHandler).toHaveBeenCalledTimes(1); + expect(searchHandler).toHaveBeenRequestedWith(async (req) => { + return req.request.url.includes("query=energy") && req.request.url.includes("types=isaacQuestionPage%2CisaacConceptPage"); + }); + }); + }); +}); diff --git a/src/test/state/actions.test.tsx b/src/test/state/actions.test.tsx index fd38b7ce3f..467854eefc 100644 --- a/src/test/state/actions.test.tsx +++ b/src/test/state/actions.test.tsx @@ -4,7 +4,6 @@ import configureMockStore from 'redux-mock-store'; import thunk from "redux-thunk"; import { fetchErrorFromParameters, - fetchSearch, middleware, registerQuestions, requestCurrentUser, @@ -15,7 +14,6 @@ import { errorResponses, questionDTOs, registeredUserDTOs, - searchResultsList, userAuthenticationSettings, userPreferencesSettings } from "../test-factory"; @@ -140,38 +138,6 @@ describe("registerQuestion action", () => { }); }); -describe("fetchSearch action", () => { - afterEach(() => { - axiosMock.reset(); - }); - - it("dispatches SEARCH_RESPONSE_SUCCESS after a successful request", async () => { - axiosMock.onGet(`/search`, {params: {types: "bar", query: "foo"}}).replyOnce(200, searchResultsList); - const store = mockStore(); - await store.dispatch(fetchSearch("foo", "bar") as any); - const expectedActions = [ - {type: ACTION_TYPE.SEARCH_REQUEST, query: "foo", types: "bar"}, - {type: ACTION_TYPE.SEARCH_RESPONSE_SUCCESS, searchResults: searchResultsList} - ]; - const actualActions = store.getActions(); - const actualIsaacActions = expectActionsToStartWithMiddlewareRegistration(actualActions); - expect(actualIsaacActions).toEqual(expectedActions); - expect(axiosMock.history.get.length).toBe(1); - }); - - it("doesn't call the API if the query is blank", async () => { - const store = mockStore(); - await store.dispatch(fetchSearch("", "types") as any); - const expectedActions = [ - {type: ACTION_TYPE.SEARCH_REQUEST, query: "", types: "types"} - ]; - const actualActions = store.getActions(); - const actualIsaacActions = expectActionsToStartWithMiddlewareRegistration(actualActions); - expect(actualIsaacActions).toEqual(expectedActions); - expect(axiosMock.history.get.length).toBe(0); - }); -}); - describe("toasts actions", () => { beforeEach(() => { jest.useFakeTimers(); @@ -327,4 +293,4 @@ describe('helper: fetchErrorPromParameters', () => { const result = fetchErrorFromParameters(['a', 1, true]); expect(result).toEqual({ parseError: 'Failed to parse "a,1,true". Each query pair must be an iterable [name, value] tuple'}); }); -}); \ No newline at end of file +}); diff --git a/src/test/state/reducers.test.tsx b/src/test/state/reducers.test.tsx index 8fbfa524cc..be224b7968 100644 --- a/src/test/state/reducers.test.tsx +++ b/src/test/state/reducers.test.tsx @@ -1,5 +1,5 @@ import {mapValues, union} from "lodash"; -import {questionDTOs, registeredUserDTOs, searchResultsList} from "../test-factory"; +import {questionDTOs, registeredUserDTOs} from "../test-factory"; import {ACTION_TYPE} from "../../app/services"; import {Action, AppQuestionDTO, PotentialUser} from "../../IsaacAppTypes"; import {GameboardDTO} from "../../IsaacApiTypes"; @@ -11,7 +11,6 @@ import { gameboardsSlice, questions, rootReducer, - search, selectors, toasts, userSlice @@ -134,30 +133,6 @@ describe("questions reducer", () => { }); }); -describe("search reducer", () => { - it("returns null as an initial value", () => { - const actualState = search(undefined, ignoredTestAction); - expect(actualState).toBe(null); - }); - - it("returns the previous state by default", () => { - const previousStates = [null, {searchResults: searchResultsList}]; - previousStates.map((previousState) => { - const actualNextState = search(previousState, ignoredTestAction); - expect(actualNextState).toEqual(previousState); - }); - }); - - it("should replace the list of search results on ", () => { - const unitsAction: Action = {type: ACTION_TYPE.SEARCH_RESPONSE_SUCCESS, searchResults: searchResultsList}; - const previousStates = [null, {searchResults: {totalResults: 0, results: []}}]; - previousStates.map((previousState) => { - const actualNextState = search(previousState, unitsAction); - expect(actualNextState).toEqual({searchResults: searchResultsList}); - }); - }); -}); - describe("toasts reducer", () => { const sampleToast = { title: "Title",