refactor: replace 5s polling with Firestore onSnapshot in mentor course#125
Conversation
The mentor course page (MentorCourse.svelte) and topics tab
(CourseTopics.svelte) were polling every 5s via startVisibilityPolling
to pick up changes from collaborators. Replace with Firestore
onSnapshot subscriptions so changes propagate in real time without
the wasteful interval traffic.
- mentora-api: add subscribeToCourse, subscribeToCourseTopics,
subscribeToCourseAssignments, subscribeToCourseQuestionnaires,
exposed via coursesSubscribe.get and {topics,assignments,
questionnaires}Subscribe.listForCourse on MentoraAPI.
- MentorCourse.svelte: derive courseTitle/announcements from
reactive courseState; subscribe in $effect with authReady wait;
drop manual loadCourseData() reloads after CRUD (subscription
fires automatically).
- CourseTopics.svelte: three subscriptions (topics + assignments +
questionnaires) combined via $effect into the local topics state;
drop manual loadData() reloads; preserve PostHog instrumentation.
- Delete unused apps/mentora/src/lib/features/polling/visibility.ts.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Deploying mentora-app with
|
| Latest commit: |
085f3e5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8b70363a.mentora-app.pages.dev |
| Branch Preview URL: | https://refactor-onsnapshot-mentor-c.mentora-app.pages.dev |
There was a problem hiding this comment.
Pull request overview
Refactors the mentor course experience to use Firestore real-time subscriptions (onSnapshot) instead of 5s visibility-based polling, enabling near-instant collaborator updates and reducing redundant network traffic.
Changes:
- Added new Firestore subscription helpers in
mentora-apifor course, topics, assignments, and questionnaires, and exposed them onMentoraAPI. - Updated mentor course UI (course page + topics tab) to derive state from reactive subscription-backed
ReactiveStatecontainers and removed manual reload/polling flows. - Removed the now-unused visibility polling utility.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mentora-api/src/lib/api/topics.ts | Adds subscribeToCourseTopics using Firestore onSnapshot with course filter + order. |
| packages/mentora-api/src/lib/api/questionnaires.ts | Adds subscribeToCourseQuestionnaires subscription helper. |
| packages/mentora-api/src/lib/api/courses.ts | Adds subscribeToCourse for real-time updates of a single course doc. |
| packages/mentora-api/src/lib/api/assignments.ts | Adds subscribeToCourseAssignments subscription helper. |
| packages/mentora-api/src/lib/api/api.svelte.ts | Exposes new subscription helpers on MentoraAPI (coursesSubscribe.get, topics/assignments/questionnairesSubscribe.listForCourse). |
| apps/mentora/src/routes/courses/[id]/MentorCourse.svelte | Replaces polling-based loading with a course subscription and derives UI fields from the subscribed state. |
| apps/mentora/src/lib/features/polling/visibility.ts | Deletes unused visibility polling helper. |
| apps/mentora/src/lib/components/course/mentor/CourseTopics.svelte | Replaces polling + manual reloads with live subscriptions and derives UI topics from subscribed topic/assignment/questionnaire data. |
| $effect(() => { | ||
| const id = courseId; | ||
| if (!id) { | ||
| courseState.cleanup(); | ||
| return; | ||
| } | ||
|
|
||
| let disposed = false; | ||
|
|
||
| (async () => { | ||
| if (!api.isAuthenticated) { | ||
| await api.authReady; | ||
| } | ||
| if (disposed || courseId !== id) return; | ||
| api.coursesSubscribe.get(id, courseState); | ||
| })(); | ||
|
|
||
| return () => { | ||
| disposed = true; | ||
| courseState.cleanup(); | ||
| }; |
There was a problem hiding this comment.
courseState.cleanup() only unsubscribes; it does not clear the last loaded course value/error. When navigating between course IDs within the same route instance, the previous course title/announcements can remain visible until the first snapshot arrives. Consider resetting courseState (e.g., set(null) + setError(null)) before (re)subscribing, or enhancing cleanup() to optionally clear value/error/loading.
| if (id) { | ||
| const now = Date.now(); | ||
| let newAnnouncements = [...currentAnnouncements]; | ||
| // Edit | ||
| newAnnouncements = newAnnouncements.map((a) => | ||
| // Edit existing announcement | ||
| const newAnnouncements = currentAnnouncements.map((a) => | ||
| a.id === id | ||
| ? { ...a, content: formattedContent, updatedAt: now } | ||
| : a, | ||
| ); | ||
|
|
||
| const res = await api.courses.update(courseId, { | ||
| await api.courses.update(courseId, { | ||
| announcements: newAnnouncements, | ||
| }); | ||
|
|
||
| if (res.success) { | ||
| loadCourseData(); | ||
| } | ||
| } else { | ||
| // Create | ||
| const res = await api.courses.createAnnouncement( | ||
| courseId, | ||
| formattedContent, | ||
| ); | ||
| if (res.success) { | ||
| loadCourseData(); | ||
| } | ||
| // Create new announcement via backend | ||
| await api.courses.createAnnouncement(courseId, formattedContent); | ||
| } |
There was a problem hiding this comment.
The API methods here return an APIResult (success=false) rather than throwing; awaiting without checking the result means failed updates/creates will be silently ignored and the UI modal will still close. Capture the returned APIResult for both update and createAnnouncement, and surface an error (and/or keep local state unchanged) when success is false.
| async function handleDeleteAnnouncement(id: string | number) { | ||
| if (!courseId || !fullCourse) return; | ||
|
|
||
| const currentAnnouncements = fullCourse.announcements || []; | ||
| const newAnnouncements = currentAnnouncements.filter( | ||
| (a) => String(a.id) !== String(id), | ||
| ); | ||
|
|
||
| const res = await api.courses.update(courseId, { | ||
| await api.courses.update(courseId, { | ||
| announcements: newAnnouncements, | ||
| }); |
There was a problem hiding this comment.
Same issue as save: api.courses.update returns APIResult and won't throw on failure, but the result is ignored. Please check res.success (and display an error / avoid implying deletion succeeded) so failed deletes don't silently desync the UI.
| $effect(() => { | ||
| const id = courseId; | ||
| if (!id) { | ||
| topicsState.cleanup(); | ||
| assignmentsState.cleanup(); | ||
| questionnairesState.cleanup(); | ||
| return; | ||
| } | ||
|
|
||
| let disposed = false; | ||
|
|
||
| (async () => { | ||
| if (!api.isAuthenticated) { | ||
| await api.authReady; | ||
| } | ||
| if (disposed || courseId !== id) return; | ||
| api.topicsSubscribe.listForCourse(id, topicsState); | ||
| api.assignmentsSubscribe.listForCourse(id, assignmentsState); | ||
| api.questionnairesSubscribe.listForCourse(id, questionnairesState); | ||
| })(); | ||
|
|
||
| return () => { | ||
| stopPolling?.(); | ||
| stopPolling = null; | ||
| disposed = true; | ||
| topicsState.cleanup(); | ||
| assignmentsState.cleanup(); | ||
| questionnairesState.cleanup(); | ||
| }; |
There was a problem hiding this comment.
topicsState/assignmentsState/questionnairesState.cleanup() only unsubscribes and leaves the previous .value intact. If courseId changes (or becomes falsy), the UI can continue rendering the previous course’s topics until the new snapshots arrive. Consider clearing the state values (and resetting topics to []) when courseId changes / on cleanup.
| } catch (e) { | ||
| console.error(e); | ||
| errorMessage = m.mentor_assignment_save_failed(); | ||
| await loadData(); | ||
| } |
There was a problem hiding this comment.
The calls inside this try/catch return APIResult objects and typically won’t throw on request failure (success=false). As written, failures will bypass the catch, but the UI has already been optimistically updated and analytics will still be captured. Capture and check the APIResult(s); on failure, show an error and revert the optimistic title change (or re-sync from the subscribed state).
| }, | ||
| ); | ||
| showAssignmentModal = false; | ||
| await loadData(); | ||
| } catch (e) { | ||
| console.error(e); | ||
| errorMessage = m.mentor_assignment_save_failed(); |
There was a problem hiding this comment.
In this flow, several awaited api.* calls (topics.update / questionnaires.update / assignments.update) return APIResult rather than throwing. That means errors won’t be caught here, but PostHog events will still fire and the modal will still close. Please check each APIResult.success and only capture analytics / close the modal on success; otherwise surface an error and keep the editor open so the user can retry.
| assignment_id: assignmentId, | ||
| assignment_type: assignment.type, | ||
| }); | ||
| await loadData(); | ||
| } catch (e) { | ||
| console.error(e); | ||
| errorMessage = m.mentor_assignment_delete_failed(); |
There was a problem hiding this comment.
Same APIResult vs throw issue here: api.topics.update / api.questionnaires.delete / api.assignments.delete return {success:false} on failure, so the catch may never run and the delete analytics event can be emitted even when deletion fails. Capture and validate the APIResult(s) before capturing PostHog and before leaving the UI in a deleted state.
| topic_id: topicId, | ||
| assignment_count: topic?.assignments.length ?? 0, | ||
| }); | ||
| await loadData(); | ||
| } | ||
|
|
||
| async function askDeleteTopic(topicId: string) { |
There was a problem hiding this comment.
api.topics.delete returns an APIResult (success=false) on failure, so this code can still capture a "topic_deleted" PostHog event even if the delete did not succeed. Please check the delete result and only emit analytics (and update UI) when success is true; otherwise surface an error.
Summary
startVisibilityPollingto pick up collaborator edits. Replaced with FirestoreonSnapshotsubscriptions so changes propagate in real time without the wasteful interval traffic.subscribe*helpers inmentora-api(course / topics / assignments / questionnaires) modeled on the existingsubscribeToConversationpattern, exposed viacoursesSubscribe.getand{topics,assignments,questionnaires}Subscribe.listForCourseonMentoraAPI.MentorCourse.svelteandCourseTopics.svelteto use the subscriptions, derive UI state via\$derived/\$effect, and drop manualloadCourseData()/loadData()reloads after CRUD (subscription auto-fires). PostHog instrumentation added in #upstream is preserved.apps/mentora/src/lib/features/polling/visibility.ts.Test plan
topic_created,topic_updated,topic_deleted,topics_reordered,assignment_*,assignments_reorderedevents still fire🤖 Generated with Claude Code