Skip to content

refactor: replace 5s polling with Firestore onSnapshot in mentor course #125

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.

Sign up for GitHub

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

Merged
TakalaWang merged 1 commit into from
Apr 8, 2026
Merged

refactor: replace 5s polling with Firestore onSnapshot in mentor course #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 108 additions & 95 deletions apps/mentora/src/lib/components/course/mentor/CourseTopics.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -63,7 +62,10 @@
let currentAssignment = $state<Assignment | undefined>(undefined);
let topics = $state<Topic[]>([]);

let stopPolling: (() => void) | null = null;
const topicsState = api.createState<ApiTopic[]>();
const assignmentsState = api.createState<ApiAssignment[]>();
const questionnairesState = api.createState<ApiQuestionnaire[]>();

let errorMessage = $state<string | null>(null);
let showDeleteModal = $state(false);
let pendingDelete = $state<{
Expand All @@ -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();
};
Comment on lines +77 to 103

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
});

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<string, ApiAssignment> = {};
remoteAssignments.forEach((assignment) => {
assignmentsMap[assignment.id] = assignment;
});

const assignmentsMap: Record<string, ApiAssignment> = {};
assignRes.data.forEach((assignment) => {
assignmentsMap[assignment.id] = assignment;
});
const questionnairesMap: Record<string, ApiQuestionnaire> = {};
remoteQuestionnaires.forEach((questionnaire) => {
questionnairesMap[questionnaire.id] = questionnaire;
});

const questionnairesMap: Record<string, ApiQuestionnaire> = {};
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[] }>) {
Expand Down Expand Up @@ -221,7 +239,6 @@
topic_id: res.data,
topic_index: topics.length,
});
await loadData();
}
}

Expand Down Expand Up @@ -280,7 +297,6 @@
} catch (e) {
console.error(e);
errorMessage = m.mentor_assignment_save_failed();
await loadData();
}
Comment on lines 297 to 300

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
}

Expand All @@ -292,7 +308,6 @@
topic_id: topicId,
assignment_count: topic?.assignments.length ?? 0,
});
await loadData();
}

async function askDeleteTopic(topicId: string) {
Comment on lines 308 to 313

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -457,7 +472,6 @@
},
);
showAssignmentModal = false;
await loadData();
} catch (e) {
console.error(e);
errorMessage = m.mentor_assignment_save_failed();
Comment on lines 472 to 477

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -507,7 +521,6 @@
assignment_id: assignmentId,
assignment_type: assignment.type,
});
await loadData();
} catch (e) {
console.error(e);
errorMessage = m.mentor_assignment_delete_failed();
Comment on lines 521 to 526

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Expand Down
59 changes: 0 additions & 59 deletions apps/mentora/src/lib/features/polling/visibility.ts
View file
Open in desktop

This file was deleted.

Loading
Loading