diff --git a/scripts/test-me-tasks.mjs b/scripts/test-me-tasks.mjs index a99e0254..2a4d8ef1 100644 --- a/scripts/test-me-tasks.mjs +++ b/scripts/test-me-tasks.mjs @@ -1,5 +1,6 @@ /** - * Unit tests for me/tasks API helpers (patch + board state merge). + * Unit tests for me/tasks API helpers (board state merge). + * Path encoding lives in scripts/test-paths.mjs (encodeTaskPathParam). * Run: node --experimental-strip-types scripts/test-me-tasks.mjs */ import { diff --git a/scripts/test-paths.mjs b/scripts/test-paths.mjs index bb41f331..e0b22c13 100644 --- a/scripts/test-paths.mjs +++ b/scripts/test-paths.mjs @@ -19,6 +19,8 @@ import { flatClassPath, flatTaskPath, isValidTaskRouteRef, + encodeTaskPathParam, + normalizeMeTaskPointer, } from "../src/common/paths.ts"; let failed = 0; @@ -145,6 +147,33 @@ assert(!isValidTaskRouteRef("t@mvla.net/abc123"), "rejects email/classId (class, assert(!isValidTaskRouteRef("a~b~c~d"), "rejects four-segment refs"); assert(!isValidTaskRouteRef("classId~"), "rejects trailing empty task id"); +console.log("\n--- me/tasks path encoding (tilde, not %2F) ---\n"); + +assertEq( + normalizeMeTaskPointer("classA/taskB"), + { path: "classA/taskB", tildeRef: "classA~taskB" }, + "normalize slash" +); +assertEq( + normalizeMeTaskPointer("classA~taskB"), + { path: "classA/taskB", tildeRef: "classA~taskB" }, + "normalize tilde" +); +assertEq( + normalizeMeTaskPointer("teacher@school.edu/classA/taskB"), + { path: "classA/taskB", tildeRef: "classA~taskB" }, + "normalize legacy email path" +); +assert( + encodeTaskPathParam("classA/taskB") === encodeURIComponent("classA~taskB"), + "encode prefers tilde, not %2F" +); +assert(!encodeTaskPathParam("classA/taskB").includes("%2F"), "encoded path must not contain %2F"); +assert( + encodeTaskPathParam("classA~taskB") === encodeURIComponent("classA~taskB"), + "encode tilde passthrough" +); + console.log("\n--- done ---\n"); if (failed) { console.error(`${failed} assertion(s) failed`); diff --git a/src/common/meTaskState.ts b/src/common/meTaskState.ts index 5b4ff647..38ddb907 100644 --- a/src/common/meTaskState.ts +++ b/src/common/meTaskState.ts @@ -3,8 +3,6 @@ * @module common/meTaskState */ -import { flatTaskPath } from "./paths"; - /** Per-task personal state returned by me/tasks and board payloads. */ export interface MeTaskState { ref: string; @@ -16,6 +14,10 @@ export interface MeTaskState { workspace_id: string | null; } +function flatTaskPath(classId: string, taskId: string): string { + return [classId, taskId].join("/"); +} + /** Merge board task row fields into a MeTaskState map entry. */ export function meTaskStateFromBoardTask( task: Record, diff --git a/src/common/meTasks.ts b/src/common/meTasks.ts index 78d6e0aa..542a2b66 100644 --- a/src/common/meTasks.ts +++ b/src/common/meTasks.ts @@ -1,16 +1,25 @@ /** - * Personal task state API — PATCH /api/v1/me/tasks/:taskPath (completed, note). - * Falls back to done/undo shims; stubs locally when routes 404 mid-dev. + * Personal task state API — PATCH /api/v1/me/tasks/:taskId (completed, note). + * Completion prefers POST done/undo shims (same contract Brief uses). + * + * Path params use tilde refs (`classId~taskId`). Slash/`%2F` forms are fragile on + * single-segment Express routes behind Firebase Hosting and previously caused + * silent local stubs that looked done until reload. * * @module common/meTasks */ import { ApiFetchError, apiFetch } from "@/common/apiFetch"; import { applyMeTaskState, type MeTaskState } from "@/common/meTaskState"; -import { flatTaskPath } from "@/common/paths"; +import { + encodeTaskPathParam, + flatTaskPath, + normalizeMeTaskPointer, +} from "@/common/paths"; export type { MeTaskState }; export { applyMeTaskState, meTaskStateFromBoardTask } from "@/common/meTaskState"; +export { encodeTaskPathParam, normalizeMeTaskPointer } from "@/common/paths"; export interface PatchMeTaskBody { completed?: boolean; @@ -18,40 +27,51 @@ export interface PatchMeTaskBody { } const stubStates = new Map(); +const ORG_DOMAIN_FALLBACK = process.env.VUE_APP_ORG_DOMAIN || "mvla.net"; + +function orgDomain(): string { + return ORG_DOMAIN_FALLBACK; +} + +function pointerFor(taskPath: string) { + return normalizeMeTaskPointer(taskPath, orgDomain()); +} -function encodeTaskPath(taskPath: string): string { - return encodeURIComponent(taskPath.replace(/~/g, "/")); +function isoFromCompletedAt(value: unknown): string | null { + if (typeof value === "string" && value) return value; + if (typeof value === "number" && Number.isFinite(value)) { + return new Date(value).toISOString(); + } + return null; } function normalizeTaskState(raw: unknown, fallbackPath: string): MeTaskState | null { if (!raw || typeof raw !== "object") return null; const row = raw as Record; + const fromId = typeof row.id === "string" && row.id ? row.id : ""; const path = (typeof row.path === "string" && row.path) || - (typeof row.ref === "string" && row.ref.includes("/") ? row.ref : fallbackPath); - const ref = typeof row.ref === "string" && row.ref ? row.ref : path; + (fromId.includes("/") || fromId.includes("~") ? fromId.replace(/~/g, "/") : "") || + (typeof row.ref === "string" && row.ref.includes("/") ? row.ref : "") || + fallbackPath; + const pointer = pointerFor(path) || pointerFor(fallbackPath); + const canonicalPath = pointer?.path || path.replace(/~/g, "/"); + const ref = + (typeof row.ref === "string" && row.ref) || + pointer?.tildeRef || + canonicalPath; return { ref, - path, + path: canonicalPath, completed: row.completed === true, - completed_at: - typeof row.completed_at === "string" - ? row.completed_at - : row.completed_at === null - ? null - : null, + completed_at: isoFromCompletedAt(row.completed_at), note: typeof row.note === "string" ? row.note : row.note === null ? null : null, - note_updated_at: - typeof row.note_updated_at === "string" - ? row.note_updated_at - : row.note_updated_at === null - ? null - : null, + note_updated_at: isoFromCompletedAt(row.note_updated_at), workspace_id: typeof row.workspace_id === "string" ? row.workspace_id @@ -62,9 +82,11 @@ function normalizeTaskState(raw: unknown, fallbackPath: string): MeTaskState | n } function stubTaskState(taskPath: string, patch: PatchMeTaskBody): MeTaskState { - const path = taskPath.replace(/~/g, "/"); + const pointer = pointerFor(taskPath); + const path = pointer?.path || taskPath.replace(/~/g, "/"); + const ref = pointer?.tildeRef || path; const prev = stubStates.get(path) || { - ref: path, + ref, path, completed: false, completed_at: null, @@ -72,7 +94,7 @@ function stubTaskState(taskPath: string, patch: PatchMeTaskBody): MeTaskState { note_updated_at: null, workspace_id: null, }; - const next: MeTaskState = { ...prev }; + const next: MeTaskState = { ...prev, ref, path }; if (patch.completed !== undefined) { next.completed = patch.completed; next.completed_at = patch.completed ? new Date().toISOString() : null; @@ -85,49 +107,216 @@ function stubTaskState(taskPath: string, patch: PatchMeTaskBody): MeTaskState { return next; } +function isMissingRoute(err: unknown): boolean { + return err instanceof ApiFetchError && (err.status === 404 || err.status === 405 || err.status === 501); +} + /** Build canonical flat task path from classId + taskId or an existing ref/path. */ export function taskPathFromParts(classId: string, taskId: string): string { return flatTaskPath(classId, taskId); } +async function postDoneOrUndo(encoded: string, completed: boolean, fallbackPath: string): Promise { + const suffix = completed ? "done" : "undo"; + const payload = await apiFetch(`/api/v1/me/tasks/${encoded}/${suffix}`, { method: "POST" }); + const parsed = normalizeTaskState(payload, fallbackPath); + if (parsed) { + return { ...parsed, completed }; + } + // done/undo envelopes sometimes only include id/completed — synthesize from request. + return { + ref: fallbackPath.includes("~") ? fallbackPath : fallbackPath.replace(/\//g, "~"), + path: fallbackPath.replace(/~/g, "/"), + completed, + completed_at: completed ? new Date().toISOString() : null, + note: null, + note_updated_at: null, + workspace_id: null, + }; +} + /** - * PATCH personal task state. Tries PATCH, then done/undo shims; stubs on 404. + * PATCH personal task state. + * Completion: POST done/undo first (Brief contract), then PATCH; never stub completed. + * Notes: PATCH, with local stub only when the route is missing mid-dev. */ export async function patchMeTask(taskPath: string, body: PatchMeTaskBody): Promise { - const normalized = taskPath.replace(/~/g, "/"); - const encoded = encodeTaskPath(normalized); + const pointer = pointerFor(taskPath); + if (!pointer) { + throw new Error(`Invalid task path: ${taskPath}`); + } + const { path: normalized, tildeRef } = pointer; + const encoded = encodeTaskPathParam(tildeRef, orgDomain()); const base = `/api/v1/me/tasks/${encoded}`; + // Prefer done/undo for completion — same routes Brief batch/single uses successfully. + if (body.completed !== undefined && body.note === undefined) { + try { + return await postDoneOrUndo(encoded, body.completed, normalized); + } catch (err) { + if (!(err instanceof ApiFetchError) || !isMissingRoute(err)) { + throw err; + } + // Fall through to PATCH when done/undo missing (older deploys). + } + + try { + const payload = await apiFetch(base, { + method: "PATCH", + body: { completed: body.completed }, + }); + const parsed = normalizeTaskState(payload, normalized); + if (parsed) return parsed; + return { + ref: tildeRef, + path: normalized, + completed: body.completed, + completed_at: body.completed ? new Date().toISOString() : null, + note: null, + note_updated_at: null, + workspace_id: null, + }; + } catch (err) { + // Never stub completed — that was the production "looks done until reload" bug. + throw err; + } + } + try { const payload = await apiFetch(base, { method: "PATCH", body }); const parsed = normalizeTaskState(payload, normalized); if (parsed) return parsed; } catch (err) { if (err instanceof ApiFetchError) { - if (err.status === 404 && body.completed === true) { + // If PATCH missing but we only needed completed, try done/undo. + if (isMissingRoute(err) && body.completed !== undefined) { try { - const payload = await apiFetch(`${base}/done`, { method: "POST" }); - const parsed = normalizeTaskState(payload, normalized); - if (parsed) return parsed; - } catch { - /* fall through to stub */ + const doneState = await postDoneOrUndo(encoded, body.completed, normalized); + if (body.note === undefined) return doneState; + } catch (doneErr) { + if (body.note === undefined) throw doneErr; } } - if (err.status === 404 && body.completed === false) { - try { - const payload = await apiFetch(`${base}/undo`, { method: "POST" }); - const parsed = normalizeTaskState(payload, normalized); - if (parsed) return parsed; - } catch { - /* fall through to stub */ - } - } - if (err.status === 404 || err.status === 405 || err.status === 501) { + if (isMissingRoute(err) && body.completed === undefined) { + // Notes-only mid-dev stub when routes absent. return stubTaskState(normalized, body); } } throw err; } + if (body.completed !== undefined) { + return { + ref: tildeRef, + path: normalized, + completed: body.completed, + completed_at: body.completed ? new Date().toISOString() : null, + note: typeof body.note === "string" ? body.note : body.note === null ? null : null, + note_updated_at: body.note ? new Date().toISOString() : null, + workspace_id: null, + }; + } + return stubTaskState(normalized, body); } + +export interface BatchMeTasksResult { + succeeded: number; + failed: number; + states: MeTaskState[]; + errors: { id: string; error: string }[]; +} + +/** + * Batch mark done/undo via POST /api/v1/me/tasks/done|undo (Brief contract). + * Body uses tilde refs. Falls back to per-task patchMeTask when batch route is missing. + */ +export async function patchMeTasksCompleted( + taskPaths: string[], + completed: boolean +): Promise { + const pointers = taskPaths + .map((p) => pointerFor(p)) + .filter((p): p is NonNullable => !!p); + if (!pointers.length) { + throw new Error("No valid task paths for batch complete"); + } + + const tildeIds = [...new Set(pointers.map((p) => p.tildeRef))]; + const pathByTilde = new Map(pointers.map((p) => [p.tildeRef, p.path])); + + try { + const suffix = completed ? "done" : "undo"; + const payload = await apiFetch<{ + results?: Array<{ + id?: string; + ok?: boolean; + path?: string; + completed?: boolean; + completed_at?: number | string; + error?: string; + }>; + succeeded?: number; + failed?: number; + }>(`/api/v1/me/tasks/${suffix}`, { + method: "POST", + body: { task_ids: tildeIds }, + }); + + const states: MeTaskState[] = []; + const errors: { id: string; error: string }[] = []; + for (const row of payload?.results || []) { + const id = typeof row.id === "string" ? row.id : ""; + if (!row.ok) { + errors.push({ id, error: row.error || "failed" }); + continue; + } + const fallback = pathByTilde.get(id) || id.replace(/~/g, "/"); + const parsed = normalizeTaskState( + { + ...row, + path: row.path || fallback, + completed: row.completed ?? completed, + }, + fallback + ); + if (parsed) states.push({ ...parsed, completed }); + } + + if (errors.length && !states.length) { + throw new ApiFetchError(errors[0]?.error || "Batch complete failed", 400); + } + + return { + succeeded: typeof payload?.succeeded === "number" ? payload.succeeded : states.length, + failed: typeof payload?.failed === "number" ? payload.failed : errors.length, + states, + errors, + }; + } catch (err) { + if (!isMissingRoute(err)) throw err; + } + + // Fallback: sequential single-task writes (still no stub for completed). + const states: MeTaskState[] = []; + const errors: { id: string; error: string }[] = []; + for (const pointer of pointers) { + try { + states.push(await patchMeTask(pointer.path, { completed })); + } catch (e) { + errors.push({ + id: pointer.tildeRef, + error: e instanceof Error ? e.message : String(e), + }); + } + } + if (!states.length && errors.length) { + throw new Error(errors[0].error); + } + return { + succeeded: states.length, + failed: errors.length, + states, + errors, + }; +} diff --git a/src/common/paths.ts b/src/common/paths.ts index d5f134fb..d020b0bc 100644 --- a/src/common/paths.ts +++ b/src/common/paths.ts @@ -283,6 +283,43 @@ export function flatTaskPath(classId: string, taskId: string): string { return [classId, taskId].join("/"); } +/** + * Canonical flat slash path + board tilde ref for a personal-task API pointer. + * Accepts classId/taskId, classId~taskId, or legacy email/local prefixed forms. + */ +export function normalizeMeTaskPointer( + taskPath: string, + orgDomain: string = "mvla.net" +): { path: string; tildeRef: string } | null { + const trimmed = (taskPath || "").trim(); + if (!trimmed) return null; + const ids = writeTaskIds(trimmed, orgDomain); + if (!ids) { + const parts = trimmed.replace(/~/g, "/").split("/").filter(Boolean); + if (parts.length === 2 && !looksLikeEmail(parts[0])) { + return { path: flatTaskPath(parts[0], parts[1]), tildeRef: `${parts[0]}~${parts[1]}` }; + } + return null; + } + return { + path: flatTaskPath(ids.classId, ids.taskId), + tildeRef: `${ids.classId}~${ids.taskId}`, + }; +} + +/** + * Encode a task identity for `/api/v1/me/tasks/:taskId`. + * Prefers tilde refs (`classId~taskId`) — never embeds raw `/` / `%2F` in the segment. + */ +export function encodeTaskPathParam( + taskPath: string, + orgDomain: string = "mvla.net" +): string { + const normalized = normalizeMeTaskPointer(taskPath, orgDomain); + const tilde = normalized?.tildeRef || taskPath.replace(/\//g, "~"); + return encodeURIComponent(tilde); +} + /** * Short share/view ref when classId is known (drops email/local prefix). * Class → `classId`; task → `classId~taskId`. diff --git a/src/common/workspace.ts b/src/common/workspace.ts index 26819ca0..8b6ae812 100644 --- a/src/common/workspace.ts +++ b/src/common/workspace.ts @@ -6,6 +6,7 @@ */ import { ApiFetchError, apiFetch } from "@/common/apiFetch"; +import { encodeTaskPathParam } from "@/common/paths"; export interface WorkspaceFile { id: string; @@ -61,7 +62,7 @@ const stubWorkspaces = new Map(); const stubTaskWorkspaceLinks = new Map(); function encodeTaskPath(taskPath: string): string { - return encodeURIComponent(taskPath.replace(/~/g, "/")); + return encodeTaskPathParam(taskPath); } function normalizeWorkspace(raw: unknown): Workspace | null { diff --git a/src/firebase/index.ts b/src/firebase/index.ts index c0691e7a..6c52b8f5 100644 --- a/src/firebase/index.ts +++ b/src/firebase/index.ts @@ -150,16 +150,18 @@ function setupSnapshot(uid: string | undefined): void { syncClassListeners(store.active_doc?.classes || nextClasses); } - // finished[] lives on the user doc; updating account_doc is enough for calendar - // (finished_tasks getter + hide_finished). Re-stamp tasks only if needed for reactivity. + // finished[] lives on the user doc; merge into task_states so is_task_completed + // stays correct after API writes (and after reload when board overlay lags). if (finishedChanged) { prevFinished = [...nextFinished]; - _status.log("⬥ User finished[] changed — store already updated via account_doc"); + _status.log("⬥ User finished[] changed — syncing task_states + restamping tasks"); + store.sync_finished_from_user_doc(nextFinished); if (store.classes?.length) { store.get_tasks(); } } else if (prevFinished === null) { prevFinished = [...nextFinished]; + store.sync_finished_from_user_doc(nextFinished); } }, (err) => { diff --git a/src/store/index.ts b/src/store/index.ts index 8f6c186a..2664b9f1 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -64,8 +64,10 @@ import { applyMeTaskState, meTaskStateFromBoardTask, patchMeTask, + patchMeTasksCompleted, type MeTaskState, } from "@/common/meTasks"; +import { normalizeMeTaskPointer } from "@/common/paths"; import { actingAsLabel, isActingAsLinked, @@ -1046,10 +1048,25 @@ export const useMainStore: StoreDefinition = defineStore({ if (!ref) throw "No reference(s) provided"; const paths: string[] = Array.isArray(ref) ? ref : [ref]; - for (const taskRef of paths) { - const path = this.ref_to_path(taskRef) || String(taskRef).replace(/~/g, "/"); + const resolved = paths.map((taskRef) => { + const path = + this.ref_to_path(taskRef) || + normalizeMeTaskPointer(String(taskRef))?.path || + String(taskRef).replace(/~/g, "/"); if (!path) throw "Invalid ref"; - const state = await patchMeTask(path, { completed: finished }); + return path; + }); + + if (resolved.length > 1) { + const batch = await patchMeTasksCompleted(resolved, finished); + for (const state of batch.states) { + this.apply_task_state(state); + } + if (batch.failed && !batch.succeeded) { + throw batch.errors[0]?.error || "Batch complete failed"; + } + } else { + const state = await patchMeTask(resolved[0], { completed: finished }); this.apply_task_state(state); } @@ -1066,15 +1083,25 @@ export const useMainStore: StoreDefinition = defineStore({ */ is_task_completed(ref: string): boolean { if (!ref) return false; - const path = this.ref_to_path(ref) || ref.replace(/~/g, "/"); + const pointer = normalizeMeTaskPointer(ref); + const path = this.ref_to_path(ref) || pointer?.path || ref.replace(/~/g, "/"); + const tilde = pointer?.tildeRef || (path ? path.replace(/\//g, "~") : ""); const state = this.task_states?.[ref] || (path ? this.task_states?.[path] : undefined) || + (tilde ? this.task_states?.[tilde] : undefined) || (ref.includes("~") ? this.task_states?.[ref.split("~").join("/")] : undefined); if (state) return state.completed === true; - if (this.active_doc?.finished?.includes(ref)) return true; + const finished = this.active_doc?.finished || []; + if ( + finished.includes(ref) || + (path && finished.includes(path)) || + (tilde && finished.includes(tilde)) + ) { + return true; + } const task = (this.tasks as ProcessedTaskInfo[])?.find( - (t) => t.ref === ref || t.ref === path + (t) => t.ref === ref || t.ref === path || t.ref === tilde ); return task?.completed === true; }, @@ -1107,11 +1134,26 @@ export const useMainStore: StoreDefinition = defineStore({ const state = meTaskStateFromBoardTask(row, task.class_id, task.id); map = applyMeTaskState(map, state); } - for (const entry of board.finished || []) { - const path = String(entry).replace(/~/g, "/"); - const existing = map[path] || map[entry]; + map = this.merge_finished_pointers(map, board.finished || []); + this.task_states = map; + this.patch_task_completion_flags(); + }, + /** + * Merge users.finished[] (or board.finished) pointers into task_states as completed. + * Keeps live user-doc snapshots in sync when API writes land without a board re-fetch. + */ + merge_finished_pointers( + map: Record, + finished: string[] + ): Record { + let next = map; + for (const entry of finished || []) { + const pointer = normalizeMeTaskPointer(String(entry)); + const path = pointer?.path || String(entry).replace(/~/g, "/"); + const tilde = pointer?.tildeRef || path.replace(/\//g, "~"); + const existing = next[path] || next[entry] || next[tilde]; const state: MeTaskState = existing || { - ref: path, + ref: tilde, path, completed: true, completed_at: null, @@ -1119,8 +1161,41 @@ export const useMainStore: StoreDefinition = defineStore({ note_updated_at: null, workspace_id: null, }; - map = applyMeTaskState(map, { ...state, completed: true }); + next = applyMeTaskState(next, { ...state, completed: true }); + } + return next; + }, + /** Apply users/{uid}.finished[] from a live snapshot into task_states. */ + sync_finished_from_user_doc(finished: string[] | null | undefined): void { + if (!Array.isArray(finished)) return; + const finishedSet = new Set(); + for (const entry of finished) { + const pointer = normalizeMeTaskPointer(String(entry)); + if (pointer) { + finishedSet.add(pointer.path); + finishedSet.add(pointer.tildeRef); + } + finishedSet.add(String(entry)); + } + + let map = { ...(this.task_states || {}) }; + const seenPaths = new Set(); + for (const state of Object.values(map)) { + if (!state?.path || seenPaths.has(state.path)) continue; + seenPaths.add(state.path); + const isDone = + finishedSet.has(state.path) || + finishedSet.has(state.ref) || + finishedSet.has(state.path.replace(/\//g, "~")); + if (state.completed !== isDone) { + map = applyMeTaskState(map, { + ...state, + completed: isDone, + completed_at: isDone ? state.completed_at : null, + }); + } } + map = this.merge_finished_pointers(map, finished); this.task_states = map; this.patch_task_completion_flags(); },