From c58ed12e4773e98f0d1cd7072d7aa44720f75158 Mon Sep 17 00:00:00 2001 From: tianma-if Date: Fri, 28 Aug 2026 19:59:56 +0800 Subject: [PATCH] fix: make attachment deletion recoverable Fixes #314 --- apps/api/src/index.ts | 11 ++- apps/api/src/resource-routes.test.mjs | 69 +++++++++++++++++++ apps/api/src/resource-routes.ts | 29 ++++---- apps/api/src/resource-service.ts | 1 + apps/web/src/components/AssetsPane.tsx | 57 ++++++++++++++- .../src/components/dialogs/ConfirmDialogs.tsx | 3 + packages/shared/src/i18n/en-US.ts | 3 + packages/shared/src/i18n/zh-CN.ts | 3 + 8 files changed, 160 insertions(+), 16 deletions(-) diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 1bb0c115..17cc64b4 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -3735,14 +3735,19 @@ const shouldSnapshotMemoRevision = async ( return Date.parse(now) - Date.parse(latest.created_at) >= REVISION_SNAPSHOT_INTERVAL_MS; }; -const getResourceRow = async (db: D1Database, workspaceId: string, id: string): Promise => +const getResourceRow = async ( + db: D1Database, + workspaceId: string, + id: string, + includeDeleted = false, +): Promise => db .prepare( `SELECT r.id, r.memo_id, r.original_memo_id, r.bucket_name, r.object_key, r.storage_config_id, r.kind, r.mime_type, - r.filename, r.byte_size, r.sha256, r.width, r.height, r.created_at, r.updated_at + r.filename, r.byte_size, r.sha256, r.width, r.height, r.created_at, r.updated_at, r.is_deleted FROM resources r INNER JOIN memos m ON m.id = r.memo_id - WHERE r.id = ? AND m.workspace_id = ? AND r.is_deleted = 0` + WHERE r.id = ? AND m.workspace_id = ?${includeDeleted ? "" : " AND r.is_deleted = 0"}` ) .bind(id, workspaceId) .first(); diff --git a/apps/api/src/resource-routes.test.mjs b/apps/api/src/resource-routes.test.mjs index 29e064d9..8beb6c29 100644 --- a/apps/api/src/resource-routes.test.mjs +++ b/apps/api/src/resource-routes.test.mjs @@ -57,6 +57,7 @@ const createEnvironment = () => ({ const createApp = (auth = agentAuth, getResourceRow = async () => null) => { const app = new Hono(); + app.onError((error, context) => context.json({ error: { message: error.message } }, 500)); app.use("/api/v1/*", async (context, next) => { context.set("auth", auth); await next(); @@ -175,4 +176,72 @@ describe("resource route contracts", () => { expect(response.headers.get("Content-Range")).toBe("bytes */256"); expect(reads).toBe(0); }); + + test("deletes object bytes before marking the resource deleted", async () => { + const events = []; + const environment = createEnvironment(); + environment.storage.resources = { + delete: async (key) => { events.push(`object:${key}`); }, + }; + environment.storage.db.batch = async () => { events.push("database"); return []; }; + let includeDeleted = false; + const response = await createApp( + { ...agentAuth, scopes: ["write:resources"] }, + async (_database, _workspaceId, _resourceId, requestedIncludeDeleted) => { + includeDeleted = requestedIncludeDeleted; + return { ...resourceRow, storage_config_id: null, is_deleted: 0 }; + }, + ).request( + "/api/v1/resources/res_1", + { method: "DELETE" }, + environment, + ); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ ok: true }); + expect(includeDeleted).toBe(true); + expect(events).toEqual(["object:resource-key", "database"]); + }); + + test("leaves the database active when object deletion fails so the request can be retried", async () => { + const environment = createEnvironment(); + let batches = 0; + environment.storage.resources = { + delete: async () => { throw new Error("storage unavailable"); }, + }; + environment.storage.db.batch = async () => { batches += 1; return []; }; + const response = await createApp( + { ...agentAuth, scopes: ["write:resources"] }, + async () => ({ ...resourceRow, storage_config_id: null, is_deleted: 0 }), + ).request( + "/api/v1/resources/res_1", + { method: "DELETE" }, + environment, + ); + + expect(response.status).toBe(500); + expect(batches).toBe(0); + }); + + test("retries object cleanup for an already deleted resource without duplicating its audit event", async () => { + const environment = createEnvironment(); + let deletes = 0; + let batches = 0; + environment.storage.resources = { + delete: async () => { deletes += 1; }, + }; + environment.storage.db.batch = async () => { batches += 1; return []; }; + const response = await createApp( + { ...agentAuth, scopes: ["write:resources"] }, + async () => ({ ...resourceRow, storage_config_id: null, is_deleted: 1 }), + ).request( + "/api/v1/resources/res_1", + { method: "DELETE" }, + environment, + ); + + expect(response.status).toBe(200); + expect(deletes).toBe(1); + expect(batches).toBe(0); + }); }); diff --git a/apps/api/src/resource-routes.ts b/apps/api/src/resource-routes.ts index b1e99f2b..e33f82f9 100644 --- a/apps/api/src/resource-routes.ts +++ b/apps/api/src/resource-routes.ts @@ -49,6 +49,7 @@ type ResourceRouteDependencies = { database: DatabaseAdapter, workspaceId: string, resourceId: string, + includeDeleted?: boolean, ) => Promise; }; @@ -233,23 +234,27 @@ export const registerResourceRoutes = ( context.env.storage.db, getWorkspaceId(context), resourceId, + true, ); if (!resource) return notFound(context, "Resource not found"); - const now = isoNow(); - const actor = getAuditActor(context); - await context.env.storage.db.batch([ - context.env.storage.db.prepare( - `UPDATE resources SET is_deleted = 1, deleted_at = ?, updated_at = ? WHERE id = ?`, - ).bind(now, now, resourceId), - auditStatement(context.env.storage.db, actor.actorType, actor.actorId, "resource.delete", "resource", resourceId, { - memoId: resource.memo_id, - filename: resource.filename, - byteSize: resource.byte_size, - }), - ]); const source = await resolveObjectStorage(context.env, resource.storage_config_id); await source.store.delete(resource.object_key); + + if (!resource.is_deleted) { + const now = isoNow(); + const actor = getAuditActor(context); + await context.env.storage.db.batch([ + context.env.storage.db.prepare( + `UPDATE resources SET is_deleted = 1, deleted_at = ?, updated_at = ? WHERE id = ?`, + ).bind(now, now, resourceId), + auditStatement(context.env.storage.db, actor.actorType, actor.actorId, "resource.delete", "resource", resourceId, { + memoId: resource.memo_id, + filename: resource.filename, + byteSize: resource.byte_size, + }), + ]); + } return context.json({ ok: true }); }); }; diff --git a/apps/api/src/resource-service.ts b/apps/api/src/resource-service.ts index 9ed4b975..a6f84be9 100644 --- a/apps/api/src/resource-service.ts +++ b/apps/api/src/resource-service.ts @@ -32,6 +32,7 @@ export type ResourceRow = { height: number | null; created_at: string; updated_at: string; + is_deleted?: number; }; export type ResourceListRow = ResourceRow & { diff --git a/apps/web/src/components/AssetsPane.tsx b/apps/web/src/components/AssetsPane.tsx index 0499d748..343e81c2 100644 --- a/apps/web/src/components/AssetsPane.tsx +++ b/apps/web/src/components/AssetsPane.tsx @@ -1,5 +1,5 @@ import { useCallback, useMemo, useState } from "react"; -import { useQuery } from "@tanstack/react-query"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useTranslation } from "react-i18next"; import Lightbox from "yet-another-react-lightbox"; import Zoom from "yet-another-react-lightbox/plugins/zoom"; @@ -18,6 +18,7 @@ import { List, X, Loader2, + Trash2, } from "lucide-react"; import { Button } from "@/components/ui/button"; @@ -29,6 +30,7 @@ import { isPdfAttachment, type ResourceListItem } from "@edgeever/shared"; import { PdfViewer } from "@/components/pdf/PdfViewer"; import { PdfThumbnail } from "@/components/pdf/PdfThumbnail"; import { AttachmentFileIcon } from "@/components/attachments/AttachmentFileIcon"; +import { AppConfirmDialog } from "@/components/dialogs/ConfirmDialogs"; export const formatBytes = (bytes: number) => { if (!Number.isFinite(bytes) || bytes <= 0) { @@ -66,6 +68,7 @@ interface AssetsPaneProps { export const AssetsPane = ({ onClose, repository }: AssetsPaneProps) => { const { t } = useTranslation(); + const queryClient = useQueryClient(); // States const [searchQuery, setSearchQuery] = useState(""); @@ -75,6 +78,7 @@ export const AssetsPane = ({ onClose, repository }: AssetsPaneProps) => { }); const [lightboxIndex, setLightboxIndex] = useState(null); const [pdfPreview, setPdfPreview] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); // Query resources const resourcesQuery = useQuery({ @@ -90,6 +94,19 @@ export const AssetsPane = ({ onClose, repository }: AssetsPaneProps) => { attachmentCount: 0, }; + const deleteMutation = useMutation({ + mutationFn: (resourceId: string) => repository.deleteResource(resourceId), + onSuccess: async () => { + setDeleteTarget(null); + await queryClient.invalidateQueries({ queryKey: ["resources"] }); + }, + }); + + const requestResourceDelete = (resource: ResourceListItem) => { + deleteMutation.reset(); + setDeleteTarget(resource); + }; + // Filter Logic const filteredResources = useMemo(() => { return resources.filter((resource) => { @@ -307,6 +324,18 @@ export const AssetsPane = ({ onClose, repository }: AssetsPaneProps) => { key={resource.id} className="group relative flex flex-col overflow-hidden rounded-xl border border-slate-200 bg-white shadow-sm transition-all duration-200 hover:border-emerald-500/40 hover:shadow-md" > + + + {/* Thumbnail area */}
{
{/* Right Actions */} + + + { onNext={pdfResources.length > 1 ? showNextPdf : undefined} /> ) : null} + {deleteTarget ? ( + { + deleteMutation.reset(); + setDeleteTarget(null); + }} + onConfirm={() => deleteMutation.mutate(deleteTarget.id)} + /> + ) : null} ); }; diff --git a/apps/web/src/components/dialogs/ConfirmDialogs.tsx b/apps/web/src/components/dialogs/ConfirmDialogs.tsx index 6827c246..dc59fea3 100644 --- a/apps/web/src/components/dialogs/ConfirmDialogs.tsx +++ b/apps/web/src/components/dialogs/ConfirmDialogs.tsx @@ -17,6 +17,7 @@ export const AppConfirmDialog = ({ cancelLabel, confirmLabel, description, + error, hideCancel = false, isWorking = false, title, @@ -28,6 +29,7 @@ export const AppConfirmDialog = ({ cancelLabel?: string; confirmLabel: string; description: string; + error?: string | null; hideCancel?: boolean; isWorking?: boolean; title: string; @@ -60,6 +62,7 @@ export const AppConfirmDialog = ({ {description} + {error ?

{error}

: null} diff --git a/packages/shared/src/i18n/en-US.ts b/packages/shared/src/i18n/en-US.ts index e33b2270..82bdd757 100644 --- a/packages/shared/src/i18n/en-US.ts +++ b/packages/shared/src/i18n/en-US.ts @@ -934,6 +934,9 @@ export const enUS = { fromMemo: "From: {{source}}", sourceMemo: "Source note: {{source}}", openInNewWindow: "Open in new window", + deleteTitle: "Delete attachment", + deleteDescription: "The attachment will be permanently removed from storage. Related links in the note will no longer open. This cannot be undone.", + deleteAria: "Delete attachment {{filename}}", }, tagsDialog: { title: "Tags", diff --git a/packages/shared/src/i18n/zh-CN.ts b/packages/shared/src/i18n/zh-CN.ts index 42e66ca6..bc77a123 100644 --- a/packages/shared/src/i18n/zh-CN.ts +++ b/packages/shared/src/i18n/zh-CN.ts @@ -934,6 +934,9 @@ export const zhCN = { fromMemo: "来自：{{source}}", sourceMemo: "来源笔记：{{source}}", openInNewWindow: "在新窗口打开", + deleteTitle: "删除附件", + deleteDescription: "附件将从存储空间中永久删除，笔记中的相关链接将无法再打开。此操作无法撤销。", + deleteAria: "删除附件 {{filename}}", }, tagsDialog: { title: "标签",