From dbbefe6843b77b8243ff0f05bdadab649d296f3a 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 332f52eae9f8f8b01e1f2efe6f641f57a6787542 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 --- src/course-home/data/apiHooks.ts | 11 +- src/courseware/CoursewareContainer.test.jsx | 2 +- src/courseware/CoursewareContainer.tsx | 3 + .../course/course-exit/CourseExit.jsx | 28 ++- .../course/course-exit/CourseExit.test.jsx | 33 +++- .../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 | 177 +----------------- src/courseware/data/statusBridge.test.ts | 113 +++++++++++ src/courseware/data/statusBridge.ts | 73 ++++++++ src/courseware/data/thunks.js | 137 ++------------ src/data/http-error.ts | 7 + .../data/modelStoreBridge.test.ts | 25 +++ src/data/modelStoreBridge.ts | 6 +- src/index.jsx | 6 +- src/product-tours/ProductTours.test.jsx | 24 +-- src/queryClient.test.ts | 13 ++ src/queryClient.ts | 20 +- src/setupTest.js | 28 ++- 22 files changed, 508 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/data/apiHooks.ts b/src/course-home/data/apiHooks.ts index e7d38249db..7fb4eb2d60 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), +// 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) => useQuery< +{ courseAccess?: { hasAccess: boolean } }, +RequestError +>({ + queryKey: courseHomeQueryKeys.metadata(courseId!), queryFn: () => getCourseHomeCourseMetadata(courseId, 'outline'), + enabled: !!courseId, meta: { modelType: 'courseHomeMeta', courseId }, }); 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..3d2d483421 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 { TabPage } from '../../../tab-page'; +import { useCoursewareMetadata } 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,21 @@ const CourseExit = () => { ); }; +const CourseExit = () => { + const { courseId } = useParams(); + const metadataQuery = useCoursewareMetadata(courseId); + const courseHomeMetaQuery = useCourseHomeMeta(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..5b19da142d 100644 --- a/src/courseware/course/course-exit/CourseExit.test.jsx +++ b/src/courseware/course/course-exit/CourseExit.test.jsx @@ -1,19 +1,23 @@ 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 { 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, } 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 +55,25 @@ 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(() => { diff --git a/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js b/src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js index cdc1b12302..1772c11add 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') }); }); 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..82e4e6af42 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) }); + }, + }, ); }; 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..ac434e10fe 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; @@ -55,169 +54,6 @@ describe('Data layer integration tests', () => { store = initializeStore(); }); - 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(); - - 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, - }); - - 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: 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, - }); - - 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)); - }); - }); - describe('Test fetchSequence', () => { it('Should result in fetch failure if error occurs', async () => { axiosMock.onGet(sequenceUrl).networkError(); @@ -252,7 +88,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 +261,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..cd391b88f1 --- /dev/null +++ b/src/courseware/data/statusBridge.test.ts @@ -0,0 +1,113 @@ +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 with null detail/code for a non-403 error', () => { + render(errored({ response: { status: 500 } }), success(), access); + expect(mockDispatch).toHaveBeenCalledWith(fetchCourseFailure({ + courseId, + errorMessage: null, + errorCode: null, + })); + }); +}); + +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 })); + }); +}); diff --git a/src/courseware/data/statusBridge.ts b/src/courseware/data/statusBridge.ts new file mode 100644 index 0000000000..791e9cebb1 --- /dev/null +++ b/src/courseware/data/statusBridge.ts @@ -0,0 +1,73 @@ +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); + + useEffect(() => { + if (!courseId) { + return; + } + if (metadataQuery.isPending || outlineQuery.isPending || courseHomeMetaQuery.isPending) { + dispatch(fetchCourseRequest({ courseId })); + return; + } + if (metadataQuery.isSuccess && courseHomeMetaQuery.isSuccess) { + const { hasAccess } = courseHomeMetaQuery.data?.courseAccess ?? {}; + if (hasAccess && outlineQuery.isSuccess) { + dispatch(fetchCourseSuccess({ courseId })); + } else { + dispatch(fetchCourseDenied({ courseId })); + } + return; + } + const { error } = courseHomeMetaQuery; + const is403 = error?.response?.status === 403; + dispatch(fetchCourseFailure({ + courseId, + errorMessage: is403 ? (error?.response?.data?.detail ?? null) : null, + errorCode: is403 ? (error?.response?.data?.error_code ?? null) : null, + })); + }, [courseId, metadataQuery, outlineQuery, courseHomeMetaQuery, 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(); + + useEffect(() => { + if (!courseId) { + return; + } + if (metadataQuery.isPending || courseHomeMetaQuery.isPending) { + dispatch(fetchCourseRequest({ courseId })); + return; + } + if (metadataQuery.isSuccess && courseHomeMetaQuery.data?.courseAccess?.hasAccess) { + dispatch(fetchCourseSuccess({ courseId })); + } else { + dispatch(fetchCourseDenied({ courseId })); + } + }, [courseId, metadataQuery, courseHomeMetaQuery, 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..3332bc2a74 --- /dev/null +++ b/src/data/http-error.ts @@ -0,0 +1,7 @@ +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 +); 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/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/queryClient.test.ts b/src/queryClient.test.ts index 757da0bbbc..80f088f4d7 100644 --- a/src/queryClient.test.ts +++ b/src/queryClient.test.ts @@ -20,6 +20,7 @@ describe('app query cache', () => { queryCache: createAppQueryCache(store), }); loggingService.logError.mockReset(); + loggingService.logInfo.mockReset(); }); it('reports query errors through onError', async () => { @@ -32,6 +33,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'], diff --git a/src/queryClient.ts b/src/queryClient.ts index 7c9a4eacab..0ed221e458 100644 --- a/src/queryClient.ts +++ b/src/queryClient.ts @@ -1,14 +1,28 @@ -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 } 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 createQueryClient = (store: Store) => new QueryClient({ diff --git a/src/setupTest.js b/src/setupTest.js index fa950ea380..ec6af6ec9e 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(