fix: phase 3 refactoring

REFACTOR_DUPLICATION.md

@@ -9,7 +9,7 @@ Scope: Reduce high-impact code duplication in production and test-adjacent areas
 |---|---|---|---|
 | 1 | Electron API contract | ✅ Completed | Shared contract now covers renderer store data interfaces too. |
 | 2 | Tag mutation workflow | ✅ Completed | Shared task workflow now used by delete/merge/rename paths. |
-| 3 | Post-media single/batch ops | ⚠️ Partial | Core primitives extracted; some duplicate link/unlink logic remains. |
+| 3 | Post-media single/batch ops | ✅ Completed | Single/batch link and unlink now share common internals. |
 | 4 | Project mapping + delete guards | ✅ Completed | Guard + mapper extracted and used in target methods. |
 | 5 | Metadata sync loop scaffolding | ✅ Completed | `runSyncLoop` extracted and reused by both sync directions. |
 | 6 | Shared color utility | ✅ Completed | Shared `getContrastColor` utility is in use. |
@@ -128,7 +128,7 @@ Refactor shared behavior into private helpers while preserving all task/event se
 
 ## Phase 3 — Unify Single/Batch Post-Media Operations
 
-Status: ⚠️ Partially complete
+Status: ✅ Completed
 
 ### Problem
 `PostMediaEngine` duplicates linking/unlinking logic across single and batch methods:
@@ -148,7 +148,7 @@ Create common primitives and make single/batch methods compose them.
 
 ### Progress Check
 - Completed: shared primitives exist (`createPostMediaLink`, `removePostMediaLink`, sidecar helpers).
-- Remaining: small duplicate segments remain between single/batch paths and can still be collapsed further.
+- Completed: remaining single/batch duplicate link/unlink segments were collapsed into shared internal paths.
 
 ### Acceptance Criteria
 - No behavior regressions in link/unlink semantics.

src/main/engine/PostMediaEngine.ts

@@ -98,6 +98,47 @@ export class PostMediaEngine extends EventEmitter {
     return this.createLinkData(link);
   }
 
+  private async getLinkState(postId: string): Promise<{
+    existingByMediaId: Map<string, PostMediaLinkData>;
+    nextSortOrder: number;
+  }> {
+    const existingLinks = await this.getLinkedMediaForPost(postId);
+    const existingByMediaId = new Map(existingLinks.map((link) => [link.mediaId, link]));
+    const maxSortOrder = existingLinks.length > 0
+      ? Math.max(...existingLinks.map((link) => link.sortOrder))
+      : -1;
+
+    return {
+      existingByMediaId,
+      nextSortOrder: maxSortOrder + 1,
+    };
+  }
+
+  private async tryLinkMediaToPost(
+    postId: string,
+    mediaId: string,
+    state: {
+      existingByMediaId: Map<string, PostMediaLinkData>;
+      nextSortOrder: number;
+    },
+    createdAt: Date
+  ): Promise<{ linked: true; link: PostMediaLinkData } | { linked: false; existing: PostMediaLinkData }> {
+    const existing = state.existingByMediaId.get(mediaId);
+    if (existing) {
+      return { linked: false, existing };
+    }
+
+    const link = await this.createPostMediaLink(postId, mediaId, state.nextSortOrder, createdAt);
+    state.nextSortOrder++;
+    state.existingByMediaId.set(mediaId, link);
+
+    return { linked: true, link };
+  }
+
+  private async unlinkMediaFromPostCore(postId: string, mediaId: string): Promise<void> {
+    await this.removePostMediaLink(postId, mediaId);
+  }
+
   private async removePostMediaLink(postId: string, mediaId: string): Promise<void> {
     const db = this.getDb();
 
@@ -123,19 +164,14 @@ export class PostMediaEngine extends EventEmitter {
    * Link a media file to a post
    */
   async linkMediaToPost(postId: string, mediaId: string): Promise<PostMediaLinkData> {
-    const existingLinks = await this.getLinkedMediaForPost(postId);
+    const state = await this.getLinkState(postId);
-    const existing = existingLinks.find(link => link.mediaId === mediaId);
+    const result = await this.tryLinkMediaToPost(postId, mediaId, state, new Date());
-    if (existing) {
+
-      return existing;
+    if (!result.linked) {
+      return result.existing;
     }
 
-    // Get current highest sortOrder for this post
+    const linkData = result.link;
-    const maxSortOrder = existingLinks.length > 0
-      ? Math.max(...existingLinks.map(l => l.sortOrder))
-      : -1;
-
-    const now = new Date();
-    const linkData = await this.createPostMediaLink(postId, mediaId, maxSortOrder + 1, now);
 
     this.emit('mediaLinked', linkData);
     return linkData;
@@ -145,7 +181,7 @@ export class PostMediaEngine extends EventEmitter {
    * Unlink a media file from a post
    */
   async unlinkMediaFromPost(postId: string, mediaId: string): Promise<void> {
-    await this.removePostMediaLink(postId, mediaId);
+    await this.unlinkMediaFromPostCore(postId, mediaId);
 
     this.emit('mediaUnlinked', { postId, mediaId });
   }
@@ -160,28 +196,17 @@ export class PostMediaEngine extends EventEmitter {
     const skipped: string[] = [];
     const uniqueMediaIds = this.getUniqueMediaIds(mediaIds);
 
-    // Get all existing links for this post to check what's already linked
+    const state = await this.getLinkState(postId);
-    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 uniqueMediaIds) {
-      // Skip if already linked
+      const result = await this.tryLinkMediaToPost(postId, mediaId, state, now);
-      if (existingMediaIds.has(mediaId)) {
+      if (!result.linked) {
         skipped.push(mediaId);
         continue;
       }
 
-      maxSortOrder++;
-      await this.createPostMediaLink(postId, mediaId, maxSortOrder, now);
-
       linked.push(mediaId);
-      existingMediaIds.add(mediaId); // Track to avoid duplicates within batch
     }
 
     // Emit a single batch event instead of per-item events
@@ -201,7 +226,7 @@ export class PostMediaEngine extends EventEmitter {
     const uniqueMediaIds = this.getUniqueMediaIds(mediaIds);
 
     for (const mediaId of uniqueMediaIds) {
-      await this.removePostMediaLink(postId, mediaId);
+      await this.unlinkMediaFromPostCore(postId, mediaId);
 
       unlinked.push(mediaId);
     }

tests/engine/PostMediaEngine.test.ts

@@ -395,6 +395,32 @@ describe('PostMediaEngine', () => {
       const sortOrders = insertedValues.map(v => v.sortOrder);
       expect(sortOrders).toEqual([0, 1, 2]);
     });
+
+    it('should produce same persisted link state as single-link path for one media item', async () => {
+      const postId = 'post-1';
+      const mediaId = 'media-1';
+
+      selectMockData = [];
+      mockGetMedia.mockResolvedValue(createMockMedia({ id: mediaId, linkedPostIds: [] }));
+
+      await engine.linkMediaToPost(postId, mediaId);
+      const singleInsert = insertedValues[0];
+      const singleUpdateArgs = mockUpdateMedia.mock.calls[0][1];
+
+      insertedValues = [];
+      mockUpdateMedia.mockClear();
+      selectMockData = [];
+
+      const batchResult = await engine.linkManyToPost(postId, [mediaId]);
+      const batchInsert = insertedValues[0];
+      const batchUpdateArgs = mockUpdateMedia.mock.calls[0][1];
+
+      expect(batchResult).toEqual({ linked: [mediaId], skipped: [] });
+      expect(singleInsert.postId).toBe(batchInsert.postId);
+      expect(singleInsert.mediaId).toBe(batchInsert.mediaId);
+      expect(singleInsert.sortOrder).toBe(batchInsert.sortOrder);
+      expect(singleUpdateArgs).toEqual(batchUpdateArgs);
+    });
   });
 
   describe('unlinkManyFromPost', () => {
@@ -481,6 +507,30 @@ describe('PostMediaEngine', () => {
       expect(deleteCallCount).toBe(2);
       expect(mockUpdateMedia).toHaveBeenCalledTimes(2);
     });
+
+    it('should produce same sidecar and delete effects as single-unlink path for one media item', async () => {
+      const postId = 'post-1';
+      const mediaId = 'media-1';
+
+      mockGetMedia.mockResolvedValue(createMockMedia({ id: mediaId, linkedPostIds: [postId, 'other-post'] }));
+
+      await engine.unlinkMediaFromPost(postId, mediaId);
+      const singleDeleteCount = deleteCallCount;
+      const singleUpdateArgs = mockUpdateMedia.mock.calls[0][1];
+
+      deleteCallCount = 0;
+      deleteCalled = false;
+      mockUpdateMedia.mockClear();
+
+      const batchResult = await engine.unlinkManyFromPost(postId, [mediaId]);
+      const batchDeleteCount = deleteCallCount;
+      const batchUpdateArgs = mockUpdateMedia.mock.calls[0][1];
+
+      expect(batchResult).toEqual({ unlinked: [mediaId] });
+      expect(singleDeleteCount).toBe(1);
+      expect(batchDeleteCount).toBe(1);
+      expect(singleUpdateArgs).toEqual(batchUpdateArgs);
+    });
   });
 
   describe('getLinkedPostsForMedia', () => {