From e2cd93bfb68c1296ff71ea6779483393b33113bc Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 19 Aug 2026 01:30:45 -0400 Subject: [PATCH] refactor: convert the discussion tab from Redux to React Query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-wrap the discussion tab (metadata-only: no tab-data fetch, so TabPage's tabDataQuery becomes optional). Also convert CourseAccessErrorPage — the other fetchDiscussionTab caller — to useCourseHomeMeta, letting fetchDiscussionTab be deleted. Ordered below the live tab so the shared fetchTab helper retires cleanly there. Part of #1975; closes #2003. Co-Authored-By: Claude Opus 4.8 --- src/course-home/data/thunks.js | 4 --- .../discussion-tab/DiscussionTab.jsx | 24 +++++++++++--- .../discussion-tab/DiscussionTab.test.jsx | 10 ++---- src/generic/CourseAccessErrorPage.jsx | 20 +++-------- src/generic/CourseAccessErrorPage.test.jsx | 33 +++++++++++-------- src/index.jsx | 6 ++-- src/tab-page/TabPage.tsx | 6 ++-- 7 files changed, 51 insertions(+), 52 deletions(-) diff --git a/src/course-home/data/thunks.js b/src/course-home/data/thunks.js index 64849b7140..b18c7670f5 100644 --- a/src/course-home/data/thunks.js +++ b/src/course-home/data/thunks.js @@ -84,10 +84,6 @@ export function fetchLiveTab(courseId) { return fetchTab(courseId, 'live', getLiveTabIframe); } -export function fetchDiscussionTab(courseId) { - return fetchTab(courseId, 'discussion'); -} - export async function deprecatedSaveCourseGoal(courseId, goalKey) { return deprecatedPostCourseGoals(courseId, goalKey); } diff --git a/src/course-home/discussion-tab/DiscussionTab.jsx b/src/course-home/discussion-tab/DiscussionTab.jsx index 83588cfe79..a3b24c55a7 100644 --- a/src/course-home/discussion-tab/DiscussionTab.jsx +++ b/src/course-home/discussion-tab/DiscussionTab.jsx @@ -1,12 +1,12 @@ import { getConfig } from '@edx/frontend-platform'; import React, { useState } from 'react'; -import { useSelector } from 'react-redux'; import { useParams, generatePath, useNavigate } from 'react-router-dom'; import { useIFrameHeight, useIFramePluginEvents } from '../../generic/hooks'; +import { useCourseHomeMeta } from '../data/apiHooks'; +import { TabWithTimer } from '../../tab-page'; -const DiscussionTab = () => { - const { courseId } = useSelector(state => state.courseHome); - const { path } = useParams(); +const DiscussionTabContent = () => { + const { courseId, path } = useParams(); const [originalPath] = useState(path); const navigate = useNavigate(); @@ -29,4 +29,20 @@ const DiscussionTab = () => { ); }; +const DiscussionTab = () => { + const { courseId } = useParams(); + + const metadataQuery = useCourseHomeMeta(courseId); + + return ( + + + + ); +}; + export default DiscussionTab; diff --git a/src/course-home/discussion-tab/DiscussionTab.test.jsx b/src/course-home/discussion-tab/DiscussionTab.test.jsx index 8e1db24705..665a56bf1e 100644 --- a/src/course-home/discussion-tab/DiscussionTab.test.jsx +++ b/src/course-home/discussion-tab/DiscussionTab.test.jsx @@ -13,9 +13,7 @@ import { createTestQueryClient, initializeMockApp, messageEvent, screen, waitFor, } from '../../setupTest'; import initializeStore from '../../store'; -import { TabContainer } from '../../tab-page'; import { appendBrowserTimezoneToUrl } from '../../utils'; -import { fetchDiscussionTab } from '../data/thunks'; import DiscussionTab from './DiscussionTab'; initializeMockApp(); @@ -31,17 +29,13 @@ describe('DiscussionTab', () => { store = initializeStore(); component = ( - + - - - )} + element={} /> diff --git a/src/generic/CourseAccessErrorPage.jsx b/src/generic/CourseAccessErrorPage.jsx index 644f409787..11fc3642f0 100644 --- a/src/generic/CourseAccessErrorPage.jsx +++ b/src/generic/CourseAccessErrorPage.jsx @@ -1,13 +1,11 @@ -import React, { useEffect } from 'react'; +import React from 'react'; import { useParams, Navigate } from 'react-router-dom'; -import { useDispatch, useSelector } from 'react-redux'; import { useIntl } from '@edx/frontend-platform/i18n'; import { FooterSlot } from '@edx/frontend-component-footer'; -import { LOADED, LOADING } from '@src/constants'; import HeaderSlot from '../plugin-slots/HeaderSlot'; import useActiveEnterpriseAlert from '../alerts/active-enteprise-alert'; import { AlertList } from './user-messages'; -import { fetchDiscussionTab } from '../course-home/data/thunks'; +import { useCourseHomeMeta } from '../course-home/data/apiHooks'; import PageLoading from './PageLoading'; import messages from '../tab-page/messages'; @@ -15,18 +13,10 @@ const CourseAccessErrorPage = () => { const intl = useIntl(); const { courseId } = useParams(); - const dispatch = useDispatch(); const activeEnterpriseAlert = useActiveEnterpriseAlert(courseId); - useEffect(() => { - dispatch(fetchDiscussionTab(courseId)); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [courseId]); + const metadataQuery = useCourseHomeMeta(courseId); - const { - courseStatus, - } = useSelector(state => state.courseHome); - - if (courseStatus === LOADING) { + if (metadataQuery.isPending) { return ( <> @@ -37,7 +27,7 @@ const CourseAccessErrorPage = () => { ); } - if (courseStatus === LOADED) { + if (metadataQuery.data?.courseAccess?.hasAccess) { return ; } return ( diff --git a/src/generic/CourseAccessErrorPage.test.jsx b/src/generic/CourseAccessErrorPage.test.jsx index 340e5d07b9..89e161b327 100644 --- a/src/generic/CourseAccessErrorPage.test.jsx +++ b/src/generic/CourseAccessErrorPage.test.jsx @@ -4,22 +4,14 @@ import { Routes, Route } from 'react-router-dom'; import { initializeTestStore, render, screen } from '../setupTest'; import CourseAccessErrorPage from './CourseAccessErrorPage'; -const mockDispatch = jest.fn(); -const mockNavigate = jest.fn(); -let mockCourseStatus; +let mockMetadataQuery; -jest.mock('react-redux', () => ({ - ...jest.requireActual('react-redux'), - useDispatch: () => mockDispatch, - useSelector: () => ({ courseStatus: mockCourseStatus }), +jest.mock('../course-home/data/apiHooks', () => ({ + useCourseHomeMeta: () => mockMetadataQuery, })); jest.mock('./PageLoading', () => function () { return
; }); -jest.mock('react-router-dom', () => ({ - ...(jest.requireActual('react-router-dom')), - useNavigate: () => mockNavigate, -})); describe('CourseAccessErrorPage', () => { let courseId; @@ -32,7 +24,7 @@ describe('CourseAccessErrorPage', () => { }); it('Displays loading in start on page rendering', () => { - mockCourseStatus = 'loading'; + mockMetadataQuery = { isPending: true, data: undefined }; render( } /> @@ -44,7 +36,7 @@ describe('CourseAccessErrorPage', () => { }); it('Redirect user to homepage if user has access', () => { - mockCourseStatus = 'loaded'; + mockMetadataQuery = { isPending: false, data: { courseAccess: { hasAccess: true } } }; render( } /> @@ -55,7 +47,20 @@ describe('CourseAccessErrorPage', () => { }); it('For access denied it should render access denied page', () => { - mockCourseStatus = 'denied'; + mockMetadataQuery = { isPending: false, data: { courseAccess: { hasAccess: false } } }; + + render( + + } /> + , + { wrapWithRouter: true }, + ); + expect(screen.getByTestId('access-denied-main')).toBeInTheDocument(); + expect(window.location.pathname).toBe(accessDeniedUrl); + }); + + it('For a failed metadata query it should render access denied page', () => { + mockMetadataQuery = { isPending: false, isError: true, data: undefined }; render( diff --git a/src/index.jsx b/src/index.jsx index 0f33148bb6..7aa5f076a6 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -10,7 +10,7 @@ import { createRoot } from 'react-dom/client'; import { Routes, Route } from 'react-router-dom'; import { Helmet } from 'react-helmet'; -import { fetchDiscussionTab, fetchLiveTab } from './course-home/data/thunks'; +import { fetchLiveTab } from './course-home/data/thunks'; import DiscussionTab from './course-home/discussion-tab/DiscussionTab'; import messages from './i18n'; @@ -100,9 +100,7 @@ subscribe(APP_READY, () => { path={DECODE_ROUTES.DISCUSSION} element={( - - - + )} /> diff --git a/src/tab-page/TabPage.tsx b/src/tab-page/TabPage.tsx index d3689e0d95..68ff8509e7 100644 --- a/src/tab-page/TabPage.tsx +++ b/src/tab-page/TabPage.tsx @@ -27,7 +27,7 @@ import { TourProvider } from '../product-tours/TourContext'; // metadata query is typed to only the field this file reads, not the whole (untyped) shape. export type CourseStatus = StatusValue | { metadataQuery: UseQueryResult<{ courseAccess?: { hasAccess: boolean } }>; - tabDataQuery: UseQueryResult; + tabDataQuery?: UseQueryResult; }; export interface TabPageProps { @@ -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 (tabDataQuery?.isPending) { return { ...view, isLoading: true }; } if (!metadataQuery.data?.courseAccess?.hasAccess) { return { ...view, isDenied: true }; } - if (tabDataQuery.isError) { return { ...view, isError: true }; } + if (tabDataQuery?.isError) { return { ...view, isError: true }; } return view; };