feat(profile): limit language logos to 5 in course progress items #2705
Conversation
- Display max 5 language logos per course progress item - Prioritize completed languages first - Sort by recency (completed or last submission date) - Show recent incomplete languages if fewer than 5 completed ones 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
WalkthroughThis pull request updates the course progress list item component. The Handlebars template now iterates over a newly computed list, Changes
Sequence Diagram(s)sequenceDiagram
participant Component as CourseProgressListItemComponent
participant Data as Participation Data
participant Template as Handlebars Template
participant Logo as LanguageLogo Component
Component->>Data: Retrieve course participation data
Data-->>Component: Return data set
Component->>Component: Compute languagesToDisplay (sort & filter, limit to 5)
Component-->>Template: Provide languagesToDisplay
Template->>Template: Iterate over languagesToDisplay with "each"
Template->>Logo: Render LanguageLogo with isCompleted check
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Bundle ReportChanges will increase total bundle size by 2.12kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: client-array-pushAssets Changed:
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/user-page/course-progress-list-item/index.js (1)
41-43: Consider adding a reusable date comparison functionThe date comparison logic is duplicated in both sorting operations. Consider extracting this to a reusable helper function for better maintainability.
+ // Helper function for date comparisons + const compareByDate = (dateField) => (a, b) => + new Date(b[dateField]).getTime() - new Date(a[dateField]).getTime(); get languagesToDisplay() { // First, get all completed course participations sorted by completion time (most recent first) - const completedParticipations = this.completedCourseParticipations.sort( - (a, b) => new Date(b.completedAt).getTime() - new Date(a.completedAt).getTime() - ); + const completedParticipations = this.completedCourseParticipations.sort(compareByDate('completedAt')); // Next, get incomplete participations sorted by activity time (most recent first) const incompleteParticipations = this.args.courseParticipations .filter(p => !p.isCompleted) - .sort((a, b) => new Date(b.lastSubmissionAt).getTime() - new Date(a.lastSubmissionAt).getTime()); + .sort(compareByDate('lastSubmissionAt'));🧰 Tools
🪛 ESLint
[error] 42-42: Insert
,(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/components/user-page/course-progress-list-item/index.hbs(1 hunks)app/components/user-page/course-progress-list-item/index.js(2 hunks)
🧰 Additional context used
🪛 ESLint
app/components/user-page/course-progress-list-item/index.js
[error] 42-42: Insert ,
(prettier/prettier)
[error] 44-44: Delete ····
(prettier/prettier)
[error] 47-47: Replace p with (p)
(prettier/prettier)
[error] 49-49: Delete ····
(prettier/prettier)
[error] 54-54: Delete ····
(prettier/prettier)
[error] 56-59: Replace ⏎······...completedParticipations,⏎······...incompleteParticipations.slice(0,·MAX_LANGUAGES_TO_DISPLAY·-·completedParticipations.length)⏎···· with ...completedParticipations,·...incompleteParticipations.slice(0,·MAX_LANGUAGES_TO_DISPLAY·-·completedParticipations.length)
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
app/components/user-page/course-progress-list-item/index.hbs (1)
48-48: Good implementation of the language logo limitation requirementThe change from
@courseParticipationstothis.languagesToDisplaycorrectly implements the requirement to limit and prioritize language logos as specified in the PR objectives.app/components/user-page/course-progress-list-item/index.js (2)
6-6: Good use of a named constant for the language limitUsing a constant
MAX_LANGUAGES_TO_DISPLAYmakes the code more maintainable and the intention clear.
46-48:❓ Verification inconclusive
Verify that lastSubmissionAt is always present
The code assumes
lastSubmissionAtis always available on incomplete participations. If this field could be null or undefined for new participations with no submissions, it might cause sorting issues.
🏁 Script executed:
#!/bin/bash # Check if there are any course participations without lastSubmissionAt rg -A2 -B2 "lastSubmissionAt" --include="*.js" --include="*.ts"Length of output: 505
Action: Confirm that every incomplete course participation has a valid
lastSubmissionAtvalue
- The sorting logic in the component assumes that
lastSubmissionAtis always present. If any participation is missing this field (i.e., it is null or undefined), sorting could produce unexpected results.- Please manually verify through your data source or schema definition that every incomplete course participation includes a valid
lastSubmissionAt. If there's any chance that new participations might not have submissions (and thus lack this field), consider handling such cases (e.g., filtering them out or providing a default value) before sorting.🧰 Tools
🪛 ESLint
[error] 47-47: Replace
pwith(p)(prettier/prettier)
| get languagesToDisplay() { | ||
| // First, get all completed course participations sorted by completion time (most recent first) | ||
| const completedParticipations = this.completedCourseParticipations.sort( | ||
| (a, b) => new Date(b.completedAt).getTime() - new Date(a.completedAt).getTime() | ||
| ); | ||
|
|
||
| // Next, get incomplete participations sorted by activity time (most recent first) | ||
| const incompleteParticipations = this.args.courseParticipations | ||
| .filter(p => !p.isCompleted) | ||
| .sort((a, b) => new Date(b.lastSubmissionAt).getTime() - new Date(a.lastSubmissionAt).getTime()); | ||
|
|
||
| // If we have 5 or fewer completed participations, show all of them | ||
| if (completedParticipations.length >= MAX_LANGUAGES_TO_DISPLAY) { | ||
| return completedParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY); | ||
| } | ||
|
|
||
| // Otherwise, show all completed participations + recent incomplete ones (up to 5 total) | ||
| return [ | ||
| ...completedParticipations, | ||
| ...incompleteParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY - completedParticipations.length) | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implementation correctly satisfies all requirements, but has formatting issues
The implementation correctly:
- Prioritizes completed languages
- Sorts by recency (completion date or last submission date)
- Displays incomplete languages if fewer than 5 completed ones exist
However, there are some formatting issues flagged by ESLint that should be fixed.
get languagesToDisplay() {
// First, get all completed course participations sorted by completion time (most recent first)
const completedParticipations = this.completedCourseParticipations.sort(
- (a, b) => new Date(b.completedAt).getTime() - new Date(a.completedAt).getTime()
+ (a, b) => new Date(b.completedAt).getTime() - new Date(a.completedAt).getTime(),
);
// Next, get incomplete participations sorted by activity time (most recent first)
const incompleteParticipations = this.args.courseParticipations
- .filter(p => !p.isCompleted)
+ .filter((p) => !p.isCompleted)
.sort((a, b) => new Date(b.lastSubmissionAt).getTime() - new Date(a.lastSubmissionAt).getTime());
// If we have 5 or fewer completed participations, show all of them
if (completedParticipations.length >= MAX_LANGUAGES_TO_DISPLAY) {
return completedParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY);
}
// Otherwise, show all completed participations + recent incomplete ones (up to 5 total)
- return [
- ...completedParticipations,
- ...incompleteParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY - completedParticipations.length)
- ];
+ return [...completedParticipations, ...incompleteParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY - completedParticipations.length)];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| get languagesToDisplay() { | |
| // First, get all completed course participations sorted by completion time (most recent first) | |
| const completedParticipations = this.completedCourseParticipations.sort( | |
| (a, b) => new Date(b.completedAt).getTime() - new Date(a.completedAt).getTime() | |
| ); | |
| // Next, get incomplete participations sorted by activity time (most recent first) | |
| const incompleteParticipations = this.args.courseParticipations | |
| .filter(p => !p.isCompleted) | |
| .sort((a, b) => new Date(b.lastSubmissionAt).getTime() - new Date(a.lastSubmissionAt).getTime()); | |
| // If we have 5 or fewer completed participations, show all of them | |
| if (completedParticipations.length >= MAX_LANGUAGES_TO_DISPLAY) { | |
| return completedParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY); | |
| } | |
| // Otherwise, show all completed participations + recent incomplete ones (up to 5 total) | |
| return [ | |
| ...completedParticipations, | |
| ...incompleteParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY - completedParticipations.length) | |
| ]; | |
| } | |
| get languagesToDisplay() { | |
| // First, get all completed course participations sorted by completion time (most recent first) | |
| const completedParticipations = this.completedCourseParticipations.sort( | |
| (a, b) => new Date(b.completedAt).getTime() - new Date(a.completedAt).getTime(), | |
| ); | |
| // Next, get incomplete participations sorted by activity time (most recent first) | |
| const incompleteParticipations = this.args.courseParticipations | |
| .filter((p) => !p.isCompleted) | |
| .sort((a, b) => new Date(b.lastSubmissionAt).getTime() - new Date(a.lastSubmissionAt).getTime()); | |
| // If we have 5 or fewer completed participations, show all of them | |
| if (completedParticipations.length >= MAX_LANGUAGES_TO_DISPLAY) { | |
| return completedParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY); | |
| } | |
| // Otherwise, show all completed participations + recent incomplete ones (up to 5 total) | |
| return [...completedParticipations, ...incompleteParticipations.slice(0, MAX_LANGUAGES_TO_DISPLAY - completedParticipations.length)]; | |
| } |
🧰 Tools
🪛 ESLint
[error] 42-42: Insert ,
(prettier/prettier)
[error] 44-44: Delete ····
(prettier/prettier)
[error] 47-47: Replace p with (p)
(prettier/prettier)
[error] 49-49: Delete ····
(prettier/prettier)
[error] 54-54: Delete ····
(prettier/prettier)
[error] 56-59: Replace ⏎······...completedParticipations,⏎······...incompleteParticipations.slice(0,·MAX_LANGUAGES_TO_DISPLAY·-·completedParticipations.length)⏎···· with ...completedParticipations,·...incompleteParticipations.slice(0,·MAX_LANGUAGES_TO_DISPLAY·-·completedParticipations.length)
(prettier/prettier)
Checklist:
[percy]in the message to trigger)Summary by CodeRabbit
New Features
Refactor