feat(studio): owner-only "Make editable in the studio" route and conversion job
POST /api/studio/courses/:id/convert (owner only, CSRF-guarded by core) queues studio.convert-import; GET /api/studio/jobs/:id reports progress. Read-only responses say whether the caller may convert. Resolves TODO(studio) (ruling R9). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
df6d235070
commit
afa2338ad3
5 files changed
+201
-18
No files matched your search
@@ -1,5 +1,5 @@
|
||||
// Server entry of @stc/feature-studio. The browser imports "@stc/feature-studio/web" instead (no server code).
|
||||
export { studioFeature, OUTSIDE_MESSAGE, IMPORTED_MESSAGE, uploadMaxBytes, UPLOAD_EXTENSIONS, type StudioFeature, type StudioFeatureOptions } from "./server/feature";
|
||||
export { studioFeature, OUTSIDE_MESSAGE, IMPORTED_MESSAGE, CONVERT_JOB, uploadMaxBytes, UPLOAD_EXTENSIONS, type StudioFeature, type StudioFeatureOptions } from "./server/feature";
|
||||
export { createDraftWriter, type DraftWriter, type DraftWriterOptions } from "./server/draft-writer";
|
||||
export { createStudioStore, contentHash, isAllowedRemote, ownerRequiredFor, type StudioStore, type Identity, type DraftStatus, type PublishResult, type SyncResult } from "./server/store";
|
||||
export { createPublishedWatcher, type PublishedWatcher } from "./server/watch";
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { existsSync, realpathSync } from "node:fs";
|
||||
import { basename, extname, resolve, sep } from "node:path";
|
||||
import type { AuthUser, Feature, RequestContext, RouteHandler, ServerContext } from "@stc/contracts";
|
||||
import type { AuthUser, Feature, JobHandler, RequestContext, RouteHandler, ServerContext } from "@stc/contracts";
|
||||
import { canView, serveMedia } from "@stc/core";
|
||||
import { ManifestSchema, parseManifest, resolveInside, type LoadedCourse } from "@stc/course-format";
|
||||
import { aclRepo } from "@stc/db";
|
||||
@@ -10,7 +10,8 @@ import { checkRelPath, createStudioStore, MANIFEST, ownerRequiredFor, StudioErro
|
||||
import { createPublishedWatcher, type PublishedWatcher } from "./watch";
|
||||
|
||||
export const OUTSIDE_MESSAGE = "This course is managed outside STCourses";
|
||||
export const IMPORTED_MESSAGE = "Imported course: read-only in the studio for now";
|
||||
export const IMPORTED_MESSAGE = "Imported course: not editable in the studio until the owner makes it editable";
|
||||
export const CONVERT_JOB = "studio.convert-import";
|
||||
|
||||
/** Upload cap in bytes from AppConfig.uploadMaxMb (STC_UPLOAD_MAX_MB, default 100; review I2). */
|
||||
export function uploadMaxBytes(mb: number | undefined): number {
|
||||
@@ -44,6 +45,7 @@ async function readCapped(req: Request, max: number): Promise<Uint8Array<ArrayBu
|
||||
}
|
||||
|
||||
export interface StudioFeature extends Feature {
|
||||
jobs: Record<string, JobHandler>;
|
||||
store: StudioStore;
|
||||
watcher: PublishedWatcher;
|
||||
draftWriter: DraftWriter;
|
||||
@@ -99,13 +101,22 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = {
|
||||
const readOnlyMessage = (c: LoadedCourse): string => {
|
||||
const kind = c.manifest?.source?.kind;
|
||||
const underImports = c.root === importedRoot || c.root.startsWith(importedRoot + sep);
|
||||
if (kind && kind !== "authored" && underImports) {
|
||||
// TODO(studio): convert imported course to a studio-managed repo (git init into <dataDir>/studio/repos/<id>.git
|
||||
// plus a published worktree), so authors can edit it and the owner-only visibility rule applies on publish.
|
||||
return IMPORTED_MESSAGE;
|
||||
}
|
||||
if (kind && kind !== "authored" && underImports) return IMPORTED_MESSAGE;
|
||||
return OUTSIDE_MESSAGE;
|
||||
};
|
||||
const isOwner = (u: AuthUser) => u.roles.includes("owner");
|
||||
/** v1.1 S: only the owner may turn an imported course into a studio course ("Make editable in the studio"). */
|
||||
const convertible = (u: AuthUser, courseId: string) => isOwner(u) && store.isConvertibleImport(courseId);
|
||||
|
||||
/** The conversion job: payload `{ courseId, actor }`, checked again by the store under the course lock. */
|
||||
const convertJob: JobHandler = async (payload, jc) => {
|
||||
const p = (payload ?? {}) as { courseId?: unknown; actor?: { id?: unknown; name?: unknown; email?: unknown } };
|
||||
const courseId = typeof p.courseId === "string" ? p.courseId : "";
|
||||
const actor = { name: String(p.actor?.name ?? "STCourses"), email: String(p.actor?.email ?? "studio@stcourses.invalid") };
|
||||
const r = await store.convertImported(courseId, actor, { onProgress: (f, m) => jc.reportProgress(f, m) });
|
||||
await afterPublishedChange(courseId);
|
||||
return { courseId, ...r };
|
||||
};
|
||||
|
||||
const afterPublishedChange = async (courseId: string) => {
|
||||
await ctx.courses.reload(courseId);
|
||||
@@ -188,7 +199,11 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = {
|
||||
if (all) {
|
||||
for (const c of ctx.courses.list()) {
|
||||
if (isStudioCourse(c) || managed.includes(c.id)) continue;
|
||||
courses.push({ id: c.id, managed: false, title: c.manifest?.title ?? c.id, pitch: c.manifest?.pitch ?? null, accent: c.manifest?.accent ?? null, publishedStatus: c.status, message: readOnlyMessage(c) });
|
||||
const message = readOnlyMessage(c);
|
||||
courses.push({
|
||||
id: c.id, managed: false, title: c.manifest?.title ?? c.id, pitch: c.manifest?.pitch ?? null, accent: c.manifest?.accent ?? null, publishedStatus: c.status, message,
|
||||
imported: message === IMPORTED_MESSAGE, convertible: convertible(user, c.id),
|
||||
});
|
||||
}
|
||||
}
|
||||
return json({ courses, canCreate: all });
|
||||
@@ -212,12 +227,38 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = {
|
||||
if (!store.isManaged(courseId)) {
|
||||
const c = ctx.courses.get(courseId);
|
||||
if (!c) return fail(404, "no such course");
|
||||
return json({ readOnly: true, message: readOnlyMessage(c), course: { id: c.id, title: c.manifest?.title ?? c.id, managed: false, root: c.root }, diagnostics: c.diagnostics });
|
||||
const message = readOnlyMessage(c);
|
||||
return json({ readOnly: true, message, imported: message === IMPORTED_MESSAGE, convertible: convertible(user, c.id), course: { id: c.id, title: c.manifest?.title ?? c.id, managed: false, root: c.root }, diagnostics: c.diagnostics });
|
||||
}
|
||||
return json(await courseState(courseId, user));
|
||||
}),
|
||||
},
|
||||
|
||||
"/api/studio/courses/:courseId/convert": {
|
||||
POST: guard(async (_req, rc, user) => {
|
||||
const courseId = rc.params.courseId ?? "";
|
||||
if (!isOwner(user)) return fail(403, "only the owner can make an imported course editable in the studio");
|
||||
if (store.isManaged(courseId)) return fail(409, `course "${courseId}" is already a studio course`);
|
||||
if (!store.isConvertibleImport(courseId)) {
|
||||
const c = ctx.courses.get(courseId);
|
||||
return c ? fail(409, `${readOnlyMessage(c) === IMPORTED_MESSAGE ? "this imported course" : OUTSIDE_MESSAGE}: only imported courses in ${importedRoot} can be made editable`) : fail(404, "no such imported course");
|
||||
}
|
||||
const pending = (await ctx.jobs.list(CONVERT_JOB)).find((j) => (j.payload as { courseId?: string } | null)?.courseId === courseId && (j.status === "queued" || j.status === "running"));
|
||||
if (pending) return fail(409, "this course is already being made editable");
|
||||
const jobId = await ctx.jobs.enqueue(CONVERT_JOB, { courseId, actor: { id: user.id, name: who(user).name, email: who(user).email } });
|
||||
return json({ jobId }, 202);
|
||||
}),
|
||||
},
|
||||
|
||||
"/api/studio/jobs/:jobId": {
|
||||
GET: guard(async (_req, rc, user) => {
|
||||
if (!isOwner(user)) return fail(403, "only the owner can follow studio jobs");
|
||||
const j = await ctx.jobs.get(rc.params.jobId ?? "");
|
||||
if (!j || !j.kind.startsWith("studio.")) return fail(404, "no such job");
|
||||
return json({ id: j.id, kind: j.kind, status: j.status, progress: j.progress, message: j.message, error: j.error, result: j.result });
|
||||
}),
|
||||
},
|
||||
|
||||
"/api/studio/courses/:courseId/file": {
|
||||
GET: course(async (_req, rc, user, courseId) => {
|
||||
const path = rc.url.searchParams.get("path") ?? "";
|
||||
@@ -364,5 +405,5 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = {
|
||||
watcher.start();
|
||||
}
|
||||
|
||||
return { name: "studio", routes, multipartRoutes: ["/api/studio/courses/:courseId/upload"], store, watcher, draftWriter, stop: () => watcher.stop() };
|
||||
return { name: "studio", routes, jobs: { [CONVERT_JOB]: convertJob }, multipartRoutes: ["/api/studio/courses/:courseId/upload"], store, watcher, draftWriter, stop: () => watcher.stop() };
|
||||
}
|
||||
@@ -1,6 +1,6 @@
|
||||
import { resolve } from "node:path";
|
||||
import type { AppConfig, AuthUser, ServerContext } from "@stc/contracts";
|
||||
import { createApp, resolveConfig } from "@stc/core";
|
||||
import type { AppConfig, AuthUser, JobQueue, ServerContext } from "@stc/contracts";
|
||||
import { createApp, createJobQueue, resolveConfig } from "@stc/core";
|
||||
import { createCourseRepository } from "@stc/course-format";
|
||||
import { connect, migrate } from "@stc/db";
|
||||
import { studioFeature } from "../src/server/feature";
|
||||
@@ -34,10 +34,14 @@ export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite:/
|
||||
const config = resolveConfig({ port: 0, databaseUrl: "sqlite://:memory:", dataDir, courseDirs: extraCourseDirs, secret: "t", publicUrl: "http://x", ...configOverrides });
|
||||
const courses = await createCourseRepository(studioCourseDirs(config));
|
||||
const ctx = {
|
||||
config, db, courses, jobs: null as never,
|
||||
config, db, courses, jobs: null as unknown as JobQueue,
|
||||
auth: { userFromRequest: async (req: Request) => users[req.headers.get("x-user") ?? ""] ?? null },
|
||||
} satisfies ServerContext;
|
||||
const feature = studioFeature(ctx, { watch: false });
|
||||
// the real queue, as composeServer builds it from the features' jobs
|
||||
const queue = createJobQueue(db, feature.jobs ?? {}, { pollMs: 20 });
|
||||
ctx.jobs = queue;
|
||||
queue.start();
|
||||
const app = createApp(ctx, [feature]);
|
||||
const call = async (as: string | null, method: string, path: string, body?: unknown) => {
|
||||
const headers: Record<string, string> = { origin: "http://x" };
|
||||
@@ -51,5 +55,14 @@ export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite:/
|
||||
try { json = JSON.parse(text); } catch { json = text; }
|
||||
return { status: r.status, json };
|
||||
};
|
||||
return { ctx, db, dataDir, feature, app, call, stop: async () => { feature.stop(); await db.close(); await dropSchema(); } };
|
||||
/** Polls a job until it is done or failed. */
|
||||
const waitJob = async (id: string) => {
|
||||
for (let i = 0; i < 500; i++) {
|
||||
const j = await queue.get(id);
|
||||
if (j && (j.status === "done" || j.status === "failed")) return j;
|
||||
await Bun.sleep(20);
|
||||
}
|
||||
throw new Error(`job ${id} did not finish`);
|
||||
};
|
||||
return { ctx, db, dataDir, feature, app, call, waitJob, stop: async () => { feature.stop(); await queue.stop(); await db.close(); await dropSchema(); } };
|
||||
}
|
||||
@@ -0,0 +1,128 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { existsSync, readFileSync } from "node:fs";
|
||||
import { join, resolve } from "node:path";
|
||||
import { REPO, setupApp } from "./app-helpers";
|
||||
import { makeImportedCourse, VIDEO, videoBytes } from "./import-fixture";
|
||||
|
||||
let env: Awaited<ReturnType<typeof setupApp>>;
|
||||
afterEach(() => env?.stop());
|
||||
|
||||
const PG = process.env.STC_TEST_PG_URL;
|
||||
const dbs: [string, string][] = [["sqlite", "sqlite://:memory:"], ...(PG ? [["postgres", PG] as [string, string]] : [])];
|
||||
|
||||
async function convert(as = "owner", id = "imp") {
|
||||
const r = await env.call(as, "POST", `/api/studio/courses/${id}/convert`);
|
||||
expect(r.status).toBe(202);
|
||||
const job = await env.waitJob(r.json.jobId);
|
||||
expect(job.error).toBeNull();
|
||||
expect(job.status).toBe("done");
|
||||
return job;
|
||||
}
|
||||
|
||||
describe.each(dbs)("convert routes [%s]", (_name, dbUrl) => {
|
||||
test("the owner converts; the course is then served from the published worktree, still restricted, ACL kept", async () => {
|
||||
env = await setupApp([], dbUrl, (d) => makeImportedCourse(d));
|
||||
const { call, ctx, db } = env;
|
||||
await db.sql`INSERT INTO course_acl (course_id, principal_type, principal_id) VALUES ('imp', 'user', 'u-lea')`;
|
||||
expect(ctx.courses.get("imp")?.root).toBe(join(env.dataDir, "courses/imp"));
|
||||
|
||||
const ro = (await call("owner", "GET", "/api/studio/courses/imp")).json;
|
||||
expect(ro).toMatchObject({ readOnly: true, convertible: true });
|
||||
expect((await call("ada", "GET", "/api/studio/courses/imp")).json).toMatchObject({ readOnly: true, convertible: false });
|
||||
expect((await call("owner", "GET", "/api/studio/courses")).json.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, imported: true, convertible: true }));
|
||||
|
||||
const job = await convert();
|
||||
expect(job.result).toMatchObject({ courseId: "imp", media: 3, resumed: false });
|
||||
const c = ctx.courses.get("imp")!;
|
||||
expect(c.root).toBe(join(env.dataDir, "worktrees/imp/published"));
|
||||
expect(c.status).toBe("ok");
|
||||
expect(c.manifest?.visibility).toBe("restricted");
|
||||
expect(ctx.courses.list().filter((x) => x.id === "imp")).toHaveLength(1);
|
||||
expect(await db.sql`SELECT principal_id FROM course_acl WHERE course_id = 'imp'`).toEqual([{ principal_id: "u-lea" }]);
|
||||
|
||||
// media through core's route, Range included
|
||||
const r = await env.app.fetch(new Request(`http://x/api/media/imp/${VIDEO.split("/").map(encodeURIComponent).join("/")}`, { headers: { "x-user": "lea", range: "bytes=10-19" } }));
|
||||
expect(r.status).toBe(206);
|
||||
expect(new Uint8Array(await r.arrayBuffer())).toEqual(videoBytes(4096, 7).slice(10, 20));
|
||||
// anonymous users still see nothing
|
||||
expect((await env.app.fetch(new Request(`http://x/api/media/imp/${encodeURI(VIDEO)}`))).status).toBe(404);
|
||||
|
||||
// the studio now edits it
|
||||
const st = (await call("owner", "GET", "/api/studio/courses/imp")).json;
|
||||
expect(st.readOnly).toBe(false);
|
||||
expect(st.files).toContain(VIDEO);
|
||||
expect((await call("owner", "GET", "/api/studio/courses")).json.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: true }));
|
||||
});
|
||||
|
||||
test("an author edits and publishes a lesson title but cannot make the course public; the owner can", async () => {
|
||||
env = await setupApp([], dbUrl, (d) => makeImportedCourse(d));
|
||||
const { call, ctx } = env;
|
||||
await convert();
|
||||
let st = (await call("ada", "GET", "/api/studio/courses/imp")).json;
|
||||
const m = structuredClone(st.manifestRaw);
|
||||
m.modules[0].lessons[0].title = "Welcome aboard";
|
||||
expect((await call("ada", "PUT", "/api/studio/courses/imp/files", { message: "Rename", manifest: m, manifestBaseHash: st.manifestHash })).status).toBe(200);
|
||||
expect((await call("ada", "POST", "/api/studio/courses/imp/publish")).json).toMatchObject({ ok: true, published: true });
|
||||
expect(ctx.courses.get("imp")?.lessonIndex.get("l1")?.lesson.title).toBe("Welcome aboard");
|
||||
|
||||
st = (await call("ada", "GET", "/api/studio/courses/imp")).json;
|
||||
const pub = structuredClone(st.manifestRaw);
|
||||
pub.visibility = "public";
|
||||
expect((await call("ada", "PUT", "/api/studio/courses/imp/files", { message: "Go public", manifest: pub, manifestBaseHash: st.manifestHash })).status).toBe(403);
|
||||
st = (await call("owner", "GET", "/api/studio/courses/imp")).json;
|
||||
expect((await call("owner", "PUT", "/api/studio/courses/imp/files", { message: "Go public", manifest: { ...st.manifestRaw, visibility: "public" }, manifestBaseHash: st.manifestHash })).status).toBe(200);
|
||||
expect((await call("owner", "POST", "/api/studio/courses/imp/publish")).status).toBe(200);
|
||||
expect(ctx.courses.get("imp")?.manifest?.visibility).toBe("public");
|
||||
});
|
||||
});
|
||||
|
||||
describe("convert route refusals", () => {
|
||||
test("only the owner; cross-site requests are refused; nothing changes", async () => {
|
||||
env = await setupApp([], undefined, (d) => makeImportedCourse(d));
|
||||
for (const who of ["ada", "lea"]) expect((await env.call(who, "POST", "/api/studio/courses/imp/convert")).status).toBe(403);
|
||||
expect((await env.call(null, "POST", "/api/studio/courses/imp/convert")).status).toBe(401);
|
||||
const forged = await env.app.fetch(new Request("http://x/api/studio/courses/imp/convert", { method: "POST", headers: { "x-user": "owner", origin: "http://evil.example" } }));
|
||||
expect(forged.status).toBe(403);
|
||||
const noOrigin = await env.app.fetch(new Request("http://x/api/studio/courses/imp/convert", { method: "POST", headers: { "x-user": "owner" } }));
|
||||
expect(noOrigin.status).toBe(403);
|
||||
expect(existsSync(join(env.dataDir, "courses/imp/stcourse.yaml"))).toBe(true);
|
||||
expect(existsSync(join(env.dataDir, "studio/repos/imp.git"))).toBe(false);
|
||||
expect(await env.ctx.jobs.list("studio.convert-import")).toEqual([]);
|
||||
});
|
||||
|
||||
test("overlay, studio and unknown courses; a second request while one is pending", async () => {
|
||||
env = await setupApp([resolve(REPO, "fixtures/courses")], undefined, (d) => makeImportedCourse(d));
|
||||
expect((await env.call("owner", "POST", "/api/studio/courses/sample/convert")).status).toBe(409);
|
||||
expect((await env.call("owner", "POST", "/api/studio/courses/nope/convert")).status).toBe(404);
|
||||
await env.call("owner", "POST", "/api/studio/courses", { id: "c1", title: "C" });
|
||||
expect((await env.call("owner", "POST", "/api/studio/courses/c1/convert")).status).toBe(409);
|
||||
|
||||
await env.ctx.jobs.enqueue("studio.convert-import", { courseId: "imp", actor: { id: "u-own", name: "O", email: "o@x" } });
|
||||
// whether the first job is still queued, running or already done, a second request is refused
|
||||
const r = await env.call("owner", "POST", "/api/studio/courses/imp/convert");
|
||||
expect(r.status).toBe(409);
|
||||
});
|
||||
|
||||
test("job status is owner-only and limited to studio jobs", async () => {
|
||||
env = await setupApp([], undefined, (d) => makeImportedCourse(d));
|
||||
const { jobId } = (await env.call("owner", "POST", "/api/studio/courses/imp/convert")).json;
|
||||
expect((await env.call("ada", "GET", `/api/studio/jobs/${jobId}`)).status).toBe(403);
|
||||
await env.waitJob(jobId);
|
||||
const j = (await env.call("owner", "GET", `/api/studio/jobs/${jobId}`)).json;
|
||||
expect(j).toMatchObject({ id: jobId, status: "done", progress: 1, error: null });
|
||||
const other = await env.ctx.jobs.enqueue("import.transcribe", { courseId: "x" });
|
||||
expect((await env.call("owner", "GET", `/api/studio/jobs/${other}`)).status).toBe(404);
|
||||
});
|
||||
|
||||
test("a failed conversion reports the error and leaves the import as it was", async () => {
|
||||
env = await setupApp([], undefined, (d) => makeImportedCourse(d));
|
||||
const manifest = join(env.dataDir, "courses/imp/stcourse.yaml");
|
||||
// the route only checks that this is an imported course; the job validates the manifest fully
|
||||
await Bun.write(manifest, readFileSync(manifest, "utf8").replace("id: imp", "id: other"));
|
||||
const { jobId } = (await env.call("owner", "POST", "/api/studio/courses/imp/convert")).json;
|
||||
const j = await env.waitJob(jobId);
|
||||
expect(j.status).toBe("failed");
|
||||
expect(j.error).toContain('"other"');
|
||||
expect(existsSync(join(env.dataDir, "studio/repos/imp.git"))).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { existsSync, mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { IMPORTED_MESSAGE } from "../src/server/feature";
|
||||
import { isAllowedRemote, ownerRequiredFor } from "../src/server/store";
|
||||
import { ada, extGit, newStore, tempDir } from "./helpers";
|
||||
import { setupApp } from "./app-helpers";
|
||||
@@ -229,11 +230,11 @@ describe("I4: layout and imported courses", () => {
|
||||
});
|
||||
expect(env.ctx.courses.get("imp")?.status).toBe("ok");
|
||||
const list = (await env.call("ada", "GET", "/api/studio/courses")).json;
|
||||
expect(list.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, message: "Imported course: read-only in the studio for now" }));
|
||||
expect((await env.call("ada", "GET", "/api/studio/courses/imp")).json.message).toBe("Imported course: read-only in the studio for now");
|
||||
expect(list.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, message: IMPORTED_MESSAGE }));
|
||||
expect((await env.call("ada", "GET", "/api/studio/courses/imp")).json.message).toBe(IMPORTED_MESSAGE);
|
||||
const w = await env.call("ada", "PUT", "/api/studio/courses/imp/files", { message: "x", files: [{ path: "a.md", content: "x" }] });
|
||||
expect(w.status).toBe(403);
|
||||
expect(w.json.error).toBe("Imported course: read-only in the studio for now");
|
||||
expect(w.json.error).toBe(IMPORTED_MESSAGE);
|
||||
expect((await env.call("ada", "POST", "/api/studio/courses", { id: "imp", title: "Dup" })).status).toBe(409);
|
||||
// studio repos are not in the importer's folder
|
||||
expect((await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" })).status).toBe(201);
|
||||
|
||||
Reference in new issue
Block a user