diff --git a/src/courseware/course/sidebar/ARCHITECTURE.md b/src/courseware/course/sidebar/ARCHITECTURE.md index ca17f0bcd4..fba3d7bfb4 100644 --- a/src/courseware/course/sidebar/ARCHITECTURE.md +++ b/src/courseware/course/sidebar/ARCHITECTURE.md @@ -157,8 +157,10 @@ _Example with built-in widgets:_ **Responsibilities:** - Calculate `initialSidebar` based on available RIGHT sidebar panels - Manage `currentSidebar` state (shared by both sidebars) +- Call `widget.prefetch()` on mount for data preloading - Handle unit shift logic for RIGHT sidebar panels - Provide context to both left and right sidebar components +- Wrap children in widget `Provider` components (if defined) **Key Logic:** ```javascript diff --git a/src/courseware/course/sidebar/README.md b/src/courseware/course/sidebar/README.md index 8b32dbe70d..8b4b6f2c13 100644 --- a/src/courseware/course/sidebar/README.md +++ b/src/courseware/course/sidebar/README.md @@ -24,6 +24,7 @@ Each widget must provide: isAvailable: (context) => boolean, // Optional: check if widget should be shown enabled: boolean, // Whether widget is enabled Provider?: ReactComponent, // Optional: React Provider for Panel↔Trigger shared state + prefetch?: (params) => void, // Optional: called on mount to preload data into Redux store } ``` @@ -55,12 +56,25 @@ The `isAvailable` function receives a context object with: { courseId: string, unitId: string, - course: object, // Merged coursewareMeta + courseHomeMeta (verifiedMode, enrollmentMode, courseModes, …) - unit: object, // discussionTopics model for the current unit (id, enabledInContext, …) + course: object, // Merged coursewareMeta + courseHomeMeta (verifiedMode, enrollmentMode, courseModes, tabs, …) } ``` -Widgets pick whatever they need from `course` or `unit` — the sidebar makes no assumptions about which fields any given widget requires. +Widgets pick whatever they need from `course` — the sidebar makes no assumptions about which fields any given widget requires. + +### Prefetch + +The optional `prefetch` function is called once by `SidebarContextProvider` on mount. It receives: + +```javascript +{ + courseId: string, + course: object, // Merged coursewareMeta + courseHomeMeta + dispatch: Function, // Redux dispatch +} +``` + +Use this to preload data (e.g., API calls that populate the Redux store) so that your `Sidebar` and `Trigger` components can read from the store without their own side effects. ## Adding Widgets diff --git a/src/courseware/course/sidebar/SidebarContextProvider.jsx b/src/courseware/course/sidebar/SidebarContextProvider.jsx index 0f01a068b1..315fceb0e0 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.jsx @@ -1,8 +1,9 @@ import { breakpoints, useWindowSize } from '@openedx/paragon'; import PropTypes from 'prop-types'; import { - useState, useMemo, useCallback, useRef, + useState, useMemo, useCallback, useRef, useEffect, } from 'react'; +import { useDispatch } from 'react-redux'; import { useSearchParams } from 'react-router-dom'; import { useModel } from '@src/generic/model-store'; @@ -30,7 +31,8 @@ const SidebarProvider = ({ }) => { const courseHomeMeta = useModel('courseHomeMeta', courseId); const coursewareMeta = useModel('coursewareMeta', courseId); - const unit = useModel('discussionTopics', unitId); + + const dispatch = useDispatch(); const { width } = useWindowSize(); const shouldDisplayFullScreen = width < breakpoints.extraLarge.minWidth; const shouldDisplaySidebarOpen = width > breakpoints.extraLarge.minWidth; @@ -39,6 +41,13 @@ const SidebarProvider = ({ // Build registry of enabled widgets const enabledWidgets = useMemo(() => getEnabledWidgets(), []); + + useEffect(() => { + enabledWidgets.forEach(widget => widget.prefetch?.({ + courseId, course: { ...coursewareMeta, ...courseHomeMeta }, dispatch, + })); + }, [courseId, courseHomeMeta, coursewareMeta, enabledWidgets, dispatch]); + const SIDEBARS = useMemo(() => buildSidebarsRegistry(enabledWidgets), [enabledWidgets]); const SIDEBAR_ORDER = useMemo(() => getSidebarOrder(enabledWidgets), [enabledWidgets]); @@ -48,15 +57,14 @@ const SidebarProvider = ({ courseId, unitId, course: { ...coursewareMeta, ...courseHomeMeta }, - unit, }; return enabledWidgets.filter(widget => { if (widget.isAvailable) { return widget.isAvailable(context); } - return true; // If no isAvailable function, widget is always available + return true; }); - }, [enabledWidgets, courseId, unitId, coursewareMeta, courseHomeMeta, unit]); + }, [enabledWidgets, courseId, unitId, coursewareMeta, courseHomeMeta]); // Helper to get the first available panel based on priority const getFirstAvailablePanel = useCallback(() => { @@ -76,15 +84,11 @@ const SidebarProvider = ({ const [currentSidebar, setCurrentSidebar] = useState(initialSidebar); - // Track if user has manually toggled sidebar within current unit const hasUserToggledRef = useRef(false); - const previousUnitIdRef = useRef(null); // Start null so first render triggers unit shift logic - // Track which unit set COURSE_OUTLINE (to prevent immediate switching) + const previousUnitIdRef = useRef(null); const courseOutlineSetByUnitRef = useRef(null); - // Track if this is initial page load (to allow data loading switches) const isInitialLoadRef = useRef(true); - // Apply unit navigation behavior useUnitShiftBehavior({ unitId, currentSidebar, @@ -100,7 +104,6 @@ const SidebarProvider = ({ isInitialLoadRef, }); - // Sync with async data loading useSidebarSync({ initialSidebar, currentSidebar, @@ -133,6 +136,7 @@ const SidebarProvider = ({ // Switch to new sidebar or hide the current sidebar const newSidebar = sidebarId === currentSidebar ? null : sidebarId; + setCurrentSidebar(newSidebar); setSidebarId(courseId, newSidebar); }, [currentSidebar, courseId]); @@ -160,6 +164,7 @@ const SidebarProvider = ({ SIDEBAR_ORDER, availableSidebarIds, ]); + const renderWithWidgetProviders = useCallback((content) => enabledWidgets .filter(w => w.Provider) .reduceRight((acc, widget) => { diff --git a/src/courseware/course/sidebar/SidebarContextProvider.test.jsx b/src/courseware/course/sidebar/SidebarContextProvider.test.jsx index d664825822..f18d5d25d9 100644 --- a/src/courseware/course/sidebar/SidebarContextProvider.test.jsx +++ b/src/courseware/course/sidebar/SidebarContextProvider.test.jsx @@ -7,6 +7,19 @@ import { MemoryRouter } from 'react-router-dom'; import SidebarContext from './SidebarContext'; import SidebarProvider from './SidebarContextProvider'; +jest.mock('react-redux', () => ({ + useDispatch: jest.fn(() => jest.fn()), +})); + +jest.mock('@edx/frontend-platform', () => ({ + getConfig: jest.fn(() => ({})), + ensureConfig: jest.fn(), +})); + +jest.mock('@src/courseware/data/thunks', () => ({ + getCourseDiscussionTopics: jest.fn(() => ({ type: 'MOCK_ACTION' })), +})); + jest.mock('@src/generic/model-store', () => ({ useModel: jest.fn(() => null), })); diff --git a/src/courseware/course/sidebar/hooks/useResponsiveBehavior.js b/src/courseware/course/sidebar/hooks/useResponsiveBehavior.js index c36bf19acc..266244a929 100644 --- a/src/courseware/course/sidebar/hooks/useResponsiveBehavior.js +++ b/src/courseware/course/sidebar/hooks/useResponsiveBehavior.js @@ -20,7 +20,7 @@ import { * @param {Function} params.setCurrentSidebar - Update current sidebar state * @param {Function} params.getFirstAvailablePanel - Get first available widget * @param {string} params.courseId - Current course ID - * @param {Function} params.hasUserToggledRef - Ref tracking user manual toggles + * @param {Object} params.hasUserToggledRef - Ref tracking user manual toggles */ export function useResponsiveBehavior({ shouldDisplaySidebarOpen, diff --git a/src/courseware/course/sidebar/hooks/useSidebarSync.js b/src/courseware/course/sidebar/hooks/useSidebarSync.js index a5e325ef7e..04394ad97e 100644 --- a/src/courseware/course/sidebar/hooks/useSidebarSync.js +++ b/src/courseware/course/sidebar/hooks/useSidebarSync.js @@ -19,8 +19,8 @@ import { * @param {string} params.courseId - Current course ID * @param {string} params.unitId - Current unit ID * @param {boolean} params.shouldDisplayFullScreen - Whether in mobile view - * @param {Function} params.hasUserToggledRef - Ref tracking user manual toggles - * @param {Function} params.courseOutlineSetByUnitRef - Ref tracking COURSE_OUTLINE auto-set + * @param {Object} params.hasUserToggledRef - Ref tracking user manual toggles + * @param {Object} params.courseOutlineSetByUnitRef - Ref tracking COURSE_OUTLINE auto-set */ export function useSidebarSync({ initialSidebar, diff --git a/src/widgets/discussions/DiscussionsTrigger.jsx b/src/widgets/discussions/DiscussionsTrigger.jsx index f44c2c00ec..7fdb4f08c3 100644 --- a/src/widgets/discussions/DiscussionsTrigger.jsx +++ b/src/widgets/discussions/DiscussionsTrigger.jsx @@ -1,18 +1,14 @@ -import { ensureConfig, getConfig } from '@edx/frontend-platform'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Icon } from '@openedx/paragon'; import { QuestionAnswer } from '@openedx/paragon/icons'; import PropTypes from 'prop-types'; -import { useContext, useEffect, useMemo } from 'react'; -import { useDispatch } from 'react-redux'; +import { useContext } from 'react'; import { useModel } from '@src/generic/model-store'; import { WIDGETS } from '@src/constants'; -import { getCourseDiscussionTopics } from '@src/courseware/data/thunks'; import SidebarTriggerBase from '@src/courseware/course/sidebar/common/TriggerBase'; import SidebarContext from '@src/courseware/course/sidebar/SidebarContext'; import messages from './messages'; -ensureConfig(['DISCUSSIONS_MFE_BASE_URL']); export const ID = WIDGETS.DISCUSSIONS; const DiscussionsTrigger = ({ @@ -21,22 +17,8 @@ const DiscussionsTrigger = ({ const intl = useIntl(); const { unitId, - courseId, } = useContext(SidebarContext); - const dispatch = useDispatch(); - const { tabs } = useModel('courseHomeMeta', courseId); const topic = useModel('discussionTopics', unitId); - const baseUrl = getConfig().DISCUSSIONS_MFE_BASE_URL; - const edxProvider = useMemo( - () => tabs?.find(tab => tab.slug === 'discussion'), - [tabs], - ); - - useEffect(() => { - if (baseUrl && edxProvider) { - dispatch(getCourseDiscussionTopics(courseId)); - } - }, [courseId, baseUrl, edxProvider, dispatch]); if (!topic?.id || !topic?.enabledInContext) { return null; diff --git a/src/widgets/discussions/DiscussionsTrigger.test.jsx b/src/widgets/discussions/DiscussionsTrigger.test.jsx index 067d971f88..44dcd1ff64 100644 --- a/src/widgets/discussions/DiscussionsTrigger.test.jsx +++ b/src/widgets/discussions/DiscussionsTrigger.test.jsx @@ -7,6 +7,7 @@ import { fireEvent, initializeMockApp, initializeTestStore, render, screen, } from '@src/setupTest'; import { buildTopicsFromUnits } from '@src/courseware/data/__factories__/discussionTopics.factory'; +import { getCourseDiscussionTopics } from '@src/courseware/data/thunks'; import SidebarContext from '@src/courseware/course/sidebar/SidebarContext'; import DiscussionsTrigger from './DiscussionsTrigger'; @@ -41,6 +42,8 @@ describe('Discussions Trigger', () => { ); axiosMock.onGet(`${getConfig().LMS_BASE_URL}/api/discussion/v2/course_topics/${courseId}`) .reply(200, buildTopicsFromUnits(state.models.units)); + + await store.dispatch(getCourseDiscussionTopics(courseId)); }); const SidebarWrapper = ({ contextValue, onClick }) => ( @@ -63,16 +66,16 @@ describe('Discussions Trigger', () => { const clickTrigger = jest.fn(); renderWithProvider({}, clickTrigger); - const discussionsTrigger = await screen.findByRole('button', { name: /Show discussions tray/i }); + const discussionsTrigger = screen.getByRole('button', { name: /Show discussions tray/i }); expect(discussionsTrigger).toBeInTheDocument(); fireEvent.click(discussionsTrigger); expect(clickTrigger).toHaveBeenCalledTimes(1); }); - it('doesn\'t show up if unit has no discussion associated with it', async () => { + it('doesn\'t show up if unit has no discussion associated with it', () => { const clickTrigger = jest.fn(); renderWithProvider({ unitId: 'has-no-discussion' }, clickTrigger); - expect(await screen.queryByRole('button', { name: /Show discussions tray/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /Show discussions tray/i })).not.toBeInTheDocument(); }); }); diff --git a/src/widgets/discussions/README.md b/src/widgets/discussions/README.md index 0898ff427c..7436724bcc 100644 --- a/src/widgets/discussions/README.md +++ b/src/widgets/discussions/README.md @@ -8,13 +8,16 @@ Built-in right-sidebar widget that embeds the Discussions MFE in an iframe for t |-------|-------| | `id` | `DISCUSSIONS` | | `priority` | `10` (highest built-in priority) | -| `isAvailable` | `({ unit }) => !!(unit?.id && unit?.enabledInContext)` | +| `isAvailable` | `({ course }) => !!(baseUrl && hasDiscussionTab)` | +| `prefetch` | Dispatches `getCourseDiscussionTopics` when discussion tab exists | ## Availability -Only shown when the current unit has a discussion topic enabled in context. Both conditions must be true: -- `topic.id` — a discussion topic exists for the unit -- `topic.enabledInContext` — discussions are enabled for this context +The widget is registered as available at **course level** — it appears in the sidebar trigger bar whenever: +- `DISCUSSIONS_MFE_BASE_URL` is configured +- The course has a `discussion` tab + +At **render time**, the `DiscussionsTrigger` and `DiscussionsSidebar` components additionally check the unit-level discussion topic (`topic?.id && topic?.enabledInContext`). This two-tier approach keeps the trigger visible across the course while gracefully rendering nothing for units without an active discussion topic. ## Exports @@ -38,3 +41,7 @@ const config = { ], }; ``` + +## Prefetch + +The widget defines a `prefetch` function that is called by `SidebarContextProvider` on mount. It dispatches `getCourseDiscussionTopics(courseId)` to preload discussion topic data into the Redux store, so that the `DiscussionsTrigger` and `DiscussionsSidebar` can read per-unit topic availability from the `discussionTopics` model without needing their own data-fetching side effects. diff --git a/src/widgets/discussions/widgetConfig.js b/src/widgets/discussions/widgetConfig.js index f443c2bed8..0c5ae1ad6b 100644 --- a/src/widgets/discussions/widgetConfig.js +++ b/src/widgets/discussions/widgetConfig.js @@ -1,7 +1,24 @@ +import { ensureConfig, getConfig } from '@edx/frontend-platform'; +import { getCourseDiscussionTopics } from '@src/courseware/data/thunks'; import DiscussionsSidebar from './DiscussionsSidebar'; import DiscussionsTrigger, { ID } from './DiscussionsTrigger'; -export const discussionsIsAvailable = ({ unit }) => !!(unit?.id && unit?.enabledInContext); +ensureConfig(['DISCUSSIONS_MFE_BASE_URL']); + +export const discussionsIsAvailable = ({ course }) => { + const baseUrl = getConfig().DISCUSSIONS_MFE_BASE_URL; + const hasDiscussionTab = course?.tabs?.some(tab => tab.slug === 'discussion'); + + return !!(baseUrl && hasDiscussionTab); +}; + +export const discussionsPrefetch = ({ courseId, course, dispatch }) => { + const baseUrl = getConfig().DISCUSSIONS_MFE_BASE_URL; + const discussionTab = course?.tabs?.find(tab => tab.slug === 'discussion'); + if (baseUrl && discussionTab) { + dispatch(getCourseDiscussionTopics(courseId)); + } +}; export const discussionsWidgetConfig = { id: ID, @@ -9,5 +26,6 @@ export const discussionsWidgetConfig = { Sidebar: DiscussionsSidebar, Trigger: DiscussionsTrigger, isAvailable: discussionsIsAvailable, + prefetch: discussionsPrefetch, enabled: true, };