Conversation
|
Visit the preview URL for this PR (updated for commit 048068c): https://extendafamily-7613e--pr182-jimmy-learner-progre-jdskjm1n.web.app (expires Sun, 15 Feb 2026 22:57:18 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f8bb7b5cd838ab636347dc54fd0ab08ab3715d31 |
bb5204a to
ff64d02
Compare
ff64d02 to
fd1cdf5
Compare
c3bc52f to
56a7821
Compare
5af69d1 to
7cb6bcc
Compare
d39031d to
4f3d7df
Compare
- Learner MC view option color states - PDF type for uploads - Admin last page press next button opens up Add page menu
f1bc7a9 to
b238eae
Compare
b238eae to
fc4a2de
Compare
bddcb9d to
eb97d4b
Compare
001457c to
048068c
Compare
There was a problem hiding this comment.
Pull request overview
Implements “Learner Progress” across the stack by introducing backend progress tracking APIs/models, surfacing progress in frontend contexts and UI, and adding a new Text Input activity type plus admin module-editing locking to reduce editing conflicts.
Changes:
- Added learner progress persistence + REST APIs (course/module/activity completion, last-viewed page) and facilitator learner-list enrichment with progress.
- Added new
TextInputactivity type (types, backend creation/mapping, authoring/editor + learner viewer + preview support). - Updated module viewing UX: activity completion display + feedback gating, publish CTA, and socket-based admin editing lock flow.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/types/UserTypes.ts | Adds optional per-user course progress summary type. |
| frontend/src/types/CourseTypes.ts | Renames/extends question types and adds TextInput activity typing/guards. |
| frontend/src/hooks/useCourseModules.ts | Adds module list caching in hook. |
| frontend/src/hooks/useActivity.ts | Restricts autosave updates based on user role. |
| frontend/src/contexts/FeedbacksContext.tsx | Refetches feedbacks when authenticated user changes. |
| frontend/src/contexts/CourseUnitsContext.tsx | Adds progress state + helpers (completed modules/dates) in course units context. |
| frontend/src/constants/ActivityLabels.tsx | Updates icons/labels for updated question types. |
| frontend/src/components/user_management/UserTable.tsx | Displays learner course progress percentage in user management table. |
| frontend/src/components/profile/ProfilePicture.tsx | Adds hover overlay UX for clickable avatar upload. |
| frontend/src/components/pages/ViewModulePage.tsx | Adds progress completion handling, TextInput support, publish CTA, and module editing lock UX. |
| frontend/src/components/pages/ModuleLockedModal.tsx | New modal when module editing lock is held by another admin. |
| frontend/src/components/pages/Home.tsx | Layout adjustments for learner home page sections. |
| frontend/src/components/pages/FinishedModules.tsx | Filters/labels finished modules using progress context. |
| frontend/src/components/learners/NavButton.tsx | Switches to react-router Link navigation and updates sizing. |
| frontend/src/components/learners/HomePageSidebar.tsx | Displays real progress percentage in progress ring. |
| frontend/src/components/learners/CourseCard.tsx | Adjusts card sizing for layout. |
| frontend/src/components/finished_modules/UnitSection.tsx | Plumbs module completion date getter down to grid. |
| frontend/src/components/finished_modules/ModulesGrid.tsx | Passes completion date into module cards. |
| frontend/src/components/finished_modules/FinishedModulesContent.tsx | Wires completion date getter through finished modules UI. |
| frontend/src/components/feedback/feedback-admin-view/FeedbackAdminView.tsx | Layout adjustments to make header/content stretch correctly. |
| frontend/src/components/courses/moduleViewing/learner-giving-feedback/SurveySlides.tsx | Prevents duplicate feedback submission by checking prior feedback. |
| frontend/src/components/course_viewing/text-input/TextInputViewer.tsx | New learner viewer for TextInput activity. |
| frontend/src/components/course_viewing/table/TableViewer.tsx | Supports externally-controlled “completed” state and correct-answer display. |
| frontend/src/components/course_viewing/sidebar/UnitSidebar.tsx | Adds loading state handling from context for unit sidebar. |
| frontend/src/components/course_viewing/multiple-choice/MultipleChoiceViewer.tsx | Supports externally-controlled “completed” state and correct-answer display. |
| frontend/src/components/course_viewing/multiple-choice/MultipleChoiceViewOption.tsx | Visual styling updates for selected/correct states and interactions. |
| frontend/src/components/course_viewing/modals/WrongAnswerModal.tsx | Stabilizes randomized title per open and memoizes it. |
| frontend/src/components/course_viewing/matching/MatchingViewer.tsx | Supports externally-controlled “completed” state and preserves shuffle across re-renders. |
| frontend/src/components/course_viewing/library-viewing/ModuleCardLearner.tsx | Displays “finished on” date when provided. |
| frontend/src/components/course_viewing/library-viewing/ModuleCardAdmin.tsx | Improves thumbnail image styling/fit. |
| frontend/src/components/course_viewing/ModuleSidebarThumbnails.tsx | Placeholder export-only file added. |
| frontend/src/components/course_viewing/CourseViewingPage.tsx | Layout/search field sizing adjustments and import ordering. |
| frontend/src/components/course_viewing/CourseModulesGrid.tsx | Improves module fetch error messaging. |
| frontend/src/components/course_authoring/text-input/TextInputSidebar.tsx | New authoring sidebar for TextInput configuration. |
| frontend/src/components/course_authoring/text-input/TextInputEditor.tsx | New TextInput editor main canvas. |
| frontend/src/components/course_authoring/editorComponents/TypographyTextField.tsx | Adds HeaderLargeTextField variant for authoring UI. |
| frontend/src/components/course_authoring/editorComponents/PreviewLearnerModal.tsx | Adds learner preview interactions + supports TextInput. |
| frontend/src/components/course_authoring/EmptyModuleEditing.tsx | PDF accept filter + activity menu styling tweaks. |
| frontend/src/components/common/navbar/UserButton.tsx | Uses user profile picture; rearranges account/logout actions. |
| frontend/src/components/auth/SignupPendingPage.tsx | Changes “refresh” flow to retry/login reset behavior. |
| frontend/src/components/auth/ResetPassword.tsx | Adds icon + themed styling for reset password action. |
| frontend/src/components/auth/MyAccountButton.tsx | Adds icon + role-based color styling. |
| frontend/src/components/auth/Logout.tsx | Adds icon + error-color styling. |
| frontend/src/components/auth/CreatePasswordPage.tsx | Alters post-password-change behavior (logout vs local status update). |
| frontend/src/components/auth/CreatePasswordConfirmationPage.tsx | Adjusts redirect flow and clears auth context. |
| frontend/src/APIClients/UserAPIClient.ts | Fixes error message for getCurrentUser failure. |
| frontend/src/APIClients/ProgressAPIClient.ts | New API client for progress endpoints. |
| frontend/src/APIClients/FeedbackAPIClient.ts | Adds hasFeedback check helper. |
| frontend/src/APIClients/AuthAPIClient.ts | Removes auth storage key earlier during logout flow. |
| frontend/src/APIClients/ActivityAPIClient.ts | Adds update failure alert and hasFeedback helper. |
| backend/utilities/activityModelMapper.ts | Adds TextInput activity model mapping. |
| backend/types/progressTypes.ts | Adds DTOs for progress tracking and progress-enriched views. |
| backend/types/activityTypes.ts | Removes unused Custom question type. |
| backend/sockets/moduleEditing.ts | Adds socket-based module editing lock handlers. |
| backend/services/interfaces/learnerProgressService.ts | New progress service interface. |
| backend/services/interfaces/feedbackService.ts | Adds hasFeedback to feedback service interface. |
| backend/services/implementations/userService.ts | Adjusts status update payload and facilitator approval sets status active. |
| backend/services/implementations/learnerProgressService.ts | New service implementing progress tracking and aggregation. |
| backend/services/implementations/feedbackService.ts | Implements hasFeedback query. |
| backend/services/implementations/courseModuleService.ts | Uses populated pages and simplifies module retrieval. |
| backend/services/implementations/activityService.ts | Initializes TextInput activity data on create. |
| backend/server.ts | Registers module editing socket handlers and mounts progress router. |
| backend/rest/userRoutes.ts | Enriches facilitator learners list with course progress. |
| backend/rest/progressRoutes.ts | New progress REST API routes. |
| backend/rest/feedbackRoutes.ts | Adds /feedbacks/check route. |
| backend/rest/courseRoutes.ts | Includes module progress in learner responses (module + unit module lists). |
| backend/rest/activityRoutes.ts | Adds completion status on activity fetch for learners. |
| backend/package.json | Removes engines constraint. |
| backend/models/learnerprogress.mgmodel.ts | New Mongoose model for learner progress. |
| backend/models/activity.mgmodel.ts | Adds units field to TextInput activity model. |
| backend/emails/learnerInvite.ts | Updates logo asset usage and dimensions. |
| backend/emails/forgotPassword.ts | Updates logo asset usage and dimensions. |
| backend/emails/facilitatorVerification.ts | Updates logo asset usage and dimensions. |
| backend/emails/facilitatorRejected.ts | Updates logo asset usage and dimensions. |
| backend/emails/facilitatorApproved.ts | Updates logo asset usage and dimensions. |
| backend/emails/constants.ts | Updates logo URL + introduces logo width/height constants. |
| backend/emails/adminInvite.ts | Updates logo asset usage and dimensions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return () => { | ||
| if (hasEditingLock) { | ||
| socket.emit("moduleEditing:releaseLock", { | ||
| moduleId: requestedModuleId, | ||
| userId, |
There was a problem hiding this comment.
Cleanup only emits moduleEditing:releaseLock when hasEditingLock is true, but because the effect re-runs on hasEditingLock changes, the cleanup may run with hasEditingLock=false and never release the server-side lock. Use a ref that is updated in the lock-acquired handler (or always emit release for the module/user on unmount/module change).
| - requires moduleId and learnerId in query params | ||
| - Ex. /feedbacks/check?moduleId=123&learnerId=456 | ||
| */ | ||
| feedbackRouter.get( | ||
| "/check", | ||
| isAuthorizedByRole(new Set(["Learner"])), | ||
| async (req, res) => { | ||
| try { | ||
| const { moduleId, learnerId } = req.query; | ||
| if (!moduleId || !learnerId) { | ||
| res.status(400).send("moduleId and learnerId are required"); | ||
| return; | ||
| } | ||
| const hasFeedback = await feedbackService.hasFeedback( | ||
| learnerId as string, |
There was a problem hiding this comment.
Security: /feedbacks/check trusts learnerId from query params. Any authenticated learner could supply a different learnerId and probe other learners’ feedback status. Derive the learnerId from the access token on the server (and ignore/omit learnerId from the request).
| - requires moduleId and learnerId in query params | |
| - Ex. /feedbacks/check?moduleId=123&learnerId=456 | |
| */ | |
| feedbackRouter.get( | |
| "/check", | |
| isAuthorizedByRole(new Set(["Learner"])), | |
| async (req, res) => { | |
| try { | |
| const { moduleId, learnerId } = req.query; | |
| if (!moduleId || !learnerId) { | |
| res.status(400).send("moduleId and learnerId are required"); | |
| return; | |
| } | |
| const hasFeedback = await feedbackService.hasFeedback( | |
| learnerId as string, | |
| - requires moduleId in query params | |
| - Ex. /feedbacks/check?moduleId=123 | |
| */ | |
| feedbackRouter.get( | |
| "/check", | |
| isAuthorizedByRole(new Set(["Learner"])), | |
| async (req, res) => { | |
| try { | |
| const { moduleId } = req.query; | |
| const learnerIdFromToken = (req as any).user?.id; | |
| if (!moduleId) { | |
| res.status(400).send("moduleId is required"); | |
| return; | |
| } | |
| if (!learnerIdFromToken) { | |
| res.status(401).send("Authenticated learner not found"); | |
| return; | |
| } | |
| const hasFeedback = await feedbackService.hasFeedback( | |
| learnerIdFromToken as string, |
| const refetchCourseProgress = async () => { | ||
| try { | ||
| const [progress, fullProgress] = await Promise.all([ | ||
| ProgressAPIClient.getCourseProgress(), | ||
| ProgressAPIClient.getLearnerProgress(), |
There was a problem hiding this comment.
When refetchCourseProgress fails (or when authenticatedUser becomes null), courseProgress/learnerProgress are left as-is, which can display stale progress from a previous user/session. Clear these states when there is no authenticated user and/or before starting the fetch, and consider setting them to null on failure.
| useEffect(() => { | ||
| refetchCourseUnits(); | ||
| }, []); | ||
| refetchCourseProgress(); | ||
| }, [authenticatedUser]); |
There was a problem hiding this comment.
This effect refetches units/progress on authenticatedUser changes, but it does not reset progress state when the user logs out. If the progress fetch fails due to missing/expired token, stale progress will remain visible. Consider clearing progress state when authenticatedUser is falsy before calling the APIs.
| // Module-level cache so it persists across component remounts and unitId changes | ||
| const moduleDataCache: Record<string, CourseModule[]> = {}; | ||
|
|
||
| const useCourseModules = (unitId: string) => { | ||
| const [courseModules, setCourseModules] = useState<CourseModule[]>([]); | ||
| const [loading, setLoading] = useState<boolean>(true); | ||
| const cached = moduleDataCache[unitId]; | ||
| const [courseModules, setCourseModules] = useState<CourseModule[]>( | ||
| cached ?? [], | ||
| ); | ||
| const [loading, setLoading] = useState<boolean>(!cached); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| if (!unitId) return; // Prevent API call if unitId is empty | ||
|
|
||
| const fetchCourseModules = async () => { | ||
| try { | ||
| setLoading(true); | ||
| setError(null); | ||
|
|
||
| const data = await CourseAPIClient.getModules(unitId); | ||
| setCourseModules(data); | ||
| } catch (err: unknown) { | ||
| if (err instanceof Error) { | ||
| setError(err.message); | ||
| } else { | ||
| setError("An unknown error occurred"); | ||
| } | ||
| } finally { | ||
| setLoading(false); | ||
| const fetchCourseModules = useCallback(async () => { | ||
| if (moduleDataCache[unitId]) { | ||
| setCourseModules(moduleDataCache[unitId]); | ||
| setLoading(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The module-level cache has no invalidation path, so after creating/deleting/reordering modules (or any server-side update), navigating away and back will reuse stale moduleDataCache[unitId] and skip refetching. Consider either removing this cache, adding a TTL, or exposing a way to bypass/clear it (e.g., refetch({ force: true }) or clearing the entry when mutations occur).
| }, [ | ||
| role, | ||
| socket, | ||
| requestedModuleId, | ||
| userId, |
There was a problem hiding this comment.
The editing-lock effect depends on hasEditingLock, which will cause the effect to re-run when the lock is acquired and emit moduleEditing:acquireLock multiple times. It also makes cleanup logic use a potentially stale hasEditingLock value. Remove hasEditingLock from the dependency list and track lock ownership in a ref (or always attempt a best-effort release on cleanup).
| const sendActivityUpdate = useCallback(() => { | ||
| if (!hasUnsavedChanges.current || !activityRef.current) { | ||
| if ( | ||
| role !== "Administrator" || | ||
| !hasUnsavedChanges.current || | ||
| !activityRef.current | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
Autosave is gated only by role === "Administrator", so administrators without the new editing lock can still send updateActivity requests. Also, hasUnsavedChanges is set based on object reference inequality, so setActivity(currentPageObject) can mark unsaved changes even when just loading/viewing an activity, triggering an unintended PATCH after 2s. Consider adding an enabled/canEdit flag to useActivity and resetting lastSentActivityRef/hasUnsavedChanges when loading an activity from the server.
| const handleBackToCourse = () => { | ||
| if (unitId) { | ||
| window.location.href = `${Routes.COURSE_PAGE}?unitId=${unitId}`; | ||
| } else { | ||
| window.location.href = Routes.COURSE_PAGE; | ||
| } | ||
| onClose(); |
There was a problem hiding this comment.
Navigation here uses window.location.href with ?unitId=..., but the course page uses the selectedUnit query param elsewhere. This will break returning to the correct unit and also forces a full page reload. Prefer history.push/Link and use ?selectedUnit=${unitId} for consistency.
| const completeActivity = async ( | ||
| activityId: string, | ||
| moduleId: string, | ||
| ): Promise<{ success: boolean; moduleCompleted?: boolean } | null> => { | ||
| const bearerToken = `Bearer ${getLocalStorageObjProperty( |
There was a problem hiding this comment.
completeActivity is typed as returning { success: boolean; moduleCompleted?: boolean }, but the backend /progress/activity/complete route returns { progress, moduleProgress }. Update this return type (and any callers) to match the actual API response shape to avoid silent runtime/typing mismatches.
| @@ -0,0 +1 @@ | |||
| export {}; | |||
There was a problem hiding this comment.
This file appears to be a leftover placeholder (export {}) and is unused. It should be removed to avoid confusion and dead code in the codebase.
Notion ticket link
Learner Progress
Implementation description
Steps to test
What should reviewers focus on?
Checklist