From 085f3e540f52d845cdcd1aaec5bd2579371e933c Mon Sep 17 00:00:00 2001 From: TakalaWang Date: Thu, 9 Apr 2026 00:23:23 +0800 Subject: [PATCH] refactor: replace 5s polling with Firestore onSnapshot for mentor course 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) --- .../course/mentor/CourseTopics.svelte | 203 ++++++++++-------- .../src/lib/features/polling/visibility.ts | 59 ----- .../routes/courses/[id]/MentorCourse.svelte | 110 +++++----- .../mentora-api/src/lib/api/api.svelte.ts | 24 ++- .../mentora-api/src/lib/api/assignments.ts | 48 ++++- packages/mentora-api/src/lib/api/courses.ts | 37 ++++ .../mentora-api/src/lib/api/questionnaires.ts | 48 ++++- packages/mentora-api/src/lib/api/topics.ts | 50 ++++- 8 files changed, 358 insertions(+), 221 deletions(-) delete mode 100644 apps/mentora/src/lib/features/polling/visibility.ts diff --git a/apps/mentora/src/lib/components/course/mentor/CourseTopics.svelte b/apps/mentora/src/lib/components/course/mentor/CourseTopics.svelte index 80281605..9d0311c6 100644 --- a/apps/mentora/src/lib/components/course/mentor/CourseTopics.svelte +++ b/apps/mentora/src/lib/components/course/mentor/CourseTopics.svelte @@ -5,8 +5,8 @@ api, type Assignment as ApiAssignment, type Questionnaire as ApiQuestionnaire, + type Topic as ApiTopic, } from "$lib/api"; - import { onMount } from "svelte"; import TopicCard from "./topics/TopicCard.svelte"; import AssignmentFormModal from "./topics/AssignmentFormModal.svelte"; import PopupModal from "$lib/components/ui/PopupModal.svelte"; @@ -15,7 +15,6 @@ dndzone, SHADOW_ITEM_MARKER_PROPERTY_NAME, } from "svelte-dnd-action"; - import { startVisibilityPolling } from "$lib/features/polling/visibility"; import posthog from "posthog-js"; import { mapMentorQuestionsFromApi, @@ -63,7 +62,10 @@ let currentAssignment = $state(undefined); let topics = $state([]); - let stopPolling: (() => void) | null = null; + const topicsState = api.createState(); + const assignmentsState = api.createState(); + const questionnairesState = api.createState(); + let errorMessage = $state(null); let showDeleteModal = $state(false); let pendingDelete = $state<{ @@ -72,113 +74,129 @@ assignmentId?: string; } | null>(null); - onMount(() => { - stopPolling = startVisibilityPolling(() => loadData(), { - intervalMs: 5000, - runImmediately: true, - onError: (error) => - console.error("Failed to refresh topics", error), - }); + $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(); }; }); - function toIso(timestamp: number | null | undefined): string | undefined { - return timestamp - ? new Date(timestamp).toISOString().slice(0, 16) - : undefined; - } + // Combine the three subscriptions into the UI topics shape whenever any + // of them changes. DnD writes to `topics` directly during a drag; once the + // drop is finalized and persisted via api.topics.update, the subscription + // fires and re-runs this effect with the canonical order. + $effect(() => { + const remoteTopics = topicsState.value; + const remoteAssignments = assignmentsState.value; + const remoteQuestionnaires = questionnairesState.value; - async function loadData() { - if (!courseId) return; + if (!remoteTopics || !remoteAssignments || !remoteQuestionnaires) { + return; + } errorMessage = null; - try { - const [topicsRes, assignRes, questRes] = await Promise.all([ - api.topics.listForCourse(courseId), - api.assignments.listForCourse(courseId), - api.questionnaires.listForCourse(courseId), - ]); - if (!topicsRes.success || !assignRes.success || !questRes.success) { - return; - } + const assignmentsMap: Record = {}; + remoteAssignments.forEach((assignment) => { + assignmentsMap[assignment.id] = assignment; + }); - const assignmentsMap: Record = {}; - assignRes.data.forEach((assignment) => { - assignmentsMap[assignment.id] = assignment; - }); + const questionnairesMap: Record = {}; + remoteQuestionnaires.forEach((questionnaire) => { + questionnairesMap[questionnaire.id] = questionnaire; + }); - const questionnairesMap: Record = {}; - questRes.data.forEach((questionnaire) => { - questionnairesMap[questionnaire.id] = questionnaire; - }); + topics = [...remoteTopics] + .sort((a, b) => (a.order || 0) - (b.order || 0)) + .map((topic) => { + const items: Assignment[] = []; - topics = topicsRes.data - .sort((a, b) => (a.order || 0) - (b.order || 0)) - .map((topic) => { - const items: Assignment[] = []; - - if (topic.contents && topic.contentTypes) { - topic.contents.forEach((id, index) => { - const type = topic.contentTypes?.[index]; - if (!type) { - return; - } - - if (type === "questionnaire") { - const questionnaire = questionnairesMap[id]; - if (!questionnaire) { - return; - } - - items.push({ - id: questionnaire.id, - title: questionnaire.title, - type: "questionnaire", - dueAt: toIso(questionnaire.dueAt), - startAt: toIso(questionnaire.startAt), - topicId: topic.id, - questions: mapMentorQuestionsFromApi( - questionnaire.questions, - ), - }); - return; - } - - const assignment = assignmentsMap[id]; - if (!assignment) { - return; - } + if (topic.contents && topic.contentTypes) { + topic.contents.forEach((id, index) => { + const type = topic.contentTypes?.[index]; + if (!type) return; + + if (type === "questionnaire") { + const questionnaire = questionnairesMap[id]; + if (!questionnaire) return; items.push({ - id: assignment.id, - title: assignment.title, - type: "dialogue", - dueAt: toIso(assignment.dueAt), - startAt: toIso(assignment.startAt), + id: questionnaire.id, + title: questionnaire.title, + type: "questionnaire", + dueAt: toIso(questionnaire.dueAt), + startAt: toIso(questionnaire.startAt), topicId: topic.id, - introduction: assignment.question ?? "", - prompt: assignment.prompt, + questions: mapMentorQuestionsFromApi( + questionnaire.questions, + ), }); + return; + } + + const assignment = assignmentsMap[id]; + if (!assignment) return; + + items.push({ + id: assignment.id, + title: assignment.title, + type: "dialogue", + dueAt: toIso(assignment.dueAt), + startAt: toIso(assignment.startAt), + topicId: topic.id, + introduction: assignment.question ?? "", + prompt: assignment.prompt, }); - } + }); + } - return { - id: topic.id, - title: topic.title, - description: topic.description || "", - assignments: items, - order: topic.order || 0, - }; - }); - } catch (e) { - console.error("Failed to load topics", e); + return { + id: topic.id, + title: topic.title, + description: topic.description || "", + assignments: items, + order: topic.order || 0, + }; + }); + }); + + $effect(() => { + const error = + topicsState.error ?? + assignmentsState.error ?? + questionnairesState.error; + if (error) { + console.error("Failed to load topics", error); errorMessage = m.mentor_topic_load_failed(); } + }); + + function toIso(timestamp: number | null | undefined): string | undefined { + return timestamp + ? new Date(timestamp).toISOString().slice(0, 16) + : undefined; } function handleTopicDndConsider(e: CustomEvent<{ items: Topic[] }>) { @@ -221,7 +239,6 @@ topic_id: res.data, topic_index: topics.length, }); - await loadData(); } } @@ -280,7 +297,6 @@ } catch (e) { console.error(e); errorMessage = m.mentor_assignment_save_failed(); - await loadData(); } } @@ -292,7 +308,6 @@ topic_id: topicId, assignment_count: topic?.assignments.length ?? 0, }); - await loadData(); } async function askDeleteTopic(topicId: string) { @@ -457,7 +472,6 @@ }, ); showAssignmentModal = false; - await loadData(); } catch (e) { console.error(e); errorMessage = m.mentor_assignment_save_failed(); @@ -507,7 +521,6 @@ assignment_id: assignmentId, assignment_type: assignment.type, }); - await loadData(); } catch (e) { console.error(e); errorMessage = m.mentor_assignment_delete_failed(); diff --git a/apps/mentora/src/lib/features/polling/visibility.ts b/apps/mentora/src/lib/features/polling/visibility.ts deleted file mode 100644 index 597984ad..00000000 --- a/apps/mentora/src/lib/features/polling/visibility.ts +++ /dev/null @@ -1,59 +0,0 @@ -interface VisibilityPollingOptions { - intervalMs: number; - runImmediately?: boolean; - onError?: (error: unknown) => void; -} - -export function startVisibilityPolling( - task: () => Promise | void, - options: VisibilityPollingOptions, -): () => void { - const { intervalMs, runImmediately = true, onError } = options; - let timer: ReturnType | null = null; - - const runTask = async () => { - try { - await task(); - } catch (error) { - onError?.(error); - } - }; - - const startTimer = () => { - if (timer) return; - timer = setInterval(() => { - if (document.visibilityState === "visible") { - void runTask(); - } - }, intervalMs); - }; - - const stopTimer = () => { - if (!timer) return; - clearInterval(timer); - timer = null; - }; - - const handleVisibilityChange = () => { - if (document.visibilityState === "visible") { - void runTask(); - startTimer(); - return; - } - stopTimer(); - }; - - if (runImmediately && document.visibilityState === "visible") { - void runTask(); - } - startTimer(); - document.addEventListener("visibilitychange", handleVisibilityChange); - - return () => { - stopTimer(); - document.removeEventListener( - "visibilitychange", - handleVisibilityChange, - ); - }; -} diff --git a/apps/mentora/src/routes/courses/[id]/MentorCourse.svelte b/apps/mentora/src/routes/courses/[id]/MentorCourse.svelte index 9bc94f1d..c20f386c 100644 --- a/apps/mentora/src/routes/courses/[id]/MentorCourse.svelte +++ b/apps/mentora/src/routes/courses/[id]/MentorCourse.svelte @@ -7,19 +7,18 @@ import CourseSettings from "$lib/components/course/mentor/CourseSettings.svelte"; import CourseSubmissions from "$lib/components/course/mentor/CourseSubmissions.svelte"; import CourseWallet from "$lib/components/course/mentor/CourseWallet.svelte"; - import { onMount } from "svelte"; import { api } from "$lib"; import type { Course } from "$lib/api"; import { formatMentoraDateTime } from "$lib/features/datetime/format"; - import { startVisibilityPolling } from "$lib/features/polling/visibility"; // Props const courseId = $derived(page.params.id); // State let activeTab = $state("dashboard"); // dashboard, topics, members, settings - let courseTitle = $state("Loading..."); - let fullCourse = $state(null); + const courseState = api.createState(); + const fullCourse = $derived(courseState.value); + const courseTitle = $derived(courseState.value?.title ?? "Loading..."); interface Announcement { id: string; @@ -27,7 +26,6 @@ createdDate: string; [key: string]: unknown; } - let announcements = $state([]); function parseAnnouncementContent(rawValue: unknown) { const normalized = String(rawValue ?? "") @@ -53,48 +51,51 @@ }; } - async function loadCourseData() { - if (courseId) { - const courseResult = await api.courses.get(courseId); - if (courseResult.success) { - fullCourse = courseResult.data; - courseTitle = courseResult.data.title; - // Map API announcements to UI format - announcements = (courseResult.data.announcements || []).map( - (a) => { - const content = - a.content ?? - (typeof a === "object" && a !== null && "title" in a - ? a.title - : ""); - const parsed = parseAnnouncementContent(content); - - return { - id: a.id, - title: parsed.title, - content: parsed.body, - createdDate: formatDate(a.createdAt), - }; - }, - ) as Announcement[]; - } - } - } - function formatDate( ts: number | Date | { toDate: () => Date } | null | undefined, ) { return formatMentoraDateTime(ts); } - onMount(() => { - const stopPolling = startVisibilityPolling(() => loadCourseData(), { - intervalMs: 5000, - runImmediately: true, - onError: (error) => - console.error("Failed to refresh course", error), - }); - return () => stopPolling(); + const announcements = $derived( + (courseState.value?.announcements ?? []).map((a) => { + const content = + a.content ?? + (typeof a === "object" && a !== null && "title" in a + ? (a as { title: string }).title + : ""); + const parsed = parseAnnouncementContent(content); + + return { + id: a.id, + title: parsed.title, + content: parsed.body, + createdDate: formatDate(a.createdAt), + }; + }), + ); + + $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(); + }; }); function handleTabChange(tab: string) { @@ -118,30 +119,19 @@ 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); } } @@ -153,13 +143,9 @@ (a) => String(a.id) !== String(id), ); - const res = await api.courses.update(courseId, { + await api.courses.update(courseId, { announcements: newAnnouncements, }); - - if (res.success) { - loadCourseData(); - } } diff --git a/packages/mentora-api/src/lib/api/api.svelte.ts b/packages/mentora-api/src/lib/api/api.svelte.ts index 3443aeea..fa45a5b3 100644 --- a/packages/mentora-api/src/lib/api/api.svelte.ts +++ b/packages/mentora-api/src/lib/api/api.svelte.ts @@ -3,14 +3,17 @@ * Extends base MentoraClient with $state reactivity and subscriptions */ import type { User } from 'firebase/auth'; -import type { UserProfile } from 'mentora-firebase'; +import type { Assignment, Questionnaire, Topic, UserProfile } from 'mentora-firebase'; import * as AnnouncementsModule from './announcements.js'; +import * as AssignmentsModule from './assignments.js'; import { MentoraClient, type MentoraClientConfig } from './client.js'; import * as CoursesModule from './courses.js'; import * as ConversationsModule from './conversations.js'; import { ProfileWatcher } from './profile.svelte.js'; +import * as QuestionnairesModule from './questionnaires.js'; import type { ReactiveState } from './state.svelte.js'; import { createState } from './state.svelte.js'; +import * as TopicsModule from './topics.js'; import * as UsersModule from './users.js'; import type { Conversation } from './conversations.js'; import type { Announcement } from './announcements.js'; @@ -95,7 +98,24 @@ export class MentoraAPI extends MentoraClient { coursesSubscribe = { listMine: (state: ReactiveState, options?: import('./types.js').QueryOptions): void => - CoursesModule.subscribeToMyCourses(this._config, state, options) + CoursesModule.subscribeToMyCourses(this._config, state, options), + get: (courseId: string, state: ReactiveState): void => + CoursesModule.subscribeToCourse(this._config, courseId, state) + }; + + topicsSubscribe = { + listForCourse: (courseId: string, state: ReactiveState): void => + TopicsModule.subscribeToCourseTopics(this._config, courseId, state) + }; + + assignmentsSubscribe = { + listForCourse: (courseId: string, state: ReactiveState): void => + AssignmentsModule.subscribeToCourseAssignments(this._config, courseId, state) + }; + + questionnairesSubscribe = { + listForCourse: (courseId: string, state: ReactiveState): void => + QuestionnairesModule.subscribeToCourseQuestionnaires(this._config, courseId, state) }; announcementsSubscribe = { diff --git a/packages/mentora-api/src/lib/api/assignments.ts b/packages/mentora-api/src/lib/api/assignments.ts index b0240ecd..4ca36de2 100644 --- a/packages/mentora-api/src/lib/api/assignments.ts +++ b/packages/mentora-api/src/lib/api/assignments.ts @@ -1,9 +1,20 @@ /** * Assignment operations */ -import { collection, deleteDoc, doc, getDoc, setDoc, updateDoc } from 'firebase/firestore'; +import { + collection, + deleteDoc, + doc, + getDoc, + onSnapshot, + query, + setDoc, + updateDoc, + where +} from 'firebase/firestore'; import { Assignments, type Assignment } from 'mentora-firebase'; import { callBackend } from './backend.js'; +import type { ReactiveState } from './state.svelte.js'; import { failure, tryCatch, @@ -32,6 +43,41 @@ export async function getAssignment( }); } +/** + * Subscribe to assignments for a course + */ +export function subscribeToCourseAssignments( + config: MentoraAPIConfig, + courseId: string, + state: ReactiveState +): void { + state.setLoading(true); + const q = query( + collection(config.db, Assignments.collectionPath()), + where('courseId', '==', courseId) + ); + + const unsubscribe = onSnapshot( + q, + (snapshot) => { + try { + const data = snapshot.docs.map((doc) => Assignments.schema.parse(doc.data())); + state.set(data); + state.setError(null); + } catch (error) { + state.setError(error instanceof Error ? error.message : 'Parse error'); + } + state.setLoading(false); + }, + (error) => { + state.setError(error.message); + state.setLoading(false); + } + ); + + state.attachUnsubscribe(unsubscribe); +} + /** * List assignments for a course (via backend) */ diff --git a/packages/mentora-api/src/lib/api/courses.ts b/packages/mentora-api/src/lib/api/courses.ts index d1b0289a..13f0b9c1 100644 --- a/packages/mentora-api/src/lib/api/courses.ts +++ b/packages/mentora-api/src/lib/api/courses.ts @@ -61,6 +61,43 @@ export async function getCourse( }); } +/** + * Subscribe to a single course's changes + */ +export function subscribeToCourse( + config: MentoraAPIConfig, + courseId: string, + state: ReactiveState +): void { + state.setLoading(true); + const docRef = doc(config.db, Courses.docPath(courseId)); + + const unsubscribe = onSnapshot( + docRef, + (snapshot) => { + if (snapshot.exists()) { + try { + const data = Courses.schema.parse(snapshot.data()); + state.set({ id: snapshot.id, ...data }); + state.setError(null); + } catch (error) { + state.setError(error instanceof Error ? error.message : 'Parse error'); + } + } else { + state.set(null); + state.setError('Course not found'); + } + state.setLoading(false); + }, + (error) => { + state.setError(error.message); + state.setLoading(false); + } + ); + + state.attachUnsubscribe(unsubscribe); +} + /** * List courses owned by current user */ diff --git a/packages/mentora-api/src/lib/api/questionnaires.ts b/packages/mentora-api/src/lib/api/questionnaires.ts index 01727574..0269d943 100644 --- a/packages/mentora-api/src/lib/api/questionnaires.ts +++ b/packages/mentora-api/src/lib/api/questionnaires.ts @@ -1,9 +1,20 @@ /** * Questionnaire operations */ -import { collection, deleteDoc, doc, getDoc, setDoc, updateDoc } from 'firebase/firestore'; +import { + collection, + deleteDoc, + doc, + getDoc, + onSnapshot, + query, + setDoc, + updateDoc, + where +} from 'firebase/firestore'; import { Questionnaires, type Questionnaire } from 'mentora-firebase'; import { callBackend } from './backend.js'; +import type { ReactiveState } from './state.svelte.js'; import { failure, tryCatch, @@ -31,6 +42,41 @@ export async function getQuestionnaire( }); } +/** + * Subscribe to questionnaires for a course + */ +export function subscribeToCourseQuestionnaires( + config: MentoraAPIConfig, + courseId: string, + state: ReactiveState +): void { + state.setLoading(true); + const q = query( + collection(config.db, Questionnaires.collectionPath()), + where('courseId', '==', courseId) + ); + + const unsubscribe = onSnapshot( + q, + (snapshot) => { + try { + const data = snapshot.docs.map((doc) => Questionnaires.schema.parse(doc.data())); + state.set(data); + state.setError(null); + } catch (error) { + state.setError(error instanceof Error ? error.message : 'Parse error'); + } + state.setLoading(false); + }, + (error) => { + state.setError(error.message); + state.setLoading(false); + } + ); + + state.attachUnsubscribe(unsubscribe); +} + /** * List questionnaires for a course (via backend) */ diff --git a/packages/mentora-api/src/lib/api/topics.ts b/packages/mentora-api/src/lib/api/topics.ts index c8ff3ce1..511b677f 100644 --- a/packages/mentora-api/src/lib/api/topics.ts +++ b/packages/mentora-api/src/lib/api/topics.ts @@ -1,9 +1,21 @@ /** * Topic operations */ -import { collection, deleteDoc, doc, getDoc, setDoc, updateDoc } from 'firebase/firestore'; +import { + collection, + deleteDoc, + doc, + getDoc, + onSnapshot, + orderBy, + query, + setDoc, + updateDoc, + where +} from 'firebase/firestore'; import { Topics, type Topic } from 'mentora-firebase'; import { callBackend } from './backend.js'; +import type { ReactiveState } from './state.svelte.js'; import { failure, tryCatch, @@ -31,6 +43,42 @@ export async function getTopic( }); } +/** + * Subscribe to topics for a course + */ +export function subscribeToCourseTopics( + config: MentoraAPIConfig, + courseId: string, + state: ReactiveState +): void { + state.setLoading(true); + const q = query( + collection(config.db, Topics.collectionPath()), + where('courseId', '==', courseId), + orderBy('order', 'asc') + ); + + const unsubscribe = onSnapshot( + q, + (snapshot) => { + try { + const data = snapshot.docs.map((doc) => Topics.schema.parse(doc.data())); + state.set(data); + state.setError(null); + } catch (error) { + state.setError(error instanceof Error ? error.message : 'Parse error'); + } + state.setLoading(false); + }, + (error) => { + state.setError(error.message); + state.setLoading(false); + } + ); + + state.attachUnsubscribe(unsubscribe); +} + /** * List topics for a course (via backend) */