fix(plugin-forms): give the digest cron task a valid name

.changeset/forms-digest-task-name.md

@@ -0,0 +1,5 @@
+---
+"@emdash-cms/plugin-forms": patch
+---
+
+Fixes the daily digest. Creating a form with the digest on, or turning it on for an existing form, failed with an "Invalid task name" error, and a form created that way was saved with the digest on but never sent one. The digest is now scheduled, and is rescheduled or cancelled when the form's digest settings change.

packages/plugins/forms/src/digest-task.ts

@@ -0,0 +1,18 @@
+/**
+ * The cron task behind a form's daily digest.
+ *
+ * `ctx.cron` accepts a task name of letters, numbers, dashes and underscores that starts with a letter,
+ * so the form id is joined with a dash.
+ */
+
+const PREFIX = "digest-";
+
+/** The task name for one form's digest. */
+export function digestTaskName(formId: string): string {
+	return `${PREFIX}${formId}`;
+}
+
+/** The form a digest task belongs to, or null when the task is not a digest. */
+export function formIdFromDigestTask(taskName: string): string | null {
+	return taskName.startsWith(PREFIX) ? taskName.slice(PREFIX.length) : null;
+}

packages/plugins/forms/src/handlers/cron.ts

@@ -7,6 +7,7 @@
 
 import type { PluginContext, StorageCollection } from "emdash";
 
+import { digestTaskName } from "../digest-task.js";
 import { formatDigestText } from "../format.js";
 import type { FormDefinition, Submission } from "../types.js";
 
@@ -93,14 +94,14 @@ export async function handleCleanup(ctx: PluginContext) {
 /**
  * Daily digest: send summary email for a specific form.
  *
- * The cron task name contains the form ID: "digest:{formId}"
+ * The cron task name contains the form ID (see `digestTaskName`).
  */
 export async function handleDigest(formId: string, ctx: PluginContext) {
 	const form = await forms(ctx).get(formId);
 	if (!form) {
 		ctx.log.warn("Digest: form not found, cancelling", { formId });
 		if (ctx.cron) {
-			await ctx.cron.cancel(`digest:${formId}`).catch(() => {});
+			await ctx.cron.cancel(digestTaskName(formId)).catch(() => {});
 		}
 		return;
 	}

packages/plugins/forms/src/handlers/forms.ts

@@ -8,6 +8,7 @@ import type { RouteContext, StorageCollection } from "emdash";
 import { PluginRouteError } from "emdash";
 import { ulid } from "ulidx";
 
+import { digestTaskName } from "../digest-task.js";
 import type {
 	FormCreateInput,
 	FormDeleteInput,
@@ -88,7 +89,7 @@ export async function formsCreateHandler(ctx: RouteContext<FormCreateInput>) {
 
 	// Schedule digest cron if enabled
 	if (form.settings.digestEnabled && ctx.cron) {
-		await ctx.cron.schedule(`digest:${id}`, {
+		await ctx.cron.schedule(digestTaskName(id), {
 			schedule: `0 ${form.settings.digestHour} * * *`,
 		});
 	}
@@ -140,16 +141,16 @@ export async function formsUpdateHandler(ctx: RouteContext<FormUpdateInput>) {
 	// Update digest cron if settings changed
 	if (ctx.cron) {
 		if (updated.settings.digestEnabled && !existing.settings.digestEnabled) {
-			await ctx.cron.schedule(`digest:${input.id}`, {
+			await ctx.cron.schedule(digestTaskName(input.id), {
 				schedule: `0 ${updated.settings.digestHour} * * *`,
 			});
 		} else if (!updated.settings.digestEnabled && existing.settings.digestEnabled) {
-			await ctx.cron.cancel(`digest:${input.id}`);
+			await ctx.cron.cancel(digestTaskName(input.id));
 		} else if (
 			updated.settings.digestEnabled &&
 			updated.settings.digestHour !== existing.settings.digestHour
 		) {
-			await ctx.cron.schedule(`digest:${input.id}`, {
+			await ctx.cron.schedule(digestTaskName(input.id), {
 				schedule: `0 ${updated.settings.digestHour} * * *`,
 			});
 		}
@@ -175,7 +176,7 @@ export async function formsDeleteHandler(ctx: RouteContext<FormDeleteInput>) {
 
 	// Cancel digest cron
 	if (ctx.cron) {
-		await ctx.cron.cancel(`digest:${input.id}`).catch(() => {});
+		await ctx.cron.cancel(digestTaskName(input.id)).catch(() => {});
 	}
 
 	await forms(ctx).delete(input.id);

packages/plugins/forms/src/index.ts

@@ -22,6 +22,7 @@ import type { PluginDescriptor, ResolvedPlugin } from "emdash";
 import { definePlugin } from "emdash";
 
 import { version } from "../package.json";
+import { formIdFromDigestTask } from "./digest-task.js";
 import { handleCleanup, handleDigest } from "./handlers/cron.js";
 import {
 	formsCreateHandler,
@@ -112,9 +113,9 @@ export function createPlugin(_options: FormsPluginOptions = {}): ResolvedPlugin
 				handler: async (event, ctx) => {
 					if (event.name === "cleanup") {
 						await handleCleanup(ctx);
-					} else if (event.name.startsWith("digest:")) {
-						const formId = event.name.slice("digest:".length);
-						await handleDigest(formId, ctx);
+					} else {
+						const formId = formIdFromDigestTask(event.name);
+						if (formId) await handleDigest(formId, ctx);
 					}
 				},
 			},

packages/plugins/forms/tests/digest-task.test.ts

@@ -0,0 +1,87 @@
+import { describe, expect, it, vi } from "vitest";
+
+import { digestTaskName, formIdFromDigestTask } from "../src/digest-task.js";
+import { formsCreateHandler, formsUpdateHandler } from "../src/handlers/forms.js";
+import { formCreateSchema, formUpdateSchema } from "../src/schemas.js";
+import type { FormDefinition } from "../src/types.js";
+
+/** The rule `ctx.cron` applies to a task name (`validateTaskName` in core's `plugins/cron.ts`). */
+const TASK_NAME_RE = /^[a-zA-Z][a-zA-Z0-9_-]*$/;
+
+/** A plugin context whose cron refuses the names the real one refuses. */
+function context(input: unknown, stored = new Map<string, FormDefinition>()) {
+	const check = (name: string) => {
+		if (!TASK_NAME_RE.test(name)) throw new Error(`Invalid task name "${name}"`);
+	};
+	const schedule = vi.fn(async (name: string, _opts: { schedule: string }) => check(name));
+	const cancel = vi.fn(async (name: string) => check(name));
+	const ctx = {
+		input,
+		storage: {
+			forms: {
+				get: async (id: string) => stored.get(id) ?? null,
+				put: async (id: string, form: FormDefinition) => void stored.set(id, form),
+				query: async () => ({ items: [] }),
+			},
+		},
+		cron: { schedule, cancel },
+	};
+	return { ctx: ctx as never, schedule, cancel, stored };
+}
+
+const fields = [
+	{ id: "f1", type: "text", label: "Email", name: "email", required: false, width: "full" },
+];
+
+describe("the daily digest's cron task", () => {
+	it("is scheduled when a form is created with the digest on", async () => {
+		const { ctx, schedule, stored } = context(
+			formCreateSchema.parse({
+				name: "Contact",
+				slug: "contact",
+				pages: [{ fields }],
+				settings: { notifyEmails: ["editor@example.com"], digestEnabled: true, digestHour: 7 },
+			}),
+		);
+
+		const created = await formsCreateHandler(ctx);
+
+		expect(stored.has(created.id)).toBe(true);
+		expect(schedule).toHaveBeenCalledWith(`digest-${created.id}`, { schedule: "0 7 * * *" });
+	});
+
+	it("is scheduled, moved and cancelled as an existing form's digest settings change", async () => {
+		const first = context(
+			formCreateSchema.parse({
+				name: "Contact",
+				slug: "contact",
+				pages: [{ fields }],
+				settings: {},
+			}),
+		);
+		const { id } = await formsCreateHandler(first.ctx);
+		expect(first.schedule).not.toHaveBeenCalled();
+
+		const update = async (settings: Record<string, unknown>) => {
+			const next = context(formUpdateSchema.parse({ id, settings }), first.stored);
+			await formsUpdateHandler(next.ctx);
+			return next;
+		};
+
+		const on = await update({ digestEnabled: true });
+		expect(on.schedule).toHaveBeenCalledWith(`digest-${id}`, { schedule: "0 9 * * *" });
+
+		const moved = await update({ digestHour: 18 });
+		expect(moved.schedule).toHaveBeenCalledWith(`digest-${id}`, { schedule: "0 18 * * *" });
+
+		const off = await update({ digestEnabled: false });
+		expect(off.cancel).toHaveBeenCalledWith(`digest-${id}`);
+	});
+
+	it("has a name the plugin maps back to its form", () => {
+		const name = digestTaskName("01M4C00952745BYW2ZD0KB1G0N");
+		expect(name).toMatch(TASK_NAME_RE);
+		expect(formIdFromDigestTask(name)).toBe("01M4C00952745BYW2ZD0KB1G0N");
+		expect(formIdFromDigestTask("cleanup")).toBeNull();
+	});
+});