-
Notifications
You must be signed in to change notification settings - Fork 2
contribution leaderboard kinda working #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
jsb26
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds an admin-only student contribution leaderboard (global and per-category) showing topic/post counts with pagination and category filtering. Key functionality added includes backend data aggregation endpoints, an admin controller & routes, and a new UI with translations and navigation integration.
- Adds category and global leaderboard data retrieval (posts + topics, ranked)
- Introduces admin UI with category selector and pagination
- Adds navigation entry and translation resources (only en-GB for new page)
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/views/admin/partials/navigation.tpl | Adds sidebar link to new leaderboard page gated by category admin privilege |
| src/views/admin/manage/leaderboard.tpl | New template rendering leaderboard table, selector, and pagination |
| src/routes/admin.js | Registers two admin routes for global and category leaderboards |
| src/controllers/admin/leaderboard.js | Controller for assembling leaderboard view data and pagination |
| src/controllers/admin.js | Exports new leaderboard controller |
| src/categories/leaderboard.js | Implements data collection for category/global leaderboards |
| src/categories/index.js | Registers leaderboard module on Categories |
| public/src/admin/manage/leaderboard.js | Client-side init for category selector navigation |
| public/language/en-US/admin/menu.json | Adds menu label for leaderboard |
| public/language/en-GB/admin/menu.json | Adds menu label for leaderboard |
| public/language/en-GB/admin/manage/leaderboard.json | Adds translation strings for leaderboard page (no en-US equivalent added) |
| LEADERBOARD_FEATURE.md | Feature documentation added |
| IMPLEMENTATION_SUMMARY.md | Implementation summary documentation added |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Apply pagination | ||
| const paginatedData = start !== undefined && stop !== undefined ? | ||
| userData.slice(start, stop + 1) : userData; | ||
|
|
||
| return paginatedData; |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The slice here removes total length information needed later for accurate pagination; the controller then guesses total users, producing incorrect page counts. Return both the full length (e.g. userData.length) and the sliced window, or move pagination responsibility entirely to the controller so it can compute totalUsers precisely.
| [leaderboardData, categoryData, selectedData] = await Promise.all([ | ||
| categories.getLeaderboard(cid, start, stop), | ||
| categories.getCategoryData(cid), | ||
| helpers.getSelectedCategory(cid), | ||
| ]); | ||
| pageTitle = categoryData ? categoryData.name : 'Category Leaderboard'; | ||
| } else { | ||
| // Global leaderboard | ||
| [leaderboardData, selectedData] = await Promise.all([ | ||
| categories.getGlobalLeaderboard(start, stop), | ||
| helpers.getSelectedCategory(0), | ||
| ]); | ||
| } | ||
|
|
||
| // Calculate total count for pagination | ||
| const totalUsers = leaderboardData.length > 0 ? | ||
| (leaderboardData[leaderboardData.length - 1].rank + itemsPerPage) : 0; |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
totalUsers is inferred from the last rank of an already paginated subset plus itemsPerPage, which inflates counts after the first page and miscomputes pageCount. Use the real total length from the unsliced dataset (see getLeaderboard/getGlobalLeaderboard) or have those functions return { data, total } to derive pageCount = Math.ceil(total/itemsPerPage).
| [leaderboardData, categoryData, selectedData] = await Promise.all([ | |
| categories.getLeaderboard(cid, start, stop), | |
| categories.getCategoryData(cid), | |
| helpers.getSelectedCategory(cid), | |
| ]); | |
| pageTitle = categoryData ? categoryData.name : 'Category Leaderboard'; | |
| } else { | |
| // Global leaderboard | |
| [leaderboardData, selectedData] = await Promise.all([ | |
| categories.getGlobalLeaderboard(start, stop), | |
| helpers.getSelectedCategory(0), | |
| ]); | |
| } | |
| // Calculate total count for pagination | |
| const totalUsers = leaderboardData.length > 0 ? | |
| (leaderboardData[leaderboardData.length - 1].rank + itemsPerPage) : 0; | |
| const [leaderboardResult, categoryData, selectedData] = await Promise.all([ | |
| categories.getLeaderboard(cid, start, stop), | |
| categories.getCategoryData(cid), | |
| helpers.getSelectedCategory(cid), | |
| ]); | |
| leaderboardData = leaderboardResult.data; | |
| var totalUsers = leaderboardResult.total; | |
| pageTitle = categoryData ? categoryData.name : 'Category Leaderboard'; | |
| } else { | |
| // Global leaderboard | |
| const [leaderboardResult, selectedData] = await Promise.all([ | |
| categories.getGlobalLeaderboard(start, stop), | |
| helpers.getSelectedCategory(0), | |
| ]); | |
| leaderboardData = leaderboardResult.data; | |
| var totalUsers = leaderboardResult.total; | |
| } | |
| // Calculate total count for pagination |
| const [topicCreators, postAuthors] = await Promise.all([ | ||
| db.getSortedSetRevRange(`cid:${cid}:uid`, 0, -1), | ||
| db.getSortedSetRevRange(`cid:${cid}:pids`, 0, -1), | ||
| ]); | ||
|
|
||
| // Get unique UIDs from posts | ||
| const pids = postAuthors; | ||
| const postUids = pids.length ? await db.getObjectsFields( | ||
| pids.map(pid => `post:${pid}`), | ||
| ['uid'] | ||
| ).then(posts => posts.map(p => p.uid)) : []; |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Fetching all pids and then all corresponding post objects for every leaderboard request can become O(N) over all posts in large categories, causing high memory and latency. Consider maintaining (or leveraging existing) per-category user contribution sorted sets (e.g. increment counts on post/topic create) to avoid scanning all posts each time.
| userData.forEach((user, index) => { | ||
| user.rank = index + 1; | ||
| }); |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The loop variable name user shadows the imported user module in this file, which can reduce readability and increase risk of accidental misuse. Rename the iteration variable to entry or row to avoid shadowing.
| <tr> | ||
| <td class="text-center"> | ||
| {{{ if @first }}} | ||
| <span class="badge text-bg-warning fs-6">#{@value.rank}</span> |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Documentation (LEADERBOARD_FEATURE.md and IMPLEMENTATION_SUMMARY.md) states different color coding for ranks 2–3, but the template only distinguishes rank 1 vs others. Either update docs to match this implementation or extend the template logic to apply the described styling for ranks 2 and 3.
| <span class="badge text-bg-warning fs-6">#{@value.rank}</span> | |
| <span class="badge text-bg-warning fs-6">#{@value.rank}</span> | |
| {{{ else if @index == 1 }}} | |
| <span class="badge text-bg-success fs-6">#{@value.rank}</span> | |
| {{{ else if @index == 2 }}} | |
| <span class="badge text-bg-info fs-6">#{@value.rank}</span> |
|
|
||
| <div class="d-flex border-bottom py-2 m-0 sticky-top acp-page-main-header align-items-center justify-content-between flex-wrap gap-2"> | ||
| <div class=""> | ||
| <h4 class="fw-bold tracking-tight mb-0">[[admin/manage/leaderboard:title]]</h4> |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The translation keys for the leaderboard page exist only in en-GB (public/language/en-GB/admin/manage/leaderboard.json); an equivalent en-US file was not added, which will cause untranslated placeholders for en-US locale users. Add public/language/en-US/admin/manage/leaderboard.json with matching keys to maintain localization completeness.
| categories.getCategoryData(cid), | ||
| helpers.getSelectedCategory(cid), | ||
| ]); | ||
| pageTitle = categoryData ? categoryData.name : 'Category Leaderboard'; |
Copilot
AI
Oct 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The fallback string 'Category Leaderboard' is hard-coded and not internationalized, unlike other page text using translation tokens; this results in inconsistent localization. Replace with a translation key (e.g. [[admin/manage/leaderboard:category-fallback]]) and add it to the language file(s).
| pageTitle = categoryData ? categoryData.name : 'Category Leaderboard'; | |
| pageTitle = categoryData ? categoryData.name : '[[admin/manage/leaderboard:category-fallback]]'; |

Issue: #147
Add Student Contribution Leaderboard for Professors
Summary
Adds an admin-only leaderboard feature that allows professors to track and view student participation by monitoring posts and topics created within categories.
Features
admin:categoriesprivilegeChanges
Backend:
src/categories/leaderboard.js- Core leaderboard data retrievalsrc/controllers/admin/leaderboard.js- Admin controllersrc/routes/admin.jsFrontend:
src/views/admin/manage/leaderboard.tpl- Responsive table UIpublic/src/admin/manage/leaderboard.js- Client-side handlerpublic/language/en-GB/admin/manage/leaderboard.jsonScreenshots
(Add screenshots of the leaderboard here)
Testing
Access: Navigate to Admin Panel → Manage → Contribution Leaderboard