From 813d972fdffee2e7dd6cdb3b358e94f1fefce8d5 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Tue, 13 Jan 2026 18:26:02 +0200 Subject: [PATCH] Fix re-saving drafts with existing media When re-saving a draft, the code was trying to copy media files that were already in drafts storage to new locations, causing copy errors. Changes: - Add extractLocalIdFromPath() to detect if a path is already in drafts - Track loadedMediaMap in ComposerState for identifying reusable media - Only delete old media that wasn't reused during re-save - Pass loadedMediaMap when saving to enable media reuse detection - Disable pointer events on draft media preview Co-Authored-By: Claude Opus 4.5 --- src/state/drafts/hooks.ts | 137 +++++++++++++++------ src/state/drafts/storage.ts | 19 +++ src/state/drafts/storage.web.ts | 14 +++ src/view/com/composer/Composer.tsx | 2 + src/view/com/composer/drafts/DraftItem.tsx | 2 +- src/view/com/composer/state/composer.ts | 4 + 6 files changed, 140 insertions(+), 38 deletions(-) diff --git a/src/state/drafts/hooks.ts b/src/state/drafts/hooks.ts index 2123d87bf4..679ccb55a8 100644 --- a/src/state/drafts/hooks.ts +++ b/src/state/drafts/hooks.ts @@ -75,10 +75,12 @@ export function useSaveDraft() { composerState, replyTo, existingDraftId, + loadedMediaMap, }: { composerState: ComposerState replyTo?: ComposerOpts['replyTo'] existingDraftId?: string + loadedMediaMap?: Map // localId -> path/url }): Promise => { if (!did) { throw new Error('No account') @@ -87,23 +89,44 @@ export function useSaveDraft() { const now = new Date().toISOString() const draftId = existingDraftId || nanoid() - // If updating existing draft, delete old media first - if (existingDraftId) { - const existingDraft = await storage.loadDraftMeta(did, existingDraftId) - if (existingDraft) { - // Clean up old media that's no longer used - await cleanupOldMedia(did, existingDraft) + // Build a reverse map (path -> localId) for identifying reusable media + const pathToLocalId = new Map() + if (loadedMediaMap) { + for (const [localId, path] of loadedMediaMap) { + pathToLocalId.set(path, localId) } } - // Serialize the composer state + // Collect old media localIds for cleanup + let oldMediaLocalIds: Set = new Set() + if (existingDraftId) { + const existingDraft = await storage.loadDraftMeta(did, existingDraftId) + if (existingDraft) { + oldMediaLocalIds = collectMediaLocalIds(existingDraft) + } + } + + // Serialize the composer state, tracking which localIds are reused + const reusedLocalIds = new Set() const posts: StoredPostDraft[] = [] for (const post of composerState.thread.posts) { - const storedPost = await serializePost(did, post) + const storedPost = await serializePost( + did, + post, + pathToLocalId, + reusedLocalIds, + ) posts.push(storedPost) } + // Clean up old media that wasn't reused + for (const oldLocalId of oldMediaLocalIds) { + if (!reusedLocalIds.has(oldLocalId)) { + await storage.deleteMediaFromLocal(did, oldLocalId) + } + } + const draft: StoredDraft = { id: draftId, accountDid: did, @@ -162,12 +185,32 @@ export function useDeleteDraft() { }) } +/** + * Collect all media localIds from a draft + */ +function collectMediaLocalIds(draft: StoredDraft): Set { + const localIds = new Set() + for (const post of draft.posts) { + if (post.images) { + for (const image of post.images) { + localIds.add(image.localId) + } + } + if (post.video) { + localIds.add(post.video.localId) + } + } + return localIds +} + /** * Serialize a post for storage */ async function serializePost( accountDid: string, post: PostDraft, + pathToLocalId: Map, + reusedLocalIds: Set, ): Promise { const richtext: StoredRichText = { text: post.richtext.text, @@ -188,11 +231,15 @@ async function serializePost( storedPost.images = await serializeImages( accountDid, post.embed.media.images, + pathToLocalId, + reusedLocalIds, ) } else if (post.embed.media.type === 'video') { storedPost.video = await serializeVideo( accountDid, post.embed.media.video, + pathToLocalId, + reusedLocalIds, ) } else if (post.embed.media.type === 'gif') { storedPost.gif = serializeGif(post.embed.media) @@ -208,16 +255,36 @@ async function serializePost( async function serializeImages( accountDid: string, images: ComposerImage[], + pathToLocalId: Map, + reusedLocalIds: Set, ): Promise { const refs: LocalMediaRef[] = [] for (const image of images) { const path = image.transformed?.path || image.source.path - const localId = await storage.saveMediaToLocal( - accountDid, - path, - image.source.mime, - ) + + // Check if this image is already in drafts storage + // First try the pathToLocalId map (works for both native and web) + let existingLocalId: string | null | undefined = pathToLocalId.get(path) + + // On native, also check if the path is in the media directory + if (!existingLocalId) { + existingLocalId = storage.extractLocalIdFromPath(accountDid, path) + } + + let localId: string + if (existingLocalId) { + // Reuse existing media + localId = existingLocalId + reusedLocalIds.add(localId) + } else { + // Save new media + localId = await storage.saveMediaToLocal( + accountDid, + path, + image.source.mime, + ) + } refs.push({ localId, @@ -238,6 +305,8 @@ async function serializeImages( async function serializeVideo( accountDid: string, videoState: VideoState, + pathToLocalId: Map, + reusedLocalIds: Set, ): Promise { // Only save videos that have been compressed (have a video file) if (!videoState.video) { @@ -245,11 +314,24 @@ async function serializeVideo( } const video = videoState.video - const localId = await storage.saveMediaToLocal( - accountDid, - video.uri, - video.mimeType, - ) + const path = video.uri + + // Check if this video is already in drafts storage + let existingLocalId: string | null | undefined = pathToLocalId.get(path) + + if (!existingLocalId) { + existingLocalId = storage.extractLocalIdFromPath(accountDid, path) + } + + let localId: string + if (existingLocalId) { + // Reuse existing media + localId = existingLocalId + reusedLocalIds.add(localId) + } else { + // Save new media + localId = await storage.saveMediaToLocal(accountDid, path, video.mimeType) + } return { localId, @@ -284,25 +366,6 @@ function serializeGif(gifMedia: { } } -/** - * Clean up old media when updating a draft - */ -async function cleanupOldMedia( - accountDid: string, - draft: StoredDraft, -): Promise { - for (const post of draft.posts) { - if (post.images) { - for (const image of post.images) { - await storage.deleteMediaFromLocal(accountDid, image.localId) - } - } - if (post.video) { - await storage.deleteMediaFromLocal(accountDid, post.video.localId) - } - } -} - /** * Load media from storage and return paths/URLs for use in composer */ diff --git a/src/state/drafts/storage.ts b/src/state/drafts/storage.ts index 10ab020e14..1e6e3c2685 100644 --- a/src/state/drafts/storage.ts +++ b/src/state/drafts/storage.ts @@ -324,3 +324,22 @@ export async function mediaExists( const info = await getInfoAsync(path) return info.exists } + +/** + * Extract the localId from a path if it's already in drafts media storage + * Returns null if the path is not in drafts storage + */ +export function extractLocalIdFromPath( + accountDid: string, + path: string, +): string | null { + const mediaDir = getMediaDirectory(accountDid) + if (path.startsWith(mediaDir)) { + // Extract the localId from the path (it's the filename) + const localId = path.slice(mediaDir.length).replace(/^\//, '') + if (localId && !localId.includes('/')) { + return localId + } + } + return null +} diff --git a/src/state/drafts/storage.web.ts b/src/state/drafts/storage.web.ts index 2169706a66..75f3f956b8 100644 --- a/src/state/drafts/storage.web.ts +++ b/src/state/drafts/storage.web.ts @@ -349,3 +349,17 @@ export function revokeMediaUrl(url: string): void { URL.revokeObjectURL(url) } } + +/** + * Extract the localId from a path if it's already in drafts media storage + * For web, this always returns null since blob URLs don't contain localId + * The hooks layer handles tracking of web localIds separately + */ +export function extractLocalIdFromPath( + _accountDid: string, + _path: string, +): string | null { + // Web uses blob URLs which don't contain the localId + // Tracking is done via loadedMediaMap in hooks.ts + return null +} diff --git a/src/view/com/composer/Composer.tsx b/src/view/com/composer/Composer.tsx index ff85001b35..d4a2cba88c 100644 --- a/src/view/com/composer/Composer.tsx +++ b/src/view/com/composer/Composer.tsx @@ -353,6 +353,7 @@ export const ComposePost = ({ composerState, replyTo, existingDraftId: composerState.draftId, + loadedMediaMap: composerState.loadedMediaMap, }) composerDispatch({type: 'mark_saved', draftId: savedDraft.id}) onClose() @@ -368,6 +369,7 @@ export const ComposePost = ({ composerState, replyTo, existingDraftId: composerState.draftId, + loadedMediaMap: composerState.loadedMediaMap, }) composerDispatch({type: 'mark_saved', draftId: savedDraft.id}) }, [saveDraft, composerState, replyTo, composerDispatch]) diff --git a/src/view/com/composer/drafts/DraftItem.tsx b/src/view/com/composer/drafts/DraftItem.tsx index b394b87a86..a8c7e593e3 100644 --- a/src/view/com/composer/drafts/DraftItem.tsx +++ b/src/view/com/composer/drafts/DraftItem.tsx @@ -266,7 +266,7 @@ function DraftMediaPreview({post}: {post: DraftPostDisplay}) { } return ( - + {/* Images - use real embed components */} {viewImages.length === 1 && ( diff --git a/src/view/com/composer/state/composer.ts b/src/view/com/composer/state/composer.ts index 7fcb462196..52cfc54a74 100644 --- a/src/view/com/composer/state/composer.ts +++ b/src/view/com/composer/state/composer.ts @@ -106,6 +106,8 @@ export type ComposerState = { draftId?: string /** Whether the composer has been modified since loading a draft. */ isDirty: boolean + /** Map of localId -> loaded media path/URL for the current draft. Used for re-saving without re-copying media. */ + loadedMediaMap?: Map } export type ComposerAction = @@ -317,6 +319,7 @@ export function composerReducer( mutableNeedsFocusActive: true, draftId: draft.id, isDirty: false, + loadedMediaMap: loadedMedia, thread: { posts, postgate: draft.postgate || state.thread.postgate, @@ -330,6 +333,7 @@ export function composerReducer( mutableNeedsFocusActive: true, draftId: undefined, isDirty: false, + loadedMediaMap: undefined, thread: { posts: [ {