From 5bb43485c9b473552132d7dbdb0099c22a1acb21 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 20 Aug 2026 15:33:40 -0400 Subject: [PATCH 1/2] refactor: extend the model-store bridge to collection dispatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the transitional QueryCache bridge so a query can mirror collection results (and several model targets) into the models store, not just a single addModel. Adds a `meta.models` list — each entry runs a model-store action (addModel/updateModel/addModelsMap/updateModelsMap/updateModels) with an optional `source` key into the result — while keeping the existing `{ modelType, courseId }` single form. No runtime effect until a query opts in (wired up in the courseware metadata conversion that follows in this PR). Co-Authored-By: Claude Opus 4.8 --- src/course-home/data/modelStoreBridge.test.ts | 102 ++++++++++++++++++ src/data/modelStoreBridge.ts | 46 +++++++- 2 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 src/course-home/data/modelStoreBridge.test.ts diff --git a/src/course-home/data/modelStoreBridge.test.ts b/src/course-home/data/modelStoreBridge.test.ts new file mode 100644 index 0000000000..6decb8fce3 --- /dev/null +++ b/src/course-home/data/modelStoreBridge.test.ts @@ -0,0 +1,102 @@ +import { QueryCache, QueryClient } from '@tanstack/react-query'; + +import { addModelsMap } from '@src/generic/model-store'; +import initializeStore from '@src/store'; +import { bridgeToModelStore } from './modelStoreBridge'; + +type Store = ReturnType; + +const modelsOf = (store: Store) => store.getState().models as Record>; + +const runQuery = (queryClient: QueryClient, queryFn: () => unknown, meta: Record) => ( + queryClient.fetchQuery({ queryKey: [Math.random().toString()], queryFn, meta }) +); + +describe('modelStoreBridge', () => { + let store: Store; + let queryClient: QueryClient; + + beforeEach(() => { + store = initializeStore(); + queryClient = new QueryClient({ + queryCache: new QueryCache({ onSuccess: (data, query) => bridgeToModelStore(store, data, query) }), + }); + }); + + it('single form: mirrors the whole result as one model keyed by courseId', async () => { + await runQuery(queryClient, () => ({ foo: 'bar' }), { modelType: 'dates', courseId: 'course-1' }); + + expect(modelsOf(store).dates['course-1']).toEqual({ id: 'course-1', foo: 'bar' }); + }); + + it('list form addModel: mirrors the whole result as one model (using its own id)', async () => { + await runQuery( + queryClient, + () => ({ id: 'course-1', title: 'Demo' }), + { models: [{ modelType: 'coursewareMeta', strategy: 'addModel' }] }, + ); + + expect(modelsOf(store).coursewareMeta['course-1']).toEqual({ id: 'course-1', title: 'Demo' }); + }); + + it('list form: fans one result out to several collection mirrors via source keys', async () => { + await runQuery( + queryClient, + () => ({ + courses: { c1: { id: 'c1', sectionIds: ['s1'] } }, + sections: { s1: { id: 's1', title: 'Section' } }, + sequences: { q1: { id: 'q1', title: 'Sequence' } }, + }), + { + models: [ + { modelType: 'coursewareMeta', strategy: 'updateModelsMap', source: 'courses' }, + { modelType: 'sections', strategy: 'addModelsMap', source: 'sections' }, + { modelType: 'sequences', strategy: 'updateModelsMap', source: 'sequences' }, + ], + }, + ); + + const models = modelsOf(store); + expect(models.coursewareMeta.c1).toEqual({ id: 'c1', sectionIds: ['s1'] }); + expect(models.sections.s1).toEqual({ id: 's1', title: 'Section' }); + expect(models.sequences.q1).toEqual({ id: 'q1', title: 'Sequence' }); + }); + + it('updateModelsMap merges into an existing model rather than replacing it', async () => { + store.dispatch(addModelsMap({ + modelType: 'sequences', + modelsMap: { q1: { id: 'q1', unitIds: ['u1', 'u2'], activeUnitIndex: 0 } }, + })); + + await runQuery( + queryClient, + () => ({ sequences: { q1: { id: 'q1', title: 'Sequence' } } }), + { models: [{ modelType: 'sequences', strategy: 'updateModelsMap', source: 'sequences' }] }, + ); + + expect(modelsOf(store).sequences.q1).toEqual({ + id: 'q1', + title: 'Sequence', + unitIds: ['u1', 'u2'], + activeUnitIndex: 0, + }); + }); + + it('updateModels merges an array of models', async () => { + await runQuery( + queryClient, + () => ({ units: [{ id: 'u1', complete: true }, { id: 'u2', complete: false }] }), + { models: [{ modelType: 'units', strategy: 'updateModels', source: 'units' }] }, + ); + + const { units } = modelsOf(store); + expect(units.u1).toEqual({ id: 'u1', complete: true }); + expect(units.u2).toEqual({ id: 'u2', complete: false }); + }); + + it('does nothing when a query has no model-store meta', async () => { + await runQuery(queryClient, () => ({ foo: 'bar' }), {}); + + expect(store.getState().models).toEqual({}); + }); +}); diff --git a/src/data/modelStoreBridge.ts b/src/data/modelStoreBridge.ts index 4e5f89c3b7..f1f7c3eaa9 100644 --- a/src/data/modelStoreBridge.ts +++ b/src/data/modelStoreBridge.ts @@ -1,21 +1,57 @@ import type { Query } from '@tanstack/react-query'; import { Store } from 'redux'; -import { addModel } from '@src/generic/model-store'; +import { + addModel, + addModelsMap, + updateModel, + updateModels, + updateModelsMap, +} from '@src/generic/model-store'; + +type MirrorStrategy = + | 'addModel' + | 'updateModel' + | 'addModelsMap' + | 'updateModelsMap' + | 'updateModels'; + +interface ModelMirror { + modelType: string; + strategy: MirrorStrategy; + source?: string; +} interface ModelStoreMeta { modelType?: string; courseId?: string; + models?: ModelMirror[]; } // Transitional (#1977): bridge a React Query result into the model store so existing // `useModel(...)` readers (the shared TabPage/LoadedTabPage and not-yet-converted tabs) -// keep working until the model store is dissolved. A query opts in by tagging itself with -// `meta: { modelType, courseId }`. This is wired as the app QueryCache's `onSuccess` (see -// src/queryClient.ts), so it runs before observers re-render. +// keep working until the model store is dissolved. A query opts in via `meta`, either: +// `{ modelType, courseId }` — the whole result as one model keyed by courseId +// `{ models: [{ modelType, strategy, source? }] }` — one or more mirrors, each running a +// model-store action (`source` selects a key of the result; omitted = the whole result) +// This is wired as the app QueryCache's `onSuccess` (see src/queryClient.ts), so it runs +// before observers re-render. export const bridgeToModelStore = (store: Store, data: unknown, query: Query) => { - const { modelType, courseId } = (query.meta ?? {}) as ModelStoreMeta; + const { modelType, courseId, models } = (query.meta ?? {}) as ModelStoreMeta; + if (modelType) { store.dispatch(addModel({ modelType, model: { id: courseId, ...(data as Record) } })); } + + models?.forEach(({ modelType: type, strategy, source }) => { + const payload = source ? (data as Record)[source] : data; + switch (strategy) { + case 'addModel': store.dispatch(addModel({ modelType: type, model: payload })); break; + case 'updateModel': store.dispatch(updateModel({ modelType: type, model: payload })); break; + case 'addModelsMap': store.dispatch(addModelsMap({ modelType: type, modelsMap: payload })); break; + case 'updateModelsMap': store.dispatch(updateModelsMap({ modelType: type, modelsMap: payload })); break; + case 'updateModels': store.dispatch(updateModels({ modelType: type, models: payload })); break; + default: break; + } + }); }; From 85553e842a11ebe0cf36ee66a630a38aef59e33d Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 21 Aug 2026 04:06:26 -0400 Subject: [PATCH 2/2] refactor: convert the courseware metadata fetch to React Query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert fetchCourse's metadata/outline/courseHomeMeta fetches to React Query, mirroring results into the model store via the bridge so the existing useModel readers keep working. - Query hooks: useCoursewareMetadata + useCoursewareOutline (courseware/data/apiHooks + queryKeys); reuse useCourseHomeMeta (now typed — enabled guard + courseAccess). courseId is string|undefined from useParams with an `enabled` guard; key factories stay strict string. - Status: CoursewareContainer and CourseExit call transitional bridges (courseware/data/statusBridge) that mirror the combined query state into state.courseware.courseStatus/courseId, so the redirect helpers/selectors, TabPage, and the exit-page children keep working until they move to React Query. - fetchCourse is thinned to just the sidebar-toggles fetch (its full conversion is #2013); its model writes move to the model-store bridge and its status derivation to the status bridges. - Query error logging: the app QueryCache onError logs failures; a query can override the level per HTTP status via meta.logStatusAs (the outline's expected 403 -> logInfo). meta is typed globally via a Register.queryMeta augmentation, so onError and the bridge read it cast-free. - useIFrameBehavior's post-event refetch invalidates the courseware queries instead of dispatching fetchCourse. Co-Authored-By: Claude Opus 4.8 --- .../courseware-search/data/apiHooks.ts | 1 - .../courseware-search/map-search-response.js | 3 +- src/course-home/data/apiHooks.test.tsx | 43 +++++ src/course-home/data/apiHooks.ts | 14 +- src/course-home/data/queryKeys.ts | 2 +- src/course-home/dates-tab/DatesTab.jsx | 2 +- .../discussion-tab/DiscussionTab.jsx | 2 +- src/course-home/live-tab/LiveTab.jsx | 2 +- src/course-home/outline-tab/OutlineTab.jsx | 2 +- src/course-home/progress-tab/ProgressTab.jsx | 2 +- src/courseware/CoursewareContainer.test.jsx | 2 +- src/courseware/CoursewareContainer.tsx | 3 + .../course/course-exit/CourseExit.jsx | 29 ++- .../course/course-exit/CourseExit.test.jsx | 74 +++++++- .../Unit/hooks/useIFrameBehavior.test.js | 22 ++- .../sequence/Unit/hooks/useIFrameBehavior.ts | 17 +- src/courseware/data/apiHooks.test.tsx | 75 ++++++++ src/courseware/data/apiHooks.ts | 25 +++ src/courseware/data/queryKeys.ts | 7 + src/courseware/data/redux.test.js | 170 ++---------------- src/courseware/data/statusBridge.test.ts | 160 +++++++++++++++++ src/courseware/data/statusBridge.ts | 91 ++++++++++ src/courseware/data/thunks.js | 137 ++------------ src/data/http-error.ts | 15 ++ .../data/modelStoreBridge.test.ts | 25 +++ src/data/modelStoreBridge.ts | 6 +- src/generic/CourseAccessErrorPage.jsx | 2 +- src/index.jsx | 6 +- src/product-tours/ProductTours.test.jsx | 24 +-- src/product-tours/data/apiHooks.ts | 1 - src/queryClient.test.ts | 80 ++++++++- src/queryClient.ts | 34 +++- src/setupTest.js | 30 +++- 33 files changed, 764 insertions(+), 344 deletions(-) create mode 100644 src/courseware/data/apiHooks.test.tsx create mode 100644 src/courseware/data/apiHooks.ts create mode 100644 src/courseware/data/queryKeys.ts create mode 100644 src/courseware/data/statusBridge.test.ts create mode 100644 src/courseware/data/statusBridge.ts create mode 100644 src/data/http-error.ts rename src/{course-home => }/data/modelStoreBridge.test.ts (81%) diff --git a/src/course-home/courseware-search/data/apiHooks.ts b/src/course-home/courseware-search/data/apiHooks.ts index 6800d9bce1..ca3cf5d12e 100644 --- a/src/course-home/courseware-search/data/apiHooks.ts +++ b/src/course-home/courseware-search/data/apiHooks.ts @@ -7,7 +7,6 @@ import { coursewareSearchQueryKeys } from './queryKeys'; export const useCoursewareSearchEnabled = (courseId: string) => useQuery({ queryKey: coursewareSearchQueryKeys.enabled(courseId), queryFn: () => getCoursewareSearchEnabled(courseId), - retry: false, }); export const useCoursewareSearchResults = (courseId: string, keyword: string) => useQuery({ diff --git a/src/course-home/courseware-search/map-search-response.js b/src/course-home/courseware-search/map-search-response.js index 49953b3f6b..af2469b123 100644 --- a/src/course-home/courseware-search/map-search-response.js +++ b/src/course-home/courseware-search/map-search-response.js @@ -1,4 +1,5 @@ const Joi = require('joi'); +const { NonRetryableError } = require('../../data/http-error'); const endpointSchema = Joi.object({ took: Joi.number().required(), @@ -24,7 +25,7 @@ export default function mapSearchResponse(response, searchKeywords = '') { const { error, value: data } = endpointSchema.validate(response); if (error) { - throw new Error('Error in server response:', error); + throw new NonRetryableError('Error in server response:', { cause: error }); } const keywords = searchKeywords ? searchKeywords.toLowerCase().split(' ') : []; diff --git a/src/course-home/data/apiHooks.test.tsx b/src/course-home/data/apiHooks.test.tsx index 2e9e3a29da..50611c4c67 100644 --- a/src/course-home/data/apiHooks.test.tsx +++ b/src/course-home/data/apiHooks.test.tsx @@ -8,6 +8,7 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { initializeMockApp } from '../../setupTest'; import { ToastProvider, useToast } from '../../generic/ToastContext'; import { + useCourseHomeMeta, useOutlineTabData, useLiveTabData, useProgressTabData, useResetDeadlines, usePostEvent, useRequestCert, useDismissWelcomeMessage, useSaveWeeklyLearningGoal, } from './apiHooks'; @@ -325,4 +326,46 @@ describe('course-home apiHooks', () => { await waitFor(() => expect(loggingService.logError).toHaveBeenCalled()); }); }); + + describe('useCourseHomeMeta', () => { + const courseId = 'course-1'; + const metadataUrl = new RegExp(`${getConfig().LMS_BASE_URL}/api/course_home/course_metadata/`); + const tabSlugs = (data: unknown) => (data as { tabs: Array<{ slug: string }> }).tabs.map(tab => tab.slug); + + beforeEach(() => { + axiosMock.onGet(metadataUrl).reply(200, Factory.build('courseHomeMetadata')); + }); + + it('labels the shared courseware/outline tab "courseware" (the rootSlug CoursewareContainer + CourseExit pass)', async () => { + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useCourseHomeMeta(courseId, 'courseware'), { wrapper }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(tabSlugs(result.current.data)).toContain('courseware'); + expect(tabSlugs(result.current.data)).not.toContain('outline'); + }); + + it('labels the shared courseware/outline tab "outline" (the rootSlug the course-home tabs pass)', async () => { + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useCourseHomeMeta(courseId, 'outline'), { wrapper }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(tabSlugs(result.current.data)).toContain('outline'); + expect(tabSlugs(result.current.data)).not.toContain('courseware'); + }); + + it('keys the query by rootSlug so the two contexts do not share a cache entry', async () => { + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => ({ + courseware: useCourseHomeMeta(courseId, 'courseware'), + outline: useCourseHomeMeta(courseId, 'outline'), + }), { wrapper }); + + await waitFor(() => expect( + result.current.courseware.isSuccess && result.current.outline.isSuccess, + ).toBe(true)); + expect(tabSlugs(result.current.courseware.data)).toContain('courseware'); + expect(tabSlugs(result.current.outline.data)).toContain('outline'); + }); + }); }); diff --git a/src/course-home/data/apiHooks.ts b/src/course-home/data/apiHooks.ts index e7d38249db..43d1074f4c 100644 --- a/src/course-home/data/apiHooks.ts +++ b/src/course-home/data/apiHooks.ts @@ -1,6 +1,7 @@ import { logError } from '@edx/frontend-platform/logging'; import { useMutation, useQuery } from '@tanstack/react-query'; +import type { RequestError } from '@src/data/http-error'; import { useToast, ToastContent } from '@src/generic/ToastContext'; import { executePostFromPostEvent, @@ -58,9 +59,15 @@ export const usePostEvent = () => { }); }; -export const useCourseHomeMeta = (courseId: string) => useQuery({ - queryKey: courseHomeQueryKeys.metadata(courseId), - queryFn: () => getCourseHomeCourseMetadata(courseId, 'outline'), +// Typed to only what we read off this query, not the whole (untyped) endpoint shape; +// other course-home fields are read via `useModel`/the bridge (until #1977). +export const useCourseHomeMeta = (courseId: string | undefined, rootSlug: string) => useQuery< +{ courseAccess?: { hasAccess: boolean } }, +RequestError +>({ + queryKey: courseHomeQueryKeys.metadata(courseId!, rootSlug), + queryFn: () => getCourseHomeCourseMetadata(courseId, rootSlug), + enabled: !!courseId, meta: { modelType: 'courseHomeMeta', courseId }, }); @@ -79,7 +86,6 @@ export const useOutlineTabData = (courseId: string) => useQuery({ export const useLiveTabData = (courseId: string) => useQuery({ queryKey: courseHomeQueryKeys.liveTab(courseId), queryFn: () => getLiveTabIframe(courseId), - refetchOnWindowFocus: false, }); export const useProgressTabData = (courseId: string, targetUserId?: string) => useQuery({ diff --git a/src/course-home/data/queryKeys.ts b/src/course-home/data/queryKeys.ts index c79083b9fc..b02196229b 100644 --- a/src/course-home/data/queryKeys.ts +++ b/src/course-home/data/queryKeys.ts @@ -2,7 +2,7 @@ import { appId } from '@src/constants'; export const courseHomeQueryKeys = { all: [appId, 'courseHome'] as const, - metadata: (courseId: string) => [...courseHomeQueryKeys.all, 'metadata', courseId] as const, + metadata: (courseId: string, rootSlug: string) => [...courseHomeQueryKeys.all, 'metadata', courseId, rootSlug] as const, datesTab: (courseId: string) => [...courseHomeQueryKeys.all, 'datesTab', courseId] as const, outlineTab: (courseId: string) => [...courseHomeQueryKeys.all, 'outlineTab', courseId] as const, liveTab: (courseId: string) => [...courseHomeQueryKeys.all, 'liveTab', courseId] as const, diff --git a/src/course-home/dates-tab/DatesTab.jsx b/src/course-home/dates-tab/DatesTab.jsx index 85f50688cb..f5c10b0983 100644 --- a/src/course-home/dates-tab/DatesTab.jsx +++ b/src/course-home/dates-tab/DatesTab.jsx @@ -19,7 +19,7 @@ const DatesTab = () => { const intl = useIntl(); const { courseId } = useParams(); - const metadataQuery = useCourseHomeMeta(courseId); + const metadataQuery = useCourseHomeMeta(courseId, 'outline'); const tabDataQuery = useDatesTabData(courseId); const { diff --git a/src/course-home/discussion-tab/DiscussionTab.jsx b/src/course-home/discussion-tab/DiscussionTab.jsx index a3b24c55a7..2e0049fa23 100644 --- a/src/course-home/discussion-tab/DiscussionTab.jsx +++ b/src/course-home/discussion-tab/DiscussionTab.jsx @@ -32,7 +32,7 @@ const DiscussionTabContent = () => { const DiscussionTab = () => { const { courseId } = useParams(); - const metadataQuery = useCourseHomeMeta(courseId); + const metadataQuery = useCourseHomeMeta(courseId, 'outline'); return ( { const { courseId } = useParams(); - const metadataQuery = useCourseHomeMeta(courseId); + const metadataQuery = useCourseHomeMeta(courseId, 'outline'); const tabDataQuery = useLiveTabData(courseId); return ( diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx index fb38ec57e6..1083acb101 100644 --- a/src/course-home/outline-tab/OutlineTab.jsx +++ b/src/course-home/outline-tab/OutlineTab.jsx @@ -192,7 +192,7 @@ const OutlineTabContent = () => { const OutlineTab = () => { const { courseId } = useParams(); - const metadataQuery = useCourseHomeMeta(courseId); + const metadataQuery = useCourseHomeMeta(courseId, 'outline'); const tabDataQuery = useOutlineTabData(courseId); return ( diff --git a/src/course-home/progress-tab/ProgressTab.jsx b/src/course-home/progress-tab/ProgressTab.jsx index 7cfc6961e5..62039d0f26 100644 --- a/src/course-home/progress-tab/ProgressTab.jsx +++ b/src/course-home/progress-tab/ProgressTab.jsx @@ -57,7 +57,7 @@ const ProgressTabContent = () => { const ProgressTab = () => { const { courseId, targetUserId } = useParams(); - const metadataQuery = useCourseHomeMeta(courseId); + const metadataQuery = useCourseHomeMeta(courseId, 'outline'); const tabDataQuery = useProgressTabData(courseId, targetUserId); return ( diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index 815ed898d9..93cc6762a7 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -101,7 +101,7 @@ describe('CoursewareContainer', () => { component = ( - + diff --git a/src/courseware/CoursewareContainer.tsx b/src/courseware/CoursewareContainer.tsx index d6e8877058..bd189dbf52 100644 --- a/src/courseware/CoursewareContainer.tsx +++ b/src/courseware/CoursewareContainer.tsx @@ -12,6 +12,7 @@ import { getSequenceForUnitDeprecated, saveSequencePosition, } from './data'; +import { useCourseStatusBridge } from './data/statusBridge'; import { TabPage } from '../tab-page'; import type { CourseStatus } from '../tab-page/TabPage'; import type { RootState } from '../store'; @@ -234,6 +235,8 @@ const CoursewareContainer = () => { const firstSequenceId = useSelector(firstSequenceIdSelector); const sectionViaSequenceId = useSelector(sectionViaSequenceIdSelector); + useCourseStatusBridge(routeCourseId); + const latest = useRef(); const guards = useRef(); diff --git a/src/courseware/course/course-exit/CourseExit.jsx b/src/courseware/course/course-exit/CourseExit.jsx index c0d296f1d9..dd1337e71e 100644 --- a/src/courseware/course/course-exit/CourseExit.jsx +++ b/src/courseware/course/course-exit/CourseExit.jsx @@ -1,7 +1,6 @@ import { useEffect } from 'react'; -import { useSelector } from 'react-redux'; -import { Navigate } from 'react-router-dom'; +import { Navigate, useParams } from 'react-router-dom'; import CourseCelebration from './CourseCelebration'; import CourseInProgress from './CourseInProgress'; @@ -11,9 +10,13 @@ import { postUnsubscribeFromGoalReminders } from './data/api'; import { CourseExitViewCoursesPluginSlot } from '../../../plugin-slots/CourseExitPluginSlots'; import { useModel } from '../../../generic/model-store'; +import { TabWithTimer } from '../../../tab-page'; +import { useCoursewareMetadata, useCoursewareOutline } from '../../data/apiHooks'; +import { useCourseExitStatusBridge } from '../../data/statusBridge'; +import { useCourseHomeMeta } from '../../../course-home/data/apiHooks'; -const CourseExit = () => { - const { courseId } = useSelector(state => state.courseware); +const CourseExitContent = () => { + const { courseId } = useParams(); const { certificateData, courseExitPageIsActive, @@ -66,4 +69,22 @@ const CourseExit = () => { ); }; +const CourseExit = () => { + const { courseId } = useParams(); + const metadataQuery = useCoursewareMetadata(courseId); + const courseHomeMetaQuery = useCourseHomeMeta(courseId, 'courseware'); + useCoursewareOutline(courseId); + useCourseExitStatusBridge(courseId, metadataQuery, courseHomeMetaQuery); + + return ( + + + + ); +}; + export default CourseExit; diff --git a/src/courseware/course/course-exit/CourseExit.test.jsx b/src/courseware/course/course-exit/CourseExit.test.jsx index f8655fcf9d..51bc15aec8 100644 --- a/src/courseware/course/course-exit/CourseExit.test.jsx +++ b/src/courseware/course/course-exit/CourseExit.test.jsx @@ -1,19 +1,24 @@ import React from 'react'; import MockAdapter from 'axios-mock-adapter'; import { Factory } from 'rosie'; -import { getConfig } from '@edx/frontend-platform'; +import { getConfig, history } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { waitFor } from '@testing-library/react'; +import { waitFor, waitForElementToBeRemoved } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { BrowserRouter, Route, Routes } from 'react-router-dom'; +import { QueryClientProvider } from '@tanstack/react-query'; -import { fetchCourse } from '../../data'; +import { getCourseMetadata } from '../../data/api'; +import { getCourseHomeCourseMetadata } from '../../../course-home/data/api'; +import { fetchCourseSuccess } from '../../data/slice'; +import { addModel } from '../../../generic/model-store'; import { buildSimpleCourseBlocks } from '../../../shared/data/__factories__/courseBlocks.factory'; import { buildOutlineFromBlocks } from '../../data/__factories__/learningSequencesOutline.factory'; import { - initializeMockApp, logUnhandledRequests, render, screen, + createTestQueryClient, initializeMockApp, logUnhandledRequests, render, screen, } from '../../../setupTest'; import initializeStore from '../../../store'; -import { appendBrowserTimezoneToUrl, executeThunk } from '../../../utils'; +import { appendBrowserTimezoneToUrl } from '../../../utils'; import CourseCelebration from './CourseCelebration'; import CourseExit from './CourseExit'; import CourseInProgress from './CourseInProgress'; @@ -51,8 +56,27 @@ describe('Course Exit Pages', () => { } async function fetchAndRender(component) { - await executeThunk(fetchCourse(courseId), store.dispatch); - render(component, { store, wrapWithRouter: true }); + const [metadata, homeMetadata] = await Promise.all([ + getCourseMetadata(courseId), + getCourseHomeCourseMetadata(courseId, 'courseware'), + ]); + store.dispatch(addModel({ modelType: 'coursewareMeta', model: metadata })); + store.dispatch(addModel({ modelType: 'courseHomeMeta', model: { id: courseId, ...homeMetadata } })); + store.dispatch(fetchCourseSuccess({ courseId })); + history.push(`/course/${courseId}`); + render( + + + + + + + , + { store, wrapWithRouter: false }, + ); + if (screen.queryByRole('status')) { + await waitForElementToBeRemoved(() => screen.queryByRole('status')); + } } beforeEach(() => { @@ -99,6 +123,24 @@ describe('Course Exit Pages', () => { expect(screen.getByText('You’ve reached the end of the course!')).toBeInTheDocument(); }); + it('Routes to in-progress experience when the course has scheduled content', async () => { + setMetadata({ + enrollment: { is_active: true }, + user_has_passing_grade: false, + }); + const { courseBlocks } = buildSimpleCourseBlocks(courseId, courseHomeMetadata.title); + // buildOutlineFromBlocks releases every sequence; mark one unreleased so the normalized + // outline reports hasScheduledContent (see isReleased in ../../data/utils.js). + const outline = buildOutlineFromBlocks(courseBlocks); + const [scheduledSequenceId] = Object.keys(outline.outline.sequences); + outline.outline.sequences[scheduledSequenceId].accessible = false; + outline.outline.sequences[scheduledSequenceId].effective_start = tomorrow.toISOString(); + axiosMock.onGet(learningSequencesUrlRegExp).reply(200, outline); + + await fetchAndRender(); + expect(await screen.findByText('More content is coming soon!')).toBeInTheDocument(); + }); + it('Redirects if it does not match any statuses', async () => { setMetadata({ certificate_data: { @@ -110,6 +152,24 @@ describe('Course Exit Pages', () => { }); }); + describe('Course Exit access error', () => { + it('surfaces the 403 access detail instead of the generic error', async () => { + axiosMock.onGet(courseHomeMetadataUrl).reply(403, { detail: 'You are not enrolled', error_code: 'not_enrolled' }); + history.push(`/course/${courseId}`); + render( + + + + } /> + + + , + { store, wrapWithRouter: false }, + ); + expect(await screen.findByText('You are not enrolled')).toBeInTheDocument(); + }); + }); + describe('Course Celebration Experience', () => { it('Displays webview link', async () => { setMetadata({ diff --git a/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js b/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js index cdc1b12302..4f9d28257c 100644 --- a/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js +++ b/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js @@ -1,11 +1,11 @@ -import { useDispatch } from 'react-redux'; import { renderHook } from '@testing-library/react'; import { logError } from '@edx/frontend-platform/logging'; import { getConfig } from '@edx/frontend-platform'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; -import { fetchCourse } from '@src/courseware/data'; +import { coursewareQueryKeys } from '@src/courseware/data/queryKeys'; +import { courseHomeQueryKeys } from '@src/course-home/data/queryKeys'; import { useEventListener } from '@src/generic/hooks'; import { useSequenceNavigationMetadata } from '@src/courseware/course/sequence/sequence-navigation/hooks'; @@ -15,6 +15,7 @@ import useIFrameBehavior, { iframeBehaviorState } from './useIFrameBehavior'; const mockNavigate = jest.fn(); const mockMutate = jest.fn(); +const mockInvalidateQueries = jest.fn(); jest.mock('@edx/frontend-platform', () => ({ ...jest.requireActual('@edx/frontend-platform'), @@ -29,7 +30,6 @@ jest.mock('react', () => ({ })); jest.mock('react-redux', () => ({ - useDispatch: jest.fn(), useSelector: jest.fn(), })); @@ -37,8 +37,9 @@ jest.mock('@edx/frontend-platform/logging', () => ({ logError: jest.fn(), })); -jest.mock('@src/courseware/data', () => ({ - fetchCourse: jest.fn(), +jest.mock('@tanstack/react-query', () => ({ + ...jest.requireActual('@tanstack/react-query'), + useQueryClient: () => ({ invalidateQueries: mockInvalidateQueries }), })); jest.mock('@src/course-home/data/thunks', () => ({ eventTypes: { POST_EVENT: 'post_event' }, @@ -72,9 +73,6 @@ const testIFrameHeight = 42; const config = { LMS_BASE_URL: 'test-base-url' }; getConfig.mockReturnValue(config); -const dispatch = jest.fn(); -useDispatch.mockReturnValue(dispatch); - const postMessage = jest.fn(); const frame = { contentWindow: { postMessage }, @@ -351,9 +349,8 @@ describe('useIFrameBehavior hook', () => { result.current.handleIFrameLoad(); expect(sendTrackEvent).not.toHaveBeenCalled(); }); - it('registers an event handler to process fetchCourse events.', () => { + it('invalidates the courseware queries on a post event.', () => { mockState(defaultStateVals); - fetchCourse.mockReturnValue('fetch-course-action'); const { result } = renderHook(() => useIFrameBehavior(props)); result.current.handleIFrameLoad(); const event = { @@ -375,8 +372,9 @@ describe('useIFrameBehavior hook', () => { const { onSuccess } = mockMutate.mock.calls[0][1]; onSuccess(); - expect(fetchCourse).toHaveBeenCalledWith('course-1'); - expect(dispatch).toHaveBeenCalledWith('fetch-course-action'); + expect(mockInvalidateQueries).toHaveBeenCalledWith({ queryKey: coursewareQueryKeys.metadata('course-1') }); + expect(mockInvalidateQueries).toHaveBeenCalledWith({ queryKey: coursewareQueryKeys.outline('course-1') }); + expect(mockInvalidateQueries).toHaveBeenCalledWith({ queryKey: courseHomeQueryKeys.metadata('course-1', 'courseware') }); }); it('updates initial iframe visibility on load', () => { const { result } = renderHook(() => useIFrameBehavior(props)); diff --git a/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.ts b/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.ts index 89b66810ac..4385ed2159 100644 --- a/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.ts +++ b/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.ts @@ -1,14 +1,16 @@ import React, { useState } from 'react'; import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; -import { useDispatch, useSelector } from 'react-redux'; +import { useSelector } from 'react-redux'; import { useNavigate } from 'react-router-dom'; +import { useQueryClient } from '@tanstack/react-query'; import { throttle } from 'lodash'; import { logError } from '@edx/frontend-platform/logging'; -import { fetchCourse } from '@src/courseware/data'; import { usePostEvent } from '@src/course-home/data/apiHooks'; +import { courseHomeQueryKeys } from '@src/course-home/data/queryKeys'; +import { coursewareQueryKeys } from '@src/courseware/data/queryKeys'; import { eventTypes } from '@src/course-home/data/thunks'; import { useEventListener } from '@src/generic/hooks'; import { getSequenceId } from '@src/courseware/data/selectors'; @@ -34,7 +36,7 @@ const useIFrameBehavior = ({ // Do not remove this hook. See function description. useLoadBearingHook(id); - const dispatch = useDispatch(); + const queryClient = useQueryClient(); const postEvent = usePostEvent(); const activeSequenceId = useSelector(getSequenceId); const navigate = useNavigate(); @@ -164,7 +166,14 @@ const useIFrameBehavior = ({ } postEvent.mutate( { postData: event.postData, researchEventData }, - { onSuccess: () => dispatch(fetchCourse(event.postData.bodyParams.courseId)) }, + { + onSuccess: () => { + const eventCourseId = event.postData.bodyParams.courseId; + queryClient.invalidateQueries({ queryKey: coursewareQueryKeys.metadata(eventCourseId) }); + queryClient.invalidateQueries({ queryKey: coursewareQueryKeys.outline(eventCourseId) }); + queryClient.invalidateQueries({ queryKey: courseHomeQueryKeys.metadata(eventCourseId, 'courseware') }); + }, + }, ); }; diff --git a/src/courseware/data/apiHooks.test.tsx b/src/courseware/data/apiHooks.test.tsx new file mode 100644 index 0000000000..7cecd8f3e2 --- /dev/null +++ b/src/courseware/data/apiHooks.test.tsx @@ -0,0 +1,75 @@ +import type { ReactNode } from 'react'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClientProvider } from '@tanstack/react-query'; +import { Factory } from 'rosie'; +import MockAdapter from 'axios-mock-adapter'; +import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; + +import { appendBrowserTimezoneToUrl } from '../../utils'; +import { buildSimpleCourseBlocks } from '../../shared/data/__factories__/courseBlocks.factory'; +import { buildOutlineFromBlocks } from './__factories__/learningSequencesOutline.factory'; +import { createTestQueryClient, initializeMockApp } from '../../setupTest'; +import initializeStore from '../../store'; +import { normalizeLearningSequencesData } from './utils'; +import { fetchCourseSuccess } from './slice'; +import { sequenceIdsSelector } from './selectors'; +import { useCoursewareMetadata, useCoursewareOutline } from './apiHooks'; + +initializeMockApp(); + +describe('courseware apiHooks — coursewareMeta bridge', () => { + const courseMetadata = Factory.build('courseMetadata'); + const courseId = courseMetadata.id; + const { courseBlocks } = buildSimpleCourseBlocks(courseId); + const outlineResponse = buildOutlineFromBlocks(courseBlocks); + const normalizedOutline = normalizeLearningSequencesData(outlineResponse); + const expectedSectionIds = normalizedOutline.courses[courseId].sectionIds; + const expectedSequenceIds = expectedSectionIds.flatMap( + (id: string) => normalizedOutline.sections[id].sequenceIds, + ); + + let axiosMock: MockAdapter; + let store: ReturnType; + const outlineUrl = `${getConfig().LMS_BASE_URL}/api/learning_sequences/v1/course_outline/${courseId}`; + const metadataUrl = appendBrowserTimezoneToUrl(`${getConfig().LMS_BASE_URL}/api/courseware/course/${courseId}`); + + const coursewareMetaFor = (id: string) => ( + store.getState().models as { coursewareMeta?: Record } + ).coursewareMeta?.[id]; + + beforeEach(() => { + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + store = initializeStore(); + }); + + it('keeps coursewareMeta.sectionIds (and the sequence order nav needs) when metadata resolves after the outline', async () => { + let resolveMetadata: () => void = () => {}; + axiosMock.onGet(outlineUrl).reply(200, outlineResponse); + axiosMock.onGet(metadataUrl).reply(() => new Promise((resolve) => { + resolveMetadata = () => resolve([200, courseMetadata]); + })); + + const queryClient = createTestQueryClient(store); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + renderHook( + () => ({ meta: useCoursewareMetadata(courseId), outline: useCoursewareOutline(courseId) }), + { wrapper }, + ); + + // The outline resolves first and populates sectionIds. + await waitFor(() => expect(coursewareMetaFor(courseId)?.sectionIds).toEqual(expectedSectionIds)); + store.dispatch(fetchCourseSuccess({ courseId })); + expect(sequenceIdsSelector(store.getState())).toEqual(expectedSequenceIds); + + // Now let the metadata mirror land last. + resolveMetadata(); + await waitFor(() => expect(coursewareMetaFor(courseId)?.title).toBe(courseMetadata.name)); + + // sectionIds must survive. + expect(coursewareMetaFor(courseId)?.sectionIds).toEqual(expectedSectionIds); + expect(sequenceIdsSelector(store.getState())).toEqual(expectedSequenceIds); + }); +}); diff --git a/src/courseware/data/apiHooks.ts b/src/courseware/data/apiHooks.ts new file mode 100644 index 0000000000..8c5156c332 --- /dev/null +++ b/src/courseware/data/apiHooks.ts @@ -0,0 +1,25 @@ +import { useQuery } from '@tanstack/react-query'; + +import { getCourseMetadata, getLearningSequencesOutline } from './api'; +import { coursewareQueryKeys } from './queryKeys'; + +export const useCoursewareMetadata = (courseId: string | undefined) => useQuery({ + queryKey: coursewareQueryKeys.metadata(courseId!), + queryFn: () => getCourseMetadata(courseId), + enabled: !!courseId, + meta: { models: [{ modelType: 'coursewareMeta', strategy: 'updateModel' }] }, +}); + +export const useCoursewareOutline = (courseId: string | undefined) => useQuery({ + queryKey: coursewareQueryKeys.outline(courseId!), + queryFn: () => getLearningSequencesOutline(courseId), + enabled: !!courseId, + meta: { + logStatusAs: { 403: 'info' }, + models: [ + { modelType: 'coursewareMeta', strategy: 'updateModelsMap', source: 'courses' }, + { modelType: 'sections', strategy: 'addModelsMap', source: 'sections' }, + { modelType: 'sequences', strategy: 'updateModelsMap', source: 'sequences' }, + ], + }, +}); diff --git a/src/courseware/data/queryKeys.ts b/src/courseware/data/queryKeys.ts new file mode 100644 index 0000000000..e3bc7814ce --- /dev/null +++ b/src/courseware/data/queryKeys.ts @@ -0,0 +1,7 @@ +import { appId } from '@src/constants'; + +export const coursewareQueryKeys = { + all: [appId, 'courseware'] as const, + metadata: (courseId: string) => [...coursewareQueryKeys.all, 'metadata', courseId] as const, + outline: (courseId: string) => [...coursewareQueryKeys.all, 'outline', courseId] as const, +}; diff --git a/src/courseware/data/redux.test.js b/src/courseware/data/redux.test.js index 2dc89bb696..739b5611b3 100644 --- a/src/courseware/data/redux.test.js +++ b/src/courseware/data/redux.test.js @@ -4,14 +4,15 @@ import MockAdapter from 'axios-mock-adapter'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { getConfig } from '@edx/frontend-platform'; -import { FAILED, LOADING } from '@src/constants'; import * as thunks from './thunks'; import { appendBrowserTimezoneToUrl, executeThunk } from '../../utils'; import { buildSimpleCourseBlocks } from '../../shared/data/__factories__/courseBlocks.factory'; import { buildOutlineFromBlocks } from './__factories__/learningSequencesOutline.factory'; -import { initializeMockApp } from '../../setupTest'; +import { initializeMockApp, seedCoursewareModels } from '../../setupTest'; +import { getCourseMetadata } from './api'; +import { addModel } from '../../generic/model-store'; import initializeStore from '../../store'; const { loggingService } = initializeMockApp(); @@ -33,7 +34,6 @@ describe('Data layer integration tests', () => { {}, { courseId, unitBlocks, sequenceBlock: sequenceBlocks[0] }, ); - const simpleOutline = buildOutlineFromBlocks(courseBlocks); let courseUrl = `${courseBaseUrl}/${courseId}`; courseUrl = appendBrowserTimezoneToUrl(courseUrl); @@ -44,7 +44,6 @@ describe('Data layer integration tests', () => { const sequenceUrl = `${sequenceBaseUrl}/${sequenceMetadata.item_id}`; const sequenceId = sequenceBlocks[0].id; const unitId = unitBlocks[0].id; - const coursewareSidebarSettingsUrl = `${getConfig().LMS_BASE_URL}/courses/${courseId}/courseware-navigation-sidebar/toggles/`; let store; @@ -56,165 +55,25 @@ describe('Data layer integration tests', () => { }); describe('Test fetchCourse', () => { - it('Should fail to fetch course and blocks if request error happens', async () => { - axiosMock.onGet(courseUrl).networkError(); - axiosMock.onGet(learningSequencesUrlRegExp).networkError(); - axiosMock.onGet(coursewareSidebarSettingsUrl).networkError(); + const sidebarTogglesUrl = `${getConfig().LMS_BASE_URL}/courses/${courseId}/courseware-navigation-sidebar/toggles/`; - await executeThunk(thunks.fetchCourse(courseId), store.dispatch); - - expect(loggingService.logError).toHaveBeenCalled(); - expect(store.getState().courseware).toEqual(expect.objectContaining({ - courseId, - courseOutline: {}, - courseStatus: FAILED, - coursewareOutlineSidebarSettings: {}, - courseOutlineStatus: LOADING, - sequenceId: null, - sequenceMightBeUnit: false, - sequenceStatus: LOADING, - })); - }); - - it('should store errorMessage and errorCode when course_home metadata returns 403', async () => { - const errorDetail = 'This course is not currently accessible. The course team has restricted access to this content.'; - const errorCode = 'not_visible_in_catalog'; - - axiosMock.onGet(courseUrl).reply(200, courseMetadata); - axiosMock.onGet(courseHomeMetadataUrl).reply(403, { detail: errorDetail, error_code: errorCode }); - axiosMock.onGet(learningSequencesUrlRegExp).reply(200, buildOutlineFromBlocks(courseBlocks)); - axiosMock.onGet(coursewareSidebarSettingsUrl).reply(200, { enable_completion_tracking: true }); - - await executeThunk(thunks.fetchCourse(courseId), store.dispatch); - - const { courseware } = store.getState(); - expect(courseware.courseStatus).toEqual(FAILED); - expect(courseware.errorMessage).toEqual(errorDetail); - expect(courseware.errorCode).toEqual(errorCode); - }); - - it('should not store errorMessage for non-403 network errors', async () => { - axiosMock.onGet(courseUrl).networkError(); - axiosMock.onGet(courseHomeMetadataUrl).networkError(); - axiosMock.onGet(learningSequencesUrlRegExp).networkError(); - axiosMock.onGet(coursewareSidebarSettingsUrl).networkError(); - - await executeThunk(thunks.fetchCourse(courseId), store.dispatch); - - const { courseware } = store.getState(); - expect(courseware.courseStatus).toEqual(FAILED); - expect(courseware.errorMessage).toBeNull(); - expect(courseware.errorCode).toBeNull(); - }); - - it('Should fetch, normalize, and save metadata, but with denied status', async () => { - const forbiddenCourseMetadata = Factory.build('courseMetadata'); - const forbiddenCourseHomeMetadata = Factory.build('courseHomeMetadata', { - course_access: { - has_access: false, - }, - }); - const forbiddenCourseHomeUrl = appendBrowserTimezoneToUrl( - `${getConfig().LMS_BASE_URL}/api/course_home/course_metadata/${courseId}`, - ); - const forbiddenCourseBlocks = Factory.build('courseBlocks', { - courseId: forbiddenCourseMetadata.id, - }); - let forbiddenCourseUrl = `${courseBaseUrl}/${forbiddenCourseMetadata.id}`; - forbiddenCourseUrl = appendBrowserTimezoneToUrl(forbiddenCourseUrl); - - axiosMock.onGet(forbiddenCourseHomeUrl).reply(200, forbiddenCourseHomeMetadata); - axiosMock.onGet(forbiddenCourseUrl).reply(200, forbiddenCourseMetadata); - axiosMock.onGet(learningSequencesUrlRegExp).reply(200, buildOutlineFromBlocks(forbiddenCourseBlocks)); - - await executeThunk(thunks.fetchCourse(forbiddenCourseMetadata.id), store.dispatch); - - const state = store.getState(); - - expect(state.courseware.courseStatus).toEqual('denied'); - - // check that at least one key camel cased, thus course data normalized - expect(state.models.courseHomeMeta[forbiddenCourseMetadata.id].courseAccess).not.toBeUndefined(); - }); - - it('Should fetch, normalize, and save metadata', async () => { - axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata); - axiosMock.onGet(courseUrl).reply(200, courseMetadata); - axiosMock.onGet(learningSequencesUrlRegExp).reply(200, buildOutlineFromBlocks(courseBlocks)); - axiosMock.onGet(coursewareSidebarSettingsUrl).reply(200, { - enable_completion_tracking: true, - }); + it('Should store the sidebar toggles on success', async () => { + axiosMock.onGet(sidebarTogglesUrl).reply(200, { enable_completion_tracking: true }); await executeThunk(thunks.fetchCourse(courseId), store.dispatch); - const state = store.getState(); - - expect(state.courseware.courseStatus).toEqual('loaded'); - expect(state.courseware.courseId).toEqual(courseId); - expect(state.courseware.sequenceStatus).toEqual('loading'); - expect(state.courseware.sequenceId).toEqual(null); - expect(state.courseware.coursewareOutlineSidebarSettings).toEqual({ + expect(store.getState().courseware.coursewareOutlineSidebarSettings).toEqual({ enableCompletionTracking: true, }); - - // check that at least one key camel cased, thus course data normalized - expect(state.models.coursewareMeta[courseId].marketingUrl).not.toBeUndefined(); }); - it('Should fetch, normalize, and save metadata; filtering has no effect', async () => { - // Very similar to previous test, but pass back an outline for filtering - // (even though it won't actually filter down in this case). - axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata); - axiosMock.onGet(courseUrl).reply(200, courseMetadata); - axiosMock.onGet(learningSequencesUrlRegExp).reply(200, simpleOutline); - axiosMock.onGet(coursewareSidebarSettingsUrl).reply(200, { - enable_completion_tracking: false, - }); + it('Should log an error and leave the toggles unset on failure', async () => { + axiosMock.onGet(sidebarTogglesUrl).networkError(); await executeThunk(thunks.fetchCourse(courseId), store.dispatch); - const state = store.getState(); - - expect(state.courseware.courseStatus).toEqual('loaded'); - expect(state.courseware.courseId).toEqual(courseId); - expect(state.courseware.sequenceStatus).toEqual('loading'); - expect(state.courseware.sequenceId).toEqual(null); - expect(state.courseware.coursewareOutlineSidebarSettings).toEqual({ - enableCompletionTracking: false, - }); - - // check that at least one key camel cased, thus course data normalized - expect(state.models.coursewareMeta[courseId].marketingUrl).not.toBeUndefined(); - expect(state.models.sequences.length === 1); - - Object.values(state.models.sections).forEach(section => expect(section.sequenceIds.length === 1)); - }); - - it('Should fetch, normalize, and save metadata; filtering removes sequence', async () => { - // Very similar to previous test, but pass back an outline for filtering - // (even though it won't actually filter down in this case). - axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata); - axiosMock.onGet(courseUrl).reply(200, courseMetadata); - - // Create an outline with basic matching metadata, but then empty it out... - const emptyOutline = buildOutlineFromBlocks(courseBlocks); - emptyOutline.sequences = {}; - emptyOutline.sections = []; - axiosMock.onGet(learningSequencesUrlRegExp).reply(200, emptyOutline); - await executeThunk(thunks.fetchCourse(courseId), store.dispatch); - - const state = store.getState(); - - expect(state.courseware.courseStatus).toEqual('loaded'); - expect(state.courseware.courseId).toEqual(courseId); - expect(state.courseware.sequenceStatus).toEqual('loading'); - expect(state.courseware.sequenceId).toEqual(null); - - // check that at least one key camel cased, thus course data normalized - expect(state.models.coursewareMeta[courseId].marketingUrl).not.toBeUndefined(); - expect(state.models.sequences === null); - - Object.values(state.models.sections).forEach(section => expect(section.sequenceIds.length === 0)); + expect(loggingService.logError).toHaveBeenCalled(); + expect(store.getState().courseware.coursewareOutlineSidebarSettings).toEqual({}); }); }); @@ -252,7 +111,7 @@ describe('Data layer integration tests', () => { // setting course with blocks before sequence to check that blocks receive // additional information after fetchSequence call. - await executeThunk(thunks.fetchCourse(courseId), store.dispatch); + await seedCoursewareModels(store, courseId); // ensure that initial state has no additional sequence info let state = store.getState(); @@ -425,7 +284,10 @@ describe('Data layer integration tests', () => { axiosMock.onGet(courseUrlNeedSignature).reply(200, courseMetadataNeedSignature); - await executeThunk(thunks.fetchCourse(courseMetadataNeedSignature.id), store.dispatch); + store.dispatch(addModel({ + modelType: 'coursewareMeta', + model: await getCourseMetadata(courseMetadataNeedSignature.id), + })); expect( store.getState().models.coursewareMeta[courseMetadataNeedSignature.id].userNeedsIntegritySignature, ).toEqual(true); diff --git a/src/courseware/data/statusBridge.test.ts b/src/courseware/data/statusBridge.test.ts new file mode 100644 index 0000000000..673dd8a17c --- /dev/null +++ b/src/courseware/data/statusBridge.test.ts @@ -0,0 +1,160 @@ +import { renderHook } from '@testing-library/react'; + +import { useCourseHomeMeta } from '@src/course-home/data/apiHooks'; +import { useCoursewareMetadata, useCoursewareOutline } from './apiHooks'; +import { + fetchCourseDenied, + fetchCourseFailure, + fetchCourseRequest, + fetchCourseSuccess, +} from './slice'; +import { useCourseExitStatusBridge, useCourseStatusBridge } from './statusBridge'; + +const mockDispatch = jest.fn(); +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useDispatch: () => mockDispatch, +})); +jest.mock('./apiHooks'); +jest.mock('@src/course-home/data/apiHooks'); + +const courseId = 'course-v1:edX+Demo+2020'; + +type MetaQuery = ReturnType; +type OutlineQuery = ReturnType; +type HomeMetaQuery = ReturnType; + +const pending = { isPending: true, isSuccess: false, isError: false }; +const success = (data?: unknown) => ({ + isPending: false, isSuccess: true, isError: false, data, +}); +const errored = (error?: unknown) => ({ + isPending: false, isSuccess: false, isError: true, error, +}); +const access = success({ courseAccess: { hasAccess: true } }); +const noAccess = success({ courseAccess: { hasAccess: false } }); + +describe('useCourseStatusBridge', () => { + const render = (metadata: object, outline: object, courseHomeMeta: object, id: string | undefined = courseId) => { + jest.mocked(useCoursewareMetadata).mockReturnValue(metadata as MetaQuery); + jest.mocked(useCoursewareOutline).mockReturnValue(outline as OutlineQuery); + jest.mocked(useCourseHomeMeta).mockReturnValue(courseHomeMeta as HomeMetaQuery); + renderHook(() => useCourseStatusBridge(id)); + }; + + beforeEach(() => { mockDispatch.mockClear(); }); + + it('dispatches nothing without a courseId', () => { + render(success(), success(), access, ''); + expect(mockDispatch).not.toHaveBeenCalled(); + }); + + it('requests while any query is pending', () => { + render(pending, success(), access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseRequest({ courseId })); + }); + + it('succeeds when the learner has access and the outline loaded', () => { + render(success(), success(), access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseSuccess({ courseId })); + }); + + it('denies when the learner lacks access', () => { + render(success(), success(), noAccess); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseDenied({ courseId })); + }); + + it('fails with the 403 detail/code when a query errors', () => { + const error = { response: { status: 403, data: { detail: 'No access', error_code: 'course_access_redirect' } } }; + render(success(), success(), errored(error)); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ + courseId, + errorMessage: 'No access', + errorCode: 'course_access_redirect', + })); + }); + + it('fails without detail/code for a non-403 error', () => { + render(errored({ response: { status: 500 } }), success(), access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ courseId })); + }); + + it('fails without detail/code for a 403 with no body', () => { + render(success(), success(), errored({ response: { status: 403 } })); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ courseId })); + }); + + it('does not re-dispatch on re-render when the query state is unchanged', () => { + jest.mocked(useCoursewareMetadata).mockImplementation(() => success() as MetaQuery); + jest.mocked(useCoursewareOutline).mockImplementation(() => success() as OutlineQuery); + jest.mocked(useCourseHomeMeta).mockImplementation(() => access as HomeMetaQuery); + + const { rerender } = renderHook(() => useCourseStatusBridge(courseId)); + expect(mockDispatch).toHaveBeenCalledTimes(1); + + mockDispatch.mockClear(); + rerender(); + expect(mockDispatch).not.toHaveBeenCalled(); + }); +}); + +describe('useCourseExitStatusBridge', () => { + const render = (metadata: object, courseHomeMeta: object, id: string | undefined = courseId) => { + renderHook(() => useCourseExitStatusBridge(id, metadata as MetaQuery, courseHomeMeta as HomeMetaQuery)); + }; + + beforeEach(() => { mockDispatch.mockClear(); }); + + it('dispatches nothing without a courseId', () => { + render(success(), access, ''); + expect(mockDispatch).not.toHaveBeenCalled(); + }); + + it('requests while a query is pending', () => { + render(pending, access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseRequest({ courseId })); + }); + + it('succeeds when the learner has access', () => { + render(success(), access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseSuccess({ courseId })); + }); + + it('denies when the learner lacks access', () => { + render(success(), noAccess); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseDenied({ courseId })); + }); + + it('fails with the 403 detail/code when a query errors', () => { + const error = { response: { status: 403, data: { detail: 'No access', error_code: 'course_access_redirect' } } }; + render(success(), errored(error)); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ + courseId, + errorMessage: 'No access', + errorCode: 'course_access_redirect', + })); + }); + + it('fails without detail/code for a non-403 error', () => { + render(errored({ response: { status: 500 } }), access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ courseId })); + }); + + it('fails without detail/code for a 403 with no body', () => { + render(success(), errored({ response: { status: 403 } })); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ courseId })); + }); + + it('does not re-dispatch on re-render when the query state is unchanged', () => { + const { rerender } = renderHook(() => useCourseExitStatusBridge( + courseId, + success() as MetaQuery, + success({ courseAccess: { hasAccess: true } }) as HomeMetaQuery, + )); + expect(mockDispatch).toHaveBeenCalledTimes(1); + + mockDispatch.mockClear(); + rerender(); + expect(mockDispatch).not.toHaveBeenCalled(); + }); +}); diff --git a/src/courseware/data/statusBridge.ts b/src/courseware/data/statusBridge.ts new file mode 100644 index 0000000000..34228cf537 --- /dev/null +++ b/src/courseware/data/statusBridge.ts @@ -0,0 +1,91 @@ +import { useEffect } from 'react'; +import { useDispatch } from 'react-redux'; + +import { useCourseHomeMeta } from '@src/course-home/data/apiHooks'; +import { useCoursewareMetadata, useCoursewareOutline } from './apiHooks'; +import { + fetchCourseDenied, + fetchCourseFailure, + fetchCourseRequest, + fetchCourseSuccess, +} from './slice'; + +// Transitional: bridges the courseware query state into the Redux `courseStatus` field so +// the still-Redux readers (the container's redirect helpers/selectors and TabPage's string +// status) keep working; removed when those readers move to React Query. +export const useCourseStatusBridge = (courseId: string | undefined) => { + const dispatch = useDispatch(); + const metadataQuery = useCoursewareMetadata(courseId); + const outlineQuery = useCoursewareOutline(courseId); + const courseHomeMetaQuery = useCourseHomeMeta(courseId, 'courseware'); + + const { isPending: metadataPending, isSuccess: metadataSuccess } = metadataQuery; + const { isPending: outlinePending, isSuccess: outlineSuccess } = outlineQuery; + const { isPending: courseHomePending, isSuccess: courseHomeSuccess, error: courseHomeError } = courseHomeMetaQuery; + const hasAccess = courseHomeMetaQuery.data?.courseAccess?.hasAccess; + + useEffect(() => { + if (!courseId) { + return; + } + if (metadataPending || outlinePending || courseHomePending) { + dispatch(fetchCourseRequest({ courseId })); + return; + } + if (metadataSuccess && courseHomeSuccess) { + if (hasAccess && outlineSuccess) { + dispatch(fetchCourseSuccess({ courseId })); + } else { + dispatch(fetchCourseDenied({ courseId })); + } + return; + } + const { status, data } = courseHomeError?.response ?? {}; + if (status === 403 && data) { + dispatch(fetchCourseFailure({ courseId, errorMessage: data.detail, errorCode: data.error_code })); + } else { + dispatch(fetchCourseFailure({ courseId })); + } + }, [courseId, metadataPending, outlinePending, courseHomePending, + metadataSuccess, courseHomeSuccess, outlineSuccess, hasAccess, courseHomeError, dispatch]); +}; + +// The CourseExit variant: no outline query, and it takes its queries as params because +// CourseExit also feeds them to its own TabPage gating. Same transitional job — keeps the slice +// `courseId`/`courseStatus` written for the exit page's still-Redux children. +export const useCourseExitStatusBridge = ( + courseId: string | undefined, + metadataQuery: ReturnType, + courseHomeMetaQuery: ReturnType, +) => { + const dispatch = useDispatch(); + + const { isPending: metadataPending, isSuccess: metadataSuccess } = metadataQuery; + const { isPending: courseHomePending, isSuccess: courseHomeSuccess, error: courseHomeError } = courseHomeMetaQuery; + const hasAccess = courseHomeMetaQuery.data?.courseAccess?.hasAccess; + + useEffect(() => { + if (!courseId) { + return; + } + if (metadataPending || courseHomePending) { + dispatch(fetchCourseRequest({ courseId })); + return; + } + if (metadataSuccess && courseHomeSuccess) { + if (hasAccess) { + dispatch(fetchCourseSuccess({ courseId })); + } else { + dispatch(fetchCourseDenied({ courseId })); + } + return; + } + const { status, data } = courseHomeError?.response ?? {}; + if (status === 403 && data) { + dispatch(fetchCourseFailure({ courseId, errorMessage: data.detail, errorCode: data.error_code })); + } else { + dispatch(fetchCourseFailure({ courseId })); + } + }, [courseId, metadataPending, courseHomePending, metadataSuccess, courseHomeSuccess, + hasAccess, courseHomeError, dispatch]); +}; diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 165f2a4a80..b829630198 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -1,25 +1,16 @@ -import { logError, logInfo } from '@edx/frontend-platform/logging'; -import { getCourseHomeCourseMetadata } from '../../course-home/data/api'; -import { - addModel, addModelsMap, updateModel, updateModels, updateModelsMap, -} from '../../generic/model-store'; +import { logError } from '@edx/frontend-platform/logging'; +import { updateModel, updateModels } from '../../generic/model-store'; import { getBlockCompletion, getCourseDiscussionConfig, - getCourseMetadata, getCourseOutline, getCourseTopics, getCoursewareOutlineSidebarToggles, - getLearningSequencesOutline, getSequenceMetadata, postIntegritySignature, postSequencePosition, } from './api'; import { - fetchCourseDenied, - fetchCourseFailure, - fetchCourseRequest, - fetchCourseSuccess, fetchSequenceFailure, fetchSequenceRequest, fetchSequenceSuccess, @@ -30,117 +21,23 @@ import { updateCourseOutlineCompletion, } from './slice'; +// Transitional — `fetchCourse` is being dismantled; its work is moving to React Query. +// What it used to do, and what replaced it: +// - metadata / outline / courseHomeMeta fetches → the `useCoursewareMetadata` / +// `useCoursewareOutline` / `useCourseHomeMeta` query hooks (+ the model-store bridge) +// - deriving/dispatching `courseStatus` → `useCourseStatusBridge` +// Only the sidebar-toggles fetch is left; it stays on Redux until #2013 converts it and +// deletes `fetchCourse`. export function fetchCourse(courseId) { return async (dispatch) => { - dispatch(fetchCourseRequest({ courseId })); - Promise.allSettled([ - getCourseMetadata(courseId), - getLearningSequencesOutline(courseId), - getCourseHomeCourseMetadata(courseId, 'courseware'), - getCoursewareOutlineSidebarToggles(courseId), - ]).then(([ - courseMetadataResult, - learningSequencesOutlineResult, - courseHomeMetadataResult, - coursewareOutlineSidebarTogglesResult]) => { - const fetchedMetadata = courseMetadataResult.status === 'fulfilled'; - const fetchedCourseHomeMetadata = courseHomeMetadataResult.status === 'fulfilled'; - const fetchedOutline = learningSequencesOutlineResult.status === 'fulfilled'; - const fetchedCoursewareOutlineSidebarTogglesResult = coursewareOutlineSidebarTogglesResult.status === 'fulfilled'; - - if (fetchedMetadata) { - dispatch(addModel({ - modelType: 'coursewareMeta', - model: courseMetadataResult.value, - })); - } - - if (fetchedCourseHomeMetadata) { - dispatch(addModel({ - modelType: 'courseHomeMeta', - model: { - id: courseId, - ...courseHomeMetadataResult.value, - }, - })); - } - - if (fetchedOutline) { - const { - courses, sections, sequences, - } = learningSequencesOutlineResult.value; - - // This updates the course with a sectionIds array from the Learning Sequence data. - dispatch(updateModelsMap({ - modelType: 'coursewareMeta', - modelsMap: courses, - })); - dispatch(addModelsMap({ - modelType: 'sections', - modelsMap: sections, - })); - // We update for sequences because the sequence metadata may have come back first. - dispatch(updateModelsMap({ - modelType: 'sequences', - modelsMap: sequences, - })); - } - - if (fetchedCoursewareOutlineSidebarTogglesResult) { - const { - enable_completion_tracking: enableCompletionTracking, - } = coursewareOutlineSidebarTogglesResult.value; - dispatch(setCoursewareOutlineSidebarToggles( - { enableCompletionTracking }, - )); - } - - // Log errors for each request if needed. Outline failures may occur - // even if the course metadata request is successful - if (!fetchedOutline) { - const { response } = learningSequencesOutlineResult.reason; - if (response && response.status === 403) { - // 403 responses are normal - they happen when the learner is logged out. - // We'll redirect them in a moment to the outline tab by calling fetchCourseDenied() below. - logInfo(learningSequencesOutlineResult.reason); - } else { - logError(learningSequencesOutlineResult.reason); - } - } - if (!fetchedMetadata) { - logError(courseMetadataResult.reason); - } - if (!fetchedCourseHomeMetadata) { - logError(courseHomeMetadataResult.reason); - } - if (!fetchedCoursewareOutlineSidebarTogglesResult) { - logError(coursewareOutlineSidebarTogglesResult.reason); - } - if (fetchedMetadata && fetchedCourseHomeMetadata) { - if (courseHomeMetadataResult.value.courseAccess.hasAccess && fetchedOutline) { - // User has access - dispatch(fetchCourseSuccess({ courseId })); - return; - } - // User either doesn't have access or only has partial access - // (can't access course blocks) - dispatch(fetchCourseDenied({ courseId })); - return; - } - - // Definitely an error happening - // Extract error details from 403 responses - let errorMessage = null; - let errorCode = null; - if (!fetchedCourseHomeMetadata) { - const error = courseHomeMetadataResult.reason; - if (error?.response?.status === 403 && error?.response?.data) { - errorMessage = error.response.data.detail || null; - errorCode = error.response.data.error_code || null; - } - } - dispatch(fetchCourseFailure({ courseId, errorMessage, errorCode })); - }); + try { + const { + enable_completion_tracking: enableCompletionTracking, + } = await getCoursewareOutlineSidebarToggles(courseId); + dispatch(setCoursewareOutlineSidebarToggles({ enableCompletionTracking })); + } catch (error) { + logError(error); + } }; } diff --git a/src/data/http-error.ts b/src/data/http-error.ts new file mode 100644 index 0000000000..3147133316 --- /dev/null +++ b/src/data/http-error.ts @@ -0,0 +1,15 @@ +export interface RequestError { + response?: { status?: number; data?: { detail?: string; error_code?: string } }; +} + +export const getResponseStatus = (error: unknown): number | undefined => ( + (error as RequestError | null)?.response?.status +); + +export class NonRetryableError extends Error { + nonRetryable = true; +} + +export const isNonRetryable = (error: unknown): boolean => ( + (error as NonRetryableError | null)?.nonRetryable === true +); diff --git a/src/course-home/data/modelStoreBridge.test.ts b/src/data/modelStoreBridge.test.ts similarity index 81% rename from src/course-home/data/modelStoreBridge.test.ts rename to src/data/modelStoreBridge.test.ts index 6decb8fce3..ba36489e0f 100644 --- a/src/course-home/data/modelStoreBridge.test.ts +++ b/src/data/modelStoreBridge.test.ts @@ -94,6 +94,31 @@ describe('modelStoreBridge', () => { expect(units.u2).toEqual({ id: 'u2', complete: false }); }); + it('updateModel merges a single model by id (via source)', async () => { + store.dispatch(addModelsMap({ + modelType: 'coursewareMeta', + modelsMap: { 'course-1': { id: 'course-1', title: 'Old', tabs: ['outline'] } }, + })); + + await runQuery( + queryClient, + () => ({ course: { id: 'course-1', title: 'New' } }), + { models: [{ modelType: 'coursewareMeta', strategy: 'updateModel', source: 'course' }] }, + ); + + expect(modelsOf(store).coursewareMeta['course-1']).toEqual({ id: 'course-1', title: 'New', tabs: ['outline'] }); + }); + + it('ignores an unrecognized strategy', async () => { + await runQuery( + queryClient, + () => ({ id: 'course-1' }), + { models: [{ modelType: 'coursewareMeta', strategy: 'nope' }] }, + ); + + expect(store.getState().models).toEqual({}); + }); + it('does nothing when a query has no model-store meta', async () => { await runQuery(queryClient, () => ({ foo: 'bar' }), {}); diff --git a/src/data/modelStoreBridge.ts b/src/data/modelStoreBridge.ts index f1f7c3eaa9..f52b6bf136 100644 --- a/src/data/modelStoreBridge.ts +++ b/src/data/modelStoreBridge.ts @@ -22,11 +22,11 @@ interface ModelMirror { source?: string; } -interface ModelStoreMeta { +export type ModelStoreMeta = { modelType?: string; courseId?: string; models?: ModelMirror[]; -} +}; // Transitional (#1977): bridge a React Query result into the model store so existing // `useModel(...)` readers (the shared TabPage/LoadedTabPage and not-yet-converted tabs) @@ -37,7 +37,7 @@ interface ModelStoreMeta { // This is wired as the app QueryCache's `onSuccess` (see src/queryClient.ts), so it runs // before observers re-render. export const bridgeToModelStore = (store: Store, data: unknown, query: Query) => { - const { modelType, courseId, models } = (query.meta ?? {}) as ModelStoreMeta; + const { modelType, courseId, models } = query.meta ?? {}; if (modelType) { store.dispatch(addModel({ modelType, model: { id: courseId, ...(data as Record) } })); diff --git a/src/generic/CourseAccessErrorPage.jsx b/src/generic/CourseAccessErrorPage.jsx index 11fc3642f0..ddb2afc9bd 100644 --- a/src/generic/CourseAccessErrorPage.jsx +++ b/src/generic/CourseAccessErrorPage.jsx @@ -14,7 +14,7 @@ const CourseAccessErrorPage = () => { const { courseId } = useParams(); const activeEnterpriseAlert = useActiveEnterpriseAlert(courseId); - const metadataQuery = useCourseHomeMeta(courseId); + const metadataQuery = useCourseHomeMeta(courseId, 'outline'); if (metadataQuery.isPending) { return ( diff --git a/src/index.jsx b/src/index.jsx index 3a7b6221d7..e73043499b 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -23,9 +23,7 @@ import CoursewareRedirectLandingPage from './courseware/CoursewareRedirectLandin import DatesTab from './course-home/dates-tab'; import GoalUnsubscribe from './course-home/goal-unsubscribe'; import ProgressTab from './course-home/progress-tab/ProgressTab'; -import { TabContainer } from './tab-page'; -import { fetchCourse } from './courseware/data'; import { store } from './store'; import { createQueryClient } from './queryClient'; import NoticesProvider from './generic/notices'; @@ -116,9 +114,7 @@ subscribe(APP_READY, () => { path={DECODE_ROUTES.COURSE_END} element={( - - - + )} /> diff --git a/src/product-tours/ProductTours.test.jsx b/src/product-tours/ProductTours.test.jsx index 8024512916..4e78d980d1 100644 --- a/src/product-tours/ProductTours.test.jsx +++ b/src/product-tours/ProductTours.test.jsx @@ -272,17 +272,19 @@ describe('Courseware Tour', () => { component = ( - - - {DECODE_ROUTES.COURSEWARE.map((route) => ( - } - /> - ))} - - + + + + {DECODE_ROUTES.COURSEWARE.map((route) => ( + } + /> + ))} + + + ); }); diff --git a/src/product-tours/data/apiHooks.ts b/src/product-tours/data/apiHooks.ts index 463551a9e1..931268cfdf 100644 --- a/src/product-tours/data/apiHooks.ts +++ b/src/product-tours/data/apiHooks.ts @@ -8,7 +8,6 @@ export const useTourData = (username: string, enabled: boolean) => useQuery({ queryKey: tourQueryKeys.user(username), queryFn: () => getTourData(username), enabled, - refetchOnWindowFocus: false, }); export const useEndCourseHomeTour = () => { diff --git a/src/queryClient.test.ts b/src/queryClient.test.ts index 757da0bbbc..cf09c7b1e8 100644 --- a/src/queryClient.test.ts +++ b/src/queryClient.test.ts @@ -2,7 +2,8 @@ import { QueryClient } from '@tanstack/react-query'; import { configureStore } from '@reduxjs/toolkit'; import { reducer as modelsReducer } from './generic/model-store'; -import { createAppQueryCache } from './queryClient'; +import { createAppQueryCache, createQueryClient, shouldRetryQuery } from './queryClient'; +import { NonRetryableError } from './data/http-error'; import { initializeMockApp } from './setupTest'; const { loggingService } = initializeMockApp(); @@ -20,6 +21,7 @@ describe('app query cache', () => { queryCache: createAppQueryCache(store), }); loggingService.logError.mockReset(); + loggingService.logInfo.mockReset(); }); it('reports query errors through onError', async () => { @@ -32,6 +34,18 @@ describe('app query cache', () => { expect(loggingService.logError).toHaveBeenCalledWith(error, undefined); }); + it('logs a status listed in `logStatusAs` at that level instead of as an error', async () => { + const error = { response: { status: 403 } }; + await queryClient.fetchQuery({ + queryKey: ['forbidden'], + queryFn: () => Promise.reject(error), + meta: { logStatusAs: { 403: 'info' } }, + }).catch(() => {}); + + expect(loggingService.logInfo).toHaveBeenCalledWith(error, undefined); + expect(loggingService.logError).not.toHaveBeenCalled(); + }); + it('bridges successful results into the model store through onSuccess', async () => { await queryClient.fetchQuery({ queryKey: ['ok'], @@ -43,3 +57,67 @@ describe('app query cache', () => { expect(models.widget['course-1']).toEqual({ id: 'course-1', value: 42 }); }); }); + +describe('shouldRetryQuery', () => { + const httpError = (status: number) => ({ response: { status } }); + + it.each([400, 401, 403, 404, 422])('does not retry client error %s', (status) => { + expect(shouldRetryQuery(0, httpError(status))).toBe(false); + }); + + it('does not retry a NonRetryableError (our own deterministic throw)', () => { + expect(shouldRetryQuery(0, new NonRetryableError('bad response'))).toBe(false); + }); + + it.each([500, 502, 503])('retries server error %s until 3 attempts', (status) => { + expect(shouldRetryQuery(0, httpError(status))).toBe(true); + expect(shouldRetryQuery(2, httpError(status))).toBe(true); + expect(shouldRetryQuery(3, httpError(status))).toBe(false); + }); + + it('retries network (undefined-status) errors until 3 attempts', () => { + const networkError = new Error('Network Error'); + expect(shouldRetryQuery(0, networkError)).toBe(true); + expect(shouldRetryQuery(2, networkError)).toBe(true); + expect(shouldRetryQuery(3, networkError)).toBe(false); + }); + + it('is wired in as the default query retry policy on createQueryClient', () => { + const store = configureStore({ reducer: { models: modelsReducer } }); + expect(createQueryClient(store).getDefaultOptions().queries?.retry).toBe(shouldRetryQuery); + }); +}); + +describe('createQueryClient retry behavior (integration)', () => { + const attemptsFor = async (error: unknown): Promise => { + const store = configureStore({ reducer: { models: modelsReducer } }); + const queryFn = jest.fn().mockRejectedValue(error); + await createQueryClient(store).fetchQuery({ + queryKey: ['retry-test'], queryFn, retryDelay: 0, + }).catch(() => {}); + return queryFn.mock.calls.length; + }; + + it('does not retry a 4xx (1 attempt)', async () => { + expect(await attemptsFor({ response: { status: 403 } })).toBe(1); + }); + + it('does not retry a NonRetryableError (1 attempt)', async () => { + expect(await attemptsFor(new NonRetryableError('bad response'))).toBe(1); + }); + + it('retries a 5xx server error (4 attempts: 1 + 3 retries)', async () => { + expect(await attemptsFor({ response: { status: 500 } })).toBe(4); + }); + + it('retries a network error (4 attempts: 1 + 3 retries)', async () => { + expect(await attemptsFor(new Error('network'))).toBe(4); + }); +}); + +describe('createQueryClient defaults', () => { + it('disables refetchOnWindowFocus (parity with the pre-RQ no-focus-refetch behavior)', () => { + const store = configureStore({ reducer: { models: modelsReducer } }); + expect(createQueryClient(store).getDefaultOptions().queries?.refetchOnWindowFocus).toBe(false); + }); +}); diff --git a/src/queryClient.ts b/src/queryClient.ts index 7c9a4eacab..a5e13eecb8 100644 --- a/src/queryClient.ts +++ b/src/queryClient.ts @@ -1,16 +1,44 @@ -import { logError } from '@edx/frontend-platform/logging'; +import { logError, logInfo } from '@edx/frontend-platform/logging'; import { QueryCache, QueryClient } from '@tanstack/react-query'; import { Store } from 'redux'; -import { bridgeToModelStore } from './data/modelStoreBridge'; +import { bridgeToModelStore, type ModelStoreMeta } from './data/modelStoreBridge'; +import { getResponseStatus, isNonRetryable } from './data/http-error'; + +const loggers = { error: logError, info: logInfo }; +export type LogLevel = keyof typeof loggers; + +declare module '@tanstack/react-query' { + interface Register { + queryMeta: ModelStoreMeta & { logStatusAs?: Record }; + } +} // `onSuccess` bridges results into the model store (transitional, #1977); the `store` param // exists only to feed it and goes away when the bridge is removed. export const createAppQueryCache = (store: Store) => new QueryCache({ onSuccess: (data, query) => bridgeToModelStore(store, data, query), - onError: (error) => logError(error), + onError: (error, query) => { + const status = getResponseStatus(error); + const level = (status !== undefined && query.meta?.logStatusAs?.[status]) || 'error'; + loggers[level](error); + }, }); +export const shouldRetryQuery = (failureCount: number, error: unknown): boolean => { + if (isNonRetryable(error)) { + return false; + } + const status = getResponseStatus(error); + if (status !== undefined && status >= 400 && status < 500) { + return false; + } + return failureCount < 3; +}; + export const createQueryClient = (store: Store) => new QueryClient({ queryCache: createAppQueryCache(store), + defaultOptions: { + queries: { retry: shouldRetryQuery, refetchOnWindowFocus: false }, + }, }); diff --git a/src/setupTest.js b/src/setupTest.js index fa950ea380..156343060c 100755 --- a/src/setupTest.js +++ b/src/setupTest.js @@ -15,14 +15,18 @@ import { reducer as specialExamsReducer } from '@edx/frontend-lib-special-exams' import { AppProvider } from '@edx/frontend-platform/react'; import { reducer as courseHomeReducer } from './course-home/data'; import { createAppQueryCache } from './queryClient'; -import { reducer as coursewareReducer } from './courseware/data/slice'; -import { reducer as modelsReducer } from './generic/model-store'; +import { reducer as coursewareReducer, fetchCourseSuccess } from './courseware/data/slice'; +import { + reducer as modelsReducer, addModel, addModelsMap, updateModelsMap, +} from './generic/model-store'; import { UserMessagesProvider } from './generic/user-messages'; import { ToastProvider } from './generic/ToastContext'; import messages from './i18n'; import { fetchCourse, fetchSequence } from './courseware/data'; import { getCourseOutlineStructure } from './courseware/data/thunks'; +import { getCourseMetadata, getLearningSequencesOutline } from './courseware/data/api'; +import { getCourseHomeCourseMetadata } from './course-home/data/api'; import { appendBrowserTimezoneToUrl, executeThunk } from './utils'; import buildSimpleCourseAndSequenceMetadata from './courseware/data/__factories__/sequenceMetadata.factory'; import { buildOutlineFromBlocks } from './courseware/data/__factories__/learningSequencesOutline.factory'; @@ -171,6 +175,21 @@ export function logUnhandledRequests(axiosMock) { let globalStore; +export async function seedCoursewareModels(store, courseId) { + const [metadata, outline, homeMetadata] = await Promise.all([ + getCourseMetadata(courseId), + getLearningSequencesOutline(courseId), + getCourseHomeCourseMetadata(courseId, 'courseware'), + ]); + store.dispatch(addModel({ modelType: 'coursewareMeta', model: metadata })); + store.dispatch(addModel({ modelType: 'courseHomeMeta', model: { id: courseId, ...homeMetadata } })); + store.dispatch(updateModelsMap({ modelType: 'coursewareMeta', modelsMap: outline.courses })); + store.dispatch(addModelsMap({ modelType: 'sections', modelsMap: outline.sections })); + store.dispatch(updateModelsMap({ modelType: 'sequences', modelsMap: outline.sequences })); + store.dispatch(fetchCourseSuccess({ courseId })); + await executeThunk(fetchCourse(courseId), store.dispatch); +} + export async function initializeTestStore(options = {}, overrideStore = true) { const store = configureStore({ reducer: { @@ -227,8 +246,9 @@ export async function initializeTestStore(options = {}, overrideStore = true) { logUnhandledRequests(axiosMock); - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - !options.excludeFetchCourse && await executeThunk(fetchCourse(courseMetadata.id), store.dispatch); + if (!options.excludeFetchCourse) { + await seedCoursewareModels(store, courseMetadata.id); + } // eslint-disable-next-line @typescript-eslint/no-unused-expressions !options.excludeFetchOutlineSidebar && await executeThunk( @@ -247,7 +267,7 @@ export async function initializeTestStore(options = {}, overrideStore = true) { export function createTestQueryClient(store) { return new QueryClient({ defaultOptions: { - queries: { retry: false }, + queries: { retry: false, refetchOnWindowFocus: false }, mutations: { retry: false }, }, ...(store ? { queryCache: createAppQueryCache(store) } : {}),