From 1e11b0453030c771c1f9244209ecfde616c544bc Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 13 Aug 2026 20:58:25 -0400 Subject: [PATCH] refactor: convert outline tab from Redux to React Query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make OutlineTab self-wrapping — it owns useCourseHomeMeta + a new useOutlineTabData query and renders TabWithTimer itself, mirroring the dates-tab conversion. The heavy body moves into an OutlineTabContent component so the useModel('outline') reads never run against the empty model during the loading phase. Subtree courseId reads move from the courseHome slice (directly, or via useContextId in section-outline) to useParams, since the outline route no longer runs a fetch thunk to populate state.courseHome.courseId. ShiftDatesAlert now invalidates both datesTab and outlineTab and drops its fetch/useDispatch. The outline-only POST writers dismissWelcomeMessage and saveWeeklyLearningGoal become useDismissWelcomeMessage / useSaveWeeklyLearningGoal mutations, and the fetchOutlineTab thunk is deleted. proctoringPanelStatus stays in Redux (client UI state). Co-Authored-By: Claude Opus 4.8 --- src/course-home/data/apiHooks.test.tsx | 82 ++++++++++++++++++- src/course-home/data/apiHooks.ts | 27 +++++- src/course-home/data/index.js | 2 - src/course-home/data/queryKeys.ts | 1 + src/course-home/data/redux.test.js | 51 ------------ src/course-home/data/thunks.js | 15 ---- src/course-home/outline-tab/DateSummary.jsx | 6 +- src/course-home/outline-tab/OutlineTab.jsx | 31 +++++-- .../outline-tab/OutlineTab.test.jsx | 77 +++++++++++------ .../outline-tab/section-outline/Section.tsx | 4 +- .../section-outline/SequenceDueDate.tsx | 4 +- .../section-outline/SequenceTitle.tsx | 5 +- .../outline-tab/widgets/CourseDates.jsx | 6 +- .../outline-tab/widgets/CourseHandouts.jsx | 6 +- .../outline-tab/widgets/CourseTools.jsx | 6 +- .../widgets/ProctoringInfoPanel.jsx | 7 +- .../widgets/StartOrResumeCourseCard.jsx | 6 +- .../widgets/WeeklyLearningGoalCard.jsx | 16 ++-- .../outline-tab/widgets/WelcomeMessage.jsx | 7 +- .../ShiftDatesAlert.jsx | 13 +-- src/index.jsx | 6 +- src/product-tours/ProductTours.test.jsx | 32 ++++---- src/tab-page/TabPage.test.jsx | 16 ++++ src/tab-page/TabPage.tsx | 2 +- 24 files changed, 249 insertions(+), 179 deletions(-) diff --git a/src/course-home/data/apiHooks.test.tsx b/src/course-home/data/apiHooks.test.tsx index 1278d74fb9..3a157dd53d 100644 --- a/src/course-home/data/apiHooks.test.tsx +++ b/src/course-home/data/apiHooks.test.tsx @@ -6,7 +6,10 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { initializeMockApp } from '../../setupTest'; import { ToastProvider, useToast } from '../../generic/ToastContext'; -import { useResetDeadlines, usePostEvent, useRequestCert } from './apiHooks'; +import { + useOutlineTabData, useResetDeadlines, usePostEvent, useRequestCert, useDismissWelcomeMessage, + useSaveWeeklyLearningGoal, +} from './apiHooks'; const { loggingService } = initializeMockApp(); @@ -142,4 +145,81 @@ describe('course-home apiHooks', () => { await waitFor(() => expect(loggingService.logError).toHaveBeenCalled()); }); }); + + describe('useDismissWelcomeMessage', () => { + const dismissUrl = `${getConfig().LMS_BASE_URL}/api/course_home/dismiss_welcome_message`; + + it('POSTs to the dismiss url', async () => { + axiosMock.onPost(dismissUrl).reply(201); + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useDismissWelcomeMessage(), { wrapper }); + + await act(async () => { await result.current.mutateAsync({ courseId: 'course-1' }); }); + + expect(axiosMock.history.post[0].url).toEqual(dismissUrl); + }); + + it('logs the error when the POST fails', async () => { + axiosMock.onPost(dismissUrl).reply(500); + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useDismissWelcomeMessage(), { wrapper }); + + await act(async () => { + await result.current.mutateAsync({ courseId: 'course-1' }).catch(() => {}); + }); + + await waitFor(() => expect(loggingService.logError).toHaveBeenCalled()); + }); + }); + + describe('useOutlineTabData', () => { + const outlineUrl = `${getConfig().LMS_BASE_URL}/api/course_home/outline/course-1`; + + it('resolves to an empty object on a 403 (access is handled via the metadata request)', async () => { + axiosMock.onGet(outlineUrl).reply(403, {}); + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useOutlineTabData('course-1'), { wrapper }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(result.current.data).toEqual({}); + }); + + it('surfaces the error on a non-403 failure', async () => { + axiosMock.onGet(outlineUrl).reply(500); + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useOutlineTabData('course-1'), { wrapper }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + }); + }); + + describe('useSaveWeeklyLearningGoal', () => { + const goalUrl = `${getConfig().LMS_BASE_URL}/api/course_home/save_course_goal`; + + it('POSTs the weekly learning goal', async () => { + axiosMock.onPost(goalUrl).reply(200, {}); + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useSaveWeeklyLearningGoal(), { wrapper }); + + await act(async () => { + await result.current.mutateAsync({ courseId: 'course-1', daysPerWeek: 3, subscribedToReminders: true }); + }); + + expect(axiosMock.history.post[0].url).toEqual(goalUrl); + }); + + it('logs the error when the POST fails', async () => { + axiosMock.onPost(goalUrl).reply(500); + const { wrapper } = buildWrapper(); + const { result } = renderHook(() => useSaveWeeklyLearningGoal(), { wrapper }); + + await act(async () => { + await result.current.mutateAsync( + { courseId: 'course-1', daysPerWeek: 3, subscribedToReminders: true }, + ).catch(() => {}); + }); + + await waitFor(() => expect(loggingService.logError).toHaveBeenCalled()); + }); + }); }); diff --git a/src/course-home/data/apiHooks.ts b/src/course-home/data/apiHooks.ts index f5d4004ecd..ada7375297 100644 --- a/src/course-home/data/apiHooks.ts +++ b/src/course-home/data/apiHooks.ts @@ -3,7 +3,14 @@ import { useMutation, useQuery } from '@tanstack/react-query'; import { useToast, ToastContent } from '@src/generic/ToastContext'; import { - executePostFromPostEvent, getCourseHomeCourseMetadata, getDatesTabData, postCourseDeadlines, postRequestCert, + executePostFromPostEvent, + getCourseHomeCourseMetadata, + getDatesTabData, + getOutlineTabData, + postCourseDeadlines, + postDismissWelcomeMessage, + postRequestCert, + postWeeklyLearningGoal, } from './api'; import { courseHomeQueryKeys } from './queryKeys'; @@ -61,7 +68,25 @@ export const useDatesTabData = (courseId: string) => useQuery({ meta: { modelType: 'dates', courseId }, }); +export const useOutlineTabData = (courseId: string) => useQuery({ + queryKey: courseHomeQueryKeys.outlineTab(courseId), + queryFn: () => getOutlineTabData(courseId), + meta: { modelType: 'outline', courseId }, +}); + export const useRequestCert = () => useMutation({ mutationFn: ({ courseId }: { courseId: string }) => postRequestCert(courseId), onError: (error) => logError(error), }); + +export const useDismissWelcomeMessage = () => useMutation({ + mutationFn: ({ courseId }: { courseId: string }) => postDismissWelcomeMessage(courseId), + onError: (error) => logError(error), +}); + +export const useSaveWeeklyLearningGoal = () => useMutation({ + mutationFn: ({ courseId, daysPerWeek, subscribedToReminders }: { + courseId: string; daysPerWeek: number; subscribedToReminders: boolean; + }) => postWeeklyLearningGoal(courseId, daysPerWeek, subscribedToReminders), + onError: (error) => logError(error), +}); diff --git a/src/course-home/data/index.js b/src/course-home/data/index.js index 7ac41d169c..14b7826f01 100644 --- a/src/course-home/data/index.js +++ b/src/course-home/data/index.js @@ -1,8 +1,6 @@ export { - fetchOutlineTab, fetchProgressTab, deprecatedSaveCourseGoal, - saveWeeklyLearningGoal, } from './thunks'; export { reducer } from './slice'; diff --git a/src/course-home/data/queryKeys.ts b/src/course-home/data/queryKeys.ts index f8f97e53d3..15dfcd794e 100644 --- a/src/course-home/data/queryKeys.ts +++ b/src/course-home/data/queryKeys.ts @@ -4,4 +4,5 @@ export const courseHomeQueryKeys = { all: [appId, 'courseHome'] as const, metadata: (courseId: string) => [...courseHomeQueryKeys.all, 'metadata', courseId] as const, datesTab: (courseId: string) => [...courseHomeQueryKeys.all, 'datesTab', courseId] as const, + outlineTab: (courseId: string) => [...courseHomeQueryKeys.all, 'outlineTab', courseId] as const, }; diff --git a/src/course-home/data/redux.test.js b/src/course-home/data/redux.test.js index 1743b3e517..453faf9f02 100644 --- a/src/course-home/data/redux.test.js +++ b/src/course-home/data/redux.test.js @@ -42,45 +42,6 @@ describe('Data layer integration tests', () => { store = initializeStore(); }); - describe('Test fetchOutlineTab', () => { - const outlineBaseUrl = `${getConfig().LMS_BASE_URL}/api/course_home/outline`; - const outlineUrl = `${outlineBaseUrl}/${courseId}`; - - it('Should result in fetch failure if error occurs', async () => { - axiosMock.onGet(courseMetadataUrl).networkError(); - axiosMock.onGet(outlineUrl).networkError(); - - await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - - expect(loggingService.logError).toHaveBeenCalled(); - expect(store.getState().courseHome.courseStatus).toEqual('failed'); - }); - - it('Should fetch, normalize, and save metadata', async () => { - const outlineTabData = Factory.build('outlineTabData', { courseId }); - - axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeMetadata); - axiosMock.onGet(outlineUrl).reply(200, outlineTabData); - - await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - - const state = store.getState(); - expect(state.courseHome.courseStatus).toEqual('loaded'); - }); - - it.each([401, 403, 404])( - 'should result in fetch denied if course access is denied, regardless of outline API status', - async (errorStatus) => { - axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeAccessDeniedMetadata); - axiosMock.onGet(outlineUrl).reply(errorStatus, {}); - - await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - - expect(store.getState().courseHome.courseStatus).toEqual('denied'); - }, - ); - }); - describe('Test fetchProgressTab', () => { const progressBaseUrl = `${getConfig().LMS_BASE_URL}/api/course_home/progress`; @@ -148,18 +109,6 @@ describe('Data layer integration tests', () => { }); }); - describe('Test dismissWelcomeMessage', () => { - it('Should dismiss welcome message', async () => { - const dismissUrl = `${getConfig().LMS_BASE_URL}/api/course_home/dismiss_welcome_message`; - axiosMock.onPost(dismissUrl).reply(201); - - await executeThunk(thunks.dismissWelcomeMessage(courseId), store.dispatch); - - expect(axiosMock.history.post[0].url).toEqual(dismissUrl); - expect(axiosMock.history.post[0].data).toEqual(`{"course_id":"${courseId}"}`); - }); - }); - describe('Test fetchExamAttemptsData', () => { const sequenceIds = [ 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@12345', diff --git a/src/course-home/data/thunks.js b/src/course-home/data/thunks.js index 300d2bac51..dc1a37c446 100644 --- a/src/course-home/data/thunks.js +++ b/src/course-home/data/thunks.js @@ -2,11 +2,8 @@ import { logError } from '@edx/frontend-platform/logging'; import { getCourseHomeCourseMetadata, getExamsData, - getOutlineTabData, getProgressTabData, deprecatedPostCourseGoals, - postWeeklyLearningGoal, - postDismissWelcomeMessage, getLiveTabIframe, } from './api'; @@ -88,10 +85,6 @@ export function fetchProgressTab(courseId, targetUserId) { return fetchTab(courseId, 'progress', getProgressTabData, parseInt(targetUserId, 10) || targetUserId); } -export function fetchOutlineTab(courseId) { - return fetchTab(courseId, 'outline', getOutlineTabData); -} - export function fetchLiveTab(courseId) { return fetchTab(courseId, 'live', getLiveTabIframe); } @@ -100,18 +93,10 @@ export function fetchDiscussionTab(courseId) { return fetchTab(courseId, 'discussion'); } -export function dismissWelcomeMessage(courseId) { - return async () => postDismissWelcomeMessage(courseId); -} - export async function deprecatedSaveCourseGoal(courseId, goalKey) { return deprecatedPostCourseGoals(courseId, goalKey); } -export async function saveWeeklyLearningGoal(courseId, daysPerWeek, subscribedToReminders) { - return postWeeklyLearningGoal(courseId, daysPerWeek, subscribedToReminders); -} - export function fetchExamAttemptsData(courseId, sequenceIds) { return async (dispatch) => { const results = await Promise.all(sequenceIds.map(async (sequenceId) => { diff --git a/src/course-home/outline-tab/DateSummary.jsx b/src/course-home/outline-tab/DateSummary.jsx index d1e5cbc7d8..f634fbbe08 100644 --- a/src/course-home/outline-tab/DateSummary.jsx +++ b/src/course-home/outline-tab/DateSummary.jsx @@ -3,7 +3,7 @@ import { faCalendarAlt } from '@fortawesome/free-regular-svg-icons'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { FormattedDate } from '@edx/frontend-platform/i18n'; import React from 'react'; -import { useSelector } from 'react-redux'; +import { useParams } from 'react-router-dom'; import PropTypes from 'prop-types'; import { useModel } from '../../generic/model-store'; import { isLearnerAssignment } from '../dates-tab/utils'; @@ -13,9 +13,7 @@ const DateSummary = ({ dateBlock, userTimezone, }) => { - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { org, } = useModel('courseHomeMeta', courseId); diff --git a/src/course-home/outline-tab/OutlineTab.jsx b/src/course-home/outline-tab/OutlineTab.jsx index db33f55fc5..fb38ec57e6 100644 --- a/src/course-home/outline-tab/OutlineTab.jsx +++ b/src/course-home/outline-tab/OutlineTab.jsx @@ -1,5 +1,5 @@ import { useEffect, useRef, useState } from 'react'; -import { useLocation, useNavigate } from 'react-router-dom'; +import { useLocation, useNavigate, useParams } from 'react-router-dom'; import { useSelector } from 'react-redux'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; @@ -13,7 +13,7 @@ import CourseHandouts from './widgets/CourseHandouts'; import StartOrResumeCourseCard from './widgets/StartOrResumeCourseCard'; import WeeklyLearningGoalCard from './widgets/WeeklyLearningGoalCard'; import CourseTools from './widgets/CourseTools'; -import { fetchOutlineTab } from '../data'; +import { useCourseHomeMeta, useOutlineTabData } from '../data/apiHooks'; import messages from './messages'; import ShiftDatesAlert from '../suggested-schedule-messaging/ShiftDatesAlert'; import UpgradeToShiftDatesAlert from '../suggested-schedule-messaging/UpgradeToShiftDatesAlert'; @@ -27,13 +27,12 @@ import WelcomeMessage from './widgets/WelcomeMessage'; import ProctoringInfoPanel from './widgets/ProctoringInfoPanel'; import AccountActivationAlert from '../../alerts/logistration-alert/AccountActivationAlert'; import CourseHomeSectionOutlineSlot from '../../plugin-slots/CourseHomeSectionOutlineSlot'; +import { TabWithTimer } from '../../tab-page'; -const OutlineTab = () => { +const OutlineTabContent = () => { const intl = useIntl(); - const { - courseId, - proctoringPanelStatus, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); + const { proctoringPanelStatus } = useSelector(state => state.courseHome); const { isSelfPaced, @@ -146,7 +145,7 @@ const OutlineTab = () => { /> {isSelfPaced && hasDeadlines && ( <> - + )} @@ -191,4 +190,20 @@ const OutlineTab = () => { ); }; +const OutlineTab = () => { + const { courseId } = useParams(); + const metadataQuery = useCourseHomeMeta(courseId); + const tabDataQuery = useOutlineTabData(courseId); + + return ( + + + + ); +}; + export default OutlineTab; diff --git a/src/course-home/outline-tab/OutlineTab.test.jsx b/src/course-home/outline-tab/OutlineTab.test.jsx index 8613916368..df4faa74cb 100644 --- a/src/course-home/outline-tab/OutlineTab.test.jsx +++ b/src/course-home/outline-tab/OutlineTab.test.jsx @@ -5,24 +5,26 @@ import React from 'react'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; import { Factory } from 'rosie'; import { getConfig } from '@edx/frontend-platform'; +import { AppProvider } from '@edx/frontend-platform/react'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { QueryClientProvider } from '@tanstack/react-query'; import MockAdapter from 'axios-mock-adapter'; import Cookies from 'js-cookie'; import userEvent from '@testing-library/user-event'; +import { render } from '@testing-library/react'; import messages from './messages'; import { buildMinimalCourseBlocks } from '../../shared/data/__factories__/courseBlocks.factory'; import { - fireEvent, initializeMockApp, logUnhandledRequests, render, screen, waitFor, act, + createTestQueryClient, fireEvent, initializeMockApp, logUnhandledRequests, screen, waitFor, act, } from '../../setupTest'; -import { appendBrowserTimezoneToUrl, executeThunk } from '../../utils'; -import * as thunks from '../data/thunks'; +import { appendBrowserTimezoneToUrl } from '../../utils'; import initializeStore from '../../store'; import { CERT_STATUS_TYPE } from './alerts/certificate-status-alert/CertificateStatusAlert'; import OutlineTab from './OutlineTab'; -import LoadedTabPage from '../../tab-page/LoadedTabPage'; -import { TourProvider } from '../../product-tours/TourContext'; +import { UserMessagesProvider } from '../../generic/user-messages'; +import { ToastProvider } from '../../generic/ToastContext'; const mockCoursewareSearchParams = jest.fn(); @@ -71,19 +73,26 @@ describe('Outline Tab', () => { axiosMock.onGet(outlineUrl).reply(200, outlineTabData); } - async function fetchAndRender(path = '') { - await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); + async function fetchAndRender(path = '', { renderStore = store, waitForLoaded = true } = {}) { const search = path.includes('?') ? path.slice(path.indexOf('?')) : ''; await act(async () => render( - - - - } /> - - - , - { store }, + + + + + + + } /> + + + + + + , )); + if (waitForLoaded) { + await waitFor(() => expect(screen.queryByRole('status')).not.toBeInTheDocument()); + } } beforeEach(async () => { @@ -111,6 +120,23 @@ describe('Outline Tab', () => { jest.clearAllMocks(); }); + describe('Access denied', () => { + it('waits for the outline data before rendering for a denied learner (no crash)', async () => { + const testStore = initializeStore(); + setMetadata({ course_access: { has_access: false, error_code: 'authentication_required' }, is_enrolled: false }); + let resolveOutline; + axiosMock.onGet(outlineUrl).reply(() => new Promise((resolve) => { resolveOutline = resolve; })); + + await fetchAndRender('', { renderStore: testStore, waitForLoaded: false }); + + await waitFor(() => expect(testStore.getState().models.courseHomeMeta?.[courseId]).toBeDefined()); + expect(screen.getByRole('status')).toBeInTheDocument(); + + await act(async () => { resolveOutline([200, Factory.build('outlineTabData')]); }); + expect(await screen.findByTestId('private-course-alert')).toBeInTheDocument(); + }); + }); + describe('Course Outline', () => { it('displays link to start course', async () => { await fetchAndRender(); @@ -429,12 +455,10 @@ describe('Outline Tab', () => { weekly_learning_goal_enabled: true, }, }); - const spy = jest.spyOn(thunks, 'saveWeeklyLearningGoal'); - await fetchAndRender(); - const button = await screen.getByTestId('weekly-learning-goal-input-Regular'); + const button = screen.getByTestId('weekly-learning-goal-input-Regular'); fireEvent.click(button); - expect(spy).toHaveBeenCalledTimes(0); + expect(axiosMock.history.post.some(req => req.url.includes('save_course_goal'))).toBe(false); }); it('post goal via query param', async () => { @@ -443,11 +467,12 @@ describe('Outline Tab', () => { weekly_learning_goal_enabled: true, }, }); - const spy = jest.spyOn(thunks, 'saveWeeklyLearningGoal'); sendTrackEvent.mockClear(); await fetchAndRender('http://localhost/?weekly_goal=3'); - expect(spy).toHaveBeenCalledTimes(1); + await waitFor(() => expect( + axiosMock.history.post.filter(req => req.url.includes('save_course_goal')), + ).toHaveLength(1)); expect(sendTrackEvent).toHaveBeenCalledWith('enrollment.email.clicked.setgoal', {}); }); @@ -665,9 +690,8 @@ describe('Outline Tab', () => { masquerading_expired_course: true, }, }); - await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - await act(async () => render(..., { store })); - const instructorToolbar = await screen.getByTestId('instructor-toolbar'); + await fetchAndRender(); + const instructorToolbar = screen.getByTestId('instructor-toolbar'); expect(instructorToolbar).toBeInTheDocument(); expect(screen.getByText('This learner no longer has access to this course. Their access expired on', { exact: false })).toBeInTheDocument(); expect(screen.getByText('1/1/2020', { exact: false })).toBeInTheDocument(); @@ -681,9 +705,8 @@ describe('Outline Tab', () => { masquerading_expired_course: false, }, }); - await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - await act(async () => render(..., { store })); - const instructorToolbar = await screen.getByTestId('instructor-toolbar'); + await fetchAndRender(); + const instructorToolbar = screen.getByTestId('instructor-toolbar'); expect(instructorToolbar).toBeInTheDocument(); expect(screen.queryByText('This learner no longer has access to this course. Their access expired on', { exact: false })).not.toBeInTheDocument(); }); diff --git a/src/course-home/outline-tab/section-outline/Section.tsx b/src/course-home/outline-tab/section-outline/Section.tsx index f905294055..43314cd3bc 100644 --- a/src/course-home/outline-tab/section-outline/Section.tsx +++ b/src/course-home/outline-tab/section-outline/Section.tsx @@ -3,9 +3,9 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Collapsible, IconButton } from '@openedx/paragon'; import { Minus, Plus } from '@openedx/paragon/icons'; +import { useParams } from 'react-router-dom'; import { useModel } from '../../../generic/model-store'; import genericMessages from '../../../generic/messages'; -import { useContextId } from '../../../data/hooks'; import messages from '../messages'; import SectionTitle from './SectionTitle'; import SequenceLink from './SequenceLink'; @@ -27,7 +27,7 @@ const Section: React.FC = ({ section, }) => { const intl = useIntl(); - const courseId = useContextId(); + const { courseId } = useParams(); const { complete, sequenceIds, diff --git a/src/course-home/outline-tab/section-outline/SequenceDueDate.tsx b/src/course-home/outline-tab/section-outline/SequenceDueDate.tsx index 9c25379240..9d07d29cec 100644 --- a/src/course-home/outline-tab/section-outline/SequenceDueDate.tsx +++ b/src/course-home/outline-tab/section-outline/SequenceDueDate.tsx @@ -1,8 +1,8 @@ import React from 'react'; import { FormattedTime, useIntl } from '@edx/frontend-platform/i18n'; +import { useParams } from 'react-router-dom'; import { useModel } from '../../../generic/model-store'; -import { useContextId } from '../../../data/hooks'; import messages from '../messages'; interface Props { @@ -17,7 +17,7 @@ const SequenceDueDate: React.FC = ({ description, }) => { const intl = useIntl(); - const courseId = useContextId(); + const { courseId } = useParams(); let dueDateMessage: string | React.ReactNode = intl.formatMessage( messages.sequenceNoDueDate, { description: description || '' }, diff --git a/src/course-home/outline-tab/section-outline/SequenceTitle.tsx b/src/course-home/outline-tab/section-outline/SequenceTitle.tsx index ec035dfa28..3e49c6a1f7 100644 --- a/src/course-home/outline-tab/section-outline/SequenceTitle.tsx +++ b/src/course-home/outline-tab/section-outline/SequenceTitle.tsx @@ -1,12 +1,11 @@ import React from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { Link } from 'react-router-dom'; +import { Link, useParams } from 'react-router-dom'; import { Icon } from '@openedx/paragon'; import { CheckCircleOutline, CheckCircle } from '@openedx/paragon/icons'; import EffortEstimate from '../../../shared/effort-estimate'; import messages from '../messages'; -import { useContextId } from '../../../data/hooks'; interface Props { complete: boolean; @@ -24,7 +23,7 @@ const SequenceTitle: React.FC = ({ id, }) => { const intl = useIntl(); - const courseId = useContextId(); + const { courseId } = useParams(); const coursewareUrl = {title}; const displayTitle = showLink ? coursewareUrl : title; diff --git a/src/course-home/outline-tab/widgets/CourseDates.jsx b/src/course-home/outline-tab/widgets/CourseDates.jsx index fa28a6433a..a1fe22da01 100644 --- a/src/course-home/outline-tab/widgets/CourseDates.jsx +++ b/src/course-home/outline-tab/widgets/CourseDates.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { useSelector } from 'react-redux'; +import { useParams } from 'react-router-dom'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -9,9 +9,7 @@ import { useModel } from '../../../generic/model-store'; const CourseDates = () => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { userTimezone, } = useModel('courseHomeMeta', courseId); diff --git a/src/course-home/outline-tab/widgets/CourseHandouts.jsx b/src/course-home/outline-tab/widgets/CourseHandouts.jsx index 8c609531b9..522d0465bd 100644 --- a/src/course-home/outline-tab/widgets/CourseHandouts.jsx +++ b/src/course-home/outline-tab/widgets/CourseHandouts.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { useSelector } from 'react-redux'; +import { useParams } from 'react-router-dom'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -9,9 +9,7 @@ import { useModel } from '../../../generic/model-store'; const CourseHandouts = () => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { handoutsHtml, } = useModel('outline', courseId); diff --git a/src/course-home/outline-tab/widgets/CourseTools.jsx b/src/course-home/outline-tab/widgets/CourseTools.jsx index a3a7e13944..a50bb849ff 100644 --- a/src/course-home/outline-tab/widgets/CourseTools.jsx +++ b/src/course-home/outline-tab/widgets/CourseTools.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { useSelector } from 'react-redux'; +import { useParams } from 'react-router-dom'; import { sendTrackingLogEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; @@ -16,9 +16,7 @@ import LaunchCourseHomeTourButton from '../../../product-tours/newUserCourseHome const CourseTools = () => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { org } = useModel('courseHomeMeta', courseId); const { courseTools, diff --git a/src/course-home/outline-tab/widgets/ProctoringInfoPanel.jsx b/src/course-home/outline-tab/widgets/ProctoringInfoPanel.jsx index 2872f80507..b7d9baf84b 100644 --- a/src/course-home/outline-tab/widgets/ProctoringInfoPanel.jsx +++ b/src/course-home/outline-tab/widgets/ProctoringInfoPanel.jsx @@ -1,5 +1,6 @@ import React, { useState, useEffect } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; +import { useParams } from 'react-router-dom'; import camelCase from 'lodash.camelcase'; import { useIntl } from '@edx/frontend-platform/i18n'; @@ -13,9 +14,7 @@ import { useModel } from '../../../generic/model-store'; const ProctoringInfoPanel = () => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { username, } = useModel('courseHomeMeta', courseId); diff --git a/src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx b/src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx index a75e76be3e..641c1de66b 100644 --- a/src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx +++ b/src/course-home/outline-tab/widgets/StartOrResumeCourseCard.jsx @@ -2,16 +2,14 @@ import React from 'react'; import { Button, Card } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { useSelector } from 'react-redux'; +import { useParams } from 'react-router-dom'; import { sendTrackingLogEvent } from '@edx/frontend-platform/analytics'; import messages from '../messages'; import { useModel } from '../../../generic/model-store'; const StartOrResumeCourseCard = () => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { org, diff --git a/src/course-home/outline-tab/widgets/WeeklyLearningGoalCard.jsx b/src/course-home/outline-tab/widgets/WeeklyLearningGoalCard.jsx index 575cb8ad67..b9a29a4d13 100644 --- a/src/course-home/outline-tab/widgets/WeeklyLearningGoalCard.jsx +++ b/src/course-home/outline-tab/widgets/WeeklyLearningGoalCard.jsx @@ -1,5 +1,5 @@ import React, { useEffect, useState } from 'react'; -import { useLocation } from 'react-router-dom'; +import { useLocation, useParams } from 'react-router-dom'; import PropTypes from 'prop-types'; import { Form, Card, Icon } from '@openedx/paragon'; @@ -8,10 +8,9 @@ import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Email } from '@openedx/paragon/icons'; -import { useSelector } from 'react-redux'; import messages from '../messages'; import LearningGoalButton from './LearningGoalButton'; -import { saveWeeklyLearningGoal } from '../../data'; +import { useSaveWeeklyLearningGoal } from '../../data/apiHooks'; import { useModel } from '../../../generic/model-store'; import './FlagButton.scss'; @@ -20,9 +19,7 @@ const WeeklyLearningGoalCard = ({ subscribedToReminders, }) => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { isMasquerading, @@ -30,6 +27,7 @@ const WeeklyLearningGoalCard = ({ } = useModel('courseHomeMeta', courseId); const { administrator } = getAuthenticatedUser(); + const saveWeeklyLearningGoal = useSaveWeeklyLearningGoal(); const [daysPerWeekGoal, setDaysPerWeekGoal] = useState(daysPerWeek); // eslint-disable-next-line react/prop-types @@ -42,7 +40,7 @@ const WeeklyLearningGoalCard = ({ setGetReminderSelected(selectReminders); setDaysPerWeekGoal(days); if (!isMasquerading) { // don't save goal updates while masquerading - saveWeeklyLearningGoal(courseId, days, selectReminders); + saveWeeklyLearningGoal.mutate({ courseId, daysPerWeek: days, subscribedToReminders: selectReminders }); sendTrackEvent('edx.ui.lms.goal.days-per-week.changed', { org_key: org, courserun_key: courseId, @@ -60,7 +58,9 @@ const WeeklyLearningGoalCard = ({ const isGetReminderChecked = event.target.checked; setGetReminderSelected(isGetReminderChecked); if (!isMasquerading) { // don't save goal updates while masquerading - saveWeeklyLearningGoal(courseId, daysPerWeekGoal, isGetReminderChecked); + saveWeeklyLearningGoal.mutate({ + courseId, daysPerWeek: daysPerWeekGoal, subscribedToReminders: isGetReminderChecked, + }); sendTrackEvent('edx.ui.lms.goal.reminder-selected.changed', { org_key: org, courserun_key: courseId, diff --git a/src/course-home/outline-tab/widgets/WelcomeMessage.jsx b/src/course-home/outline-tab/widgets/WelcomeMessage.jsx index 40bac852e7..0bafc72e19 100644 --- a/src/course-home/outline-tab/widgets/WelcomeMessage.jsx +++ b/src/course-home/outline-tab/widgets/WelcomeMessage.jsx @@ -5,11 +5,10 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { Alert, Button, TransitionReplace } from '@openedx/paragon'; import truncate from 'truncate-html'; -import { useDispatch } from 'react-redux'; import LmsHtmlFragment from '../LmsHtmlFragment'; import messages from '../messages'; import { useModel } from '../../../generic/model-store'; -import { dismissWelcomeMessage } from '../../data/thunks'; +import { useDismissWelcomeMessage } from '../../data/apiHooks'; const WelcomeMessage = ({ courseId, nextElementRef }) => { const intl = useIntl(); @@ -37,7 +36,7 @@ const WelcomeMessage = ({ courseId, nextElementRef }) => { ); const [showShortMessage, setShowShortMessage] = useState(messageCanBeShortened); - const dispatch = useDispatch(); + const dismissWelcomeMessage = useDismissWelcomeMessage(); if (!welcomeMessageHtml) { return null; @@ -53,7 +52,7 @@ const WelcomeMessage = ({ courseId, nextElementRef }) => { onClose={() => { nextElementRef.current?.focus(); setDisplay(false); - dispatch(dismissWelcomeMessage(courseId)); + dismissWelcomeMessage.mutate({ courseId }); }} className="raised-card" actions={messageCanBeShortened ? [ diff --git a/src/course-home/suggested-schedule-messaging/ShiftDatesAlert.jsx b/src/course-home/suggested-schedule-messaging/ShiftDatesAlert.jsx index b1c82d1f0f..2eea824aac 100644 --- a/src/course-home/suggested-schedule-messaging/ShiftDatesAlert.jsx +++ b/src/course-home/suggested-schedule-messaging/ShiftDatesAlert.jsx @@ -1,5 +1,4 @@ import React from 'react'; -import { useDispatch } from 'react-redux'; import { useParams } from 'react-router-dom'; import { useQueryClient } from '@tanstack/react-query'; import PropTypes from 'prop-types'; @@ -17,11 +16,10 @@ import { courseHomeQueryKeys } from '../data/queryKeys'; import { useModel } from '../../generic/model-store'; import messages from './messages'; -const ShiftDatesAlert = ({ fetch, model }) => { +const ShiftDatesAlert = ({ model }) => { const intl = useIntl(); const { courseId } = useParams(); const queryClient = useQueryClient(); - const dispatch = useDispatch(); const { datesBannerInfo, @@ -41,9 +39,7 @@ const ShiftDatesAlert = ({ fetch, model }) => { const refreshTabData = () => { queryClient.invalidateQueries({ queryKey: courseHomeQueryKeys.datesTab(courseId) }); - if (fetch) { - dispatch(fetch(courseId)); - } + queryClient.invalidateQueries({ queryKey: courseHomeQueryKeys.outlineTab(courseId) }); }; return ( @@ -72,12 +68,7 @@ const ShiftDatesAlert = ({ fetch, model }) => { }; ShiftDatesAlert.propTypes = { - fetch: PropTypes.func, model: PropTypes.string.isRequired, }; -ShiftDatesAlert.defaultProps = { - fetch: undefined, -}; - export default ShiftDatesAlert; diff --git a/src/index.jsx b/src/index.jsx index fedd863781..5b7d6ba358 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -26,7 +26,7 @@ import GoalUnsubscribe from './course-home/goal-unsubscribe'; import ProgressTab from './course-home/progress-tab/ProgressTab'; import { TabContainer } from './tab-page'; -import { fetchOutlineTab, fetchProgressTab } from './course-home/data'; +import { fetchProgressTab } from './course-home/data'; import { fetchCourse } from './courseware/data'; import { store } from './store'; import { createQueryClient } from './queryClient'; @@ -75,9 +75,7 @@ subscribe(APP_READY, () => { path={DECODE_ROUTES.HOME} element={( - - - + )} /> diff --git a/src/product-tours/ProductTours.test.jsx b/src/product-tours/ProductTours.test.jsx index a5b87340a0..8024512916 100644 --- a/src/product-tours/ProductTours.test.jsx +++ b/src/product-tours/ProductTours.test.jsx @@ -3,29 +3,28 @@ * @jest-environment jsdom */ import React from 'react'; -import { Route, Routes } from 'react-router-dom'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; import { Factory } from 'rosie'; import { getConfig, history } from '@edx/frontend-platform'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider } from '@edx/frontend-platform/react'; +import { QueryClientProvider } from '@tanstack/react-query'; import MockAdapter from 'axios-mock-adapter'; import { waitForElementToBeRemoved } from '@testing-library/dom'; import userEvent from '@testing-library/user-event'; import * as popper from '@popperjs/core'; import { - fireEvent, initializeMockApp, logUnhandledRequests, render, screen, + act, createTestQueryClient, fireEvent, initializeMockApp, logUnhandledRequests, render, screen, waitFor, } from '../setupTest'; import initializeStore from '../store'; -import { appendBrowserTimezoneToUrl, executeThunk } from '../utils'; +import { appendBrowserTimezoneToUrl } from '../utils'; import CoursewareContainer from '../courseware/CoursewareContainer'; -import LoadedTabPage from '../tab-page/LoadedTabPage'; import { TourProvider } from './TourContext'; import ProductTours from './ProductTours'; import OutlineTab from '../course-home/outline-tab/OutlineTab'; -import * as courseHomeThunks from '../course-home/data/thunks'; import { buildSimpleCourseBlocks } from '../shared/data/__factories__/courseBlocks.factory'; import { buildOutlineFromBlocks } from '../courseware/data/__factories__/learningSequencesOutline.factory'; @@ -62,15 +61,17 @@ describe('Course Home Tours', () => { } async function fetchAndRender() { - await executeThunk(courseHomeThunks.fetchOutlineTab(courseId), store.dispatch); - render( - - - - - , - { store, wrapWithRouter: true }, - ); + await act(async () => render( + + + + } /> + + + , + { store }, + )); + await waitFor(() => expect(screen.queryByRole('status')).not.toBeInTheDocument()); } beforeEach(async () => { @@ -193,7 +194,8 @@ describe('Course Home Tours', () => { }); it('launches tour on button click', async () => { - const launchTourButton = await screen.findByRole('button', { name: 'Launch tour' }); + const buttons = await screen.findAllByRole('button', { name: 'Launch tour' }); + const launchTourButton = buttons.find((button) => !button.classList.contains('sr-only')); expect(launchTourButton).toBeInTheDocument(); fireEvent.click(launchTourButton); diff --git a/src/tab-page/TabPage.test.jsx b/src/tab-page/TabPage.test.jsx index 049e226694..d56d897ef9 100644 --- a/src/tab-page/TabPage.test.jsx +++ b/src/tab-page/TabPage.test.jsx @@ -167,6 +167,22 @@ describe('Tab Page', () => { expect(screen.queryByTestId('LoadedTabPage')).not.toBeInTheDocument(); }); + it('shows loading when access is denied but the tab-data query is still pending', () => { + render( + , + { wrapWithRouter: true }, + ); + expect(screen.getByText('Loading course page…')).toBeInTheDocument(); + expect(screen.queryByTestId('LoadedTabPage')).not.toBeInTheDocument(); + }); + it('does not render tab content when access is denied', async () => { const testStore = await initializeTestStore({ excludeFetchCourse: true, excludeFetchSequence: true }, false); testStore.dispatch(addModel({ diff --git a/src/tab-page/TabPage.tsx b/src/tab-page/TabPage.tsx index 22064f3a13..d3689e0d95 100644 --- a/src/tab-page/TabPage.tsx +++ b/src/tab-page/TabPage.tsx @@ -60,9 +60,9 @@ const deriveView = (courseStatus: CourseStatus): TabView => { const { metadataQuery, tabDataQuery } = courseStatus; if (metadataQuery.isError) { return { ...view, isError: true }; } if (metadataQuery.isPending) { return { ...view, isLoading: true }; } + if (tabDataQuery.isPending) { return { ...view, isLoading: true }; } if (!metadataQuery.data?.courseAccess?.hasAccess) { return { ...view, isDenied: true }; } if (tabDataQuery.isError) { return { ...view, isError: true }; } - if (tabDataQuery.isPending) { return { ...view, isLoading: true }; } return view; };