feat: display optional completion information on the cards#48
feat: display optional completion information on the cards#48Agrendalath merged 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates learning path and course cards to surface “optional completion” information, based on additional completion data returned from the completion-aggregator API.
Changes:
- Extend course completion fetching/mapping to include
optionalCompletiondata. - Compute and attach optional-completion flags (
hasOptionalCompletion,hasUnearnedOptionalCompletion) to learning path and course view models. - Update LearningPathCard and CourseCard UI copy to distinguish required vs optional completion.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/learningpath/data/queries.js | Computes learning path optional-completion flags from the completions map; removes unused per-course completion query usage. |
| src/learningpath/data/dataUtils.js | Expands the completions map structure and propagates optional-completion flags onto course objects. |
| src/learningpath/data/api.js | Requests optional completion data from completion-aggregator and camelCases it into the client model. |
| src/learningpath/LearningPathCard.jsx | Displays required-vs-optional completion messaging on learning path cards. |
| src/learningpath/CourseCard.jsx | Displays required-vs-optional completion messaging on course cards. |
| package.json | Bumps package version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let hasOptionalCompletion = false; | ||
| let hasUnearnedOptionalCompletion = false; | ||
| const totalCompletion = lp.steps.reduce((sum, step) => { | ||
| const completion = completionsMap[step.courseKey]; | ||
| return sum + (completion?.percent ?? 0); | ||
| const completionData = completionsMap[step.courseKey]; | ||
| const optionalPossible = completionData?.optionalCompletion?.possible ?? 0; | ||
| const optionalEarned = completionData?.optionalCompletion?.earned ?? 0; | ||
| if (optionalPossible > 0) { | ||
| hasOptionalCompletion = true; | ||
| if (optionalPossible > optionalEarned) { | ||
| hasUnearnedOptionalCompletion = true; | ||
| } | ||
| } | ||
| return sum + (completionData?.completion?.percent ?? 0); |
There was a problem hiding this comment.
useLearningPaths uses reduce to compute totalCompletion but also mutates hasOptionalCompletion/hasUnearnedOptionalCompletion as side effects inside the reducer. This makes the reducer non-pure and harder to reason about (and easier to break if the logic changes). Consider replacing this with a single for...of loop (or a reduce that returns an object containing { totalCompletion, hasOptionalCompletion, hasUnearnedOptionalCompletion }) so the aggregation is explicit and side-effect free.
Agrendalath
force-pushed
the
agrendalath/bb-9331-optional-completion
branch
from
698663d to
c342389
Compare
See open-craft/openedx-platform#823.
Private-ref: BB-9331