feat: fetch exams data on the progress page (#1829) #38
Conversation
nsprenkle
requested review from
Copilot,
jansenk,
michaelroytman and
sundarthapa2u
* feat: fetch exams data on the progress page This commit adds changes to fetch the exams data associated with all subsections relevant to the progress page. Exams data is relevant to the progress page because the status of a learner's exam attempt may influence the state of their grade. This allows children of the root ProgressPage or downstream plugin slots to access this data from the Redux store. --------- Co-authored-by: nsprenkle <[email protected]>
nsprenkle
force-pushed
the
nsprenkle/progress-page-fetch-exam-info
branch
from
9f3cf85 to
2654d91
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to fetch exam attempts data for all subsections on the progress page. The exam data is fetched asynchronously and stored in Redux, making it accessible to child components and plugin slots that need to understand the state of learner exam attempts (which can influence grade states).
Key changes:
- Introduces a custom hook
useGetExamsDatathat triggers exam data fetching when the progress page loads or when section/subsection data changes - Adds Redux state management for exam data with a new
setExamsDataaction andexamsDatastate property - Implements
getExamsDataAPI function that supports both LMS and external exams service endpoints with proper error handling for 404 responses
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/course-home/progress-tab/ProgressTab.jsx | Integrates the exam data fetching hook and extracts sequence IDs from section scores for fetching |
| src/course-home/progress-tab/hooks.jsx | Implements useGetExamsData custom hook that dispatches exam data fetch on mount and when dependencies change |
| src/course-home/progress-tab/hooks.test.jsx | Comprehensive unit tests for the exam data fetching hook covering mount, re-render, and edge cases |
| src/course-home/progress-tab/ProgressTab.test.jsx | Integration tests verifying exam data fetching behavior within the full Progress Tab component |
| src/course-home/data/thunks.js | Adds fetchExamAttemptsData thunk that fetches exam data for multiple sequences in parallel with error handling |
| src/course-home/data/slice.js | Adds examsData to Redux state and setExamsData action for storing exam attempts data |
| src/course-home/data/slice.test.js | Unit tests for the new Redux reducer verifying exam data state management |
| src/course-home/data/redux.test.js | Redux integration tests for exam data fetching thunk covering success, error, and mixed response scenarios |
| src/course-home/data/api.js | Implements getExamsData API function with conditional URL construction and 404 error handling |
| src/course-home/data/api.test.js | Unit tests for the exam data API function covering both URL patterns, error cases, and parameter encoding |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sectionScores.flatMap((section) => (section.subsections)).map((subsection) => subsection.blockKey) | ||
| ), [sectionScores]); | ||
|
|
||
| useGetExamsData(courseId, sequenceIds); |
There was a problem hiding this comment.
The sequenceIds calculation will cause unnecessary re-fetches of exam data because arrays are recreated on every render even if the content is the same. The useEffect dependency in the hook will trigger on every array reference change. Consider using a more stable dependency mechanism, such as JSON.stringify in the dependency array or useMemo with a deep comparison, to prevent redundant API calls when the actual sequence IDs haven't changed.
| useGetExamsData(courseId, sequenceIds); | |
| useGetExamsData(courseId, useMemo(() => JSON.stringify(sequenceIds), [sequenceIds])); |
| const { data } = await getAuthenticatedHttpClient().get(url); | ||
| return camelCaseObject(data); | ||
| } catch (error) { | ||
| const { httpErrorStatus } = error && error.customAttributes; |
There was a problem hiding this comment.
The error handling for 404 status is fragile. The code attempts to destructure httpErrorStatus from error.customAttributes, but if error is falsy or customAttributes is undefined, this will fail silently and httpErrorStatus will be undefined, causing the 404 check to always fail. This means 404 errors might be thrown instead of returning an empty object. Add proper null/undefined checks before accessing nested properties.
| const { httpErrorStatus } = error && error.customAttributes; | |
| const httpErrorStatus = error?.customAttributes?.httpErrorStatus; |
| const { disableProgressGraph, sectionScores } = useModel('progress', courseId); | ||
|
|
||
| const sequenceIds = useMemo(() => ( | ||
| sectionScores.flatMap((section) => (section.subsections)).map((subsection) => subsection.blockKey) |
There was a problem hiding this comment.
The code doesn't handle cases where subsections might be undefined or missing, or where blockKey might not exist on a subsection object. This could cause runtime errors when mapping over subsections or accessing blockKey. Add defensive checks or optional chaining to handle potentially missing data.
| sectionScores.flatMap((section) => (section.subsections)).map((subsection) => subsection.blockKey) | |
| sectionScores | |
| .flatMap((section) => Array.isArray(section.subsections) ? section.subsections : []) | |
| .map((subsection) => subsection?.blockKey) | |
| .filter((blockKey) => blockKey !== undefined && blockKey !== null) |
3 participants
This commit adds changes to fetch the exams data associated with all subsections relevant to the progress page. Exams data is relevant to the progress page because the status of a learner's exam attempt may influence the state of their grade.
This allows children of the root ProgressPage or downstream plugin slots to access this data from the Redux store.
Cherry-pick of openedx#1829