diff --git a/src/routes/admin.test.ts b/src/routes/admin.test.ts index 52e9fae..4b7178d 100644 --- a/src/routes/admin.test.ts +++ b/src/routes/admin.test.ts @@ -728,6 +728,79 @@ describe("Admin Routes", () => { .length; expect(indicatorCount).toBe(1); }); + + it("form-based bulk-delete also removes R2 attachments", async () => { + const r2Env = createMockEnv({ withR2: true }) as unknown as Env; + const bucket = r2Env.ATTACHMENT_BUCKET as unknown as { + put: (k: string, v: string) => Promise; + _has: (k: string) => boolean; + }; + await bucket.put("att-1", "data1"); + await bucket.put("att-2", "data2"); + + const loginForm = new FormData(); + loginForm.append("password", "test-password"); + const loginRes = await testApp.request( + "/admin/login", + { method: "POST", body: loginForm }, + r2Env, + ); + const authCookie = (loginRes.headers.get("Set-Cookie") as string).split( + ";", + )[0]; + + const feedId = "bulk-r2-feed"; + await r2Env.EMAIL_STORAGE.put( + "feeds:list", + JSON.stringify({ feeds: [{ id: feedId, title: "F" }] }), + ); + const emailKey = `feed:${feedId}:1`; + await r2Env.EMAIL_STORAGE.put( + emailKey, + JSON.stringify({ + subject: "x", + from: "a@b.c", + content: "

x

", + receivedAt: 1, + headers: {}, + attachments: [{ id: "att-1" }, { id: "att-2" }], + }), + ); + await r2Env.EMAIL_STORAGE.put( + `feed:${feedId}:metadata`, + JSON.stringify({ + emails: [ + { + key: emailKey, + subject: "x", + receivedAt: 1, + attachmentIds: ["att-1", "att-2"], + }, + ], + }), + ); + + const form = new FormData(); + form.append("emailKeys", emailKey); + const res = await testApp.request( + `/admin/feeds/${feedId}/emails/bulk-delete`, + { + method: "POST", + headers: { + Cookie: authCookie, + Origin: "https://test.getmynews.app", + }, + body: form, + }, + r2Env, + ); + + expect(res.status).toBe(302); + expect(res.headers.get("Location")).toContain("bulkDeleted"); + expect(await r2Env.EMAIL_STORAGE.get(emailKey, "json")).toBeNull(); + expect(bucket._has("att-1")).toBe(false); + expect(bucket._has("att-2")).toBe(false); + }); }); }); diff --git a/src/routes/admin/emails.tsx b/src/routes/admin/emails.tsx index bf44b11..517cbc1 100644 --- a/src/routes/admin/emails.tsx +++ b/src/routes/admin/emails.tsx @@ -8,9 +8,11 @@ import { } from "../../types"; import { logger } from "../../lib/logger"; import { Layout, clampText } from "./ui"; -import { deleteKeysWithConcurrency } from "./helpers"; +import { + deleteAttachmentsForEmails, + deleteKeysWithConcurrency, +} from "./helpers"; import { feedRssUrl, feedAtomUrl, feedEmailAddress } from "../../utils/urls"; -import { getAttachmentBucket } from "../../utils/attachments"; import { emailsPageScript } from "../../scripts/generated/emails-page"; type AppEnv = { Bindings: Env }; @@ -632,10 +634,11 @@ emailsRouter.post("/emails/:emailKey/delete", async (c) => { const feedMetadata = (await emailStorage.get(feedMetadataKey, { type: "json", })) as FeedMetadata | null; - const attachmentIds = - feedMetadata?.emails.find((e) => e.key === emailKey)?.attachmentIds ?? []; await emailStorage.delete(emailKey); + await deleteAttachmentsForEmails(env, feedMetadata?.emails ?? [], [ + emailKey, + ]); if (feedMetadata) { feedMetadata.emails = feedMetadata.emails.filter( @@ -644,13 +647,6 @@ emailsRouter.post("/emails/:emailKey/delete", async (c) => { await emailStorage.put(feedMetadataKey, JSON.stringify(feedMetadata)); } - const attachmentBucket = getAttachmentBucket(env); - if (attachmentBucket && attachmentIds.length > 0) { - await Promise.allSettled( - attachmentIds.map((id) => attachmentBucket.delete(id)), - ); - } - if (wantsJson) return c.json({ ok: true, emailKey, feedId }); return c.redirect(`/admin/feeds/${feedId}/emails`); } catch (error) { @@ -714,13 +710,10 @@ emailsRouter.post("/feeds/:feedId/emails/bulk-delete", async (c) => { } const candidates = emailKeys.filter((key) => allowedKeys.has(key)); - const candidateSet = new Set(candidates); - const r2AttachmentIds = feedMetadata.emails - .filter((e) => candidateSet.has(e.key)) - .flatMap((e) => e.attachmentIds ?? []); const { ok: deletedOk, failed: failedEmailKeys } = await deleteKeysWithConcurrency(emailStorage, candidates, 35); + await deleteAttachmentsForEmails(env, feedMetadata.emails, candidates); const deletedSet = new Set(deletedOk); feedMetadata.emails = feedMetadata.emails.filter( @@ -728,13 +721,6 @@ emailsRouter.post("/feeds/:feedId/emails/bulk-delete", async (c) => { ); await emailStorage.put(feedMetadataKey, JSON.stringify(feedMetadata)); - const attachmentBucket = getAttachmentBucket(env); - if (attachmentBucket && r2AttachmentIds.length > 0) { - await Promise.allSettled( - r2AttachmentIds.map((id) => attachmentBucket.delete(id)), - ); - } - return c.json({ ok: failedEmailKeys.length === 0, deletedEmailKeys: deletedOk, @@ -752,11 +738,13 @@ emailsRouter.post("/feeds/:feedId/emails/bulk-delete", async (c) => { return c.redirect(`/admin/feeds/${feedId}/emails?message=bulkDeleteNoop`); const candidates = emailKeys.filter((key) => allowedKeys.has(key)); + const { ok: deletedOk } = await deleteKeysWithConcurrency( emailStorage, candidates, 35, ); + await deleteAttachmentsForEmails(env, feedMetadata.emails, candidates); const deletedSet = new Set(deletedOk); feedMetadata.emails = feedMetadata.emails.filter( diff --git a/src/routes/admin/helpers.ts b/src/routes/admin/helpers.ts index c21da69..7119596 100644 --- a/src/routes/admin/helpers.ts +++ b/src/routes/admin/helpers.ts @@ -1,6 +1,34 @@ -import { EmailData, FeedList, FeedListItem, FeedMetadata } from "../../types"; +import { + EmailData, + EmailMetadata, + Env, + FeedList, + FeedListItem, + FeedMetadata, +} from "../../types"; import { FEEDS_LIST_KEY } from "../../config/constants"; import { logger } from "../../lib/logger"; +import { getAttachmentBucket } from "../../utils/attachments"; + +// Delete the R2 attachments belonging to the given email keys. Call before the +// emails are removed from feed metadata, while `emails` still carries their +// attachmentIds. +export async function deleteAttachmentsForEmails( + env: Env, + emails: EmailMetadata[], + keys: Iterable, +): Promise { + const keySet = new Set(keys); + const attachmentIds = emails + .filter((e) => keySet.has(e.key)) + .flatMap((e) => e.attachmentIds ?? []); + if (attachmentIds.length === 0) return; + + const bucket = getAttachmentBucket(env); + if (!bucket) return; + + await Promise.allSettled(attachmentIds.map((id) => bucket.delete(id))); +} export async function deleteKeysWithConcurrency( emailStorage: KVNamespace,