diff --git a/apps/api/src/mcp/policy.ts b/apps/api/src/mcp/policy.ts index f4882ce..3a40bd1 100644 --- a/apps/api/src/mcp/policy.ts +++ b/apps/api/src/mcp/policy.ts @@ -94,6 +94,8 @@ const toolPolicies = { update_sequence_email: MCP_SCOPES.sequencesWrite, send_sequence_email_test: MCP_SCOPES.sequencesWrite, delete_sequence_email: MCP_SCOPES.sequencesWrite, + delete_sequence: MCP_SCOPES.sequencesWrite, + preview_sequence_deletion: MCP_SCOPES.sequencesRead, start_sequence: MCP_SCOPES.sequencesWrite, pause_sequence: MCP_SCOPES.sequencesWrite, diff --git a/apps/api/src/mcp/server.test.ts b/apps/api/src/mcp/server.test.ts index 931f2b7..4d8c86c 100644 --- a/apps/api/src/mcp/server.test.ts +++ b/apps/api/src/mcp/server.test.ts @@ -67,11 +67,11 @@ describe("MCP server", () => { expect(client.getProtocolEra()).toBe("modern"); const result = await client.listTools(); - expect(result.tools).toHaveLength(109); + expect(result.tools).toHaveLength(111); expect(result.tools.map((tool) => tool.name).sort()).toEqual( Object.keys(listMcpToolPolicies()).sort(), ); - expect(new Set(result.tools.map((tool) => tool.name)).size).toBe(109); + expect(new Set(result.tools.map((tool) => tool.name)).size).toBe(111); expect(result.ttlMs).toBe(300_000); expect(result.cacheScope).toBe("private"); for (const tool of result.tools) { diff --git a/apps/api/src/mcp/tools/sequences.ts b/apps/api/src/mcp/tools/sequences.ts index 6a48e36..a56ce03 100644 --- a/apps/api/src/mcp/tools/sequences.ts +++ b/apps/api/src/mcp/tools/sequences.ts @@ -1,6 +1,7 @@ import { contactFilterSchema, emailContentInputSchema, + sequenceDeletionPreviewSchema, sequenceEmailTestResultSchema, updateSequenceBodySchema, } from "@sendlit/api-contract"; @@ -8,15 +9,18 @@ import { z } from "zod"; import { broadcastPacing } from "../../abuse/pacing-status"; import { sequenceDeliveryRates } from "../../automation/sending-health"; import { emailActionTypes, mailTypes } from "../../config/constants"; +import { sequenceDeleteBlockerMessage } from "../../sequences/delete-blockers"; import type { HydratedSequence } from "../../sequences/queries"; import { addMailToSequence, countSequences, createSequence, deleteMailFromSequence, + deleteSequence, getEmailSentCount, getSequenceBySequenceId, getSequenceClickThroughRate, + getSequenceDeletionImpact, getSequenceOpenRate, getSubscribers, getSubscribersCount, @@ -39,7 +43,12 @@ import { NOT_FOUND, planGateResult, } from "./responses"; -import { sequenceListSchema, sequenceSchema, sequenceStatsSchema } from "./schemas"; +import { + sequenceListSchema, + sequenceSchema, + sequenceStatsSchema, + successMessageSchema, +} from "./schemas"; // `sequenceEmails.sequenceId` stores its *parent* sequence's internal `id` // (see the file-level doc comment in `db/schema.ts`) — dropped here rather @@ -391,6 +400,54 @@ export function registerSequenceTools(server: McpToolRegistrar): void { }, ); + server.registerTool( + "preview_sequence_deletion", + { + description: + "Shows what delete_sequence would remove without changing anything: how many contacts are enrolled, how many emails it has sent, and whether it can be deleted now (blocker sequence_active means pause it first).", + inputSchema: { sequenceId: z.string() }, + outputSchema: sequenceDeletionPreviewSchema, + annotations: { + readOnlyHint: true, + destructiveHint: false, + openWorldHint: false, + }, + }, + async (args: any, extra: any) => { + const teamId = getTeamId(extra); + if (!teamId) return AUTH_ERROR; + const preview = await getSequenceDeletionImpact(teamId, args.sequenceId); + if (!preview) return NOT_FOUND; + return jsonResult(preview); + }, + ); + + server.registerTool( + "delete_sequence", + { + description: + "Permanently deletes a broadcast or sequence with its emails and triggers. An active one must be paused first (pause_sequence). Contacts still enrolled leave it, and its delivery, open and click history (its stats) is deleted and can't be recovered; call preview_sequence_deletion first and confirm with the user. Images only it used are deleted; images in sent mail are kept.", + inputSchema: { sequenceId: z.string() }, + outputSchema: successMessageSchema, + annotations: { + readOnlyHint: false, + destructiveHint: true, + openWorldHint: false, + }, + }, + async (args: any, extra: any) => { + const teamId = getTeamId(extra); + if (!teamId) return AUTH_ERROR; + try { + const deleted = await deleteSequence({ teamId, sequenceId: args.sequenceId }); + if (!deleted) return NOT_FOUND; + return jsonResult({ message: "Sequence deleted." }); + } catch (err: any) { + return errorResult(sequenceDeleteBlockerMessage(err.message) ?? err.message); + } + }, + ); + server.registerTool( "start_sequence", { diff --git a/apps/api/src/mcp/tools/tools.test.ts b/apps/api/src/mcp/tools/tools.test.ts index 0da94fe..ce40611 100644 --- a/apps/api/src/mcp/tools/tools.test.ts +++ b/apps/api/src/mcp/tools/tools.test.ts @@ -17,6 +17,8 @@ const mocks = { countSelection: vi.fn(), createSequence: vi.fn(), + deleteSequence: vi.fn(), + getSequenceDeletionImpact: vi.fn(), getSequenceBySequenceId: vi.fn(), getEmailSentCount: vi.fn(), getSequenceOpenRate: vi.fn(), @@ -147,6 +149,8 @@ mock.module("../../sequences/queries", () => ({ addMailToSequence: mocks.addMailToSequence, updateMailInSequence: mocks.updateMailInSequence, deleteMailFromSequence: mocks.deleteMailFromSequence, + deleteSequence: mocks.deleteSequence, + getSequenceDeletionImpact: mocks.getSequenceDeletionImpact, startSequence: mocks.startSequence, pauseSequence: mocks.pauseSequence, })); @@ -496,6 +500,54 @@ describe("MCP contact tools", () => { }); describe("MCP sequence tools", () => { + it("explains why an active sequence can't be deleted", async () => { + const tools = makeToolRegistry(registerSequenceTools); + mocks.deleteSequence.mockRejectedValueOnce(new Error("sequence_active")); + + const result = await tools + .get("delete_sequence")! + .handler({ sequenceId: "seq-1" }, auth); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("Pause it before deleting it"); + expect(tools.get("delete_sequence")!.config.annotations.destructiveHint).toBe(true); + }); + + it("deletes a sequence of the caller's team", async () => { + const tools = makeToolRegistry(registerSequenceTools); + mocks.deleteSequence.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + + const deleted = await tools + .get("delete_sequence")! + .handler({ sequenceId: "seq-1" }, auth); + expect(deleted.structuredContent).toEqual({ message: "Sequence deleted." }); + expect(mocks.deleteSequence).toHaveBeenCalledWith({ + teamId: "team-1", + sequenceId: "seq-1", + }); + await expect( + tools.get("delete_sequence")!.handler({ sequenceId: "seq-2" }, auth), + ).resolves.toEqual(NOT_FOUND); + }); + + it("previews a sequence deletion", async () => { + const tools = makeToolRegistry(registerSequenceTools); + const preview = { + status: "paused", + blocker: null, + enrolledCount: 2, + emailsSentCount: 5, + }; + mocks.getSequenceDeletionImpact.mockResolvedValueOnce(preview); + + const result = await tools + .get("preview_sequence_deletion")! + .handler({ sequenceId: "seq-1" }, auth); + + expect(result.structuredContent).toEqual(preview); + expect(mocks.getSequenceDeletionImpact).toHaveBeenCalledWith("team-1", "seq-1"); + }); + it("validates non-empty template IDs on sequence creation/add-email inputs", () => { const tools = makeToolRegistry(registerSequenceTools); diff --git a/apps/api/src/observability/instance.ts b/apps/api/src/observability/instance.ts index a332cde..a568ecb 100644 --- a/apps/api/src/observability/instance.ts +++ b/apps/api/src/observability/instance.ts @@ -81,6 +81,9 @@ const PROPERTY_ALLOWLIST = new Set([ "suppressed_count", // ESP deletion (settings/esp, organization ESPs): public esp_ id. "esp_id", + // Sequence deletion (sequences/queries.ts deleteSequence): counts only. + "enrolled_count", + "emails_sent_count", ]); /** Allowlisted keys that hold opaque record ids. They skip the package's diff --git a/apps/api/src/sequences/delete-blockers.ts b/apps/api/src/sequences/delete-blockers.ts new file mode 100644 index 0000000..9bb23c7 --- /dev/null +++ b/apps/api/src/sequences/delete-blockers.ts @@ -0,0 +1,31 @@ +/** + * Why a broadcast or sequence can't be deleted. Each code tells the caller + * what to do instead, so the REST API, MCP and dashboard explain it the same + * way. Broadcast-only blockers (#147) belong here too. + */ +export const sequenceDeleteBlockerMessages = { + // Contacts are moving through it (or a broadcast is sending), so deleting + // it would drop them mid-flow. Pausing stops new sends first. + sequence_active: + "This sequence is active. Pause it before deleting it, so no contact is dropped mid-flow.", +} as const; + +export type SequenceDeleteBlocker = keyof typeof sequenceDeleteBlockerMessages; + +export function isSequenceDeleteBlocker(code: unknown): code is SequenceDeleteBlocker { + return typeof code === "string" && Object.hasOwn(sequenceDeleteBlockerMessages, code); +} + +/** The explanation for a delete blocker, or `undefined` for any other error. */ +export function sequenceDeleteBlockerMessage(code: unknown): string | undefined { + return isSequenceDeleteBlocker(code) + ? sequenceDeleteBlockerMessages[code] + : undefined; +} + +/** The blocker for a sequence in this state, or `null` when it may be deleted. */ +export function sequenceDeleteBlocker(sequence: { + status: string; +}): SequenceDeleteBlocker | null { + return sequence.status === "active" ? "sequence_active" : null; +} diff --git a/apps/api/src/sequences/queries.test.ts b/apps/api/src/sequences/queries.test.ts index 1c9f926..bc2dfdd 100644 --- a/apps/api/src/sequences/queries.test.ts +++ b/apps/api/src/sequences/queries.test.ts @@ -47,6 +47,7 @@ const { deleteSequence, getSequenceBySequenceId, getSequenceClickThroughRate, + getSequenceDeletionImpact, getSequenceOpenRate, getSubscribers, getSubscribersCount, @@ -835,3 +836,171 @@ describe("sequence queries", () => { expect(await remainingMediaLitIds()).toEqual(["sequence-sent-media"]); }); }); + +describe("deleteSequence guards and history", () => { + async function setStatus(id: string, status: string) { + await tdb.update(sequences).set({ status }).where(eq(sequences.id, id)); + } + + /** A sequence with one contact enrolled, one delivery and one open. */ + async function sequenceWithHistory(type: "sequence" | "broadcast" = "sequence") { + const { team, contact } = await seedTeamAndContact(tdb); + const template = await makeTemplate(team.id); + const sequence = await createSequence({ + teamId: team.id, + type, + templateId: template.templateId, + }); + const email = sequence.emails[0]; + await tdb.insert(ongoingSequences).values({ + teamId: team.id, + sequenceId: sequence.id, + contactId: contact.id, + nextEmailScheduledTime: Date.now(), + }); + await tdb.insert(emailDeliveries).values({ + teamId: team.id, + sequenceId: sequence.id, + contactId: contact.id, + emailId: email.id, + }); + await tdb.insert(emailEvents).values({ + teamId: team.id, + sequenceId: sequence.id, + contactId: contact.id, + emailId: email.id, + action: EmailEventAction.OPEN, + }); + return { team, contact, sequence }; + } + + it("refuses to delete an active sequence and keeps everything", async () => { + const { team, sequence } = await sequenceWithHistory(); + await setStatus(sequence.id, "active"); + + await expect( + getSequenceDeletionImpact(team.id, sequence.sequenceId), + ).resolves.toEqual({ + status: "active", + blocker: "sequence_active", + enrolledCount: 1, + emailsSentCount: 1, + }); + await expect( + deleteSequence({ teamId: team.id, sequenceId: sequence.sequenceId }), + ).rejects.toThrow("sequence_active"); + + expect(await getSequenceBySequenceId(team.id, sequence.sequenceId)).not.toBeNull(); + expect(await tdb.select().from(ongoingSequences)).toHaveLength(1); + expect(await tdb.select().from(emailDeliveries)).toHaveLength(1); + expect(await tdb.select().from(emailEvents)).toHaveLength(1); + }); + + it("refuses to delete a sending broadcast", async () => { + const { team, sequence } = await sequenceWithHistory("broadcast"); + await setStatus(sequence.id, "active"); + + await expect( + deleteSequence({ teamId: team.id, sequenceId: sequence.sequenceId }), + ).rejects.toThrow("sequence_active"); + expect(await getSequenceBySequenceId(team.id, sequence.sequenceId)).not.toBeNull(); + }); + + it("deletes a paused sequence with its enrollments and history", async () => { + const { team, contact, sequence } = await sequenceWithHistory(); + await setStatus(sequence.id, "paused"); + + await expect( + getSequenceDeletionImpact(team.id, sequence.sequenceId), + ).resolves.toEqual({ + status: "paused", + blocker: null, + enrolledCount: 1, + emailsSentCount: 1, + }); + await expect( + deleteSequence({ teamId: team.id, sequenceId: sequence.sequenceId }), + ).resolves.toBe(true); + + expect(await getSequenceBySequenceId(team.id, sequence.sequenceId)).toBeNull(); + expect(await tdb.select().from(ongoingSequences)).toHaveLength(0); + expect(await tdb.select().from(emailDeliveries)).toHaveLength(0); + expect(await tdb.select().from(emailEvents)).toHaveLength(0); + expect(await tdb.select().from(rules)).toHaveLength(0); + expect(await tdb.select().from(sequenceEmails)).toHaveLength(0); + // The contact itself stays. + expect( + await tdb.select().from(contacts).where(eq(contacts.id, contact.id)), + ).toHaveLength(1); + }); + + it("deletes a never-started draft", async () => { + const { team } = await seedTeamAndContact(tdb); + const template = await makeTemplate(team.id); + const sequence = await createSequence({ + teamId: team.id, + type: "sequence", + templateId: template.templateId, + }); + + await expect( + getSequenceDeletionImpact(team.id, sequence.sequenceId), + ).resolves.toEqual({ + status: "draft", + blocker: null, + enrolledCount: 0, + emailsSentCount: 0, + }); + await expect( + deleteSequence({ teamId: team.id, sequenceId: sequence.sequenceId }), + ).resolves.toBe(true); + expect(await tdb.select().from(sequences)).toHaveLength(0); + }); + + it("never deletes or previews another team's sequence", async () => { + const { sequence } = await sequenceWithHistory(); + await setStatus(sequence.id, "paused"); + const other = await seedTeamAndContact(tdb); + + await expect( + getSequenceDeletionImpact(other.team.id, sequence.sequenceId), + ).resolves.toBeNull(); + await expect( + deleteSequence({ teamId: other.team.id, sequenceId: sequence.sequenceId }), + ).resolves.toBe(false); + expect(await tdb.select().from(sequences)).toHaveLength(1); + expect(await tdb.select().from(emailDeliveries)).toHaveLength(1); + }); + + it("keeps the images of an email that was delivered, even after its deliveries go", async () => { + const { team, contact } = await seedTeamAndContact(tdb); + const template = await makeTemplate(team.id); + const sequence = await createSequence({ + teamId: team.id, + type: "sequence", + templateId: template.templateId, + }); + await updateMailInSequence({ + teamId: team.id, + sequenceId: sequence.sequenceId, + emailId: sequence.emails[0].emailId, + content: emailWithImage("delivered-media"), + }); + await tdb.insert(emailDeliveries).values({ + teamId: team.id, + sequenceId: sequence.id, + contactId: contact.id, + emailId: sequence.emails[0].id, + }); + await setStatus(sequence.id, "paused"); + vi.clearAllMocks(); + + await deleteSequence({ teamId: team.id, sequenceId: sequence.sequenceId }); + + expect(deleteMedia).not.toHaveBeenCalled(); + expect((await tdb.select().from(media)).map((row) => row.mediaLitId)).toEqual([ + "delivered-media", + ]); + expect(await tdb.select().from(mediaReferences)).toHaveLength(0); + }); +}); diff --git a/apps/api/src/sequences/queries.ts b/apps/api/src/sequences/queries.ts index e7f6704..899e7b7 100644 --- a/apps/api/src/sequences/queries.ts +++ b/apps/api/src/sequences/queries.ts @@ -1,5 +1,5 @@ import type { Email as EmailContent } from "@sendlit/email-editor"; -import { and, asc, count, eq, inArray } from "drizzle-orm"; +import { and, asc, count, eq, inArray, notInArray } from "drizzle-orm"; import { assertLargeBroadcastReviewed } from "../abuse/broadcast-review"; import { assertEmailContentAllowed } from "../abuse/content-scan"; import { assertNoImportUnderReview } from "../abuse/imports"; @@ -50,6 +50,7 @@ import { captureEvent } from "../observability"; import { assertMailingAddressConfigured } from "../settings/general/queries"; import { getTeam } from "../team/queries"; import { resolveStartingTemplate } from "../templates/queries"; +import { type SequenceDeleteBlocker, sequenceDeleteBlocker } from "./delete-blockers"; import { addRule, defaultEmailContent, @@ -618,18 +619,11 @@ export async function updateMailInSequence({ } /** - * Removes the media references of sequence emails about to be deleted and - * returns the media rows to pass to `deleteOrphanedMedia` after the delete. - * Media of an email that was already sent (it has a delivery or a stored - * content version) is left out: inboxes and "view in browser" pages load - * its images by URL, so it stays in the media library until the user - * deletes it there. + * The ids of the given sequence emails that were already sent: they have a + * delivery or a stored content version. */ -async function releaseEmailMedia( - teamId: string, - emailIds: string[], -): Promise { - if (!emailIds.length) return []; +async function sentEmailIds(emailIds: string[]): Promise> { + if (!emailIds.length) return new Set(); const [delivered, versioned] = await Promise.all([ db .selectDistinct({ id: emailDeliveries.emailId }) @@ -640,14 +634,31 @@ async function releaseEmailMedia( .from(emailContentVersions) .where(inArray(emailContentVersions.sequenceEmailId, emailIds)), ]); - const sentIds = new Set([...delivered, ...versioned].map((row) => row.id)); - const draftIds = emailIds.filter((id) => !sentIds.has(id)); + return new Set([...delivered, ...versioned].map((row) => row.id)); +} + +/** + * Removes the media references of sequence emails about to be deleted and + * returns the media rows to pass to `deleteOrphanedMedia` after the delete. + * Media of an email that was already sent (`sentIds`) is left out: inboxes + * and "view in browser" pages load its images by URL, so it stays in the + * media library until the user deletes it there. Pass `sentIds` when the + * deliveries that prove it are gone by now. + */ +async function releaseEmailMedia( + teamId: string, + emailIds: string[], + sentIds?: Set, +): Promise { + if (!emailIds.length) return []; + const sent = sentIds ?? (await sentEmailIds(emailIds)); + const draftIds = emailIds.filter((id) => !sent.has(id)); const kept = new Set( await deleteMediaReferencesForResource({ teamId, resourceType: "SEQUENCE_EMAIL", - resourceInternalId: [...sentIds], + resourceInternalId: emailIds.filter((id) => sent.has(id)), }), ); const released = await deleteMediaReferencesForResource({ @@ -711,6 +722,51 @@ export async function deleteMailFromSequence({ return getSequenceBySequenceId(teamId, sequenceId); } +export interface SequenceDeletionImpact { + status: string; + /** Why it can't be deleted now, or null when it can. */ + blocker: SequenceDeleteBlocker | null; + /** Contacts currently moving through it; they stop receiving it. */ + enrolledCount: number; + /** Emails it has sent; their delivery and open/click history is deleted. */ + emailsSentCount: number; +} + +/** What deleting a sequence would remove, shown before the user confirms. */ +export async function getSequenceDeletionImpact( + teamId: string, + sequenceId: string, +): Promise { + const [sequence] = await db + .select({ id: sequences.id, status: sequences.status }) + .from(sequences) + .where(and(eq(sequences.teamId, teamId), eq(sequences.sequenceId, sequenceId))); + if (!sequence) return null; + const [[enrolled], [sent]] = await Promise.all([ + db + .select({ value: count() }) + .from(ongoingSequences) + .where(eq(ongoingSequences.sequenceId, sequence.id)), + db + .select({ value: count() }) + .from(emailDeliveries) + .where(eq(emailDeliveries.sequenceId, sequence.id)), + ]); + return { + status: sequence.status, + blocker: sequenceDeleteBlocker(sequence), + enrolledCount: enrolled?.value ?? 0, + emailsSentCount: sent?.value ?? 0, + }; +} + +/** + * Permanently deletes a broadcast or sequence that isn't active (see ADR + * 0007). Contacts still enrolled in a paused sequence leave it, and its + * delivery and open/click history goes with it; the outbound message log + * and suppressions are kept. Throws a `SequenceDeleteBlocker` code when it + * can't be deleted; returns false when it doesn't exist. + */ export async function deleteSequence({ teamId, sequenceId, @@ -720,15 +776,27 @@ export async function deleteSequence({ }): Promise { const sequence = await getSequenceBySequenceId(teamId, sequenceId); if (!sequence) return false; + const blocker = sequenceDeleteBlocker(sequence); + if (blocker) throw new Error(blocker); - const mediaIds = await releaseEmailMedia( - teamId, - sequence.emails.map((email) => email.id), - ); + const impact = await getSequenceDeletionImpact(teamId, sequenceId); + const emailIds = sequence.emails.map((email) => email.id); + // Read before the delete: the deliveries that mark an email as sent + // cascade away with the sequence. + const sentIds = await sentEmailIds(emailIds); - await db + // Guarded on status so a sequence started since the read above is kept. + const removed = await db .delete(sequences) - .where(and(eq(sequences.teamId, teamId), eq(sequences.sequenceId, sequenceId))); + .where(and(eq(sequences.id, sequence.id), notInArray(sequences.status, ["active"]))) + .returning({ id: sequences.id }); + if (!removed.length) { + const current = await getSequenceBySequenceId(teamId, sequenceId); + if (!current) return false; + throw new Error(sequenceDeleteBlocker(current) ?? "sequence_active"); + } + + const mediaIds = await releaseEmailMedia(teamId, emailIds, sentIds); const media = await deleteOrphanedMedia(teamId, mediaIds); captureEvent({ @@ -739,6 +807,9 @@ export async function deleteSequence({ sequence_id: sequence.sequenceId, media_deleted_count: media.deleted, sequence_type: sequence.type, + sequence_status: sequence.status, + enrolled_count: impact?.enrolledCount ?? 0, + emails_sent_count: impact?.emailsSentCount ?? 0, }, }); diff --git a/apps/api/src/sequences/routes-purpose.test.ts b/apps/api/src/sequences/routes-purpose.test.ts index eb85e46..b982f56 100644 --- a/apps/api/src/sequences/routes-purpose.test.ts +++ b/apps/api/src/sequences/routes-purpose.test.ts @@ -6,6 +6,8 @@ const mocks = { addMailToSequence: vi.fn(), updateMailInSequence: vi.fn(), sendSequenceEmailTest: vi.fn(), + deleteSequence: vi.fn(), + getSequenceDeletionImpact: vi.fn(), }; mock.module("../auth/middleware", () => ({ @@ -34,7 +36,8 @@ mock.module("./queries", () => ({ createSequence: mocks.createSequence, addMailToSequence: mocks.addMailToSequence, countSequences: vi.fn(), - deleteSequence: vi.fn(), + deleteSequence: mocks.deleteSequence, + getSequenceDeletionImpact: mocks.getSequenceDeletionImpact, deleteMailFromSequence: vi.fn(), getEmailSentCount: vi.fn(), getSequenceBySequenceId: vi.fn(), @@ -154,3 +157,45 @@ describe("POST /sequences/:sequenceId/emails/:emailId/test", () => { ).toBe(400); }); }); + +describe("sequence delete route", () => { + it("answers 409 sequence_active for an active sequence", async () => { + mocks.deleteSequence.mockRejectedValue(new Error("sequence_active")); + + const response = await requestApp(app(), "/sequences/seq_1", { method: "DELETE" }); + + expect(response.status).toBe(409); + expect(response.json()).toEqual({ error: "sequence_active" }); + }); + + it("answers 204 once deleted and 404 when missing", async () => { + mocks.deleteSequence.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + + expect( + (await requestApp(app(), "/sequences/seq_1", { method: "DELETE" })).status, + ).toBe(204); + expect( + (await requestApp(app(), "/sequences/seq_1", { method: "DELETE" })).status, + ).toBe(404); + expect(mocks.deleteSequence).toHaveBeenCalledWith({ + teamId: "team-1", + sequenceId: "seq_1", + }); + }); + + it("previews the deletion for the caller's team", async () => { + const preview = { + status: "paused", + blocker: null, + enrolledCount: 3, + emailsSentCount: 9, + }; + mocks.getSequenceDeletionImpact.mockResolvedValue(preview); + + const response = await requestApp(app(), "/sequences/seq_1/deletion-preview"); + + expect(response.status).toBe(200); + expect(response.json()).toEqual(preview); + expect(mocks.getSequenceDeletionImpact).toHaveBeenCalledWith("team-1", "seq_1"); + }); +}); diff --git a/apps/api/src/sequences/routes.ts b/apps/api/src/sequences/routes.ts index 468d69e..c2535fa 100644 --- a/apps/api/src/sequences/routes.ts +++ b/apps/api/src/sequences/routes.ts @@ -6,6 +6,7 @@ import { requireAuth } from "../auth/middleware"; import { requireTeam } from "../auth/require-team"; import { sequenceDeliveryRates } from "../automation/sending-health"; import { planGateHttp } from "../billing/errors"; +import { captureEvent } from "../observability"; import { MAILING_ADDRESS_REQUIRED } from "../settings/general/constants"; import { omitInternal } from "../utils/public"; import { serializeDates } from "../utils/serialize"; @@ -14,6 +15,7 @@ import { updateWebPublishing, WebPublishingError, } from "../web/queries"; +import { isSequenceDeleteBlocker } from "./delete-blockers"; import { addMailToSequence, countSequences, @@ -23,6 +25,7 @@ import { getEmailSentCount, getSequenceBySequenceId, getSequenceClickThroughRate, + getSequenceDeletionImpact, getSequenceOpenRate, getSubscribers, getSubscribersCount, @@ -211,13 +214,33 @@ const impl = s.router(contract.sequences, { } }, remove: async ({ params, req }) => { - const deleted = await deleteSequence({ - teamId: (req as any).teamId, - sequenceId: params.sequenceId, - }); + const teamId = (req as any).teamId; + let deleted: boolean; + try { + deleted = await deleteSequence({ teamId, sequenceId: params.sequenceId }); + } catch (err: any) { + if (isSequenceDeleteBlocker(err.message)) { + captureEvent({ + event: "sequence_delete_blocked", + source: "sequences.remove", + teamId, + properties: { sequence_id: params.sequenceId, reason: err.message }, + }); + return { status: 409, body: { error: err.message } }; + } + throw err; + } if (!deleted) return { status: 404, body: { error: "Sequence not found" } }; return { status: 204, body: undefined }; }, + deletionPreview: async ({ params, req }) => { + const preview = await getSequenceDeletionImpact( + (req as any).teamId, + params.sequenceId, + ); + if (!preview) return { status: 404, body: { error: "Sequence not found" } }; + return { status: 200, body: preview }; + }, start: async ({ params, req }) => { try { const sequence = await startSequence({ diff --git a/apps/docs/content/docs/developers/errors.mdx b/apps/docs/content/docs/developers/errors.mdx index 7e549a7..e510378 100644 --- a/apps/docs/content/docs/developers/errors.mdx +++ b/apps/docs/content/docs/developers/errors.mdx @@ -44,6 +44,7 @@ template or transactional delivery. | `422` | `event_name_limit_reached` | The event name is new and the team already has 500. Nothing was stored. | | `409` | `event_definition_exists` | An event with that name is already registered. | | `409` | `event_definition_in_use` | The event can't be deleted while a sequence uses it as its trigger. | +| `409` | `sequence_active` | An active sequence, or a sending broadcast, can't be deleted. Pause it first. | | `404` | `event_definition_not_found` | No event with that name is registered. | | `400` | `invalid_trigger_type` | The trigger type isn't allowed for this broadcast or sequence. | | `400` | `invalid_trigger_data` | The trigger value is invalid, for example an event name over 128 characters or starting with `$`. | diff --git a/apps/docs/content/docs/developers/mcp.mdx b/apps/docs/content/docs/developers/mcp.mdx index c2494a8..f3c561c 100644 --- a/apps/docs/content/docs/developers/mcp.mdx +++ b/apps/docs/content/docs/developers/mcp.mdx @@ -119,6 +119,7 @@ Broadcasts use the sequence resource with one email and an audience filter. For - `list_sequences`, `get_sequence`, `create_sequence`, `update_sequence` - `add_sequence_email`, `update_sequence_email`, `delete_sequence_email` +- `preview_sequence_deletion`, `delete_sequence` (destructive: pause an active sequence first; its enrollments and stats are deleted) - `send_sequence_email_test` sends a `[Test]` copy of one email to you (or up to five addresses) - `start_sequence`, `pause_sequence`, `get_sequence_stats`, `get_sequence_subscribers` - `get_broadcast_web_publishing`, `update_broadcast_web_publishing`, `list_web_issues`, `get_web_issue` ([web archive](/email-marketing/web-archive)) diff --git a/apps/docs/content/docs/email-marketing/sequences.mdx b/apps/docs/content/docs/email-marketing/sequences.mdx index eed897f..1e55ef2 100644 --- a/apps/docs/content/docs/email-marketing/sequences.mdx +++ b/apps/docs/content/docs/email-marketing/sequences.mdx @@ -119,3 +119,28 @@ paused or before it is first activated, including custom events, are not replayed when it is activated. Sequences pause themselves on high bounce or complaint rates like [broadcasts](/email-marketing/broadcasts#automatic-pause-on-bounces-and-complaints). + +## Delete a sequence + +Delete a sequence from the **⋯** menu on the Sequences list, or with +**Delete** on the sequence page. An active sequence can't be deleted: pause it +first, so no contact is dropped in the middle of a send. The API answers +`409 sequence_active`. + +Deleting a sequence is permanent and can't be undone. The confirmation says +what goes with it: + +- Contacts still enrolled leave the sequence and get no more of its emails. + The contacts themselves are kept. +- Its stats go too: the delivery, open and click history of every email it + sent. The outbound message log and the suppression list are kept, so + bounces and unsubscribes still protect your sender reputation. +- Images only this sequence used are deleted from the media library. Images in + emails it already sent are kept, because inboxes still load them. + +If you might want the numbers later, export or note them before you delete. +To stop a sequence but keep its history, pause it instead. + +With the API, `GET /sequences/{sequenceId}/deletion-preview` returns the +enrolled-contact and sent-email counts, then `DELETE /sequences/{sequenceId}` +deletes it. MCP clients use `preview_sequence_deletion` and `delete_sequence`. diff --git a/apps/web/app/(dashboard)/sequences/[sequenceId]/page.tsx b/apps/web/app/(dashboard)/sequences/[sequenceId]/page.tsx index f2ac01a..b17b86e 100644 --- a/apps/web/app/(dashboard)/sequences/[sequenceId]/page.tsx +++ b/apps/web/app/(dashboard)/sequences/[sequenceId]/page.tsx @@ -12,12 +12,14 @@ import { type SequenceStats, TagEditor, } from "@sendlit/email-blocks"; -import { Check, Pause, Pencil, Play } from "lucide-react"; +import { Check, Pause, Pencil, Play, Trash2 } from "lucide-react"; import Link from "next/link"; +import { useRouter } from "next/navigation"; import { use, useEffect, useState } from "react"; import { AutoPauseBanner } from "@/components/dashboard/auto-pause-banner"; import { Banner } from "@/components/dashboard/banner"; import { useSetBreadcrumb } from "@/components/dashboard/breadcrumb-context"; +import { DeleteSequenceDialog } from "@/components/dashboard/delete-sequence-dialog"; import { DeliverySourcePicker } from "@/components/dashboard/delivery-source-picker"; import { MARKETING_EMAIL_EDITOR_BLOCKS, @@ -111,6 +113,8 @@ export default function SequenceEditorPage({ const [templates, setTemplates] = useState([]); const [templatesLoading, setTemplatesLoading] = useState(false); const [sendingOptions, setSendingOptions] = useState([]); + const [deleteOpen, setDeleteOpen] = useState(false); + const router = useRouter(); useSetBreadcrumb([ { label: "Sequences", href: "/sequences" }, @@ -292,18 +296,39 @@ export default function SequenceEditorPage({ {sequence.status} - {sequence.status !== "completed" && ( - - )} + {sequence.status !== "completed" && ( + + )} + + router.push("/sequences")} + /> + {error && ( diff --git a/apps/web/components/dashboard/delete-sequence-dialog.test.tsx b/apps/web/components/dashboard/delete-sequence-dialog.test.tsx new file mode 100644 index 0000000..7244c83 --- /dev/null +++ b/apps/web/components/dashboard/delete-sequence-dialog.test.tsx @@ -0,0 +1,83 @@ +import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"; +import { registerDom } from "@sendlit/test-utils/dom"; + +registerDom(); + +const getSequenceDeletionPreview = mock(async (_sequenceId: string) => ({ + status: "paused", + blocker: null as "sequence_active" | null, + enrolledCount: 3, + emailsSentCount: 12, +})); +const deleteSequence = mock(async (_sequenceId: string) => undefined); + +mock.module("@/lib/api", () => ({ getSequenceDeletionPreview, deleteSequence })); + +const { act, cleanup, fireEvent, render, screen } = await import( + "@testing-library/react" +); +const { DeleteSequenceDialog } = await import("./delete-sequence-dialog"); + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +function confirmButton() { + return screen.getByRole("button", { name: "Delete sequence" }) as HTMLButtonElement; +} + +beforeEach(() => { + getSequenceDeletionPreview.mockClear(); + deleteSequence.mockClear(); +}); +afterEach(cleanup); + +describe("DeleteSequenceDialog", () => { + it("says how many contacts are enrolled and that stats are lost, then deletes", async () => { + const onDeleted = mock(); + const onOpenChange = mock(); + render( + , + ); + await settle(); + + expect(getSequenceDeletionPreview).toHaveBeenCalledWith("seq_1"); + expect(screen.getByText('Delete "Onboarding"?')).toBeTruthy(); + expect(screen.getByText(/3 contacts are\s+enrolled/)).toBeTruthy(); + expect(screen.getByText(/Stats for 12 sent emails/)).toBeTruthy(); + + fireEvent.click(confirmButton()); + await settle(); + + expect(deleteSequence).toHaveBeenCalledWith("seq_1"); + expect(onDeleted).toHaveBeenCalled(); + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it("refuses to delete an active sequence and says to pause it", async () => { + getSequenceDeletionPreview.mockResolvedValueOnce({ + status: "active", + blocker: "sequence_active", + enrolledCount: 1, + emailsSentCount: 1, + }); + render( + , + ); + await settle(); + + expect(screen.getByText(/Pause it before deleting it/)).toBeTruthy(); + expect(confirmButton().disabled).toBe(true); + expect(deleteSequence).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/components/dashboard/delete-sequence-dialog.tsx b/apps/web/components/dashboard/delete-sequence-dialog.tsx new file mode 100644 index 0000000..9cbb226 --- /dev/null +++ b/apps/web/components/dashboard/delete-sequence-dialog.tsx @@ -0,0 +1,124 @@ +"use client"; + +import type { SequenceDeletionPreview } from "@sendlit/api-contract"; +import { useEffect, useState } from "react"; +import { DeleteConfirmationDialog } from "@/components/dashboard/delete-confirmation-dialog"; +import { Loading } from "@/components/dashboard/loading"; +import { deleteSequence, getSequenceDeletionPreview } from "@/lib/api"; +import { ApiError } from "@/lib/api-client"; + +const ACTIVE_MESSAGE = + "This sequence is active. Pause it before deleting it, so no contact is dropped mid-flow."; + +function plural(count: number, one: string, many: string) { + return `${count.toLocaleString()} ${count === 1 ? one : many}`; +} + +/** + * Confirms deleting a sequence. Shows how many contacts are enrolled and how + * many emails it sent, because both go with it (ADR 0007), and refuses while + * the sequence is active. + */ +export function DeleteSequenceDialog({ + sequence, + onOpenChange, + onDeleted, +}: { + /** The sequence to delete; the dialog is open while this is set. */ + sequence: { sequenceId: string; title: string } | null; + onOpenChange: (open: boolean) => void; + onDeleted: () => Promise | void; +}) { + const sequenceId = sequence?.sequenceId; + const [preview, setPreview] = useState(null); + const [loadError, setLoadError] = useState(null); + + useEffect(() => { + setPreview(null); + setLoadError(null); + if (!sequenceId) return; + let cancelled = false; + getSequenceDeletionPreview(sequenceId) + .then((result) => { + if (!cancelled) setPreview(result); + }) + .catch((err) => { + if (!cancelled) { + setLoadError( + err instanceof ApiError ? err.message : "Couldn't load this sequence.", + ); + } + }); + return () => { + cancelled = true; + }; + }, [sequenceId]); + + const blocked = preview?.blocker === "sequence_active"; + const name = sequence?.title || "Untitled sequence"; + + return ( + + error instanceof ApiError && error.message === "sequence_active" + ? ACTIVE_MESSAGE + : "Unable to delete this sequence. Please try again." + } + onConfirm={async () => { + if (!sequenceId) return; + await deleteSequence(sequenceId); + await onDeleted(); + }} + > + {loadError ? ( +

+ {loadError} +

+ ) : !preview ? ( + + ) : blocked ? ( +

+ {ACTIVE_MESSAGE} +

+ ) : ( +
    +
  • + {preview.enrolledCount > 0 ? ( + <> + + {plural(preview.enrolledCount, "contact is", "contacts are")} enrolled + {" "} + and will get no more of its emails. The contacts themselves are kept. + + ) : ( + "No contacts are enrolled." + )} +
    • +
  • + {preview.emailsSentCount > 0 ? ( + <> + + Stats for{" "} + {plural(preview.emailsSentCount, "sent email", "sent emails")} + {" "} + (deliveries, opens and clicks) are deleted and can't be recovered. To + keep them, leave the sequence paused instead; it sends nothing. + + ) : ( + "It hasn't sent any email, so no stats are lost." + )} +
    • +
+ )} + + ); +} diff --git a/apps/web/components/dashboard/sequence-list.tsx b/apps/web/components/dashboard/sequence-list.tsx index 13896f9..2c2d834 100644 --- a/apps/web/components/dashboard/sequence-list.tsx +++ b/apps/web/components/dashboard/sequence-list.tsx @@ -1,11 +1,12 @@ "use client"; import type { MailType, Sequence } from "@sendlit/email-blocks"; -import { Pause, Play } from "lucide-react"; +import { MoreHorizontal, Pause, Play, Trash2 } from "lucide-react"; import { useRouter } from "next/navigation"; import { useEffect, useState } from "react"; import { Banner } from "@/components/dashboard/banner"; import { useSetBreadcrumb } from "@/components/dashboard/breadcrumb-context"; +import { DeleteSequenceDialog } from "@/components/dashboard/delete-sequence-dialog"; import { ListPagination } from "@/components/dashboard/list-pagination"; import { Loading } from "@/components/dashboard/loading"; import { NewSequenceDialog } from "@/components/dashboard/new-sequence-dialog"; @@ -14,6 +15,12 @@ import { ScrollablePage } from "@/components/dashboard/scrollable-page"; import { SendErrorMessage } from "@/components/dashboard/send-error-message"; import { Badge } from "@/components/ui/codelit/badge"; import { Card, CardContent } from "@/components/ui/codelit/card"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@/components/ui/codelit/dropdown-menu"; import { IconButton } from "@/components/ui/codelit/icon-button"; import { listSequences, pauseSequence, startSequence } from "@/lib/api"; import { ApiError } from "@/lib/api-client"; @@ -46,6 +53,9 @@ export function SequenceListPage({ const [page, setPage] = useState(1); const [error, setError] = useState(null); const [loadingPage, setLoadingPage] = useState(false); + const [sequencePendingDelete, setSequencePendingDelete] = useState( + null, + ); useSetBreadcrumb([{ label: title }]); @@ -98,6 +108,17 @@ export function SequenceListPage({ } /> + { + if (!open) setSequencePendingDelete(null); + }} + onDeleted={async () => { + const lastOnPage = sequences?.length === 1 && page > 1; + await load(lastOnPage ? page - 1 : page); + }} + /> + {error && ( @@ -175,6 +196,38 @@ export function SequenceListPage({ )} )} + {/* Broadcast delete and archive arrive with #147. */} + {type === "sequence" && ( + + + e.stopPropagation()} + > + + + + e.stopPropagation()} + > + setSequencePendingDelete(sequence)} + > + + {sequence.status === "active" + ? "Delete (pause first)" + : "Delete"} + + + + )} ))} diff --git a/apps/web/lib/api.ts b/apps/web/lib/api.ts index 9781ffe..93e5b7b 100644 --- a/apps/web/lib/api.ts +++ b/apps/web/lib/api.ts @@ -15,6 +15,7 @@ import { type PaidPlan, type RenameTagResult, type Reputation, + type SequenceDeletionPreview, type SignupForm, type SignupFormInput, type TagReferences, @@ -716,6 +717,13 @@ export function pauseSequence(sequenceId: string) { return unwrap(client.sequences.pause({ params: { sequenceId } })); } +/** Enrolled contacts and sent emails a delete would remove, and any blocker. */ +export function getSequenceDeletionPreview(sequenceId: string) { + return unwrap( + client.sequences.deletionPreview({ params: { sequenceId } }), + ); +} + export function getSequenceStats(sequenceId: string) { return unwrap(client.sequences.stats({ params: { sequenceId } })); } diff --git a/docs/decisions/0007-delete-paused-sequences-with-their-history.md b/docs/decisions/0007-delete-paused-sequences-with-their-history.md new file mode 100644 index 0000000..5a9ce28 --- /dev/null +++ b/docs/decisions/0007-delete-paused-sequences-with-their-history.md @@ -0,0 +1,55 @@ +# ADR 0007: Delete paused sequences with their history; archive is for broadcasts + +Status: accepted +Date: 2026-10-29 + +## Context + +`DELETE /sequences/:sequenceId` had no status guard (#148). Deleting a +sequence cascades through `ongoing_sequences`, `rules`, `sequence_entries`, +`email_deliveries` and `email_events`, so deleting an active sequence dropped +contacts mid-flow, and deleting one that had sent mail silently removed its +stats. We had to choose between archiving a sequence that has sent mail +(stats kept, hidden from the list) and hard deleting it after an explicit +warning. + +Email marketing platforms treat automations and campaigns differently: + +- Mailchimp Customer Journeys and ActiveCampaign automations must be paused + or turned off before they can be deleted, and deleting removes the + automation together with its contacts' progress and its reporting. + Klaviyo deletes flows the same way, after a confirmation. +- Sent campaigns, by contrast, are archived or kept for their report and + public archive page (Mailchimp's campaign archive). + +A sequence's value is the running automation, not a one-off report, and a +paused sequence already keeps its stats for as long as anyone wants them. + +## Decision + +- An active broadcast or sequence can't be deleted: `409 sequence_active`. + Pausing stops sends first. The check is repeated in the delete statement, + so one started in between is kept. +- A paused, draft or completed sequence is hard deleted, with its emails, + triggers, enrollments, and delivery, open and click history. Before + confirming, the dashboard and MCP show what goes with it: + `GET /sequences/:sequenceId/deletion-preview` returns the enrolled-contact + and sent-email counts, and the dialog says the stats can't be recovered. + Pausing remains the way to stop a sequence and keep its numbers. +- Records that protect sending are kept: `outbound_messages` (its link to + the delivery is set null), suppressions and the contacts themselves. +- Media follows ADR 0006: images of sent emails are kept, the rest is + deleted when nothing else uses it. +- Blocker codes live in `apps/api/src/sequences/delete-blockers.ts`, shared + by REST, MCP and the dashboard. Broadcasts use the same table and delete + path; #147 adds archive for sent broadcasts and its own blocker there, + rather than a separate mechanism. + +## Consequences + +- No new column or "Archived" filter for sequences; deleted sequences are + gone, and so are their per-sequence stats. +- A deleted sequence's sends still count against plan usage, because usage + is counted from `outbound_messages` and usage buckets, not `email_deliveries`. +- If customers ask to keep automation reports after deletion, archive can be + extended from broadcasts (#147) to sequences without changing this API. diff --git a/packages/api-contract/src/contract.ts b/packages/api-contract/src/contract.ts index 56eea6e..7f4b876 100644 --- a/packages/api-contract/src/contract.ts +++ b/packages/api-contract/src/contract.ts @@ -115,6 +115,7 @@ import { listSequencesQuerySchema, listSubscribersQuerySchema, sendSequenceEmailTestBodySchema, + sequenceDeletionPreviewSchema, sequenceEmailTestResultSchema, sequenceSchema, sequenceStatsSchema, @@ -620,10 +621,18 @@ const sequencesContract = c.router( remove: { method: "DELETE", path: "/sequences/:sequenceId", - responses: { 204: c.noBody(), 404: errorSchema }, + responses: { 204: c.noBody(), 404: errorSchema, 409: errorSchema }, summary: "Delete a broadcast or sequence", description: - "Permanently deletes the broadcast or sequence. Images it uses that nothing else uses are deleted from the media library and storage; images used by sent mail are kept.", + "Permanently deletes the broadcast or sequence, its emails and triggers. An active one can't be deleted: pause it first (409 `sequence_active`). Contacts still enrolled in a paused sequence leave it, and its delivery, open and click history is deleted too, so its stats are gone; check GET /sequences/{sequenceId}/deletion-preview first. The outbound message log and suppressions are kept. Images it uses that nothing else uses are deleted from the media library and storage; images used by sent mail are kept.", + }, + deletionPreview: { + method: "GET", + path: "/sequences/:sequenceId/deletion-preview", + responses: { 200: sequenceDeletionPreviewSchema, 404: errorSchema }, + summary: "Preview what deleting a broadcast or sequence removes", + description: + "How many contacts are enrolled and how many emails it has sent, and whether it can be deleted right now. Nothing is changed.", }, addEmail: { method: "POST", diff --git a/packages/api-contract/src/mcp-parity.ts b/packages/api-contract/src/mcp-parity.ts index a545be8..f68d511 100644 --- a/packages/api-contract/src/mcp-parity.ts +++ b/packages/api-contract/src/mcp-parity.ts @@ -139,10 +139,8 @@ export const mcpParityManifest: readonly McpParityManifestEntry[] = [ required("sequences.list", "list_sequences", "read"), required("sequences.get", "get_sequence", "read"), required("sequences.update", "update_sequence", "write"), - restOnly( - "sequences.remove", - "Deleting a whole broadcast or sequence (and its delivery history) is REST/dashboard-only; MCP can remove individual sequence emails and pause sequences.", - ), + required("sequences.remove", "delete_sequence", "destructive"), + required("sequences.deletionPreview", "preview_sequence_deletion", "read"), required("sequences.addEmail", "add_sequence_email", "write"), required("sequences.updateEmail", "update_sequence_email", "write"), required("contacts.import", "import_contacts", "write"), diff --git a/packages/api-contract/src/schemas/sequences.ts b/packages/api-contract/src/schemas/sequences.ts index fde01a0..d803a5f 100644 --- a/packages/api-contract/src/schemas/sequences.ts +++ b/packages/api-contract/src/schemas/sequences.ts @@ -290,6 +290,26 @@ export const sequenceStatsSchema = z.object({ complaintRate: z.number().optional(), }); +export const sequenceDeletionPreviewSchema = z.object({ + status: z.string().describe("The broadcast or sequence's current status"), + blocker: z + .enum(["sequence_active"]) + .nullable() + .describe( + "Why it can't be deleted right now (sequence_active: pause it first), or null when it can", + ), + enrolledCount: z + .number() + .int() + .describe("Contacts currently in the sequence; deleting it stops their emails"), + emailsSentCount: z + .number() + .int() + .describe("Emails it has sent; their delivery, open and click history is deleted"), +}); + +export type SequenceDeletionPreview = z.infer; + export const listSubscribersQuerySchema = z.object({ page: z.coerce.number().int().min(1).optional(), limit: z.coerce.number().int().min(1).optional(),