From 51f58d608d31200131b9725b539401c0e41f4edf Mon Sep 17 00:00:00 2001 From: hugo Date: Sat, 14 Feb 2026 22:23:41 +0100 Subject: [PATCH] fix: better updating of links from photo_album --- src/main/engine/PostMediaEngine.ts | 96 ++++++++++ src/main/ipc/handlers.ts | 12 ++ src/main/preload.ts | 2 + src/renderer/components/Editor/Editor.tsx | 56 ++---- .../LinkedMediaPanel/LinkedMediaPanel.tsx | 19 ++ src/renderer/types/electron.d.ts | 2 + tests/engine/PostMediaEngine.test.ts | 166 ++++++++++++++++++ tests/ipc/handlers.test.ts | 28 +++ 8 files changed, 344 insertions(+), 37 deletions(-) diff --git a/src/main/engine/PostMediaEngine.ts b/src/main/engine/PostMediaEngine.ts index c3db20c..1ab1cac 100644 --- a/src/main/engine/PostMediaEngine.ts +++ b/src/main/engine/PostMediaEngine.ts @@ -114,6 +114,102 @@ export class PostMediaEngine extends EventEmitter { this.emit('mediaUnlinked', { postId, mediaId }); } + /** + * Batch link multiple media files to a post. + * Only emits a single event at the end instead of per-item events. + * Skips media that are already linked. + */ + async linkManyToPost(postId: string, mediaIds: string[]): Promise<{ linked: string[]; skipped: string[] }> { + const db = getDatabase().getLocal(); + const linked: string[] = []; + const skipped: string[] = []; + + // Get all existing links for this post to check what's already linked + const existingLinks = await this.getLinkedMediaForPost(postId); + const existingMediaIds = new Set(existingLinks.map(l => l.mediaId)); + + let maxSortOrder = existingLinks.length > 0 + ? Math.max(...existingLinks.map(l => l.sortOrder)) + : -1; + + const now = new Date(); + + for (const mediaId of mediaIds) { + // Skip if already linked + if (existingMediaIds.has(mediaId)) { + skipped.push(mediaId); + continue; + } + + maxSortOrder++; + const link: NewPostMediaLink = { + id: uuidv4(), + projectId: this.currentProjectId, + postId, + mediaId, + sortOrder: maxSortOrder, + createdAt: now, + }; + + await db.insert(postMedia).values(link); + + // Update the media sidecar to include this post + const media = await getMediaEngine().getMedia(mediaId); + if (media) { + const linkedPostIds = media.linkedPostIds || []; + if (!linkedPostIds.includes(postId)) { + await getMediaEngine().updateMedia(mediaId, { + linkedPostIds: [...linkedPostIds, postId], + }); + } + } + + linked.push(mediaId); + existingMediaIds.add(mediaId); // Track to avoid duplicates within batch + } + + // Emit a single batch event instead of per-item events + if (linked.length > 0) { + this.emit('mediaBatchLinked', { postId, mediaIds: linked }); + } + + return { linked, skipped }; + } + + /** + * Batch unlink multiple media files from a post. + * Only emits a single event at the end instead of per-item events. + */ + async unlinkManyFromPost(postId: string, mediaIds: string[]): Promise<{ unlinked: string[] }> { + const db = getDatabase().getLocal(); + const unlinked: string[] = []; + + for (const mediaId of mediaIds) { + await db.delete(postMedia).where( + and( + eq(postMedia.postId, postId), + eq(postMedia.mediaId, mediaId) + ) + ); + + // Update the media sidecar to remove this post + const media = await getMediaEngine().getMedia(mediaId); + if (media) { + const linkedPostIds = (media.linkedPostIds || []).filter(id => id !== postId); + await getMediaEngine().updateMedia(mediaId, { linkedPostIds }); + } + + unlinked.push(mediaId); + } + + // Emit a single batch event instead of per-item events + if (unlinked.length > 0) { + this.emit('mediaBatchUnlinked', { postId, mediaIds: unlinked }); + } + + return { unlinked }; + } + /** * Get all media linked to a post, ordered by sortOrder */ diff --git a/src/main/ipc/handlers.ts b/src/main/ipc/handlers.ts index 1ae5890..b5676b7 100644 --- a/src/main/ipc/handlers.ts +++ b/src/main/ipc/handlers.ts @@ -640,6 +640,16 @@ export function registerIpcHandlers(): void { return engine.unlinkMediaFromPost(postId, mediaId); }); + safeHandle('postMedia:linkMany', async (_, postId: string, mediaIds: string[]) => { + const engine = getPostMediaEngine(); + return engine.linkManyToPost(postId, mediaIds); + }); + + safeHandle('postMedia:unlinkMany', async (_, postId: string, mediaIds: string[]) => { + const engine = getPostMediaEngine(); + return engine.unlinkManyFromPost(postId, mediaIds); + }); + safeHandle('postMedia:getForPost', async (_, postId: string) => { const engine = getPostMediaEngine(); return engine.getLinkedMediaForPost(postId); @@ -982,6 +992,8 @@ export function registerIpcHandlers(): void { postMediaEngine.on('mediaLinked', forwardEvent('postMedia:linked')); postMediaEngine.on('mediaUnlinked', forwardEvent('postMedia:unlinked')); + postMediaEngine.on('mediaBatchLinked', forwardEvent('postMedia:batchLinked')); + postMediaEngine.on('mediaBatchUnlinked', forwardEvent('postMedia:batchUnlinked')); postMediaEngine.on('mediaReordered', forwardEvent('postMedia:reordered')); postMediaEngine.on('rebuilt', forwardEvent('postMedia:rebuilt')); diff --git a/src/main/preload.ts b/src/main/preload.ts index 47780cc..c9b2761 100644 --- a/src/main/preload.ts +++ b/src/main/preload.ts @@ -69,6 +69,8 @@ contextBridge.exposeInMainWorld('electronAPI', { postMedia: { link: (postId: string, mediaId: string) => ipcRenderer.invoke('postMedia:link', postId, mediaId), unlink: (postId: string, mediaId: string) => ipcRenderer.invoke('postMedia:unlink', postId, mediaId), + linkMany: (postId: string, mediaIds: string[]) => ipcRenderer.invoke('postMedia:linkMany', postId, mediaIds), + unlinkMany: (postId: string, mediaIds: string[]) => ipcRenderer.invoke('postMedia:unlinkMany', postId, mediaIds), getForPost: (postId: string) => ipcRenderer.invoke('postMedia:getForPost', postId), getForMedia: (mediaId: string) => ipcRenderer.invoke('postMedia:getForMedia', mediaId), getMediaDataForPost: (postId: string) => ipcRenderer.invoke('postMedia:getMediaDataForPost', postId), diff --git a/src/renderer/components/Editor/Editor.tsx b/src/renderer/components/Editor/Editor.tsx index fe8bed8..da7443f 100644 --- a/src/renderer/components/Editor/Editor.tsx +++ b/src/renderer/components/Editor/Editor.tsx @@ -288,10 +288,8 @@ const hydratePhotoArchive = async ( // No photo_archive macros - unlink any previously linked and clear state const previouslyLinked = loadPreviouslyLinkedIds(postId); if (previouslyLinked.size > 0) { - console.log(`[photo_archive] No macros found, unlinking ${previouslyLinked.size} previously linked media`); - for (const mediaId of previouslyLinked) { - await window.electronAPI?.postMedia.unlink(postId, mediaId); - } + console.log(`[photo_archive] No macros found, batch unlinking ${previouslyLinked.size} previously linked media`); + await window.electronAPI?.postMedia.unlinkMany(postId, Array.from(previouslyLinked)); localStorage.removeItem(getPhotoArchiveLinkedKey(postId)); } return; @@ -444,19 +442,31 @@ const doHydratePhotoArchive = async ( console.log(`[photo_archive] Should link ${shouldBeLinkedIds.size} media IDs`); - // Phase 2: Unlink media that was previously linked but is no longer needed - // Simple set difference: previouslyLinkedIds - shouldBeLinkedIds + // Phase 2: Batch unlink media that was previously linked but is no longer needed + const idsToUnlink: string[] = []; for (const mediaId of previouslyLinkedIds) { if (!shouldBeLinkedIds.has(mediaId)) { - console.log(`[photo_archive] Unlinking ${mediaId} - no longer in range`); - await window.electronAPI?.postMedia.unlink(postId, mediaId); + idsToUnlink.push(mediaId); } } + if (idsToUnlink.length > 0) { + console.log(`[photo_archive] Batch unlinking ${idsToUnlink.length} media items`); + await window.electronAPI?.postMedia.unlinkMany(postId, idsToUnlink); + } + // Save current linked IDs for next hydration saveLinkedIds(postId, shouldBeLinkedIds); - // Phase 3: Link new media and render + // Phase 3: Batch link all media that should be linked and render + // Use linkMany which internally skips already linked items + const idsToLink = Array.from(shouldBeLinkedIds); + if (idsToLink.length > 0) { + console.log(`[photo_archive] Batch linking ${idsToLink.length} media items`); + await window.electronAPI?.postMedia.linkMany(postId, idsToLink); + } + + // Phase 4: Render galleries (no more link/unlink calls here) for (const { element, mode, year, month, images, monthlyImages, showYearInLabel } of archiveData) { const archiveContainer = element.querySelector('.photo-archive-container'); if (!archiveContainer) continue; @@ -467,14 +477,6 @@ const doHydratePhotoArchive = async ( if (mode === 'single-month' && month !== undefined && images && year) { // Single month view - // Link images to the post - for (const img of images) { - const isLinked = await window.electronAPI?.postMedia.isLinked(postId, img.id); - if (!isLinked) { - await window.electronAPI?.postMedia.link(postId, img.id); - } - } - if (images.length === 0) { archiveContainer.innerHTML = `
No photos found for ${FULL_MONTH_NAMES[month - 1]} ${year}
`; continue; @@ -482,16 +484,6 @@ const doHydratePhotoArchive = async ( html = buildMonthGallery(month, year, images, onImageClick, false); } else if (mode === 'recent' && monthlyImages) { // Recent mode - keys are "YYYY-MM" strings - // Link all images to the post - for (const imgs of monthlyImages.values()) { - for (const img of imgs) { - const isLinked = await window.electronAPI?.postMedia.isLinked(postId, img.id); - if (!isLinked) { - await window.electronAPI?.postMedia.link(postId, img.id); - } - } - } - if (monthlyImages.size === 0) { archiveContainer.innerHTML = `
No recent photos found
`; continue; @@ -510,16 +502,6 @@ const doHydratePhotoArchive = async ( }).join(''); } else if (mode === 'full-year' && monthlyImages && year) { // Full year view - keys are month numbers - // Link all images to the post - for (const imgs of monthlyImages.values()) { - for (const img of imgs) { - const isLinked = await window.electronAPI?.postMedia.isLinked(postId, img.id); - if (!isLinked) { - await window.electronAPI?.postMedia.link(postId, img.id); - } - } - } - if (monthlyImages.size === 0) { archiveContainer.innerHTML = `
No photos found for ${year}
`; continue; diff --git a/src/renderer/components/LinkedMediaPanel/LinkedMediaPanel.tsx b/src/renderer/components/LinkedMediaPanel/LinkedMediaPanel.tsx index 2ef3b32..403840d 100644 --- a/src/renderer/components/LinkedMediaPanel/LinkedMediaPanel.tsx +++ b/src/renderer/components/LinkedMediaPanel/LinkedMediaPanel.tsx @@ -78,12 +78,31 @@ export const LinkedMediaPanel: React.FC = ({ } }; + // Also handle batch events (single refresh for multiple link/unlink operations) + const handleBatchLinked = (...args: unknown[]) => { + const data = args[0] as { postId: string; mediaIds: string[] } | undefined; + if (data?.postId === postId) { + loadLinkedMedia(); + } + }; + + const handleBatchUnlinked = (...args: unknown[]) => { + const data = args[0] as { postId: string; mediaIds: string[] } | undefined; + if (data?.postId === postId) { + loadLinkedMedia(); + } + }; + const unsubLinked = window.electronAPI?.on('postMedia:linked', handleLinked); const unsubUnlinked = window.electronAPI?.on('postMedia:unlinked', handleUnlinked); + const unsubBatchLinked = window.electronAPI?.on('postMedia:batchLinked', handleBatchLinked); + const unsubBatchUnlinked = window.electronAPI?.on('postMedia:batchUnlinked', handleBatchUnlinked); return () => { unsubLinked?.(); unsubUnlinked?.(); + unsubBatchLinked?.(); + unsubBatchUnlinked?.(); }; }, [postId, loadLinkedMedia]); diff --git a/src/renderer/types/electron.d.ts b/src/renderer/types/electron.d.ts index 97746f7..ca2b209 100644 --- a/src/renderer/types/electron.d.ts +++ b/src/renderer/types/electron.d.ts @@ -319,6 +319,8 @@ export interface ElectronAPI { postMedia: { link: (postId: string, mediaId: string) => Promise; unlink: (postId: string, mediaId: string) => Promise; + linkMany: (postId: string, mediaIds: string[]) => Promise<{ linked: string[]; skipped: string[] }>; + unlinkMany: (postId: string, mediaIds: string[]) => Promise<{ unlinked: string[] }>; getForPost: (postId: string) => Promise; getForMedia: (mediaId: string) => Promise; getMediaDataForPost: (postId: string) => Promise>; diff --git a/tests/engine/PostMediaEngine.test.ts b/tests/engine/PostMediaEngine.test.ts index 697c58e..891258b 100644 --- a/tests/engine/PostMediaEngine.test.ts +++ b/tests/engine/PostMediaEngine.test.ts @@ -280,6 +280,172 @@ describe('PostMediaEngine', () => { }); }); + describe('linkManyToPost', () => { + it('should link multiple media files to a post in a single batch', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2', 'media-3']; + + // No existing links + selectMockData = []; + + // Setup mock media for each + mockGetMedia.mockImplementation((id: string) => + Promise.resolve(createMockMedia({ id, linkedPostIds: [] })) + ); + + const result = await engine.linkManyToPost(postId, mediaIds); + + expect(result.linked).toHaveLength(3); + expect(result.skipped).toHaveLength(0); + expect(insertedValues).toHaveLength(3); + }); + + it('should skip already linked media and include them in skipped array', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2', 'media-3']; + + // media-1 is already linked + selectMockData = [ + { id: 'link-1', projectId: 'test-project', postId, mediaId: 'media-1', sortOrder: 0, createdAt: new Date() }, + ]; + + mockGetMedia.mockImplementation((id: string) => + Promise.resolve(createMockMedia({ id, linkedPostIds: id === 'media-1' ? [postId] : [] })) + ); + + const result = await engine.linkManyToPost(postId, mediaIds); + + expect(result.linked).toHaveLength(2); + expect(result.linked).toContain('media-2'); + expect(result.linked).toContain('media-3'); + expect(result.skipped).toHaveLength(1); + expect(result.skipped).toContain('media-1'); + }); + + it('should emit mediaBatchLinked event once at the end', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2']; + selectMockData = []; + + mockGetMedia.mockResolvedValue(createMockMedia({ id: 'any', linkedPostIds: [] })); + + const handler = vi.fn(); + engine.on('mediaBatchLinked', handler); + // Should NOT emit individual mediaLinked events + const individualHandler = vi.fn(); + engine.on('mediaLinked', individualHandler); + + await engine.linkManyToPost(postId, mediaIds); + + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith({ postId, mediaIds: expect.arrayContaining(['media-1', 'media-2']) }); + expect(individualHandler).not.toHaveBeenCalled(); + }); + + it('should not emit event if no media was linked', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1']; + + // media-1 is already linked + selectMockData = [ + { id: 'link-1', projectId: 'test-project', postId, mediaId: 'media-1', sortOrder: 0, createdAt: new Date() }, + ]; + + mockGetMedia.mockResolvedValue(createMockMedia({ id: 'media-1', linkedPostIds: [postId] })); + + const handler = vi.fn(); + engine.on('mediaBatchLinked', handler); + + await engine.linkManyToPost(postId, mediaIds); + + expect(handler).not.toHaveBeenCalled(); + }); + + it('should update sortOrder incrementally for batch-linked media', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2', 'media-3']; + selectMockData = []; + + mockGetMedia.mockResolvedValue(createMockMedia({ linkedPostIds: [] })); + + await engine.linkManyToPost(postId, mediaIds); + + // Check that sort orders are sequential + const sortOrders = insertedValues.map(v => v.sortOrder); + expect(sortOrders).toEqual([0, 1, 2]); + }); + }); + + describe('unlinkManyFromPost', () => { + it('should unlink multiple media files from a post in a single batch', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2', 'media-3']; + + mockGetMedia.mockImplementation((id: string) => + Promise.resolve(createMockMedia({ id, linkedPostIds: [postId] })) + ); + + const result = await engine.unlinkManyFromPost(postId, mediaIds); + + expect(result.unlinked).toHaveLength(3); + // deleteCalled flag is set to true when any delete is called + expect(deleteCalled).toBe(true); + }); + + it('should emit mediaBatchUnlinked event once at the end', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2']; + + mockGetMedia.mockResolvedValue(createMockMedia({ linkedPostIds: [postId] })); + + const handler = vi.fn(); + engine.on('mediaBatchUnlinked', handler); + // Should NOT emit individual mediaUnlinked events + const individualHandler = vi.fn(); + engine.on('mediaUnlinked', individualHandler); + + await engine.unlinkManyFromPost(postId, mediaIds); + + expect(handler).toHaveBeenCalledTimes(1); + expect(handler).toHaveBeenCalledWith({ postId, mediaIds: expect.arrayContaining(['media-1', 'media-2']) }); + expect(individualHandler).not.toHaveBeenCalled(); + }); + + it('should not emit event if no media was unlinked', async () => { + const postId = 'post-1'; + const mediaIds: string[] = []; + + const handler = vi.fn(); + engine.on('mediaBatchUnlinked', handler); + + await engine.unlinkManyFromPost(postId, mediaIds); + + expect(handler).not.toHaveBeenCalled(); + }); + + it('should update media sidecars to remove postId', async () => { + const postId = 'post-1'; + const mediaIds = ['media-1', 'media-2']; + + mockGetMedia.mockImplementation((id: string) => + Promise.resolve(createMockMedia({ id, linkedPostIds: [postId, 'other-post'] })) + ); + + await engine.unlinkManyFromPost(postId, mediaIds); + + // Both media should have their sidecars updated + expect(mockUpdateMedia).toHaveBeenCalledTimes(2); + expect(mockUpdateMedia).toHaveBeenCalledWith( + 'media-1', + expect.objectContaining({ linkedPostIds: ['other-post'] }) + ); + expect(mockUpdateMedia).toHaveBeenCalledWith( + 'media-2', + expect.objectContaining({ linkedPostIds: ['other-post'] }) + ); + }); + }); + describe('getLinkedPostsForMedia', () => { it('should return all posts linked to a media file', async () => { const mediaId = 'media-1'; diff --git a/tests/ipc/handlers.test.ts b/tests/ipc/handlers.test.ts index 2b13a38..946604e 100644 --- a/tests/ipc/handlers.test.ts +++ b/tests/ipc/handlers.test.ts @@ -130,6 +130,8 @@ const mockPostMediaEngine = { setProjectContext: vi.fn(), linkMediaToPost: vi.fn(), unlinkMediaFromPost: vi.fn(), + linkManyToPost: vi.fn(), + unlinkManyFromPost: vi.fn(), getLinkedMediaForPost: vi.fn(), getLinkedPostsForMedia: vi.fn(), reorderMediaForPost: vi.fn(), @@ -878,6 +880,32 @@ describe('IPC Handlers', () => { }); }); + describe('postMedia:linkMany', () => { + it('should batch link multiple media to post', async () => { + const batchResult = { linked: ['media-1', 'media-2'], skipped: [] }; + mockPostMediaEngine.linkManyToPost.mockResolvedValue(batchResult); + const mediaIds = ['media-1', 'media-2']; + + const result = await invokeHandler('postMedia:linkMany', 'post-1', mediaIds); + + expect(mockPostMediaEngine.linkManyToPost).toHaveBeenCalledWith('post-1', mediaIds); + expect(result).toEqual(batchResult); + }); + }); + + describe('postMedia:unlinkMany', () => { + it('should batch unlink multiple media from post', async () => { + const batchResult = { unlinked: ['media-1', 'media-2'] }; + mockPostMediaEngine.unlinkManyFromPost.mockResolvedValue(batchResult); + const mediaIds = ['media-1', 'media-2']; + + const result = await invokeHandler('postMedia:unlinkMany', 'post-1', mediaIds); + + expect(mockPostMediaEngine.unlinkManyFromPost).toHaveBeenCalledWith('post-1', mediaIds); + expect(result).toEqual(batchResult); + }); + }); + describe('postMedia:getForPost', () => { it('should return media linked to a post', async () => { const linkedMedia = [createMockMedia(), createMockMedia()];