From 9d68c8c8018b41ebe9bf2ea56b15b0abc2a1e936 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Thu, 6 Nov 2025 16:26:09 +0000 Subject: [PATCH 01/48] Migrate Question Finder to RTKQ --- src/app/components/pages/QuestionFinder.tsx | 205 +++++++++----------- src/app/state/slices/api/questionsApi.ts | 52 ++++- 2 files changed, 145 insertions(+), 112 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index d4020c62c7..cefad4ef58 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -1,5 +1,5 @@ import React, {useCallback, useEffect, useMemo, useState} from "react"; -import {AppState, clearQuestionSearch, searchQuestions, useAppDispatch, useAppSelector} from "../../state"; +import {AppState, clearQuestionSearch, useAppDispatch, useAppSelector, useSearchQuestionsQuery} from "../../state"; import debounce from "lodash/debounce"; import { arrayFromPossibleCsv, @@ -34,10 +34,9 @@ import { useQueryParams, useUrlPageTheme, } from "../../services"; -import {ContentSummaryDTO, Difficulty, ExamBoard} from "../../../IsaacApiTypes"; +import {Difficulty, ExamBoard} from "../../../IsaacApiTypes"; import {IsaacSpinner} from "../handlers/IsaacSpinner"; import {useHistory, withRouter} from "react-router"; -import {ShowLoading} from "../handlers/ShowLoading"; import {generateSubjectLandingPageCrumbFromContext, TitleAndBreadcrumb} from "../elements/TitleAndBreadcrumb"; import {MetaDescription} from "../elements/MetaDescription"; import {CanonicalHrefElement} from "../navigation/CanonicalHrefElement"; @@ -57,6 +56,9 @@ import { Link } from "react-router-dom"; import { updateTopicChoices } from "../../services"; import { PageMetadata } from "../elements/PageMetadata"; import { ResultsListContainer, ResultsListHeader } from "../elements/ListResultsContainer"; +import { QuestionSearchQuery } from "../../../IsaacAppTypes"; +import { skipToken } from "@reduxjs/toolkit/query"; +import { ShowLoadingQuery } from "../handlers/ShowLoadingQuery"; // Type is used to ensure that we check all query params if a new one is added in the future const FILTER_PARAMS = ["query", "topics", "fields", "subjects", "stages", "difficulties", "examBoards", "book", "excludeBooks", "statuses", "randomSeed"] as const; @@ -146,6 +148,13 @@ export const FilterSummary = ({filterTags, clearFilters, removeFilterTag}: Filte ; }; +const loadingPlaceholder = +
+ + +
+; + export const QuestionFinder = withRouter(() => { const dispatch = useAppDispatch(); const user = useAppSelector((state: AppState) => state && state.user); @@ -201,8 +210,6 @@ export const QuestionFinder = withRouter(() => { } }, [pageContext]); - const [disableLoadMore, setDisableLoadMore] = useState(false); - const choices = useMemo(() => { return updateTopicChoices(selections, pageContext, getAllowedTags(pageContext)); }, [selections, pageContext]); @@ -214,7 +221,8 @@ export const QuestionFinder = withRouter(() => { // this should only update when a new search is triggered, not (necessarily) when the filters change const [isCurrentSearchEmpty, setIsCurrentSearchEmpty] = useState(isEmptySearch(searchQuery, searchTopics, searchBooks, searchStages, searchDifficulties, searchExamBoards, selections)); - const {results: questions, totalResults: totalQuestions, nextSearchOffset} = useAppSelector((state: AppState) => state && state.questionSearchResult) || {}; + const [searchParams, setSearchParams] = useState(undefined); + const searchQuestionsQuery = useSearchQuestionsQuery(searchParams ?? skipToken); const debouncedSearch = useMemo(() => debounce(({ @@ -250,7 +258,7 @@ export const QuestionFinder = withRouter(() => { setIsCurrentSearchEmpty(false); - void dispatch(searchQuestions({ + setSearchParams({ querySource: "questionFinder", searchString: searchString || undefined, tags: choiceTreeLeaves.join(",") || undefined, @@ -265,9 +273,9 @@ export const QuestionFinder = withRouter(() => { statuses: questionStatusToURIComponent(questionStatuses), fasttrack: false, startIndex, - limit: SEARCH_RESULTS_PER_PAGE + 1, // request one more than we need to know if there are more results + limit: SEARCH_RESULTS_PER_PAGE, randomSeed - })); + }); }, 250), [dispatch, pageContext]); @@ -276,8 +284,6 @@ export const QuestionFinder = withRouter(() => { const searchAndUpdateURL = useCallback(() => { setPageCount(1); - setDisableLoadMore(false); - setDisplayQuestions(undefined); const filteredStages = !searchStages.length && pageContext?.stage ? pageStageToSearchStage(pageContext.stage) : searchStages; debouncedSearch({ @@ -340,32 +346,10 @@ export const QuestionFinder = withRouter(() => { } }, [searchStages]); - const questionList = useMemo(() => { - if (questions) { - if (questions.length < SEARCH_RESULTS_PER_PAGE + 1) { - setDisableLoadMore(true); - } else { - setDisableLoadMore(false); - } - - return questions.slice(0, SEARCH_RESULTS_PER_PAGE); - } - }, [questions]); - - const [displayQuestions, setDisplayQuestions] = useState([]); const [pageCount, setPageCount] = useState(1); const [validFiltersSelected, setValidFiltersSelected] = useState(false); - useEffect(() => { - if (displayQuestions && nextSearchOffset && pageCount > 1) { - setDisplayQuestions(dqs => [...dqs ?? [], ...questionList ?? []]); - } else { - setDisplayQuestions(questionList); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [questionList]); - useEffect(function onFiltersChanged() { setSearchDisabled(false); setValidFiltersSelected(searchDifficulties.length > 0 @@ -424,11 +408,6 @@ export const QuestionFinder = withRouter(() => { "Search for the perfect computer science questions to study. For revision. For homework. For the classroom." ); - const loadingPlaceholder =
- - -
; - function removeFilterTag(filter: string) { if (searchStages.includes(filter as STAGE)) { setSearchStages(searchStages.filter(f => f !== filter)); @@ -551,78 +530,86 @@ export const QuestionFinder = withRouter(() => { }} /> } - - -
- {displayQuestions && displayQuestions.length > 0 - ? <>Showing {displayQuestions.length} - : isPhy && isCurrentSearchEmpty - ? <>Select {filteringByStatus ? "more" : "some"} filters to start searching - : <>No results - } - {(totalQuestions ?? 0) > 0 && !filteringByStatus && <> of {totalQuestions}} - . -
- - - - - {displayQuestions?.length - ? isPhy - ? - : - : isAda && <>{ - isCurrentSearchEmpty - ? Please select and apply filters. - : filteringByStatus - ? Could not load any results matching the requested filters. - : No results match the requested filters. - } + 1} + thenRender={({ results: questions, totalResults: totalQuestions, nextSearchOffset, moreResultsAvailable }, isStale) => { + return <> + + +
+ <>{questions && questions.length > 0 + ? <> + Showing {questions.length} + {(totalQuestions ?? 0) > 0 && !filteringByStatus && <> of {totalQuestions}} + . + + : isPhy && isCurrentSearchEmpty + ? <>Select {filteringByStatus ? "more" : "some"} filters to start searching. + : <>No results. + } +
+ + + + <>{questions.length + ? isPhy + ? + : + : isAda && <>{ + isCurrentSearchEmpty + ? Please select and apply filters. + : filteringByStatus + ? Could not load any results matching the requested filters. + : No results match the requested filters. + } + } + + + {(questions?.length ?? 0) > 0 && + + + + + } - - - - {(displayQuestions?.length ?? 0) > 0 && - - - - - } + ; + }} + /> diff --git a/src/app/state/slices/api/questionsApi.ts b/src/app/state/slices/api/questionsApi.ts index 7fb3cc824d..7f77bd2e89 100644 --- a/src/app/state/slices/api/questionsApi.ts +++ b/src/app/state/slices/api/questionsApi.ts @@ -1,13 +1,57 @@ -import { IsaacQuestionPageDTO } from "../../../../IsaacApiTypes"; -import { CanAttemptQuestionTypeDTO } from "../../../../IsaacAppTypes"; -import { tags } from "../../../services"; +import { ContentSummaryDTO, IsaacQuestionPageDTO } from "../../../../IsaacApiTypes"; +import { CanAttemptQuestionTypeDTO, QuestionSearchQuery } from "../../../../IsaacAppTypes"; +import { SEARCH_RESULTS_PER_PAGE, tags } from "../../../services"; import { docSlice } from "../doc"; import { isaacApi } from "./baseApi"; import { onQueryLifecycleEvents } from "./utils"; +interface QuestionSearchResponseType { + results: ContentSummaryDTO[]; + totalResults: number; + nextSearchOffset?: number; + moreResultsAvailable?: boolean; // frontend only; calculated in transformResponse +} export const questionsApi = isaacApi.enhanceEndpoints({addTagTypes: ["CanAttemptQuestionType"]}).injectEndpoints({ endpoints: (build) => ({ + searchQuestions: build.query({ + query: (query: QuestionSearchQuery) => ({ + url: `/pages/questions`, + params: { + ...query, + limit: query.limit ? query.limit + 1 : SEARCH_RESULTS_PER_PAGE + 1 // fetch one extra to check if more results are available + } + }), + serializeQueryArgs: (args) => { + const { queryArgs, ...rest } = args; + const { startIndex: _startIndex, ...otherParams } = queryArgs; + // So that different queries with different pagination params still share the same cache + return { + ...rest, + queryArgs: otherParams + }; + }, + transformResponse: (response: QuestionSearchResponseType, _, arg) => { + return { + ...response, + // remove the extra result used to check for more results, so that we return the correct amount + moreResultsAvailable: response.results.length > (arg.limit ?? SEARCH_RESULTS_PER_PAGE), + results: response.results.slice(0, (arg.limit ?? SEARCH_RESULTS_PER_PAGE)) + }; + }, + merge: (currentCache, newItems) => { + currentCache.results.push(...newItems.results); + currentCache.totalResults = newItems.totalResults; + currentCache.nextSearchOffset = newItems.nextSearchOffset; + }, + forceRefetch({ currentArg, previousArg }) { + return currentArg !== previousArg; + }, + onQueryStarted: onQueryLifecycleEvents({ + errorTitle: "Unable to search for questions", + }), + }), + getQuestion: build.query({ query: (id) => ({ url: `/pages/questions/${id}` @@ -33,6 +77,8 @@ export const questionsApi = isaacApi.enhanceEndpoints({addTagTypes: ["CanAttempt }); export const { + useSearchQuestionsQuery, + useLazySearchQuestionsQuery, useGetQuestionQuery, useCanAttemptQuestionTypeQuery, } = questionsApi; From 23c8838e5419ff24e2b9240aaa61e6355f1e05e9 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Thu, 6 Nov 2025 16:52:13 +0000 Subject: [PATCH 02/48] Use RTKQ `isUninitialized` to prevent 1-frame error popup --- src/app/components/handlers/ShowLoadingQuery.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app/components/handlers/ShowLoadingQuery.tsx b/src/app/components/handlers/ShowLoadingQuery.tsx index 1657ff2e09..fe8937ed09 100644 --- a/src/app/components/handlers/ShowLoadingQuery.tsx +++ b/src/app/components/handlers/ShowLoadingQuery.tsx @@ -29,6 +29,7 @@ interface ShowLoadingQueryInfo { data?: T | NOT_FOUND_TYPE; isLoading: boolean; isFetching: boolean; + isUninitialized: boolean; isError: boolean; error?: FetchBaseQueryError | SerializedError; } @@ -38,6 +39,7 @@ export function combineQueries(firstQuery: ShowLoadingQueryInfo, sec data: isFound(firstQuery.data) && isFound(secondQuery.data) ? combineResult(firstQuery.data, secondQuery.data) : undefined, isLoading: firstQuery.isLoading || secondQuery.isLoading, isFetching: firstQuery.isFetching || secondQuery.isFetching, + isUninitialized: firstQuery.isUninitialized || secondQuery.isUninitialized, isError: firstQuery.isError || secondQuery.isError, error: firstQuery.error ?? secondQuery.error, }; @@ -75,14 +77,14 @@ type ShowLoadingQueryProps = ShowLoadingQueryErrorProps & ({ // - `maintainOnRefetch` (boolean indicating whether to keep showing the current data while refetching. use second parameter of `thenRender` to modify render tree accordingly) // - `query` (the object returned by a RTKQ useQuery hook) export function ShowLoadingQuery({query, thenRender, children, placeholder, ifError, ifNotFound, defaultErrorTitle, maintainOnRefetch}: ShowLoadingQueryProps) { - const {data, isLoading, isFetching, isError, error} = query; + const {data, isLoading, isFetching, isUninitialized, isError, error} = query; const renderError = () => ifError ? <>{ifError(error)} : ; if (isError && error) { return "status" in error && typeof error.status === "number" && [NOT_FOUND, NO_CONTENT].includes(error.status) && ifNotFound ? <>{ifNotFound} : renderError(); } const isStale = (isLoading || isFetching) && isFound(data); - const showPlaceholder = (isLoading || isFetching) && (!maintainOnRefetch || !isDefined(data)); + const showPlaceholder = (isUninitialized || isLoading || isFetching) && (!maintainOnRefetch || !isDefined(data)); if (showPlaceholder) { return placeholder ? <>{placeholder} : ; From 8bc08adc2d0fa1634d719f1977529d518b727da2 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Thu, 6 Nov 2025 16:52:42 +0000 Subject: [PATCH 03/48] Migrate subject landing page random question to RTKQ --- .../components/pages/SubjectLandingPage.tsx | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/src/app/components/pages/SubjectLandingPage.tsx b/src/app/components/pages/SubjectLandingPage.tsx index 6d3751824c..bf5471a775 100644 --- a/src/app/components/pages/SubjectLandingPage.tsx +++ b/src/app/components/pages/SubjectLandingPage.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from "react"; +import React, { useEffect, useMemo, useState } from "react"; import { RouteComponentProps, withRouter } from "react-router"; import { Button, Col, Container, Row } from "reactstrap"; import { TitleAndBreadcrumb } from "../elements/TitleAndBreadcrumb"; @@ -6,10 +6,10 @@ import { getHumanContext, isFullyDefinedContext, isSingleStageContext, useUrlPag import { ListView, ListViewCards } from "../elements/list-groups/ListView"; import { getBooksForContext, getLandingPageCardsForContext } from "./subjectLandingPageComponents"; import { below, BookInfo, DOCUMENT_TYPE, EventStatusFilter, EventTypeFilter, isStudent, nextSeed, STAGE, STAGE_TO_LEARNING_STAGE, useDeviceSize } from "../../services"; -import { AugmentedEvent, PageContextState } from "../../../IsaacAppTypes"; +import { AugmentedEvent, PageContextState, QuestionSearchQuery } from "../../../IsaacAppTypes"; import { Link } from "react-router-dom"; import { ShowLoadingQuery } from "../handlers/ShowLoadingQuery"; -import { searchQuestions, selectors, useAppDispatch, useAppSelector, useGetNewsPodListQuery, useLazyGetEventsQuery } from "../../state"; +import { selectors, useAppSelector, useGetNewsPodListQuery, useLazyGetEventsQuery, useSearchQuestionsQuery } from "../../state"; import { EventCard } from "../elements/cards/EventCard"; import debounce from "lodash/debounce"; import { IsaacSpinner } from "../handlers/IsaacSpinner"; @@ -18,23 +18,28 @@ import { NewsCard } from "../elements/cards/NewsCard"; import { BookCard } from "./BooksOverview"; import { placeholderIcon } from "../elements/PageTitle"; import { ContentSummaryDTO, IsaacPodDTO } from "../../../IsaacApiTypes"; -import {v4 as uuid_v4} from "uuid"; +import { skipToken } from "@reduxjs/toolkit/query"; + +const loadingPlaceholder =
    +
  • + +
    • +
; const RandomQuestionBanner = ({context}: {context?: PageContextState}) => { - const dispatch = useAppDispatch(); - const [randomSeed, setrandomSeed] = useState(nextSeed); + const [randomSeed, setRandomSeed] = useState(nextSeed); + + const handleGetDifferentQuestion = () => setRandomSeed(nextSeed); - const handleGetDifferentQuestion = () => setrandomSeed(nextSeed); - const [searchId, setSearchId] = useState(''); + const [searchParams, setSearchParams] = useState(undefined); + const searchQuestionsQuery = useSearchQuestionsQuery(searchParams ?? skipToken); - const searchDebounce = useCallback(debounce(() => { + const searchDebounce = useMemo(() => debounce(() => { if (!isFullyDefinedContext(context)) { return; } - const nextSearchId = uuid_v4(); - setSearchId(nextSearchId); - dispatch(searchQuestions({ + setSearchParams({ querySource: "randomQuestion", searchString: "", tags: "", @@ -51,17 +56,13 @@ const RandomQuestionBanner = ({context}: {context?: PageContextState}) => { startIndex: undefined, limit: 1, randomSeed - }, nextSearchId)); - }), [dispatch, context, randomSeed]); - - const {results: questions, searchId: questionSearchId } = useAppSelector((state) => state && state.questionSearchResult) || {}; + }); + }), [context, randomSeed]); useEffect(() => { searchDebounce(); }, [searchDebounce]); - const question = questions?.[0]; - return

Try a random question!

@@ -70,20 +71,27 @@ const RandomQuestionBanner = ({context}: {context?: PageContextState}) => {
- {question && searchId === questionSearchId - ? - :
    -
  • - -
    • -
- } + { + const question = questions?.[0]; + return question + ? + :
    +
  • + +
    • +
; + }} + />
; }; From 67ba91781778803c92cad96b9355111883457b20 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 7 Nov 2025 11:54:03 +0000 Subject: [PATCH 04/48] Show nothing on uninitialised query, rather than loading --- src/app/components/handlers/ShowLoadingQuery.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app/components/handlers/ShowLoadingQuery.tsx b/src/app/components/handlers/ShowLoadingQuery.tsx index fe8937ed09..fdfefe5c9e 100644 --- a/src/app/components/handlers/ShowLoadingQuery.tsx +++ b/src/app/components/handlers/ShowLoadingQuery.tsx @@ -84,7 +84,11 @@ export function ShowLoadingQuery({query, thenRender, children, placeholder, i } const isStale = (isLoading || isFetching) && isFound(data); - const showPlaceholder = (isUninitialized || isLoading || isFetching) && (!maintainOnRefetch || !isDefined(data)); + const showPlaceholder = (isLoading || isFetching) && (!maintainOnRefetch || !isDefined(data)); + + if (isUninitialized) { + return null; + } if (showPlaceholder) { return placeholder ? <>{placeholder} : ; From 7534b8b22e19659c2205f2f9d4bc04c89e1451c9 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 7 Nov 2025 11:55:20 +0000 Subject: [PATCH 05/48] Migrate gameboards q search to RTKQ --- .../elements/modals/QuestionSearchModal.tsx | 132 +++++++++--------- 1 file changed, 65 insertions(+), 67 deletions(-) diff --git a/src/app/components/elements/modals/QuestionSearchModal.tsx b/src/app/components/elements/modals/QuestionSearchModal.tsx index e42cd76b87..beb47dfff7 100644 --- a/src/app/components/elements/modals/QuestionSearchModal.tsx +++ b/src/app/components/elements/modals/QuestionSearchModal.tsx @@ -1,11 +1,10 @@ -import React, {lazy, Suspense, useCallback, useEffect, useMemo, useReducer, useState} from "react"; +import React, {lazy, useEffect, useMemo, useReducer, useState} from "react"; import { AppState, - clearQuestionSearch, closeActiveModal, - searchQuestions, useAppDispatch, - useAppSelector + useAppSelector, + useSearchQuestionsQuery } from "../../../state"; import debounce from "lodash/debounce"; import isEqual from "lodash/isEqual"; @@ -35,8 +34,8 @@ import { useDeviceSize, EXAM_BOARD, QUESTIONS_PER_GAMEBOARD } from "../../../services"; -import {ContentSummary, GameboardBuilderQuestions, GameboardBuilderQuestionsStackProps} from "../../../../IsaacAppTypes"; -import {AudienceContext, Difficulty, ExamBoard} from "../../../../IsaacApiTypes"; +import {ContentSummary, GameboardBuilderQuestions, GameboardBuilderQuestionsStackProps, QuestionSearchQuery} from "../../../../IsaacAppTypes"; +import {AudienceContext, ContentSummaryDTO, Difficulty, ExamBoard} from "../../../../IsaacApiTypes"; import {GroupBase} from "react-select/dist/declarations/src/types"; import {Loading} from "../../handlers/IsaacSpinner"; import {StyledSelect} from "../inputs/StyledSelect"; @@ -49,6 +48,8 @@ import { CollapsibleList } from "../CollapsibleList"; import { StyledCheckbox } from "../inputs/StyledCheckbox"; import { updateTopicChoices, initialiseListState, listStateReducer } from "../../../services"; import { HorizontalScroller } from "../inputs/HorizontalScroller"; +import { skipToken } from "@reduxjs/toolkit/query"; +import { ShowLoadingQuery } from "../../handlers/ShowLoadingQuery"; // Immediately load GameboardBuilderRow, but allow splitting const importGameboardBuilderRow = import("../GameboardBuilderRow"); @@ -72,6 +73,9 @@ export const QuestionSearchModal = ( const deviceSize = useDeviceSize(); const sublistDelimiter = " >>> "; + const [searchParams, setSearchParams] = useState(undefined); + const searchQuestionsQuery = useSearchQuestionsQuery(searchParams ?? skipToken); + const [topicSelections, setTopicSelections] = useState([]); const [searchTopics, setSearchTopics] = useState([]); const [searchQuestionName, setSearchQuestionName] = useState(""); @@ -83,7 +87,6 @@ export const QuestionSearchModal = ( if (userContext.contexts.length === 1 && !EXAM_BOARD_NULL_OPTIONS.includes(userExamBoard)) setSearchExamBoards([userExamBoard]); }, [userContext.contexts[0].examBoard]); - const [isSearching, setIsSearching] = useState(false); const [searchBook, setSearchBook] = useState([]); const isBookSearch = searchBook.length > 0; @@ -101,14 +104,9 @@ export const QuestionSearchModal = ( const modalQuestions : GameboardBuilderQuestions = {selectedQuestions, questionOrder, setSelectedQuestions, setQuestionOrder}; - const {results: questions} = useAppSelector((state: AppState) => state && state.questionSearchResult) || {}; const user = useAppSelector((state: AppState) => state && state.user); - useEffect(() => { - setIsSearching(false); - }, [questions]); - - const searchDebounce = useCallback( + const searchDebounce = useMemo(() => debounce((searchString: string, topics: string[], examBoards: string[], book: string[], stages: string[], difficulties: string[], fasttrack: boolean, startIndex: number) => { // Clear front-end sorting so as not to override ElasticSearch's match ranking setQuestionsSort({}); @@ -116,15 +114,12 @@ export const QuestionSearchModal = ( const isBookSearch = book.length > 0; // Tasty. if ([searchString, topics, book, stages, difficulties, examBoards].every(v => v.length === 0) && !fasttrack) { // Nothing to search for - dispatch(clearQuestionSearch); return; } const tags = (isBookSearch ? book : [...([topics].map((tags) => tags.join(" ")))].filter((query) => query != "")).join(","); - setIsSearching(true); - - dispatch(searchQuestions({ + setSearchParams({ querySource: "gameboardBuilder", searchString: searchString || undefined, tags: tags || undefined, @@ -134,12 +129,11 @@ export const QuestionSearchModal = ( fasttrack, startIndex, limit: 300 - })); + }); logEvent(eventLog,"SEARCH_QUESTIONS", {searchString, topics, examBoards, book, stages, difficulties, fasttrack, startIndex}); }, 250), - [] - ); + [eventLog]); const sortableTableHeaderUpdateState = (sortState: Record, setSortState: React.Dispatch>>, key: string) => (order: SortOrder) => { const newSortState = {...sortState}; @@ -155,19 +149,17 @@ export const QuestionSearchModal = ( searchDebounce(searchQuestionName, searchTopics, searchExamBoards, searchBook, searchStages, searchDifficulties, searchFastTrack, 0); },[searchDebounce, searchQuestionName, searchTopics, searchExamBoards, searchBook, searchFastTrack, searchStages, searchDifficulties]); - const sortedQuestions = useMemo(() => { - return questions && sortQuestions(isBookSearch ? {title: SortOrder.ASC} : questionsSort, creationContext)( - questions.filter(question => { - const qIsPublic = searchResultIsPublic(question, user); - if (isBookSearch) return qIsPublic; - const qTopicsMatch = - searchTopics.length === 0 || - (question.tags && question.tags.filter((tag) => searchTopics.includes(tag)).length > 0); + const sortAndFilterBySearch = (questions: ContentSummaryDTO[]) => questions && sortQuestions(isBookSearch ? {title: SortOrder.ASC} : questionsSort, creationContext)( + questions.filter(question => { + const qIsPublic = searchResultIsPublic(question, user); + if (isBookSearch) return qIsPublic; + const qTopicsMatch = + searchTopics.length === 0 || + (question.tags && question.tags.filter((tag) => searchTopics.includes(tag)).length > 0); - return qIsPublic && qTopicsMatch; - }) - ); - }, [questions, user, searchTopics, isBookSearch, questionsSort, creationContext]); + return qIsPublic && qTopicsMatch; + }) + ); const addSelectionsRow =
@@ -289,41 +281,47 @@ export const QuestionSearchModal = ( {addSelectionsRow} - }> - 6} className="my-4"> - - - - - - className={siteSpecific("w-40", "w-30")} - setOrder={sortableTableHeaderUpdateState(questionsSort, setQuestionsSort, "title")} - defaultOrder={SortOrder.ASC} - reverseOrder={SortOrder.DESC} - currentOrder={questionsSort['title']} - alignment="start" - >Question title - - - - {isAda && } - - - - {isSearching ? : sortedQuestions?.map(question => - - )} - -
TopicStageDifficultyExam boards
- - + + + + + + + className={siteSpecific("w-40", "w-30")} + setOrder={sortableTableHeaderUpdateState(questionsSort, setQuestionsSort, "title")} + defaultOrder={SortOrder.ASC} + reverseOrder={SortOrder.DESC} + currentOrder={questionsSort['title']} + alignment="start" + >Question title + + + + {isAda && } + + + + } + defaultErrorTitle="Failed to load questions." + thenRender={({results: questions}) => { + const sortedQuestions = sortAndFilterBySearch(questions); + return sortedQuestions?.map(question => + + ); + }} + /> + +
TopicStageDifficultyExam boards
+ ; }; From 9ca4f1b1abf940dbc1bdd4134d92eb648d1b9aec Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 7 Nov 2025 11:55:44 +0000 Subject: [PATCH 06/48] Supersede action/reducer setup for question search --- src/IsaacAppTypes.tsx | 4 ---- src/app/components/pages/QuestionFinder.tsx | 3 +-- src/app/services/constants.ts | 4 ---- src/app/state/actions/index.tsx | 23 --------------------- src/app/state/index.ts | 1 - src/app/state/reducers/gameboardsState.ts | 18 ---------------- src/app/state/reducers/index.ts | 2 -- 7 files changed, 1 insertion(+), 54 deletions(-) delete mode 100644 src/app/state/reducers/gameboardsState.ts diff --git a/src/IsaacAppTypes.tsx b/src/IsaacAppTypes.tsx index 1b6aff9278..c6254af377 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} diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index cefad4ef58..b112b3c03f 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -1,5 +1,5 @@ import React, {useCallback, useEffect, useMemo, useState} from "react"; -import {AppState, clearQuestionSearch, useAppDispatch, useAppSelector, useSearchQuestionsQuery} from "../../state"; +import {AppState, useAppDispatch, useAppSelector, useSearchQuestionsQuery} from "../../state"; import debounce from "lodash/debounce"; import { arrayFromPossibleCsv, @@ -239,7 +239,6 @@ export const QuestionFinder = withRouter(() => { }): void => { if (isEmptySearch(searchString, topics, book, stages, difficulties, examBoards, hierarchySelections)) { setIsCurrentSearchEmpty(true); - return void dispatch(clearQuestionSearch); } const choiceTreeLeaves = getChoiceTreeLeaves(hierarchySelections).map(leaf => leaf.value); diff --git a/src/app/services/constants.ts b/src/app/services/constants.ts index 97ed74c66b..9f9477c048 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", diff --git a/src/app/state/actions/index.tsx b/src/app/state/actions/index.tsx index d8ed976436..a36ae31216 100644 --- a/src/app/state/actions/index.tsx +++ b/src/app/state/actions/index.tsx @@ -481,29 +481,6 @@ export function setCurrentAttempt(questionId: string, attem }); } -let questionSearchCounter = 0; - -export const searchQuestions = (query: QuestionSearchQuery, searchId?: string) => async (dispatch: Dispatch) => { - const searchCount = ++questionSearchCounter; - dispatch({type: ACTION_TYPE.QUESTION_SEARCH_REQUEST}); - try { - const questionsResponse = await api.questions.search(query); - // Because some searches might take longer to return that others, check this is the most recent search still. - // Otherwise, we just discard the data. - if (searchCount === questionSearchCounter) { - dispatch({type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_SUCCESS, questionResults: questionsResponse.data, searchId}); - } - } catch (e) { - dispatch({type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_FAILURE}); - dispatch(showAxiosErrorToastIfNeeded("Failed to search for questions", e)); - } -}; - -export const clearQuestionSearch = async (dispatch: Dispatch) => { - questionSearchCounter++; - dispatch({type: ACTION_TYPE.QUESTION_SEARCH_RESPONSE_SUCCESS, questionResults: {results: [], totalResults: 0}}); -}; - export const getMyAnsweredQuestionsByDate = (userId: number | string, fromDate: number, toDate: number, perDay: boolean) => async (dispatch: Dispatch) => { dispatch({type: ACTION_TYPE.MY_QUESTION_ANSWERS_BY_DATE_REQUEST}); try { diff --git a/src/app/state/index.ts b/src/app/state/index.ts index 9936e9e923..5e0330c4ba 100644 --- a/src/app/state/index.ts +++ b/src/app/state/index.ts @@ -23,7 +23,6 @@ export * from "./reducers/quizState"; export * from "./reducers/questionState"; export * from "./reducers/notifiersState"; export * from "./reducers/groupsState"; -export * from "./reducers/gameboardsState"; export * from "./reducers/progressState"; export * from "./reducers/adminState"; diff --git a/src/app/state/reducers/gameboardsState.ts b/src/app/state/reducers/gameboardsState.ts deleted file mode 100644 index 457c0e96f9..0000000000 --- a/src/app/state/reducers/gameboardsState.ts +++ /dev/null @@ -1,18 +0,0 @@ -import {ContentSummaryDTO, SearchResultsWrapper} from "../../../IsaacApiTypes"; -import {Action} from "../../../IsaacAppTypes"; -import {ACTION_TYPE} from "../../services"; - -type QuestionSearchResultState = SearchResultsWrapper & {searchId?: string} | null; -export const questionSearchResult = (questionSearchResult: QuestionSearchResultState = null, action: Action) => { - switch(action.type) { - case ACTION_TYPE.QUESTION_SEARCH_RESPONSE_SUCCESS: { - return {...action.questionResults, searchId: action.searchId}; - } - case ACTION_TYPE.QUESTION_SEARCH_RESPONSE_FAILURE: { - return null; - } - default: { - return questionSearchResult; - } - } -}; diff --git a/src/app/state/reducers/index.ts b/src/app/state/reducers/index.ts index 6a32e87998..4282e856f6 100644 --- a/src/app/state/reducers/index.ts +++ b/src/app/state/reducers/index.ts @@ -20,7 +20,6 @@ import { testQuestions, quizAttempt, groupMemberships, - questionSearchResult, search, isaacApi, gameboardsSlice, @@ -82,7 +81,6 @@ export const rootReducer = combineReducers({ // Gameboards boards: gameboardsSlice.reducer, - questionSearchResult, // Search search, From 6f34b4f936cb004f2d12337d64c743f735b75623 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 7 Nov 2025 12:16:37 +0000 Subject: [PATCH 07/48] Fix ESLint warnings --- eslint-suppressions.json | 15 ++------------- src/app/components/pages/QuestionFinder.tsx | 5 ++--- src/app/state/actions/index.tsx | 1 - 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 35c2c4d9f6..ec5aa6c666 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -439,11 +439,8 @@ } }, "src/app/components/elements/modals/QuestionSearchModal.tsx": { - "@typescript-eslint/no-floating-promises": { - "count": 2 - }, "react-hooks/exhaustive-deps": { - "count": 3 + "count": 2 } }, "src/app/components/elements/modals/QuizSettingModal.tsx": { @@ -869,7 +866,7 @@ "count": 2 }, "@typescript-eslint/no-floating-promises": { - "count": 10 + "count": 9 }, "react-hooks/exhaustive-deps": { "count": 2 @@ -983,14 +980,6 @@ "count": 1 } }, - "src/app/components/pages/SubjectLandingPage.tsx": { - "@typescript-eslint/no-floating-promises": { - "count": 1 - }, - "react-hooks/exhaustive-deps": { - "count": 1 - } - }, "src/app/components/pages/SubjectOverviewPage.tsx": { "@typescript-eslint/no-unused-vars": { "count": 2 diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index b112b3c03f..d1238cc6de 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -1,5 +1,5 @@ import React, {useCallback, useEffect, useMemo, useState} from "react"; -import {AppState, useAppDispatch, useAppSelector, useSearchQuestionsQuery} from "../../state"; +import {AppState, useAppSelector, useSearchQuestionsQuery} from "../../state"; import debounce from "lodash/debounce"; import { arrayFromPossibleCsv, @@ -156,7 +156,6 @@ const loadingPlaceholder = ; export const QuestionFinder = withRouter(() => { - const dispatch = useAppDispatch(); const user = useAppSelector((state: AppState) => state && state.user); const params = useQueryParams(false); const history = useHistory(); @@ -276,7 +275,7 @@ export const QuestionFinder = withRouter(() => { randomSeed }); }, 250), - [dispatch, pageContext]); + [pageContext]); const filteringByStatus = Object.values(searchStatuses).some(v => v) && !Object.values(searchStatuses).every(v => v); diff --git a/src/app/state/actions/index.tsx b/src/app/state/actions/index.tsx index a36ae31216..7274567804 100644 --- a/src/app/state/actions/index.tsx +++ b/src/app/state/actions/index.tsx @@ -19,7 +19,6 @@ import { CredentialsAuthDTO, FreeTextRule, InlineContext, - QuestionSearchQuery, UserSnapshot, ValidatedChoice, } from "../../../IsaacAppTypes"; From 94a1c3a8ed01e7d14ef295e7597fce97dd496e34 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 7 Nov 2025 14:11:24 +0000 Subject: [PATCH 08/48] Use leading edge in search debounces to make more responsive This fixes the tests as the leading edge change now causes an immediate rerender, which prevents the test clicking on a stale button --- src/app/components/elements/modals/QuestionSearchModal.tsx | 2 +- src/app/components/pages/QuestionFinder.tsx | 2 +- src/app/components/pages/SubjectLandingPage.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/components/elements/modals/QuestionSearchModal.tsx b/src/app/components/elements/modals/QuestionSearchModal.tsx index beb47dfff7..f7762f3b89 100644 --- a/src/app/components/elements/modals/QuestionSearchModal.tsx +++ b/src/app/components/elements/modals/QuestionSearchModal.tsx @@ -132,7 +132,7 @@ export const QuestionSearchModal = ( }); logEvent(eventLog,"SEARCH_QUESTIONS", {searchString, topics, examBoards, book, stages, difficulties, fasttrack, startIndex}); - }, 250), + }, 250, { leading: true }), [eventLog]); const sortableTableHeaderUpdateState = (sortState: Record, setSortState: React.Dispatch>>, key: string) => (order: SortOrder) => { diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index d1238cc6de..40f362964d 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -274,7 +274,7 @@ export const QuestionFinder = withRouter(() => { limit: SEARCH_RESULTS_PER_PAGE, randomSeed }); - }, 250), + }, 250, { leading: true }), [pageContext]); diff --git a/src/app/components/pages/SubjectLandingPage.tsx b/src/app/components/pages/SubjectLandingPage.tsx index bf5471a775..536a1a86b3 100644 --- a/src/app/components/pages/SubjectLandingPage.tsx +++ b/src/app/components/pages/SubjectLandingPage.tsx @@ -57,7 +57,7 @@ const RandomQuestionBanner = ({context}: {context?: PageContextState}) => { limit: 1, randomSeed }); - }), [context, randomSeed]); + }, 250, { leading: true }), [context, randomSeed]); useEffect(() => { searchDebounce(); From 4a49b9023fb44e4589259970bc044d5041a8dd07 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 7 Nov 2025 14:18:14 +0000 Subject: [PATCH 09/48] Account for nullable `results` field in QF response --- .../elements/modals/QuestionSearchModal.tsx | 1 + src/app/components/pages/QuestionFinder.tsx | 4 ++-- src/app/state/slices/api/questionsApi.ts | 16 ++++++++++------ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/app/components/elements/modals/QuestionSearchModal.tsx b/src/app/components/elements/modals/QuestionSearchModal.tsx index f7762f3b89..d9c19587ca 100644 --- a/src/app/components/elements/modals/QuestionSearchModal.tsx +++ b/src/app/components/elements/modals/QuestionSearchModal.tsx @@ -306,6 +306,7 @@ export const QuestionSearchModal = ( placeholder={} defaultErrorTitle="Failed to load questions." thenRender={({results: questions}) => { + if (!questions) return <>; const sortedQuestions = sortAndFilterBySearch(questions); return sortedQuestions?.map(question => { {isPhy && } - - <>{questions.length + + <>{questions?.length ? isPhy ? : (arg.limit ?? SEARCH_RESULTS_PER_PAGE), - results: response.results.slice(0, (arg.limit ?? SEARCH_RESULTS_PER_PAGE)) + moreResultsAvailable: isDefined(response.results) ? response.results.length > (arg.limit ?? SEARCH_RESULTS_PER_PAGE) : undefined, + results: response.results?.slice(0, (arg.limit ?? SEARCH_RESULTS_PER_PAGE)) }; }, merge: (currentCache, newItems) => { - currentCache.results.push(...newItems.results); + if (currentCache.results) { + currentCache.results.push(...(newItems.results ?? [])); + } else { + currentCache.results = newItems.results; + } currentCache.totalResults = newItems.totalResults; currentCache.nextSearchOffset = newItems.nextSearchOffset; }, From 446ef92e954874066a7d24d85b538b5431bab8fc Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 10 Nov 2025 11:13:21 +0000 Subject: [PATCH 10/48] Enable customisable uninit placeholder logic in query --- src/app/components/handlers/ShowLoadingQuery.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/components/handlers/ShowLoadingQuery.tsx b/src/app/components/handlers/ShowLoadingQuery.tsx index fdfefe5c9e..34a498703e 100644 --- a/src/app/components/handlers/ShowLoadingQuery.tsx +++ b/src/app/components/handlers/ShowLoadingQuery.tsx @@ -52,6 +52,7 @@ export const discardResults = (): true => true; interface ShowLoadingQueryBaseProps { placeholder?: JSX.Element | JSX.Element[]; + uninitializedPlaceholder?: JSX.Element | JSX.Element[]; query: ShowLoadingQueryInfo; ifNotFound?: JSX.Element | JSX.Element[]; maintainOnRefetch?: boolean; @@ -76,7 +77,7 @@ type ShowLoadingQueryProps = ShowLoadingQueryErrorProps & ({ // - `placeholder` (React element to show while loading) // - `maintainOnRefetch` (boolean indicating whether to keep showing the current data while refetching. use second parameter of `thenRender` to modify render tree accordingly) // - `query` (the object returned by a RTKQ useQuery hook) -export function ShowLoadingQuery({query, thenRender, children, placeholder, ifError, ifNotFound, defaultErrorTitle, maintainOnRefetch}: ShowLoadingQueryProps) { +export function ShowLoadingQuery({query, thenRender, children, placeholder, uninitializedPlaceholder, ifError, ifNotFound, defaultErrorTitle, maintainOnRefetch}: ShowLoadingQueryProps) { const {data, isLoading, isFetching, isUninitialized, isError, error} = query; const renderError = () => ifError ? <>{ifError(error)} : ; if (isError && error) { @@ -87,7 +88,7 @@ export function ShowLoadingQuery({query, thenRender, children, placeholder, i const showPlaceholder = (isLoading || isFetching) && (!maintainOnRefetch || !isDefined(data)); if (isUninitialized) { - return null; + return uninitializedPlaceholder ? <>{uninitializedPlaceholder} : null; } if (showPlaceholder) { From 09c48e498180b5f5fb5249a88279d4a27e4f6275 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 10 Nov 2025 11:13:44 +0000 Subject: [PATCH 11/48] Allow non-user QF on-load computation without rerender --- src/app/components/pages/QuestionFinder.tsx | 69 ++++++++++----------- src/app/services/pageContext.ts | 12 ++-- 2 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index 0f8601e8a1..a0b0737759 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -160,10 +160,16 @@ export const QuestionFinder = withRouter(() => { const params = useQueryParams(false); const history = useHistory(); const pageContext = useUrlPageTheme(); - const [selections, setSelections] = useState([]); // we can't populate this until we have the page context + const [selections, setSelections] = useState(processTagHierarchy( + tags, + pageContext.subject ? [pageContext.subject] : arrayFromPossibleCsv(params.subjects), + arrayFromPossibleCsv(params.fields), + arrayFromPossibleCsv(params.topics), + pageContext + )); const [searchTopics, setSearchTopics] = useState(arrayFromPossibleCsv(params.topics)); const [searchQuery, setSearchQuery] = useState(params.query ? (params.query instanceof Array ? params.query[0] : params.query) : ""); - const [searchStages, setSearchStages] = useState(arrayFromPossibleCsv(params.stages) as STAGE[]); // we can't fully populate this until we have the page context + const [searchStages, setSearchStages] = useState(arrayFromPossibleCsv(params.stages) as STAGE[]); // we can't fully populate this until we have the user const [searchDifficulties, setSearchDifficulties] = useState(arrayFromPossibleCsv(params.difficulties) as Difficulty[]); const [searchExamBoards, setSearchExamBoards] = useState(arrayFromPossibleCsv(params.examBoards) as ExamBoard[]); const [searchStatuses, setSearchStatuses] = useState(getInitialQuestionStatuses(params)); @@ -194,21 +200,6 @@ export const QuestionFinder = withRouter(() => { // eslint-disable-next-line react-hooks/exhaustive-deps -- we don't want this to re-run on params change. }, [user, pageContext]); - useEffect(() => { - if (pageContext) { - // on subject-QFs, the url path (i.e. pageContext.subject) is the first tier of the hierarchy. - setSelections( - processTagHierarchy( - tags, - pageContext.subject ? [pageContext.subject] : arrayFromPossibleCsv(params.subjects), - arrayFromPossibleCsv(params.fields), - arrayFromPossibleCsv(params.topics), - pageContext - ) - ); - } - }, [pageContext]); - const choices = useMemo(() => { return updateTopicChoices(selections, pageContext, getAllowedTags(pageContext)); }, [selections, pageContext]); @@ -237,7 +228,8 @@ export const QuestionFinder = withRouter(() => { startIndex, randomSeed }): void => { if (isEmptySearch(searchString, topics, book, stages, difficulties, examBoards, hierarchySelections)) { - setIsCurrentSearchEmpty(true); + setSearchParams(undefined); + return; } const choiceTreeLeaves = getChoiceTreeLeaves(hierarchySelections).map(leaf => leaf.value); @@ -254,8 +246,6 @@ export const QuestionFinder = withRouter(() => { }); } - setIsCurrentSearchEmpty(false); - setSearchParams({ querySource: "questionFinder", searchString: searchString || undefined, @@ -532,22 +522,30 @@ export const QuestionFinder = withRouter(() => { query={searchQuestionsQuery} placeholder={loadingPlaceholder} defaultErrorTitle="Error loading questions" + uninitializedPlaceholder={ + + + {siteSpecific( + <>Select {filteringByStatus ? "more" : "some"} filters to start searching., + Please select and apply filters. + )} + + + } maintainOnRefetch={pageCount > 1} thenRender={({ results: questions, totalResults: totalQuestions, nextSearchOffset, moreResultsAvailable }, isStale) => { return <>
- <>{questions && questions.length > 0 + {questions && questions.length > 0 ? <> Showing {questions.length} {(totalQuestions ?? 0) > 0 && !filteringByStatus && <> of {totalQuestions}} . - : isPhy && isCurrentSearchEmpty - ? <>Select {filteringByStatus ? "more" : "some"} filters to start searching. - : <>No results. - } + : isPhy && <>No results. + }
- <>{questions?.length - ? isPhy - ? - : , + - : isAda && <>{ - isCurrentSearchEmpty - ? Please select and apply filters. - : filteringByStatus - ? Could not load any results matching the requested filters. - : No results match the requested filters. - } - } + ) + : isAda && filteringByStatus + ? Could not load any results matching the requested filters. + : No results match the requested filters. + } {(questions?.length ?? 0) > 0 && diff --git a/src/app/services/pageContext.ts b/src/app/services/pageContext.ts index 7cd2837e6c..c7c688244d 100644 --- a/src/app/services/pageContext.ts +++ b/src/app/services/pageContext.ts @@ -181,13 +181,13 @@ function isValidIsaacStage(stage?: string): stage is LearningStage { return typeof stage === "string" && LearningStages.includes(stage as LearningStage); } -function determinePageContextFromUrl(url: string): PageContextState { +function determinePageContextFromUrl(url: string): NonNullable { const [subject, stage] = url.split("/").filter(Boolean); return { subject: isValidIsaacSubject(subject) ? subject : undefined, stage: isValidIsaacStage(stage) ? [stage] : [], - } as PageContextState; + } as NonNullable; } /** @@ -196,12 +196,12 @@ function determinePageContextFromUrl(url: string): PageContextState { * If you want to get the current page context from redux rather than the URL, use `useAppSelector(selectors.pageContext.context)` instead. * @returns The current page context. */ -export function useUrlPageTheme(): PageContextState { +export function useUrlPageTheme(): NonNullable { const location = useLocation(); const dispatch = useAppDispatch(); // urlPageTheme mirrors the redux state, but without delay; this is never stale, but redux might be for a couple of renders - const [urlPageTheme, setUrlPageTheme] = useState(undefined); + const [urlPageTheme, setUrlPageTheme] = useState>(determinePageContextFromUrl(location.pathname)); useEffect(() => { const urlContext = determinePageContextFromUrl(location.pathname); @@ -213,7 +213,7 @@ export function useUrlPageTheme(): PageContextState { })); return () => { - setUrlPageTheme(undefined); + setUrlPageTheme({stage: undefined, subject: undefined}); dispatch(pageContextSlice.actions.updatePageContext({ subject: undefined, stage: undefined, @@ -247,4 +247,4 @@ export function sortStages(stages: Stage[]): Stage[] { const bIndex = isDefined(b) ? Object.values(STAGE).indexOf(b as STAGE) : -1; return aIndex - bIndex; }); -} \ No newline at end of file +} From 683dbeb7068eac6ea2029aa4be8f954cb7156239 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 10 Nov 2025 11:13:54 +0000 Subject: [PATCH 12/48] Fix(?) QF jest tests --- src/app/state/slices/api/questionsApi.ts | 2 +- src/mocks/data.ts | 2 +- src/test/pages/QuestionFinder.test.tsx | 39 ++++++++++++------------ 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/app/state/slices/api/questionsApi.ts b/src/app/state/slices/api/questionsApi.ts index 9260a5b0c0..665bc0b434 100644 --- a/src/app/state/slices/api/questionsApi.ts +++ b/src/app/state/slices/api/questionsApi.ts @@ -5,7 +5,7 @@ import { docSlice } from "../doc"; import { isaacApi } from "./baseApi"; import { onQueryLifecycleEvents } from "./utils"; -interface QuestionSearchResponseType { +export interface QuestionSearchResponseType { results?: ContentSummaryDTO[]; totalResults?: number; nextSearchOffset?: number; diff --git a/src/mocks/data.ts b/src/mocks/data.ts index ff75256957..35946da609 100644 --- a/src/mocks/data.ts +++ b/src/mocks/data.ts @@ -4529,7 +4529,7 @@ export const mockQuestionFinderResults = { "title": "A Bungee Jumper", "subtitle": "Step into Physics: Force and Motion Practice 9", "type": "isaacQuestionPage", - "level": 0, + "level": "0", "tags": [ "physics", "problem_solving", diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index a9760665d5..b5f441a04f 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -5,34 +5,29 @@ import shuffle from "lodash/shuffle"; import times from "lodash/times"; import flatten from "lodash/flatten"; import { buildFunctionHandler } from "../../mocks/handlers"; -import { isPhy, siteSpecific } from "../../app/services"; +import { isPhy, SEARCH_RESULTS_PER_PAGE, siteSpecific } from "../../app/services"; import userEvent from "@testing-library/user-event"; import { PageContextState } from "../../IsaacAppTypes"; import { expectPhyBreadCrumbs } from "../helpers/quiz"; -import { IsaacQuestionPageDTO } from "../../IsaacApiTypes"; +import { ContentSummaryDTO } from "../../IsaacApiTypes"; import { toggleFilter, PartialCheckboxState, Filter as F, expectPartialCheckBox } from "../../mocks/filters"; +import { QuestionSearchResponseType } from "../../app/state"; -type QuestionFinderResultsResponse = { - results: IsaacQuestionPageDTO[]; - nextSearchOffset: number; - totalResults: number; +const buildMockQuestions = (n: number, questions: QuestionSearchResponseType): ContentSummaryDTO[] => { + return Array(n).fill(null).map((_, i) => ({ ...questions.results?.[i % questions.results.length], id: `q${i}`, title: `Question ${i}: ${questions.results?.[i % questions.results.length].title}` })); }; -const buildMockQuestions = (n: number, questions: QuestionFinderResultsResponse): IsaacQuestionPageDTO[] => { - return Array(n).fill(null).map((_, i) => ({ ...questions.results[i % questions.results.length], id: `q${i}`, title: `Question ${i}: ${questions.results[i % questions.results.length].title}` })); -}; - -const buildMockQuestionFinderResults = (questions: T, start: number): QuestionFinderResultsResponse => ({ - results: questions.slice(start, start + 31), - nextSearchOffset: start + 31, - totalResults: questions.length +const buildMockQuestionFinderResults = (questions: T, start: number): QuestionSearchResponseType => ({ + results: questions.slice(start, start + SEARCH_RESULTS_PER_PAGE + 1), + nextSearchOffset: start + SEARCH_RESULTS_PER_PAGE + 1, + totalResults: questions.length, }); describe("QuestionFinder", () => { - const questions = buildMockQuestions(40, mockQuestionFinderResults as QuestionFinderResultsResponse); + const questions = buildMockQuestions(40, mockQuestionFinderResults as QuestionSearchResponseType); const resultsResponse = buildMockQuestionFinderResults(questions, 0); - const questionsWithMultipleStages = buildMockQuestions(40, mockQuestionFinderResultsWithMultipleStages as QuestionFinderResultsResponse); + const questionsWithMultipleStages = buildMockQuestions(40, mockQuestionFinderResultsWithMultipleStages as QuestionSearchResponseType); const resultsResponseWithMultipleStages = buildMockQuestionFinderResults(questionsWithMultipleStages, 0); const renderQuestionFinderPage = async ({response, queryParams, context} : RenderParameters) => { @@ -361,6 +356,12 @@ describe("QuestionFinder", () => { context: { subject: "physics", stage: ["a_level"] }, }); + await waitFor(async () => { + const x = resultsResponseWithMultipleStages; + const found = (await findQuestions()).map(getQuestionText); + expect(found.length).toEqual(30); + }); + await clickOn("Load more"); await waitFor(() => expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ @@ -423,7 +424,7 @@ type RenderParameters = { stages: string | null; randomSeed: string | null; startIndex: string | null; - }) => QuestionFinderResultsResponse; + }) => QuestionSearchResponseType; queryParams?: SearchString; context?: NonNullable; }; @@ -432,7 +433,7 @@ const findQuestions = () => screen.findByTestId("question-finder-results").then( const getQuestionText = (q: HTMLElement) => q.querySelector(isPhy ? '.question-link-title > span' : 'span.question-link-title')?.textContent; -const expectQuestions = (expectedQuestions: IsaacQuestionPageDTO[]) => waitFor(async () => { +const expectQuestions = (expectedQuestions: ContentSummaryDTO[]) => waitFor(async () => { const found = await findQuestions(); expect(found.length).toEqual(expectedQuestions.length); expect(found.map(getQuestionText)).toEqual(expectedQuestions.map(q => q.title)); @@ -458,4 +459,4 @@ const queryFilters = () => { const isInput = (element: HTMLElement): element is HTMLInputElement => { return element.tagName.toLowerCase() === 'input'; -}; \ No newline at end of file +}; From 03c37b94dfece7a8c85b2bd0c94c9d903880c923 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 10 Nov 2025 11:18:56 +0000 Subject: [PATCH 13/48] Remove unused vars --- src/app/components/pages/QuestionFinder.tsx | 3 --- src/test/pages/QuestionFinder.test.tsx | 1 - 2 files changed, 4 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index a0b0737759..c6315b7f6d 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -208,9 +208,6 @@ export const QuestionFinder = withRouter(() => { return [query, topics, books, stages, difficulties, examBoards].every(v => v.length === 0) && selections.every(v => Object.keys(v).length === 0); }; - // this should only update when a new search is triggered, not (necessarily) when the filters change - const [isCurrentSearchEmpty, setIsCurrentSearchEmpty] = useState(isEmptySearch(searchQuery, searchTopics, searchBooks, searchStages, searchDifficulties, searchExamBoards, selections)); - const [searchParams, setSearchParams] = useState(undefined); const searchQuestionsQuery = useSearchQuestionsQuery(searchParams ?? skipToken); diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index b5f441a04f..fa57ac8e84 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -357,7 +357,6 @@ describe("QuestionFinder", () => { }); await waitFor(async () => { - const x = resultsResponseWithMultipleStages; const found = (await findQuestions()).map(getQuestionText); expect(found.length).toEqual(30); }); From b2f2a9815219d3a38567120aa94ed965fa14d353 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 10 Nov 2025 16:04:23 +0000 Subject: [PATCH 14/48] Fix(??) more jest tests --- .../list-groups/AbstractListViewItem.tsx | 1 + src/mocks/filters.ts | 6 ++--- src/test/pages/QuestionFinder.test.tsx | 27 +++++++++++-------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/app/components/elements/list-groups/AbstractListViewItem.tsx b/src/app/components/elements/list-groups/AbstractListViewItem.tsx index c01958ec69..d152402d3a 100644 --- a/src/app/components/elements/list-groups/AbstractListViewItem.tsx +++ b/src/app/components/elements/list-groups/AbstractListViewItem.tsx @@ -253,6 +253,7 @@ export const AbstractListViewItem = ({title, icon, subject, subtitle, breadcrumb return {cardBody} ; diff --git a/src/mocks/filters.ts b/src/mocks/filters.ts index 7a7f3ed7d5..eded2377cb 100644 --- a/src/mocks/filters.ts +++ b/src/mocks/filters.ts @@ -79,7 +79,7 @@ export const toggleFilter = async (filter: Filter | Filter[]): Promise => } else { const regex = allowMatchesWithCount(filter); if (isPhy) { - await clickOn(regex, mainContainer()); + await clickOn(regex, sidebarContainer()); } else { await clickOn(regex); await clickOn("Apply filters"); @@ -133,11 +133,11 @@ export const expectPartialCheckBox = toExpectation((filter: Filter, state: Parti return expect(element).toHaveClass(state); }); -const mainContainer = () => screen.findByTestId('main'); +const sidebarContainer = () => screen.findByTestId('sidebar'); const findFilter = (label: string) => { const regexp = allowMatchesWithCount(label); return screen.queryByLabelText(regexp); }; -const allowMatchesWithCount = (str: string) => new RegExp(`^${str}$|^${str} \\([0-9]+\\)$|^${str}\\s*[0-9]+$`); \ No newline at end of file +const allowMatchesWithCount = (str: string) => new RegExp(`^${str}$|^${str} \\([0-9]+\\)$|^${str}\\s*[0-9]+$`); diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index fa57ac8e84..fde9d0089a 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -37,6 +37,13 @@ describe("QuestionFinder", () => { await waitForLoaded(); await setUrl({ pathname: context ? `/${context.subject}/${context.stage?.[0]}/questions` : '/questions', search: queryParams }); await waitForLoaded(); + if (context || queryParams?.includes("stages=") || queryParams?.includes("subjects=")) { + await waitFor(async () => { + expect(await within(screen.getByTestId("question-finder-results")).findAllByTestId("list-view-item")).not.toHaveLength(0); + }); + } else { + await waitFor(() => expect(screen.getByTestId("question-finder-results")).toHaveTextContent("Select some filters")); + } }; it('should render results in alphabetical order', async () => { @@ -65,10 +72,7 @@ describe("QuestionFinder", () => { it('button should shuffle questions', async () => { await withMockedRandom(async (randomSequence) => { randomSequence([1 * 10 ** -6]); - await renderQuestionFinderPage({ response }); - - await toggleFilter(F.GCSE); - await expectQuestions(questions.slice(0, 30)); + await renderQuestionFinderPage({ response, queryParams: '?stages=gcse' }); await clickOn("Shuffle"); await expectQuestions(shuffledQuestions.slice(0, 30)); @@ -79,8 +83,8 @@ describe("QuestionFinder", () => { return withMockedRandom(async (randomSequence) => { randomSequence([1 * 10 ** -6]); - await renderQuestionFinderPage({ response }); - await toggleFilter(F.GCSE); + await renderQuestionFinderPage({ response, queryParams: '?stages=gcse' }); + await clickOn("Shuffle"); await expectUrlParams("?randomSeed=1&stages=gcse"); }); @@ -127,7 +131,7 @@ describe("QuestionFinder", () => { await renderQuestionFinderPage({ response: ({ randomSeed, startIndex }) => { switch (randomSeed) { - case null: return startIndex === '0' ? resultsResponse : resultsResponsePage2;; + case null: return startIndex === '0' ? resultsResponse : resultsResponsePage2; case '1': return startIndex === '0' ? shuffledResultsResponse : shuffledResultsResponsePage2; default: throw new Error('Unexpected seed'); } @@ -437,10 +441,11 @@ const expectQuestions = (expectedQuestions: ContentSummaryDTO[]) => waitFor(asyn expect(found.length).toEqual(expectedQuestions.length); expect(found.map(getQuestionText)).toEqual(expectedQuestions.map(q => q.title)); }, { timeout: 5000 }); - -const expectPageIndicator = (content: string) => screen.findByTestId("question-finder-results").then(found => { - expect(found.querySelector('[data-testid="question-finder-results-header"]')?.textContent).toBe(content); -}); + +const expectPageIndicator = async (content: string) => await waitFor(async () => { + const results = await screen.findByTestId("question-finder-results"); + expect(results.querySelector('[data-testid="question-finder-results-header"]')?.textContent).toBe(content); +}, { timeout: 5000 }); const clearFilterTag = async (tagId: string) => { const tag = await screen.findByTestId(`filter-tag-${tagId}`); From a17366e996ff3a981e4ef3119cce5e57e64094c5 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Mon, 10 Nov 2025 16:05:24 +0000 Subject: [PATCH 15/48] Include searching in waitForLoaded --- src/test/testUtils.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/testUtils.tsx b/src/test/testUtils.tsx index 89fdc240f4..bac31cf6b7 100644 --- a/src/test/testUtils.tsx +++ b/src/test/testUtils.tsx @@ -172,6 +172,7 @@ export const enterInput = async (placeholder: string, input: string) => { export const waitForLoaded = () => waitFor(() => { expect(screen.queryAllByText("Loading...")).toHaveLength(0); + expect(screen.queryAllByText("Searching...")).toHaveLength(0); }); export const expectUrl = (text: string) => waitFor(() => { From 31832723c10ab749a7a98d85e47dca621b1291e0 Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Mon, 10 Nov 2025 17:35:03 +0000 Subject: [PATCH 16/48] tests: clickOn requires exactly one match --- src/app/components/elements/layout/SidebarLayout.tsx | 2 +- src/test/testUtils.tsx | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/app/components/elements/layout/SidebarLayout.tsx b/src/app/components/elements/layout/SidebarLayout.tsx index 8cea0a9fa5..dee4068cad 100644 --- a/src/app/components/elements/layout/SidebarLayout.tsx +++ b/src/app/components/elements/layout/SidebarLayout.tsx @@ -1078,7 +1078,7 @@ export const QuizSidebar = (props: QuizSidebarAttemptProps | QuizSidebarViewProp
- Key + Key
    diff --git a/src/test/testUtils.tsx b/src/test/testUtils.tsx index bac31cf6b7..75877d27a0 100644 --- a/src/test/testUtils.tsx +++ b/src/test/testUtils.tsx @@ -155,7 +155,7 @@ export const switchAccountTab = async (tab: ACCOUNT_TAB) => { }; export const clickOn = async (text: string | RegExp, container?: Promise) => { - const [target] = await (container ? within(await container).findAllByText(text).then(e => e) : screen.findAllByText(text)); + const target = await (container ? within(await container).findByText(text).then(e => e) : screen.findByText(text)); if (target.hasAttribute('disabled')) { throw new Error(`Can't click on disabled button ${target.textContent}`); } @@ -253,7 +253,8 @@ export const expectLinkWithEnabledBackwardsNavigation = async (text: string | un if (text === undefined) { throw new Error("Target text is undefined"); } - await clickOn(text); + const container = isPhy ? screen.findByRole("link", { name: text}) : undefined; + await clickOn(text, container); await expectUrl(targetHref); goBack(); await expectUrl(originalHref); From 38247f59d430a07e58baa577fa14c52aa1c94ee0 Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Tue, 11 Nov 2025 14:43:07 +0000 Subject: [PATCH 17/48] silence windows.scrollTo errors --- src/test/setupTests.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/setupTests.ts b/src/test/setupTests.ts index 05b9d9d796..3c76cace3f 100644 --- a/src/test/setupTests.ts +++ b/src/test/setupTests.ts @@ -1,9 +1,12 @@ +// scrollManager access this as it's interpreted, so the mocking +// needs to happen before it's imported +global.window.scrollTo = jest.fn(); + import 'core-js'; import '@testing-library/jest-dom'; import {server} from "../mocks/server"; import "./matchers"; -global.window.scrollTo = jest.fn(); global.window.alert = jest.fn(); global.window.confirm = jest.fn(() => true); global.confirm = jest.fn(() => true); From 83307b11a76a60ab03868d35f81064f4c782c003 Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Tue, 11 Nov 2025 14:48:18 +0000 Subject: [PATCH 18/48] fix ada QF tests, still expecting some failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's still flakiness. For example, the following test fails for me locally: `QuestionFinder › Question shuffling › button should shuffle questions` (it sees the same order before and after shuffling). --- src/test/pages/QuestionFinder.test.tsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index fde9d0089a..14bb237e17 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -37,13 +37,6 @@ describe("QuestionFinder", () => { await waitForLoaded(); await setUrl({ pathname: context ? `/${context.subject}/${context.stage?.[0]}/questions` : '/questions', search: queryParams }); await waitForLoaded(); - if (context || queryParams?.includes("stages=") || queryParams?.includes("subjects=")) { - await waitFor(async () => { - expect(await within(screen.getByTestId("question-finder-results")).findAllByTestId("list-view-item")).not.toHaveLength(0); - }); - } else { - await waitFor(() => expect(screen.getByTestId("question-finder-results")).toHaveTextContent("Select some filters")); - } }; it('should render results in alphabetical order', async () => { From 4db743df7f2ca69e112a07ce2ad93b6f36d1c9f2 Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Wed, 12 Nov 2025 10:25:20 +0000 Subject: [PATCH 19/48] revert this! prove tests pass without these - this commit disables the debounce, to prove that without it, tests pass consistently. - it skips a test that unreliably catches a legitimate bug, which is regrettably also on the live site. This test should be stabilized, re-enabled and the bug fixed. --- src/app/components/pages/QuestionFinder.tsx | 2 +- src/test/pages/QuestionFinder.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index c940aca7cf..520a41cff7 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -260,7 +260,7 @@ export const QuestionFinder = withRouter(() => { limit: SEARCH_RESULTS_PER_PAGE, randomSeed }); - }, 250, { leading: true }), + }, 0, { leading: true }), [pageContext]); diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index 8097b94a25..afb8999d5f 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -345,7 +345,7 @@ describe("QuestionFinder", () => { }))); }); - it('"Load more" only fetches questions for the context', async () => { + it.skip('"Load more" only fetches questions for the context', async () => { const getQuestionsWithMultipleStages = jest.fn(() => resultsResponseWithMultipleStages); await renderQuestionFinderPage({ From c7502f090e9135caa6f22eab4d5d3d1d201314df Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Wed, 12 Nov 2025 10:34:55 +0000 Subject: [PATCH 20/48] stabilize "Load more" test We assert the number of calls to make sure the assertion about the last call runs on the second call. --- src/test/pages/QuestionFinder.test.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index afb8999d5f..5a26ac35ae 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -360,10 +360,13 @@ describe("QuestionFinder", () => { await clickOn("Load more"); - await waitFor(() => expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ - tags: "physics", - stages: "a_level,further_a", - }))); + await waitFor(() => { + expect(getQuestionsWithMultipleStages).toHaveBeenCalledTimes(2); + return expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ + tags: "physics", + stages: "a_level,further_a", + })); + }); }); describe('fields and topics', () => { From aad5dec277edf68fe732024075492897e42a9f74 Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Wed, 12 Nov 2025 11:03:22 +0000 Subject: [PATCH 21/48] fix VRTs --- src/mocks/handlers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts index 0c915e7620..3093816618 100644 --- a/src/mocks/handlers.ts +++ b/src/mocks/handlers.ts @@ -223,7 +223,7 @@ export const handlers = [ status: 200, }); }), - http.get(API_PATH + "/pages/questions/", () => { + http.get(API_PATH + "/pages/questions", () => { return HttpResponse.json(mockQuestionFinderResults, { status: 200, }); From aefbc15c6eb7f88b9f0d21ef7ddd5977484cde6f Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Wed, 12 Nov 2025 11:30:12 +0000 Subject: [PATCH 22/48] Fix load more in context test --- src/app/components/pages/QuestionFinder.tsx | 2 +- src/test/pages/QuestionFinder.test.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index 520a41cff7..28aab533c0 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -168,7 +168,7 @@ export const QuestionFinder = withRouter(() => { )); const [searchTopics, setSearchTopics] = useState(arrayFromPossibleCsv(params.topics)); const [searchQuery, setSearchQuery] = useState(params.query ? (params.query instanceof Array ? params.query[0] : params.query) : ""); - const [searchStages, setSearchStages] = useState(arrayFromPossibleCsv(params.stages) as STAGE[]); // we can't fully populate this until we have the user + const [searchStages, setSearchStages] = useState(pageContext.stage ? pageStageToSearchStage(pageContext.stage) : arrayFromPossibleCsv(params.stages) as STAGE[]); // we can't fully populate this until we have the user const [searchDifficulties, setSearchDifficulties] = useState(arrayFromPossibleCsv(params.difficulties) as Difficulty[]); const [searchExamBoards, setSearchExamBoards] = useState(arrayFromPossibleCsv(params.examBoards) as ExamBoard[]); const [searchStatuses, setSearchStatuses] = useState(getInitialQuestionStatuses(params)); diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index 5a26ac35ae..3478b4ecea 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -345,7 +345,7 @@ describe("QuestionFinder", () => { }))); }); - it.skip('"Load more" only fetches questions for the context', async () => { + it('"Load more" only fetches questions for the context', async () => { const getQuestionsWithMultipleStages = jest.fn(() => resultsResponseWithMultipleStages); await renderQuestionFinderPage({ @@ -362,7 +362,7 @@ describe("QuestionFinder", () => { await waitFor(() => { expect(getQuestionsWithMultipleStages).toHaveBeenCalledTimes(2); - return expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ + expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ tags: "physics", stages: "a_level,further_a", })); From 94c33c2b637160e0deac46dfe3e8f6ff79810031 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Wed, 12 Nov 2025 11:30:38 +0000 Subject: [PATCH 23/48] Tidy up context => STAGE function --- src/app/components/pages/QuestionFinder.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index 28aab533c0..769d55bcc1 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -18,6 +18,7 @@ import { isPhy, Item, itemiseTag, + LEARNING_STAGE_TO_STAGES, LearningStage, ListParams, nextSeed, @@ -27,6 +28,7 @@ import { STAGE, STAGE_NULL_OPTIONS, stageLabelMap, + STAGES_PHY, SUBJECT_SPECIFIC_CHILDREN_MAP, TAG_ID, tags, @@ -120,13 +122,7 @@ function getInitialQuestionStatuses(params: ListParams): QuestionS export function pageStageToSearchStage(stage?: LearningStage[]): STAGE[] { if (!stage || stage.length === 0) return []; - switch (stage[0]) { - case "11_14": return [STAGE.YEAR_7_AND_8, STAGE.YEAR_9]; - case "gcse": return [STAGE.GCSE]; - case "a_level": return [STAGE.A_LEVEL, STAGE.FURTHER_A]; - case "university": return [STAGE.UNIVERSITY]; - default: return []; - } + return LEARNING_STAGE_TO_STAGES[stage[0]].filter((STAGES_PHY as readonly STAGE[]).includes); } interface FilterSummaryProps { From 08272a1e039a9338e5aff0166d8c4b88af630ffe Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Wed, 12 Nov 2025 11:41:24 +0000 Subject: [PATCH 24/48] Pass only first param in context => stage conversion --- src/app/components/pages/QuestionFinder.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index 769d55bcc1..8478da640f 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -122,7 +122,7 @@ function getInitialQuestionStatuses(params: ListParams): QuestionS export function pageStageToSearchStage(stage?: LearningStage[]): STAGE[] { if (!stage || stage.length === 0) return []; - return LEARNING_STAGE_TO_STAGES[stage[0]].filter((STAGES_PHY as readonly STAGE[]).includes); + return LEARNING_STAGE_TO_STAGES[stage[0]].filter(s => (STAGES_PHY as readonly STAGE[]).includes(s)); } interface FilterSummaryProps { @@ -256,7 +256,7 @@ export const QuestionFinder = withRouter(() => { limit: SEARCH_RESULTS_PER_PAGE, randomSeed }); - }, 0, { leading: true }), + }, 250, { leading: true }), [pageContext]); From 60675576dd307961b4e74cd1606edba8ea92d263 Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Wed, 12 Nov 2025 12:45:08 +0000 Subject: [PATCH 25/48] Fix last tests, speed up Jest --- config/jest/jest.config.common.js | 4 ++++ src/app/components/pages/QuestionFinder.tsx | 16 ++++++++++------ src/app/services/pageContext.ts | 8 +++++++- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/config/jest/jest.config.common.js b/config/jest/jest.config.common.js index 63b2885781..ca128691d5 100644 --- a/config/jest/jest.config.common.js +++ b/config/jest/jest.config.common.js @@ -25,6 +25,10 @@ module.exports = { "^.+\\.css$": "config/jest/cssTransform.js", "^(?!.*\\.(js|jsx|ts|tsx|css|json)$)": "config/jest/fileTransform.js", '^.+\\.[jt]sx?$': "config/jest/tsTransform.js", + "^.+\\.[jt]sx?$": ["ts-jest", { + tsconfig: "/tsconfig.json", + }], + }, "transformIgnorePatterns": [ "/node_modules/(?!@popperjs|katex|leaflet)", diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index 8478da640f..a0b1281f00 100644 --- a/src/app/components/pages/QuestionFinder.tsx +++ b/src/app/components/pages/QuestionFinder.tsx @@ -164,7 +164,7 @@ export const QuestionFinder = withRouter(() => { )); const [searchTopics, setSearchTopics] = useState(arrayFromPossibleCsv(params.topics)); const [searchQuery, setSearchQuery] = useState(params.query ? (params.query instanceof Array ? params.query[0] : params.query) : ""); - const [searchStages, setSearchStages] = useState(pageContext.stage ? pageStageToSearchStage(pageContext.stage) : arrayFromPossibleCsv(params.stages) as STAGE[]); // we can't fully populate this until we have the user + const [searchStages, setSearchStages] = useState(pageContext.stage?.length ? pageStageToSearchStage(pageContext.stage) : arrayFromPossibleCsv(params.stages) as STAGE[]); // we can't fully populate this until we have the user const [searchDifficulties, setSearchDifficulties] = useState(arrayFromPossibleCsv(params.difficulties) as Difficulty[]); const [searchExamBoards, setSearchExamBoards] = useState(arrayFromPossibleCsv(params.examBoards) as ExamBoard[]); const [searchStatuses, setSearchStatuses] = useState(getInitialQuestionStatuses(params)); @@ -539,10 +539,13 @@ export const QuestionFinder = withRouter(() => { : isPhy && <>No results. }
- From ed28c37ca882f4353214b17326f32b8a575bdc0b Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Tue, 18 Nov 2025 10:43:04 +0000 Subject: [PATCH 47/48] re-add parens around single param arrow function Jaycie told me we should avoid this style for consistency. --- src/app/state/slices/api/questionsApi.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/state/slices/api/questionsApi.ts b/src/app/state/slices/api/questionsApi.ts index 9d09baffc4..17cd9ad8fc 100644 --- a/src/app/state/slices/api/questionsApi.ts +++ b/src/app/state/slices/api/questionsApi.ts @@ -15,11 +15,11 @@ export interface QuestionSearchResponseType { export const questionsApi = isaacApi.enhanceEndpoints({addTagTypes: ["CanAttemptQuestionType"]}).injectEndpoints({ endpoints: (build) => ({ searchQuestions: build.query({ - query: args => ({ + query: (search) => ({ url: `/pages/questions`, params: { - ...args, - limit: (args.limit ?? SEARCH_RESULTS_PER_PAGE) + 1 // fetch one extra to check if more results are available + ...search, + limit: (search.limit ?? SEARCH_RESULTS_PER_PAGE) + 1 // fetch one extra to check if more results are available } }), serializeQueryArgs: (args) => { From 81b24c1055034bc8b47c2162f9ed5dcdc98af572 Mon Sep 17 00:00:00 2001 From: Barna Magyarkuti Date: Tue, 18 Nov 2025 11:06:30 +0000 Subject: [PATCH 48/48] undo change which ended up not being necessary we did this trying to a fix a bug where the spinner did not show up, but it turnes out the only reason it didn't show was that I had cache disabled in dev tools and throttling enabled --- src/app/components/pages/SubjectLandingPage.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/app/components/pages/SubjectLandingPage.tsx b/src/app/components/pages/SubjectLandingPage.tsx index fd5025efd0..266bfca11f 100644 --- a/src/app/components/pages/SubjectLandingPage.tsx +++ b/src/app/components/pages/SubjectLandingPage.tsx @@ -29,10 +29,7 @@ const loadingPlaceholder =
    const RandomQuestionBanner = ({context}: {context?: PageContextState}) => { const [randomSeed, setRandomSeed] = useState(nextSeed); - const handleGetDifferentQuestion = () => { - setSearchParams(skipToken); - setRandomSeed(nextSeed); - }; + const handleGetDifferentQuestion = () => setRandomSeed(nextSeed); const [searchParams, setSearchParams] = useState(skipToken); const searchQuestionsQuery = useSearchQuestionsQuery(searchParams);