feat: refactor program dashboard to use react query#12
feat: refactor program dashboard to use react query#12asharma12-sonata wants to merge 94 commits intomasterfrom
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…9.3 (openedx#739) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…edx#740) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…enedx#744) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Migrate from Redux to React Query and React Context. This modernizes state management while maintaining all existing functionality. All the redux code and files were removed, including all redux and related packages.
…nedx#803) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…edx#808) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…enedx#814) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…edx#820) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the learner program dashboard experience to be driven by React Query (instead of Redux) and wires a new /programs route into the app shell, with header updates to reflect active navigation state.
Changes:
- Added Program Dashboard “Programs List” UI (cards, progress bubbles, CTA) and associated styling/messages/tests.
- Introduced a React Query data hook for fetching program enrollments and conditionally enabled the
/programsroute via config. - Moved page
<Helmet>/ favicon + SR-only<h1>title responsibilities intoLearnerDashboardHeaderand adjusted menu active-state handling.
Reviewed changes
Copilot reviewed 26 out of 29 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.jsx | Adds header + conditional /programs route and React Query provider wiring. |
| src/index.test.jsx | Updates app registry mocks for new shell components/routes. |
| src/App.jsx | Simplifies App to only render <main> content (header/footer moved to app shell). |
| src/App.test.jsx | Removes assertions that depended on header/footer/helmet being inside App. |
| src/containers/LearnerDashboardHeader/index.jsx | Adds Helmet/favicons + SR-only page title and uses location for active tab logic. |
| src/containers/LearnerDashboardHeader/index.test.jsx | Adds coverage for SR-only title and active tab behavior. |
| src/containers/LearnerDashboardHeader/LearnerDashboardMenu.jsx | Makes menu isActive depend on current pathname; updates programs link when dashboard enabled. |
| src/containers/LearnerDashboardHeader/hooks.js | Threads pathname into menu construction hook. |
| src/containers/Dashboard/index.test.jsx | Removes page-title assertion now handled by header. |
| src/containers/AppWrapper/index.jsx | Removes wrapper component (no longer used). |
| src/containers/ProgramDashboard/index.tsx | Exposes ProgramsList container export. |
| src/containers/ProgramDashboard/ProgramsList/index.tsx | Implements Programs List page with loading/error/empty states. |
| src/containers/ProgramDashboard/ProgramsList/index.test.tsx | Adds tests for Programs List behavior (currently misaligned with implementation). |
| src/containers/ProgramDashboard/ProgramsList/ProgramListCard.tsx | Adds program card UI linking to program progress page and showing org/progress info. |
| src/containers/ProgramDashboard/ProgramsList/ProgramListCard.test.tsx | Adds tests for card rendering/linking/responsiveness. |
| src/containers/ProgramDashboard/ProgramsList/ProgressCategoryBubbles.tsx | Adds progress summary bubbles component. |
| src/containers/ProgramDashboard/ProgramsList/ProgressCategoryBubbles.test.tsx | Adds unit test for progress bubbles rendering. |
| src/containers/ProgramDashboard/ProgramsList/ExploreProgramsCTA.tsx | Adds CTA card/button for exploring programs/courses. |
| src/containers/ProgramDashboard/ProgramsList/ExploreProgramsCTA.test.tsx | Adds unit tests for CTA behavior and URL fallback. |
| src/containers/ProgramDashboard/ProgramsList/messages.ts | Adds i18n strings for Programs List UI and error messaging. |
| src/containers/ProgramDashboard/ProgramsList/index.scss | Adds truncate utility classes for card text. |
| src/containers/ProgramDashboard/data/api.ts | Adds React Query hook to fetch programs list from LMS endpoint. |
| src/containers/ProgramDashboard/data/api.test.ts | Adds API test (currently importing a non-exported symbol). |
| src/containers/ProgramDashboard/data/types.d.ts | Adds TypeScript types for program dashboard data structures. |
| src/containers/ProgramDashboard/api.ts | Adds standalone API function duplicating data/api.ts logic. |
| src/custom.d.ts | Adds TS module declaration for importing .png assets. |
| package.json | Bumps @edx/brand version. |
Comments suppressed due to low confidence (3)
src/App.jsx:24
- The variables from
useMasquerade()/useInitializeLearnerHome()are declared twice in this component (they are re-declared again a few lines below), which will throw a "Identifier has already been declared" syntax error. Remove the duplicate block and keep a single set of hook calls/derived values.
export const App = () => {
const { formatMessage } = useIntl();
const { masqueradeUser } = useMasquerade();
const { data, isError } = useInitializeLearnerHome();
const hasNetworkFailure = !masqueradeUser && isError;
const supportEmail = data?.platformSettings?.supportEmail || undefined;
src/App.jsx:30
- This second block re-declares
masqueradeUser,data,isError,hasNetworkFailure, andsupportEmailthat were already declared above, causing a runtime parse error. Delete this duplicate block (and keep the/* istanbul ignore next */only where it’s actually needed).
/* istanbul ignore next */
React.useEffect(() => {
if (getConfig().HOTJAR_APP_ID) {
try {
initializeHotjar({
src/index.jsx:33
QueryClient/QueryClientProviderandContextProvidersare imported twice, which will fail linting (and is easy to miss in reviews). Remove the duplicate import statements and keep a single import for each symbol.
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import ContextProviders from 'data/context';
import { configuration } from './config';
import messages from './i18n';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| getConfig, | ||
| mergeConfig, | ||
| } from '@edx/frontend-platform'; | ||
| import { FooterSlot } from '@edx/frontend-component-footer'; | ||
|
|
||
| import LearnerDashboardHeader from 'containers/LearnerDashboardHeader'; | ||
| import { ProgramsList } from './containers/ProgramDashboard'; |
There was a problem hiding this comment.
FooterSlot is imported but never rendered. Since FooterSlot was removed from App.jsx, this likely drops the footer entirely; either render <FooterSlot /> here (e.g., after Routes) or remove the import if the footer is intentionally removed.
| })); | ||
|
|
||
| jest.mock('containers/LearnerDashboardHeader', () => 'LearnerDashboardHeader'); | ||
| jest.mock('containers/ProgramDashboard', () => 'ProgramDashboard'); |
There was a problem hiding this comment.
This mock path doesn’t match the actual import in src/index.jsx (which imports ./containers/ProgramDashboard). As written, the real module will be loaded during the test, likely breaking the unit test isolation. Update the mock to target the same module specifier used in src/index.jsx (or change the import to use the aliased path consistently).
| jest.mock('containers/ProgramDashboard', () => 'ProgramDashboard'); | |
| jest.mock('./containers/ProgramDashboard', () => 'ProgramDashboard'); |
| import ProgramsList from '.'; | ||
| import { useProgramsListData } from '../data/api'; | ||
| import ProgramListCard from './ProgramListCard'; | ||
| import ExploreProgramsCTA from './ExploreProgramsCTA'; | ||
| import messages from './messages'; | ||
|
|
||
| // Mock API and external utilities | ||
| jest.mock('../data/api', () => ({ | ||
| useProgramsListData: jest.fn(), | ||
| })); | ||
| jest.mock('@edx/frontend-platform/logging', () => ({ |
There was a problem hiding this comment.
ProgramsList uses the useProgramsListData React Query hook, but this test mocks/calls getProgramsListData from ../data/api, which doesn’t exist in that module. Update the test to mock useProgramsListData and drive the component via { data, isLoading, isError } states (and remove the getProgramsListData expectations).
| describe('ProgramsList', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| // Set up a successful mock API response by default | ||
| (useProgramsListData as jest.Mock).mockResolvedValue(mockApiData); | ||
| }); | ||
|
|
||
| const renderComponent = () => render( | ||
| <IntlProvider locale="en"> | ||
| <ProgramsList /> | ||
| </IntlProvider>, | ||
| ); | ||
|
|
||
| it('renders header text and ExploreProgramsCTA', async () => { | ||
| renderComponent(); | ||
| await waitFor(() => { | ||
| expect(screen.getByText(messages.programsListHeaderText.defaultMessage)).toBeInTheDocument(); | ||
| expect(screen.getByTestId('explore-programs-cta')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('fetches program data on mount', async () => { | ||
| renderComponent(); | ||
|
|
||
| expect(useProgramsListData).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('renders ProgramListCard components upon successful API response', async () => { |
There was a problem hiding this comment.
The behavior in these tests (resolved/rejected promises and logError expectations) is based on an imperative getProgramsListData call, but the component doesn’t call such a function or logError directly. Once the tests are switched to mock useProgramsListData, adjust these cases to assert the rendered loading/error UI instead of expecting logError/promise handling here.
| import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; | ||
| import { useProgramsListData } from './api'; | ||
|
|
There was a problem hiding this comment.
./api (in this folder) exports useProgramsListData, not getProgramsListData, so this test won’t compile/pass. Either export a named fetcher (e.g., getProgramsListData) from data/api.ts and use it in the hook, or rewrite the test to validate the hook behavior via React Query (using QueryClientProvider and mocking the HTTP client).
| import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; | ||
| import { getConfig } from '@edx/frontend-platform'; | ||
|
|
||
| export async function useProgramsListData() { | ||
| const url = `${getConfig().LMS_BASE_URL}/api/dashboard/v0/programs/`; | ||
| const response = await getAuthenticatedHttpClient().get(url); | ||
| return response; | ||
| } |
There was a problem hiding this comment.
This standalone getProgramsListData duplicates the endpoint logic already implemented in data/api.ts and currently has no in-repo references. Consider deleting it to avoid dead code, or centralize on a single fetch function that both the React Query hook and any callers/tests share.
| import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; | |
| import { getConfig } from '@edx/frontend-platform'; | |
| export async function useProgramsListData() { | |
| const url = `${getConfig().LMS_BASE_URL}/api/dashboard/v0/programs/`; | |
| const response = await getAuthenticatedHttpClient().get(url); | |
| return response; | |
| } |
| <Card | ||
| className="program-list-card" | ||
| isClickable | ||
| as={Link} | ||
| to={`${getConfig().LMS_BASE_URL}/dashboard/programs/${program.uuid}`} | ||
| data-testid="program-list-card" |
There was a problem hiding this comment.
react-router-dom’s Link is intended for in-app navigation; passing an absolute LMS URL (from LMS_BASE_URL) can lead to incorrect href generation or blocked navigation when the LMS is on a different origin. Prefer rendering an anchor (as="a"/href) for external navigation, or use a relative route (e.g., /programs/...) if this is meant to stay within the MFE.
ae495ae to
6d961e6
Compare
This PR includes changes from openedx#751
Updated program dashboard code that was earlier using redux now using react query.