From 26ca07c7ad48b548066cadb565d692c7d69f6516 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 13:53:37 +0000 Subject: [PATCH 1/9] Migrate site search to RTKQ; fix debouncing --- src/app/components/elements/SearchInputs.tsx | 36 ++++---- src/app/components/pages/Search.tsx | 90 +++++++++----------- src/app/state/slices/api/searchApi.ts | 22 +++++ 3 files changed, 83 insertions(+), 65 deletions(-) create mode 100644 src/app/state/slices/api/searchApi.ts diff --git a/src/app/components/elements/SearchInputs.tsx b/src/app/components/elements/SearchInputs.tsx index 6b0e1804d7..20bf718222 100644 --- a/src/app/components/elements/SearchInputs.tsx +++ b/src/app/components/elements/SearchInputs.tsx @@ -1,8 +1,9 @@ -import React, {ChangeEvent, FormEvent, useEffect, useRef, useState} from "react"; +import React, {ChangeEvent, FormEvent, useMemo, useRef, useState} from "react"; import {Button, Form, Input, InputGroup, InputProps, Label} from "reactstrap"; import {ifKeyIsEnter, pushSearchToHistory, SEARCH_CHAR_LENGTH_LIMIT, siteSpecific} from "../../services"; import classNames from "classnames"; -import { useHistory, useLocation } from "react-router"; +import { useHistory } from "react-router"; +import { debounce } from "lodash"; interface SearchInputProps { setSearchText: (s: string) => void; @@ -23,22 +24,23 @@ 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 === "") { - if (searchInputRef.current) searchInputRef.current.focus(); - } else { - onSearch?.(searchText); - pushSearchToHistory(history, searchText, []); - } - } + const doSearch = useMemo(() => { + return debounce((text: string) => { + if (text === "") { + if (searchInputRef.current) searchInputRef.current.focus(); + } else { + onSearch?.(text); + } + }, 500, { leading: true, trailing: true }); + }, [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]); - - return
+ return { + e.preventDefault(); + doSearch(searchText); + }} + >
{ value: T; @@ -57,70 +62,54 @@ 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]); - - function updateSearchUrl(e?: FormEvent) { - if (e) {e.preventDefault();} - pushSearchToHistory(history, queryState || "", filtersState.map(deitemise)); - } + const searchResult = useSearchRequestQuery(queryState ? {query: queryState, types: filtersState.map(deitemise).join(",")} : skipToken); // 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]); + const onUpdate = useMemo(() => { + return throttle((query: Nullable, filters: Item[]) => { + updateSearchUrl(history, query, filters); + }, 500); + }, [history]); 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]); + onUpdate(queryState, filtersState); + }, [queryState, filtersState, onUpdate]); // 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?.data?.results) ? {searchResult?.data?.results.length} : null}

@@ -147,12 +136,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/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; From 2daddc4e4057fd6e3b6398fc1a48279d70dff866 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 13:56:43 +0000 Subject: [PATCH 2/9] Remove axios actions /search methods, tests --- src/IsaacAppTypes.tsx | 7 ---- src/app/components/elements/SearchInputs.tsx | 5 ++- src/app/services/constants.ts | 7 ---- src/app/state/actions/index.tsx | 14 -------- src/app/state/reducers/searchState.ts | 15 -------- src/test/state/actions.test.tsx | 36 +------------------- 6 files changed, 3 insertions(+), 81 deletions(-) delete mode 100644 src/app/state/reducers/searchState.ts diff --git a/src/IsaacAppTypes.tsx b/src/IsaacAppTypes.tsx index 94a3dad922..a7481f7802 100644 --- a/src/IsaacAppTypes.tsx +++ b/src/IsaacAppTypes.tsx @@ -118,10 +118,6 @@ export type Action = | {type: ACTION_TYPE.QUESTION_UNLOCK; questionId: string} | {type: ACTION_TYPE.QUESTION_SET_CURRENT_ATTEMPT; questionId: string; attempt: Immutable>} - | {type: ACTION_TYPE.QUESTION_SEARCH_REQUEST} - | {type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_SUCCESS; questionResults: ApiTypes.SearchResultsWrapper, searchId?: string} - | {type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_FAILURE} - | {type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_REQUEST} | {type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_SUCCESS; myAnsweredQuestionsByDate: ApiTypes.AnsweredQuestionsByDate} | {type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_FAILURE} @@ -142,9 +138,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 20bf718222..70a309e79e 100644 --- a/src/app/components/elements/SearchInputs.tsx +++ b/src/app/components/elements/SearchInputs.tsx @@ -1,8 +1,7 @@ -import React, {ChangeEvent, FormEvent, useMemo, useRef, useState} from "react"; +import React, {ChangeEvent, useMemo, 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 } from "react-router"; import { debounce } from "lodash"; interface SearchInputProps { diff --git a/src/app/services/constants.ts b/src/app/services/constants.ts index c8d263f940..3ff1ee2d23 100644 --- a/src/app/services/constants.ts +++ b/src/app/services/constants.ts @@ -190,10 +190,6 @@ export enum ACTION_TYPE { QUESTION_UNLOCK = "QUESTION_UNLOCK", QUESTION_SET_CURRENT_ATTEMPT = "QUESTION_SET_CURRENT_ATTEMPT", - QUESTION_SEARCH_REQUEST = "QUESTION_SEARCH_REQUEST", - QUESTION_SEARCH_RESPONSE_SUCCESS = "QUESTION_SEARCH_RESPONSE_SUCCESS", - QUESTION_SEARCH_RESPONSE_FAILURE = "QUESTION_SEARCH_RESPONSE_FAILURE", - MY_QUESTION_ANSWERS_BY_DATE_REQUEST = "MY_QUESTION_ANSWERS_BY_DATE_REQUEST", MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_SUCCESS = "MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_SUCCESS", MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_FAILURE = "MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_FAILURE", @@ -214,9 +210,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/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/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 +}); From 3ff55bbb312e1b16836acded83dde5adc15ab4d2 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 14:12:33 +0000 Subject: [PATCH 3/9] Restore question search axios state; fix imports --- src/IsaacAppTypes.tsx | 4 ++++ src/app/services/constants.ts | 4 ++++ src/app/state/index.ts | 2 +- src/app/state/reducers/index.ts | 4 ---- src/test/state/reducers.test.tsx | 27 +-------------------------- 5 files changed, 10 insertions(+), 31 deletions(-) diff --git a/src/IsaacAppTypes.tsx b/src/IsaacAppTypes.tsx index a7481f7802..3a2996738e 100644 --- a/src/IsaacAppTypes.tsx +++ b/src/IsaacAppTypes.tsx @@ -118,6 +118,10 @@ export type Action = | {type: ACTION_TYPE.QUESTION_UNLOCK; questionId: string} | {type: ACTION_TYPE.QUESTION_SET_CURRENT_ATTEMPT; questionId: string; attempt: Immutable>} + | {type: ACTION_TYPE.QUESTION_SEARCH_REQUEST} + | {type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_SUCCESS; questionResults: ApiTypes.SearchResultsWrapper, searchId?: string} + | {type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_FAILURE} + | {type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_REQUEST} | {type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_SUCCESS; myAnsweredQuestionsByDate: ApiTypes.AnsweredQuestionsByDate} | {type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_FAILURE} diff --git a/src/app/services/constants.ts b/src/app/services/constants.ts index 3ff1ee2d23..979fe3e6bb 100644 --- a/src/app/services/constants.ts +++ b/src/app/services/constants.ts @@ -190,6 +190,10 @@ export enum ACTION_TYPE { QUESTION_UNLOCK = "QUESTION_UNLOCK", QUESTION_SET_CURRENT_ATTEMPT = "QUESTION_SET_CURRENT_ATTEMPT", + QUESTION_SEARCH_REQUEST = "QUESTION_SEARCH_REQUEST", + QUESTION_SEARCH_RESPONSE_SUCCESS = "QUESTION_SEARCH_RESPONSE_SUCCESS", + QUESTION_SEARCH_RESPONSE_FAILURE = "QUESTION_SEARCH_RESPONSE_FAILURE", + MY_QUESTION_ANSWERS_BY_DATE_REQUEST = "MY_QUESTION_ANSWERS_BY_DATE_REQUEST", MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_SUCCESS = "MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_SUCCESS", MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_FAILURE = "MY_QUESTION_ANSWERS_BY_DATE_RESPONSE_FAILURE", 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/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", From f1d8c1fbbf91b4c3bc95c1faa9d43963c85b0f36 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 14:22:18 +0000 Subject: [PATCH 4/9] Deduplicate debounce logic I was toying between using debounce and throttle here, but went with a debounce with both leading and trailing set. This makes it instantly responsive on first use, but waits until the timeout has passed entirely once if there are multiple presses. Truthfully this probably doesn't *need* a debounce here since it's unlikely someone would spam enter alongside typing their query, but it doesn't hurt. --- src/app/components/elements/SearchInputs.tsx | 17 +++++++---------- src/app/components/pages/Search.tsx | 15 ++++++++------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/app/components/elements/SearchInputs.tsx b/src/app/components/elements/SearchInputs.tsx index 70a309e79e..031911a171 100644 --- a/src/app/components/elements/SearchInputs.tsx +++ b/src/app/components/elements/SearchInputs.tsx @@ -1,8 +1,7 @@ -import React, {ChangeEvent, useMemo, useRef, useState} from "react"; +import React, {ChangeEvent, useCallback, useRef, useState} from "react"; import {Button, Form, Input, InputGroup, InputProps, Label} from "reactstrap"; import {ifKeyIsEnter, SEARCH_CHAR_LENGTH_LIMIT, siteSpecific} from "../../services"; import classNames from "classnames"; -import { debounce } from "lodash"; interface SearchInputProps { setSearchText: (s: string) => void; @@ -23,14 +22,12 @@ function withSearch(Component: React.FC) { const [searchText, setSearchText] = useState(initialValue ?? ""); const searchInputRef = useRef(null); - const doSearch = useMemo(() => { - return debounce((text: string) => { - if (text === "") { - if (searchInputRef.current) searchInputRef.current.focus(); - } else { - onSearch?.(text); - } - }, 500, { leading: true, trailing: true }); + const doSearch = useCallback((text: string) => { + if (text === "") { + if (searchInputRef.current) searchInputRef.current.focus(); + } else { + onSearch?.(text); + } }, [onSearch]); return
{ @@ -80,13 +79,15 @@ export const Search = withRouter((props: RouteComponentProps) => { const [filtersState, setFiltersState] = useState[]>(initialFilters.map(itemise)); const [queryState, setQueryState] = useState(urlQuery); - const searchResult = useSearchRequestQuery(queryState ? {query: queryState, types: filtersState.map(deitemise).join(",")} : skipToken); + const [searchQuery, setSearchQuery] = useState<{query: string; types: string} | typeof skipToken>(skipToken); + const searchResult = useSearchRequestQuery(searchQuery); - // Trigger update to search url on query or filter change + // Trigger update to query on state change const onUpdate = useMemo(() => { - return throttle((query: Nullable, filters: Item[]) => { + return debounce((query: Nullable, filters: Item[]) => { + setSearchQuery(query ? {query, types: filters.map(deitemise).join(",")} : skipToken); updateSearchUrl(history, query, filters); - }, 500); + }, 500, {leading: true, trailing: true}); }, [history]); useEffect(() => { From d984b622386b838b58efd06fc88fb300bdf15f38 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 14:26:15 +0000 Subject: [PATCH 5/9] Add comment --- src/app/components/pages/Search.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/components/pages/Search.tsx b/src/app/components/pages/Search.tsx index bf53ac687b..c5cb50b269 100644 --- a/src/app/components/pages/Search.tsx +++ b/src/app/components/pages/Search.tsx @@ -79,6 +79,7 @@ export const Search = withRouter((props: RouteComponentProps) => { const [filtersState, setFiltersState] = useState[]>(initialFilters.map(itemise)); const [queryState, setQueryState] = useState(urlQuery); + // 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); From 7b70e7821e3cbde5f5bbc6a0e8c515108b7a8450 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 14:28:29 +0000 Subject: [PATCH 6/9] Prefer displaying query `currentData` over stale `data` --- src/app/components/pages/Search.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/components/pages/Search.tsx b/src/app/components/pages/Search.tsx index c5cb50b269..cfc28f8e94 100644 --- a/src/app/components/pages/Search.tsx +++ b/src/app/components/pages/Search.tsx @@ -111,7 +111,7 @@ export const Search = withRouter((props: RouteComponentProps) => {

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

From 69dcb86fa3242cce6c07f43b741dbfc92c41c707 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Tue, 11 Nov 2025 16:33:19 +0000 Subject: [PATCH 7/9] Add tests for site search --- src/app/components/elements/SearchInputs.tsx | 3 +- .../list-groups/AbstractListViewItem.tsx | 1 + src/test/pages/SiteSearch.test.tsx | 74 +++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/test/pages/SiteSearch.test.tsx diff --git a/src/app/components/elements/SearchInputs.tsx b/src/app/components/elements/SearchInputs.tsx index 031911a171..b261ca57c9 100644 --- a/src/app/components/elements/SearchInputs.tsx +++ b/src/app/components/elements/SearchInputs.tsx @@ -31,7 +31,8 @@ function withSearch(Component: React.FC) { }, [onSearch]); return { e.preventDefault(); doSearch(searchText); diff --git a/src/app/components/elements/list-groups/AbstractListViewItem.tsx b/src/app/components/elements/list-groups/AbstractListViewItem.tsx index 701fefa5be..a19f1f6c4f 100644 --- a/src/app/components/elements/list-groups/AbstractListViewItem.tsx +++ b/src/app/components/elements/list-groups/AbstractListViewItem.tsx @@ -258,6 +258,7 @@ export const AbstractListViewItem = ({title, icon, subject, subtitle, breadcrumb return {cardBody} ; 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"); + }); + }); + }); +}); From a2e1ceef7f6a021d082902a0ffb0954dbdcd0df0 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 17 Nov 2025 14:57:37 +0000 Subject: [PATCH 8/9] Fix header search --- src/app/components/site/phy/NavigationMenuPhy.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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)}`)} />
} ; From 7d2d8871cce8ce1e6c1184b746f5a397735751a2 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 17 Nov 2025 14:58:24 +0000 Subject: [PATCH 9/9] Allow multiple search inputs on one page to stay in sync --- src/app/components/elements/SearchInputs.tsx | 9 ++++++++- src/app/components/pages/Search.tsx | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/app/components/elements/SearchInputs.tsx b/src/app/components/elements/SearchInputs.tsx index b261ca57c9..622270d6de 100644 --- a/src/app/components/elements/SearchInputs.tsx +++ b/src/app/components/elements/SearchInputs.tsx @@ -1,4 +1,4 @@ -import React, {ChangeEvent, useCallback, useRef, useState} from "react"; +import React, {ChangeEvent, useCallback, useEffect, useRef, useState} from "react"; import {Button, Form, Input, InputGroup, InputProps, Label} from "reactstrap"; import {ifKeyIsEnter, SEARCH_CHAR_LENGTH_LIMIT, siteSpecific} from "../../services"; import classNames from "classnames"; @@ -30,6 +30,13 @@ function withSearch(Component: React.FC) { } }, [onSearch]); + 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 { onUpdate(queryState, filtersState); }, [queryState, filtersState, onUpdate]); + useEffect(function triggerSearchOnUrlChange() { + setQueryState(urlQuery); + }, [urlQuery]); + // Process results and add shortcut responses const shortcutAndFilterResults = (results?: ContentSummaryDTO[]) => {