diff --git a/config/jest/jest.config.common.js b/config/jest/jest.config.common.js index 63b2885781..f4c2e6c570 100644 --- a/config/jest/jest.config.common.js +++ b/config/jest/jest.config.common.js @@ -24,7 +24,10 @@ module.exports = { "transform": { "^.+\\.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/config/jest/tsTransform.js b/config/jest/tsTransform.js deleted file mode 100644 index 64f88e5f22..0000000000 --- a/config/jest/tsTransform.js +++ /dev/null @@ -1,3 +0,0 @@ -const config = {}; - -module.exports = require("ts-jest").default.createTransformer(config); \ No newline at end of file diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 35c2c4d9f6..a7ee5a7bf9 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -350,9 +350,6 @@ } }, "src/app/components/elements/layout/SidebarLayout.tsx": { - "@stylistic/indent": { - "count": 1 - }, "@typescript-eslint/no-explicit-any": { "count": 4 }, @@ -439,11 +436,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": { @@ -731,7 +725,7 @@ "count": 4 }, "@typescript-eslint/no-unused-vars": { - "count": 3 + "count": 2 }, "jsx-a11y/anchor-is-valid": { "count": 1 @@ -869,7 +863,7 @@ "count": 2 }, "@typescript-eslint/no-floating-promises": { - "count": 10 + "count": 9 }, "react-hooks/exhaustive-deps": { "count": 2 @@ -924,7 +918,7 @@ }, "src/app/components/pages/QuestionFinder.tsx": { "react-hooks/exhaustive-deps": { - "count": 2 + "count": 1 } }, "src/app/components/pages/QuickQuizzes.tsx": { @@ -983,14 +977,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 @@ -1251,11 +1237,6 @@ "count": 1 } }, - "src/app/state/slices/api/baseApi.ts": { - "@typescript-eslint/no-unused-vars": { - "count": 1 - } - }, "src/app/state/slices/api/contentApi.ts": { "@typescript-eslint/no-unused-vars": { "count": 2 diff --git a/src/IsaacAppTypes.tsx b/src/IsaacAppTypes.tsx index 3a2996738e..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} diff --git a/src/app/components/elements/layout/SidebarLayout.tsx b/src/app/components/elements/layout/SidebarLayout.tsx index 68c4b465c0..33cc2a5baf 100644 --- a/src/app/components/elements/layout/SidebarLayout.tsx +++ b/src/app/components/elements/layout/SidebarLayout.tsx @@ -1084,7 +1084,7 @@ export const QuizSidebar = (props: QuizSidebarAttemptProps | QuizSidebarViewProp
- Key + Key
    diff --git a/src/app/components/elements/modals/QuestionSearchModal.tsx b/src/app/components/elements/modals/QuestionSearchModal.tsx index e42cd76b87..3636d8a87c 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(skipToken); + const searchQuestionsQuery = useSearchQuestionsQuery(searchParams); + 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; + return setSearchParams(skipToken); } 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), - [] - ); + }, 250, { leading: true }), + [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,48 @@ 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}) => { + if (!questions) return <>; + const sortedQuestions = sortAndFilterBySearch(questions); + return sortedQuestions?.map(question => + + ); + }} + /> + +
    TopicStageDifficultyExam boards
    +     ; }; diff --git a/src/app/components/handlers/ShowLoadingQuery.tsx b/src/app/components/handlers/ShowLoadingQuery.tsx index 1657ff2e09..34a498703e 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, }; @@ -50,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; @@ -74,8 +77,8 @@ 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) { - const {data, isLoading, isFetching, isError, error} = query; +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) { return "status" in error && typeof error.status === "number" && [NOT_FOUND, NO_CONTENT].includes(error.status) && ifNotFound ? <>{ifNotFound} : renderError(); @@ -84,6 +87,10 @@ export function ShowLoadingQuery({query, thenRender, children, placeholder, i const isStale = (isLoading || isFetching) && isFound(data); const showPlaceholder = (isLoading || isFetching) && (!maintainOnRefetch || !isDefined(data)); + if (isUninitialized) { + return uninitializedPlaceholder ? <>{uninitializedPlaceholder} : null; + } + if (showPlaceholder) { return placeholder ? <>{placeholder} : ; } diff --git a/src/app/components/pages/QuestionFinder.tsx b/src/app/components/pages/QuestionFinder.tsx index b9ff70dd6a..338cd0f4ad 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, useAppSelector, useSearchQuestionsQuery} from "../../state"; import debounce from "lodash/debounce"; import { arrayFromPossibleCsv, @@ -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, @@ -34,10 +36,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"; @@ -56,6 +57,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; @@ -118,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(s => (STAGES_PHY as readonly STAGE[]).includes(s)); } interface FilterSummaryProps { @@ -145,16 +143,28 @@ export const FilterSummary = ({filterTags, clearFilters, removeFilterTag}: Filte
    ; }; +const loadingPlaceholder = +
    + + +
    +    ; + export const QuestionFinder = withRouter(() => { - const dispatch = useAppDispatch(); const user = useAppSelector((state: AppState) => state && state.user); 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(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)); @@ -185,23 +195,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 [disableLoadMore, setDisableLoadMore] = useState(false); - const choices = useMemo(() => { return updateTopicChoices(selections, pageContext, getAllowedTags(pageContext)); }, [selections, pageContext]); @@ -210,10 +203,8 @@ 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 {results: questions, totalResults: totalQuestions, nextSearchOffset} = useAppSelector((state: AppState) => state && state.questionSearchResult) || {}; + const [searchParams, setSearchParams] = useState(skipToken); + const searchQuestionsQuery = useSearchQuestionsQuery(searchParams); const debouncedSearch = useMemo(() => debounce(({ @@ -229,8 +220,8 @@ export const QuestionFinder = withRouter(() => { startIndex, randomSeed }): void => { if (isEmptySearch(searchString, topics, book, stages, difficulties, examBoards, hierarchySelections)) { - setIsCurrentSearchEmpty(true); - return void dispatch(clearQuestionSearch); + setSearchParams(skipToken); + return; } const choiceTreeLeaves = getChoiceTreeLeaves(hierarchySelections).map(leaf => leaf.value); @@ -247,9 +238,7 @@ export const QuestionFinder = withRouter(() => { }); } - setIsCurrentSearchEmpty(false); - - void dispatch(searchQuestions({ + setSearchParams({ querySource: "questionFinder", searchString: searchString || undefined, tags: choiceTreeLeaves.join(",") || undefined, @@ -264,19 +253,17 @@ 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]); + }); + }, 250, { leading: true }), + [pageContext]); const filteringByStatus = Object.values(searchStatuses).some(v => v) && !Object.values(searchStatuses).every(v => v); const searchAndUpdateURL = useCallback(() => { setPageCount(1); - setDisableLoadMore(false); - setDisplayQuestions(undefined); const filteredStages = !searchStages.length && pageContext?.stage ? pageStageToSearchStage(pageContext.stage) : searchStages; debouncedSearch({ @@ -339,32 +326,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 @@ -385,7 +350,7 @@ export const QuestionFinder = withRouter(() => { searchAndUpdateURL(); } - }, [searchDifficulties, searchTopics, searchExamBoards, searchStages, searchBooks, excludeBooks, selections, searchStatuses, searchQuery]); + }, [searchDifficulties, searchTopics, searchExamBoards, searchStages, searchBooks, excludeBooks, selections, searchStatuses]); const clearFilters = useCallback(() => { setSearchDifficulties([]); @@ -408,6 +373,7 @@ export const QuestionFinder = withRouter(() => { const debouncedSearchHandler = useMemo(() => debounce((searchTerm: string) => { + setRandomSeed(undefined); setSearchQuery(searchTerm); }, 500), [setSearchQuery]); @@ -423,11 +389,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)); @@ -512,10 +473,7 @@ export const QuestionFinder = withRouter(() => { { - debouncedSearchHandler(e.target.value); - setRandomSeed(undefined); // This random seed reset is for Ada only! This is managed in the filtersChanged useEffect for Phy - }} + onChange={(e) => debouncedSearchHandler(e.target.value)} onSearch={searchAndUpdateURL} /> @@ -550,70 +508,88 @@ export const QuestionFinder = withRouter(() => { }} /> } - - -
    - {displayQuestions && displayQuestions.length > 0 - ? <>Showing {displayQuestions.length} - : isPhy && isCurrentSearchEmpty - ? <>Select {filteringByStatus ? "more" : "some"} filters to start searching - : <>No results + + + {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 + ? <> + Showing {questions.length} + {(totalQuestions ?? 0) > 0 && !filteringByStatus && <> of {totalQuestions}} + . + + : isPhy && <>No results. + } +
    + +     + + {questions?.length + ? + : isAda && (filteringByStatus + ? Could not load any results matching the requested filters. + : No results match the requested filters. + ) + } + +     + {(questions?.length ?? 0) > 0 && + + + + + } - {(totalQuestions ?? 0) > 0 && !filteringByStatus && <> of {totalQuestions}} - . -
    - -     - - - {displayQuestions?.length - ? - : isAda && isCurrentSearchEmpty - ? Please select and apply filters. - : filteringByStatus - ? Could not load any results matching the requested filters. - : No results match the requested filters. - } - - -     - {(displayQuestions?.length ?? 0) > 0 && - - - - - } + ; + }} + /> diff --git a/src/app/components/pages/SubjectLandingPage.tsx b/src/app/components/pages/SubjectLandingPage.tsx index 6d3751824c..266bfca11f 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(skipToken); + const searchQuestionsQuery = useSearchQuestionsQuery(searchParams); - 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) || {}; + }); + }, 250, { leading: true }), [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 + ? + :
      +
    • + No questions found. +
      • +
    ; + }} + />
    ; }; diff --git a/src/app/services/api.ts b/src/app/services/api.ts index d5ce7f1f2b..bb801bd52d 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -3,7 +3,7 @@ import {API_PATH, IMAGE_PATH, securePadCredentials, TAG_ID} from "./"; import * as ApiTypes from "../../IsaacApiTypes"; import {AuthenticationProvider, TestCaseDTO} from "../../IsaacApiTypes"; import * as AppTypes from "../../IsaacAppTypes"; -import {Choice, CredentialsAuthDTO, QuestionSearchQuery} from "../../IsaacAppTypes"; +import {Choice, CredentialsAuthDTO} from "../../IsaacAppTypes"; import {handleApiGoneAway, handleServerError} from "../state"; import {Immutable} from "immer"; @@ -123,11 +123,6 @@ export const api = { } }, questions: { - search: (query: QuestionSearchQuery): AxiosPromise> => { - return endpoint.get(`/pages/questions/`, { - params: query, - }); - }, answer: (id: string, answer: Immutable): AxiosPromise => { return endpoint.post(`/questions/${id}/answer`, answer); }, diff --git a/src/app/services/constants.ts b/src/app/services/constants.ts index 979fe3e6bb..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", diff --git a/src/app/services/pageContext.ts b/src/app/services/pageContext.ts index 7cd2837e6c..3e0d4e12e0 100644 --- a/src/app/services/pageContext.ts +++ b/src/app/services/pageContext.ts @@ -181,13 +181,17 @@ 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; +} + +function isDistinctUrlContext(urlContext: PageContextState, reduxContext: PageContextState): boolean { + return urlContext?.subject !== reduxContext?.subject || urlContext?.stage?.join(",") !== reduxContext?.stage?.join(","); } /** @@ -196,16 +200,18 @@ 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); - setUrlPageTheme(urlContext); + // only update local state if an actual value has changed, not just because this is a different object + setUrlPageTheme(pt => isDistinctUrlContext(urlContext, pt) ? urlContext : pt); + dispatch(pageContextSlice.actions.updatePageContext({ subject: urlContext?.subject, stage: urlContext?.stage, @@ -213,7 +219,7 @@ export function useUrlPageTheme(): PageContextState { })); return () => { - setUrlPageTheme(undefined); + setUrlPageTheme({stage: undefined, subject: undefined}); dispatch(pageContextSlice.actions.updatePageContext({ subject: undefined, stage: undefined, @@ -247,4 +253,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 +} diff --git a/src/app/state/actions/index.tsx b/src/app/state/actions/index.tsx index 9cd727622b..f8c496e95f 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"; @@ -481,29 +480,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 5f2b1c21a2..bab174f448 100644 --- a/src/app/state/index.ts +++ b/src/app/state/index.ts @@ -22,7 +22,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 03f6557d1f..c0315fa53a 100644 --- a/src/app/state/reducers/index.ts +++ b/src/app/state/reducers/index.ts @@ -20,7 +20,6 @@ import { testQuestions, quizAttempt, groupMemberships, - questionSearchResult, isaacApi, gameboardsSlice, adminUserSearchSlice, @@ -81,7 +80,6 @@ export const rootReducer = combineReducers({ // Gameboards boards: gameboardsSlice.reducer, - questionSearchResult, // Quizzes quizAttempt, diff --git a/src/app/state/slices/api/baseApi.ts b/src/app/state/slices/api/baseApi.ts index 6180337bf4..4e29a5f042 100644 --- a/src/app/state/slices/api/baseApi.ts +++ b/src/app/state/slices/api/baseApi.ts @@ -10,7 +10,7 @@ const isaacApi = createApi({ reducerPath: "isaacApi", baseQuery: isaacBaseQuery, keepUnusedDataFor: 0, - endpoints: (build) => ({ + endpoints: () => ({ // Endpoints are injected in other files, e.g. contentApi.ts }) }); diff --git a/src/app/state/slices/api/questionsApi.ts b/src/app/state/slices/api/questionsApi.ts index 7fb3cc824d..17cd9ad8fc 100644 --- a/src/app/state/slices/api/questionsApi.ts +++ b/src/app/state/slices/api/questionsApi.ts @@ -1,13 +1,62 @@ -import { IsaacQuestionPageDTO } from "../../../../IsaacApiTypes"; -import { CanAttemptQuestionTypeDTO } from "../../../../IsaacAppTypes"; -import { tags } from "../../../services"; +import { ContentSummaryDTO, IsaacQuestionPageDTO, SearchResultsWrapper } from "../../../../IsaacApiTypes"; +import { CanAttemptQuestionTypeDTO, QuestionSearchQuery } from "../../../../IsaacAppTypes"; +import { isDefined, SEARCH_RESULTS_PER_PAGE, tags } from "../../../services"; import { docSlice } from "../doc"; import { isaacApi } from "./baseApi"; import { onQueryLifecycleEvents } from "./utils"; +export 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: (search) => ({ + url: `/pages/questions`, + params: { + ...search, + limit: (search.limit ?? 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: SearchResultsWrapper, _, args) => { + return { + ...response, + // remove the extra result used to check for more results, so that we return the correct amount + moreResultsAvailable: isDefined(response.results) ? response.results.length > (args.limit ?? SEARCH_RESULTS_PER_PAGE) : undefined, + results: response.results?.slice(0, args.limit ?? SEARCH_RESULTS_PER_PAGE) + }; + }, + merge: (currentCache, newItems) => { + if (currentCache.results) { + currentCache.results.push(...(newItems.results ?? [])); + } else { + currentCache.results = newItems.results; + } + currentCache.totalResults = newItems.totalResults; + currentCache.nextSearchOffset = newItems.nextSearchOffset; + currentCache.moreResultsAvailable = newItems.moreResultsAvailable; + }, + forceRefetch({ currentArg, previousArg }) { + return currentArg !== previousArg; + }, + onQueryStarted: onQueryLifecycleEvents({ + errorTitle: "Unable to search for questions", + }), + }), + getQuestion: build.query({ query: (id) => ({ url: `/pages/questions/${id}` @@ -33,7 +82,8 @@ export const questionsApi = isaacApi.enhanceEndpoints({addTagTypes: ["CanAttempt }); export const { + useSearchQuestionsQuery, + useLazySearchQuestionsQuery, useGetQuestionQuery, useCanAttemptQuestionTypeQuery, } = questionsApi; - diff --git a/src/mocks/data.ts b/src/mocks/data.ts index ff75256957..5e9707ae64 100644 --- a/src/mocks/data.ts +++ b/src/mocks/data.ts @@ -4,11 +4,13 @@ import {DAYS_AGO, SOME_FIXED_FUTURE_DATE, SOME_FIXED_PAST_DATE} from "../test/da import { BookingStatus, CompletionState, + ContentSummaryDTO, DetailedQuizSummaryDTO, EmailVerificationStatus, EventStatus, IsaacQuizDTO, QuizAttemptDTO, + SearchResultsWrapper, USER_ROLES, UserRole, UserSummaryWithGroupMembershipDTO @@ -4489,14 +4491,14 @@ export const mockQuestionFinderResults = { "title": "A Bag of Flour", "subtitle": "Step into Physics: Weight 6", "type": "isaacQuestionPage", - "level": 0, + "level": "0", "tags": [ "physics", "problem_solving", "mechanics" ], "url": "/api/pages/questions/itsp24_weight_class_q6", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4529,14 +4531,14 @@ 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", "mechanics" ], "url": "/api/pages/questions/itsp24_forcemotion_hw_q9", - "state": "ALL_CORRECT", + "state": CompletionState.ALL_CORRECT, "audience": [ { "stage": [ @@ -4569,14 +4571,14 @@ export const mockQuestionFinderResults = { "title": "A Car Leaving Town", "subtitle": "Step into Physics: Acceleration 11", "type": "isaacQuestionPage", - "level": 0, + "level": "0", "tags": [ "physics", "problem_solving", "mechanics" ], "url": "/api/pages/questions/itsp24_accel_class_q11", - "state": "IN_PROGRESS", + "state": CompletionState.IN_PROGRESS, "audience": [ { "stage": [ @@ -4609,7 +4611,7 @@ export const mockQuestionFinderResults = { "title": "A Car on a Motorway", "subtitle": "Step into Physics: Calculating Speed 3", "type": "isaacQuestionPage", - "level": 0, + "level": "0", "tags": [ "physics", "problem_solving", @@ -4649,14 +4651,14 @@ export const mockQuestionFinderResults = { "title": "A Cat, Cyclist, Aeroplane and Cow", "subtitle": "Step into Physics: Acceleration Practice 1", "type": "isaacQuestionPage", - "level": 0, + "level": "0", "tags": [ "physics", "problem_solving", "mechanics" ], "url": "/api/pages/questions/itsp24_accel_hw_q1", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4687,7 +4689,7 @@ export const mockQuestionFinderResults = { ], "nextSearchOffset": 5, "totalResults": 5 -}; +} satisfies SearchResultsWrapper; export const mockQuestionFinderResultsWithMultipleStages = { "results": [ @@ -4704,7 +4706,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { "fields" ], "url": "/api/pages/questions/phys_linking_22_q5", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4729,7 +4731,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { "mechanics" ], "url": "/api/pages/questions/gcse_ch2_13_q12", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4762,7 +4764,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { "wave_motion" ], "url": "/api/pages/questions/phys19_k1_q4", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4787,7 +4789,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { "phys_book_step_up" ], "url": "/api/pages/questions/step_up_35_q7", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4820,7 +4822,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { "electricity" ], "url": "/api/pages/questions/phys_linking_33_q1", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4845,7 +4847,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { "nuclear" ], "url": "/api/pages/questions/gcse_ch6_51_q2_r1", - "state": "NOT_ATTEMPTED", + "state": CompletionState.NOT_ATTEMPTED, "audience": [ { "stage": [ @@ -4868,7 +4870,7 @@ export const mockQuestionFinderResultsWithMultipleStages = { ], "nextSearchOffset": 6, "totalResults": 6 -}; +} satisfies SearchResultsWrapper; export const mockConceptsResults = { results: [{ 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/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, }); diff --git a/src/mocks/utils.ts b/src/mocks/utils.ts index 319e85d96a..9eee86509c 100644 --- a/src/mocks/utils.ts +++ b/src/mocks/utils.ts @@ -1,4 +1,5 @@ -import { persistence } from "../app/services"; +import { persistence, SEARCH_RESULTS_PER_PAGE } from "../app/services"; +import { ContentSummaryDTO, SearchResultsWrapper } from "../IsaacApiTypes"; // this makes sure `elem` implements an interface without widening its type export const recordOf = () => (elem: Record) => elem; @@ -22,3 +23,13 @@ export const mockPersistence = () => { jest.resetAllMocks(); }); }; + +export const buildMockQuestions = (n: number, questions: SearchResultsWrapper): 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}` })); +}; + +export const buildMockQuestionFinderResults = (questions: T, start: number, limit = SEARCH_RESULTS_PER_PAGE): SearchResultsWrapper => ({ + results: questions.slice(start, start + limit + 1), + nextSearchOffset: start + limit + 1, + totalResults: questions.length, +}); diff --git a/src/test/pages/QuestionFinder.test.tsx b/src/test/pages/QuestionFinder.test.tsx index 45ccd0eaeb..ad759637dd 100644 --- a/src/test/pages/QuestionFinder.test.tsx +++ b/src/test/pages/QuestionFinder.test.tsx @@ -9,30 +9,16 @@ import { isPhy, 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, SearchResultsWrapper } from "../../IsaacApiTypes"; import { toggleFilter, PartialCheckboxState, Filter as F, expectPartialCheckBox } from "../../mocks/filters"; +import { buildMockQuestionFinderResults, buildMockQuestions } from "../../mocks/utils"; -type QuestionFinderResultsResponse = { - results: IsaacQuestionPageDTO[]; - nextSearchOffset: number; - totalResults: number; -}; - -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 -}); describe("QuestionFinder", () => { - const questions = buildMockQuestions(40, mockQuestionFinderResults as QuestionFinderResultsResponse); + const questions = buildMockQuestions(40, mockQuestionFinderResults); const resultsResponse = buildMockQuestionFinderResults(questions, 0); - const questionsWithMultipleStages = buildMockQuestions(40, mockQuestionFinderResultsWithMultipleStages as QuestionFinderResultsResponse); + const questionsWithMultipleStages = buildMockQuestions(40, mockQuestionFinderResultsWithMultipleStages); const resultsResponseWithMultipleStages = buildMockQuestionFinderResults(questionsWithMultipleStages, 0); const renderQuestionFinderPage = async ({response, queryParams, context} : RenderParameters) => { @@ -50,6 +36,14 @@ describe("QuestionFinder", () => { await expectQuestions(questions.slice(0, 30)); }); + it('should disable the shuffle button when there are no results', async () => { + await renderQuestionFinderPage({ + response: () => buildMockQuestionFinderResults([], 0), + queryParams: '?stages=gcse' + }); + expect(shuffleButton()).toBeDisabled(); + }); + describe('Question shuffling', () => { const shuffledQuestions = shuffle(questions); const shuffledResultsResponse = buildMockQuestionFinderResults(shuffledQuestions, 0); @@ -70,12 +64,9 @@ 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 clickOn(shuffleButton()); await expectQuestions(shuffledQuestions.slice(0, 30)); }); }); @@ -84,9 +75,9 @@ describe("QuestionFinder", () => { return withMockedRandom(async (randomSequence) => { randomSequence([1 * 10 ** -6]); - await renderQuestionFinderPage({ response }); - await toggleFilter(F.GCSE); - await clickOn("Shuffle"); + await renderQuestionFinderPage({ response, queryParams: '?stages=gcse' }); + + await clickOn(shuffleButton()); await expectUrlParams("?randomSeed=1&stages=gcse"); }); }); @@ -132,7 +123,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'); } @@ -141,7 +132,7 @@ describe("QuestionFinder", () => { await expectQuestions(questions.slice(0, 30)); await expectPageIndicator("Showing 30 of 40."); - await clickOn("Shuffle"); + await clickOn(shuffleButton()); await expectQuestions(shuffledQuestions.slice(0, 30)); await expectPageIndicator("Showing 30 of 40."); @@ -361,12 +352,20 @@ describe("QuestionFinder", () => { context: { subject: "physics", stage: ["a_level"] }, }); + await waitFor(async () => { + const found = (await findQuestions()).map(getQuestionText); + expect(found.length).toEqual(30); + }); + await clickOn("Load more"); - await waitFor(() => expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ - tags: "physics", - stages: "a_level,further_a", - }))); + await waitFor(() => { + expect(getQuestionsWithMultipleStages).toHaveBeenCalledTimes(2); + expect(getQuestionsWithMultipleStages).toHaveBeenLastCalledWith(expect.objectContaining({ + tags: "physics", + stages: "a_level,further_a", + })); + }); }); describe('fields and topics', () => { @@ -423,7 +422,7 @@ type RenderParameters = { stages: string | null; randomSeed: string | null; startIndex: string | null; - }) => QuestionFinderResultsResponse; + }) => SearchResultsWrapper; queryParams?: SearchString; context?: NonNullable; }; @@ -432,15 +431,16 @@ const findQuestions = () => screen.findByTestId("question-finder-results").then( const getQuestionText = (q: HTMLElement) => q.querySelector('.question-link-title > span')?.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)); }, { 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}`); @@ -458,4 +458,6 @@ const queryFilters = () => { const isInput = (element: HTMLElement): element is HTMLInputElement => { return element.tagName.toLowerCase() === 'input'; -}; \ No newline at end of file +}; + +const shuffleButton = () => screen.getByRole('button', { name: "Shuffle questions" }); \ No newline at end of file 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); diff --git a/src/test/state/slices/api/questionsApi.test.ts b/src/test/state/slices/api/questionsApi.test.ts new file mode 100644 index 0000000000..3d5df2b08a --- /dev/null +++ b/src/test/state/slices/api/questionsApi.test.ts @@ -0,0 +1,156 @@ +import { QuestionSearchQuery } from "../../../../IsaacAppTypes"; +import { renderTestHook } from "../../../testUtils"; +import { useAppSelector, useSearchQuestionsQuery } from "../../../../app/state"; +import { act, waitFor } from "@testing-library/react"; +import { buildMockQuestionFinderResults, buildMockQuestions } from "../../../../mocks/utils"; +import { buildFunctionHandler } from "../../../../mocks/handlers"; +import { mockQuestionFinderResults } from "../../../../mocks/data"; +import { useState } from "react"; +import { API_PATH, SEARCH_RESULTS_PER_PAGE } from "../../../../app/services"; +import { http, HttpResponse } from "msw"; + +describe('questions API', () => { + it('calls the /pages/questions endpoint', async () => { + const { handler } = await renderQuestionsQueryHook(createSearchQuery()); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it('forwards any query parameters', async () => { + const { handler } = await renderQuestionsQueryHook(createSearchQuery({ searchString: "horse" })); + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith(expect.objectContaining({ searchString: "horse" })); + }); + + it('requests an extra question to determine whether there are more', async () => { + const { handler } = await renderQuestionsQueryHook(createSearchQuery({ limit: 5 })); + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith(expect.objectContaining({ limit: "6" })); + }); + + it('returns one less questions to comply with original request', async () => { + const { response, questions } = await renderQuestionsQueryHook(createSearchQuery({ limit: 5 })); + expect(response().data?.results).toEqual(questions.slice(0, 5)); + }); + + it('indicates when there are more results', async () => { + const { response } = await renderQuestionsQueryHook(createSearchQuery({ limit: 5 }), { totalQuestions: 10}); + expect(response().data?.moreResultsAvailable).toBe(true); + }); + + it('indicates when there are no more results', async () => { + const { response } = await renderQuestionsQueryHook(createSearchQuery({ limit: 5 }), { totalQuestions: 5}); + expect(response().data?.moreResultsAvailable).toBe(false); + }); + + it('uses a default limit of SEARCH_RESULTS_PER_PAGE', async () => { + const { response, handler, questions } = await renderQuestionsQueryHook( + createSearchQuery(), + { totalQuestions: SEARCH_RESULTS_PER_PAGE + 1} + ); + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith(expect.objectContaining({ limit: "31" })); + expect(response().data?.results).toEqual(questions.slice(0, SEARCH_RESULTS_PER_PAGE)); + expect(response().data?.moreResultsAvailable).toBe(true); + }); + + describe('paging', () => { + it('appends the new page to the existing one', async () => { + const { questions, response, setQuery } = await renderQuestionsQueryHook( + createSearchQuery({ startIndex: 0, limit: 10 }), + { totalQuestions: 20 } + ); + + expect(response().data?.results).toEqual(questions.slice(0, 10)); + expect(response().data?.moreResultsAvailable).toBe(true); + + await setQuery(createSearchQuery({ startIndex: 10, limit: 10 })); + + expect(response().data?.results).toEqual(questions); + expect(response().data?.moreResultsAvailable).toBe(false); + }); + + it('starts a new page when a filter parameter changes', async () => { + const { questions, response, setQuery } = await renderQuestionsQueryHook( + createSearchQuery({ searchString: "horse", startIndex: 0, limit: 10 }), + { totalQuestions: 20 } + ); + + expect(response().data?.results).toEqual(questions.slice(0, 10)); + expect(response().data?.moreResultsAvailable).toBe(true); + + await setQuery(createSearchQuery({ searchString: "vs", startIndex: 0, limit: 10 })); + + expect(response().data?.results).toEqual(questions.slice(0, 10)); + expect(response().data?.moreResultsAvailable).toBe(true); + }); + }); + + describe('when there was an error', () => { + it('indicates this in the response', async () => { + const { result } = await renderQuestionsQueryHookFailure(); + expect(result.current.result.isError).toBe(true); + expect(result.current.result.error).toHaveProperty('status', 401); + expect(result.current.result.data).toBe(undefined); + }); + + it('shows a toast notification that mentions this endpoint', async () => { + const { result } = await renderQuestionsQueryHookFailure(); + expect(result.current.toasts).toHaveLength(1); + expect(result.current.toasts).toHaveProperty('[0].title', 'Unable to search for questions'); + }); + }); +}); + +const renderQuestionsQueryHook = async ( + initialQuery: QuestionSearchQuery, + { totalQuestions } = { totalQuestions: 10} +) => { + const questions = buildMockQuestions(totalQuestions, mockQuestionFinderResults); + const useTest = () => { + const [query, setQuery] = useState(createSearchQuery(initialQuery)); + const response = useSearchQuestionsQuery(query); + return { response, setQuery } as const; + }; + const handler = jest.fn(({ startIndex, limit } : { startIndex: string, limit: string }) => { + return buildMockQuestionFinderResults(questions, parseInt(startIndex), parseInt(limit)); + }); + + const { result } = renderTestHook(useTest, { extraEndpoints: [ + buildFunctionHandler('/pages/questions', ['startIndex', 'limit', 'searchString'], handler) + ]}); + + await waitFor(() => expect(result.current.response.isFetching).toBe(false)); + + const response = () => result.current.response; + const setQuery = async (query: QuestionSearchQuery) => { + await act(async() => result.current.setQuery(query)); + await waitFor(() => expect(response().isFetching).toBe(false)); + }; + return { handler, questions, response, setQuery }; +}; + +const renderQuestionsQueryHookFailure = async () => { + const useTest = () => { + const toasts = useAppSelector(state => state?.toasts || []); + const result = useSearchQuestionsQuery(createSearchQuery()); + return { toasts, result }; + }; + const handler = http.get( + API_PATH + "/pages/questions", + () => HttpResponse.json({ error: 'Not Authorized' }, { status: 401 }) + ); + + const { result } = renderTestHook(useTest, { extraEndpoints: [handler]}); + await waitFor(() => expect(result.current.result.isFetching).toBe(false)); + + return { result }; +}; + +const createSearchQuery = ( + {limit, searchString, startIndex = 0} : { limit?: number, searchString?: string, startIndex?: number } = { } +): QuestionSearchQuery => { + return { + querySource: "questionFinder", + startIndex, limit, searchString + }; +}; diff --git a/src/test/testUtils.tsx b/src/test/testUtils.tsx index 89fdc240f4..9def0b01ef 100644 --- a/src/test/testUtils.tsx +++ b/src/test/testUtils.tsx @@ -10,7 +10,7 @@ import {Provider} from "react-redux"; import {IsaacApp} from "../app/components/navigation/IsaacApp"; import React from "react"; import {MemoryRouter} from "react-router"; -import {fireEvent, screen, waitFor, within, act} from "@testing-library/react"; +import {fireEvent, screen, waitFor, within, act, renderHook, RenderHookResult} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import {SOME_FIXED_FUTURE_DATE_AS_STRING} from "./dateUtils"; import * as miscUtils from '../app/services/miscUtils'; @@ -49,10 +49,7 @@ interface RenderTestEnvironmentOptions { export const renderTestEnvironment = (options?: RenderTestEnvironmentOptions) => { const {role, modifyUser, sessionExpires, PageComponent, initalRouteEntries, extraEndpoints} = options ?? {}; history.replace({ pathname: '/', search: '' }); - store.dispatch({type: ACTION_TYPE.USER_LOG_OUT_RESPONSE_SUCCESS}); - store.dispatch({type: ACTION_TYPE.ACTIVE_MODAL_CLOSE}); - store.dispatch(isaacApi.util.resetApiState()); - store.getState().toasts?.forEach(toast => toast.id && store.dispatch(removeToast(toast.id))); + resetStore(); server.resetHandlers(); if (role || modifyUser) { server.use( @@ -99,6 +96,28 @@ export const renderTestEnvironment = (options?: RenderTestEnvironmentOptions) => ); }; +export const renderTestHook = ( + render: (initialProps: Props) => Result, + { extraEndpoints }: { extraEndpoints?: HttpHandler[] } = {} +): RenderHookResult => { + resetStore(); + server.resetHandlers(); + if (extraEndpoints) { + server.use(...extraEndpoints); + } + + return renderHook(render, { + wrapper: ({children}) => {children} + }); +}; + +export const resetStore = () => { + store.dispatch({type: ACTION_TYPE.USER_LOG_OUT_RESPONSE_SUCCESS}); + store.dispatch({type: ACTION_TYPE.ACTIVE_MODAL_CLOSE}); + store.dispatch(isaacApi.util.resetApiState()); + store.getState().toasts?.forEach(toast => toast.id && store.dispatch(removeToast(toast.id))); +}; + // Clicks on the given navigation menu entry, allowing navigation around the app as a user would export const followHeaderNavLink = async (menu: string, linkName: string) => { const header = await screen.findByTestId("header"); @@ -154,14 +173,24 @@ export const switchAccountTab = async (tab: ACCOUNT_TAB) => { await userEvent.click(tabLink); }; -export const clickOn = async (text: string | RegExp, container?: Promise) => { - const [target] = await (container ? within(await container).findAllByText(text).then(e => e) : screen.findAllByText(text)); +export const clickOn = async (e: string | RegExp | HTMLElement, container?: Promise) => { + const target = await identify(e, container); if (target.hasAttribute('disabled')) { throw new Error(`Can't click on disabled button ${target.textContent}`); } await userEvent.click(target); }; +const identify = async (e: string | RegExp | HTMLElement, container?: Promise): Promise => { + if (e instanceof HTMLElement) { + return e; + } else if (container) { + return within(await container).getByText(e); + } else { + return screen.getByText(e); + } +}; + export const enterInput = async (placeholder: string, input: string) => { const textBox = await screen.findByPlaceholderText(placeholder); if (textBox.hasAttribute('disabled')) { @@ -172,6 +201,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(() => { @@ -252,7 +282,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);