diff --git a/REFACTOR_DUPLICATION.md b/REFACTOR_DUPLICATION.md index bdb035b..a1da60f 100644 --- a/REFACTOR_DUPLICATION.md +++ b/REFACTOR_DUPLICATION.md @@ -8,7 +8,7 @@ Scope: Reduce high-impact code duplication in production and test-adjacent areas | Phase | Area | Status | Notes | |---|---|---|---| | 1 | Electron API contract | ✅ Completed | Shared contract now covers renderer store data interfaces too. | -| 2 | Tag mutation workflow | ⚠️ Partial | Helpers exist; duplicate blocks still present in task flows. | +| 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. | | 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. | @@ -82,7 +82,7 @@ Create one canonical API contract and consume it from both preload and renderer ## Phase 2 — Extract Common Tag Mutation Workflow -Status: ⚠️ Partially complete +Status: ✅ Completed ### Problem `TagEngine` repeats similar logic in: @@ -106,7 +106,7 @@ Refactor shared behavior into private helpers while preserving all task/event se ### Progress Check - Completed: helper extraction has started (`queryPostsContainingTag`, `updatePostTagsAndSync`, and shared update path usage). -- Remaining: duplicate progress/task scaffolding still exists across `deleteTag` / `mergeTags` / `renameTag`. +- Completed: duplicate progress/task scaffolding was consolidated across `deleteTag` / `mergeTags` / `renameTag` via shared task workflow. ### Acceptance Criteria - Public method behavior and events unchanged. diff --git a/src/main/engine/TagEngine.ts b/src/main/engine/TagEngine.ts index 33a31bd..9b3c2a4 100644 --- a/src/main/engine/TagEngine.ts +++ b/src/main/engine/TagEngine.ts @@ -215,6 +215,38 @@ export class TagEngine extends EventEmitter { }; } + private async runTagMutationTask(options: { + taskId: string; + taskName: string; + startMessage: string; + runUpdates: (onProgress: (progress: number, message: string) => void) => Promise; + finalizeMessage: string; + finalize: () => Promise; + buildResult: (postsUpdated: number) => TResult; + onComplete: (result: TResult) => Promise; + }): Promise { + return taskManager.runTask({ + id: options.taskId, + name: options.taskName, + execute: async (onProgress) => { + onProgress(0, options.startMessage); + + const postsUpdated = await options.runUpdates(onProgress); + + onProgress(90, options.finalizeMessage); + await options.finalize(); + + onProgress(100, 'Complete'); + + const result = options.buildResult(postsUpdated); + await options.onComplete(result); + await this.saveTagsToFile(); + + return result; + }, + }); + } + /** * Returns the default internal project directory (in userData). */ @@ -408,38 +440,32 @@ export class TagEngine extends EventEmitter { const tag = await this.getTagRowOrThrow(id); const tagName = tag.name; - // Run the deletion as a background task - return taskManager.runTask({ - id: `delete-tag-${id}-${Date.now()}`, - name: `Delete tag "${tagName}"`, - execute: async (onProgress) => { - onProgress(0, `Finding posts with tag "${tagName}"...`); - + return this.runTagMutationTask({ + taskId: `delete-tag-${id}-${Date.now()}`, + taskName: `Delete tag "${tagName}"`, + startMessage: `Finding posts with tag "${tagName}"...`, + runUpdates: async (onProgress) => { const updateOperation = await this.updateMatchingPosts( tagName, (postTags) => postTags.filter((tagEntry) => tagEntry !== tagName) ); - const updated = await updateOperation.process((updatedCount, totalCount) => { + return updateOperation.process((updatedCount, totalCount) => { onProgress((updatedCount / totalCount) * 80, `Updated ${updatedCount}/${totalCount} posts...`); }); - - onProgress(90, 'Deleting tag...'); - - // Delete the tag + }, + finalizeMessage: 'Deleting tag...', + finalize: async () => { await db .delete(tags) .where(and( eq(tags.id, id), eq(tags.projectId, this.currentProjectId) )); - - onProgress(100, 'Complete'); - + }, + buildResult: (postsUpdated) => ({ success: true, postsUpdated }), + onComplete: async () => { this.emit('tagDeleted', id); - await this.saveTagsToFile(); - - return { success: true, postsUpdated: updated }; }, }); } @@ -486,19 +512,16 @@ export class TagEngine extends EventEmitter { const targetName = targetTag.name; const sourceNames = sourceTags.map(t => t.name); - // Run as background task - return taskManager.runTask({ - id: `merge-tags-${Date.now()}`, - name: `Merge tags into "${targetName}"`, - execute: async (onProgress) => { - onProgress(0, 'Finding posts to update...'); - + return this.runTagMutationTask({ + taskId: `merge-tags-${Date.now()}`, + taskName: `Merge tags into "${targetName}"`, + startMessage: 'Finding posts to update...', + runUpdates: async (onProgress) => { const updatedPostTagsById = new Map(); - // For each source tag, compute final post tags per post ID for (let index = 0; index < sourceNames.length; index++) { const sourceName = sourceNames[index]; - onProgress((index / sourceNames.length) * 80, `Processing tag "${sourceName}"...`); + onProgress(((index + 1) / sourceNames.length) * 80, `Processing tag "${sourceName}"...`); const postsWithSourceTag = await this.queryPostsContainingTag(sourceName); for (const row of postsWithSourceTag) { @@ -520,33 +543,27 @@ export class TagEngine extends EventEmitter { await this.updatePostTagsAndSync(postId, updatedTags); } - const totalPostsUpdated = updatedPostTagsById.size; - - onProgress(90, 'Deleting source tags...'); - - // Delete source tags - for (const id of sourceTagIds) { + return updatedPostTagsById.size; + }, + finalizeMessage: 'Deleting source tags...', + finalize: async () => { + for (const sourceId of sourceTagIds) { await db .delete(tags) .where(and( - eq(tags.id, id), + eq(tags.id, sourceId), eq(tags.projectId, this.currentProjectId) )); } - - onProgress(100, 'Complete'); - - const result: MergeTagsResult = { - success: true, - postsUpdated: totalPostsUpdated, - tagsDeleted: sourceTagIds.length, - targetTag: targetName, - }; - + }, + buildResult: (postsUpdated) => ({ + success: true, + postsUpdated, + tagsDeleted: sourceTagIds.length, + targetTag: targetName, + }), + onComplete: async (result) => { this.emit('tagsMerged', result); - await this.saveTagsToFile(); - - return result; }, }); } @@ -596,25 +613,22 @@ export class TagEngine extends EventEmitter { throw new Error(`Tag "${newName}" already exists`); } - // Run as background task - return taskManager.runTask({ - id: `rename-tag-${id}-${Date.now()}`, - name: `Rename tag "${oldName}" to "${newName}"`, - execute: async (onProgress) => { - onProgress(0, 'Finding posts to update...'); - + return this.runTagMutationTask({ + taskId: `rename-tag-${id}-${Date.now()}`, + taskName: `Rename tag "${oldName}" to "${newName}"`, + startMessage: 'Finding posts to update...', + runUpdates: async (onProgress) => { const updateOperation = await this.updateMatchingPosts( oldName, (postTags) => postTags.map((tagEntry) => tagEntry === oldName ? newName : tagEntry) ); - const updated = await updateOperation.process((updatedCount, totalCount) => { + return updateOperation.process((updatedCount, totalCount) => { onProgress((updatedCount / totalCount) * 80, `Updated ${updatedCount}/${totalCount} posts...`); }); - - onProgress(90, 'Updating tag record...'); - - // Update the tag name + }, + finalizeMessage: 'Updating tag record...', + finalize: async () => { await db .update(tags) .set({ @@ -625,20 +639,15 @@ export class TagEngine extends EventEmitter { eq(tags.id, id), eq(tags.projectId, this.currentProjectId) )); - - onProgress(100, 'Complete'); - - const result: RenameTagResult = { - success: true, - postsUpdated: updated, - oldName, - newName, - }; - + }, + buildResult: (postsUpdated) => ({ + success: true, + postsUpdated, + oldName, + newName, + }), + onComplete: async (result) => { this.emit('tagRenamed', result); - await this.saveTagsToFile(); - - return result; }, }); } diff --git a/tests/engine/TagEngine.test.ts b/tests/engine/TagEngine.test.ts index b096361..83b7eb1 100644 --- a/tests/engine/TagEngine.test.ts +++ b/tests/engine/TagEngine.test.ts @@ -9,6 +9,7 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { TagEngine, TagData, TagWithCount, MergeTagsResult, DeleteTagResult } from '../../src/main/engine/TagEngine'; import { resetMockCounters } from '../utils/factories'; import { getDatabase } from '../../src/main/database'; +import { taskManager } from '../../src/main/engine/TaskManager'; // Create mock data stores const mockTags = new Map(); @@ -135,6 +136,18 @@ vi.mock('../../src/main/engine/PostEngine', () => ({ describe('TagEngine', () => { let tagEngine: TagEngine; + const captureProgressForNextTask = (): Array<{ progress: number; message: string }> => { + const progressCalls: Array<{ progress: number; message: string }> = []; + + vi.mocked(taskManager.runTask).mockImplementationOnce(async (task: any) => { + return task.execute((progress: number, message: string) => { + progressCalls.push({ progress, message }); + }); + }); + + return progressCalls; + }; + beforeEach(() => { vi.clearAllMocks(); mockTags.clear(); @@ -320,6 +333,29 @@ describe('TagEngine', () => { expect(result.postsUpdated).toBe(1); expect(mockPostEngine.syncPublishedPostFile).toHaveBeenCalledTimes(1); }); + + it('should report stable progress checkpoints during delete', async () => { + mockSelectDataQueue = [ + [{ id: 'tag-1', name: 'react', color: null, projectId: 'default', createdAt: new Date(), updatedAt: new Date() }], + ]; + mockLocalClient.execute.mockResolvedValueOnce({ + rows: [ + { id: 'post-1', tags: '["react", "typescript"]' }, + { id: 'post-2', tags: '["react"]' }, + ], + }); + + const progressCalls = captureProgressForNextTask(); + await tagEngine.deleteTag('tag-1'); + + expect(progressCalls).toEqual([ + { progress: 0, message: 'Finding posts with tag "react"...' }, + { progress: 40, message: 'Updated 1/2 posts...' }, + { progress: 80, message: 'Updated 2/2 posts...' }, + { progress: 90, message: 'Deleting tag...' }, + { progress: 100, message: 'Complete' }, + ]); + }); }); describe('mergeTags', () => { @@ -386,6 +422,29 @@ describe('TagEngine', () => { expect(result.postsUpdated).toBe(1); expect(mockPostEngine.syncPublishedPostFile).toHaveBeenCalledTimes(1); }); + + it('should report stable progress checkpoints during merge', async () => { + mockSelectDataQueue = [ + [{ id: 'tag-1', name: 'js', projectId: 'default', createdAt: new Date(), updatedAt: new Date() }], + [{ id: 'tag-2', name: 'javascript', projectId: 'default', createdAt: new Date(), updatedAt: new Date() }], + [{ id: 'tag-3', name: 'ecmascript', projectId: 'default', createdAt: new Date(), updatedAt: new Date() }], + ]; + + mockLocalClient.execute + .mockResolvedValueOnce({ rows: [{ id: 'post-1', tags: '["js"]' }] }) + .mockResolvedValueOnce({ rows: [{ id: 'post-2', tags: '["javascript"]' }] }); + + const progressCalls = captureProgressForNextTask(); + await tagEngine.mergeTags(['tag-1', 'tag-2'], 'tag-3'); + + expect(progressCalls).toEqual([ + { progress: 0, message: 'Finding posts to update...' }, + { progress: 40, message: 'Processing tag "js"...' }, + { progress: 80, message: 'Processing tag "javascript"...' }, + { progress: 90, message: 'Deleting source tags...' }, + { progress: 100, message: 'Complete' }, + ]); + }); }); describe('renameTags (batch rename)', () => { @@ -440,6 +499,30 @@ describe('TagEngine', () => { expect(result.postsUpdated).toBe(1); expect(mockPostEngine.syncPublishedPostFile).toHaveBeenCalledTimes(1); }); + + it('should report stable progress checkpoints during rename', async () => { + mockSelectDataQueue = [ + [{ id: 'tag-1', name: 'old-name', projectId: 'default', createdAt: new Date(), updatedAt: new Date() }], + [], + ]; + mockLocalClient.execute.mockResolvedValueOnce({ + rows: [ + { id: 'post-1', tags: '["old-name"]' }, + { id: 'post-2', tags: '["old-name", "other"]' }, + ], + }); + + const progressCalls = captureProgressForNextTask(); + await tagEngine.renameTag('tag-1', 'new-name'); + + expect(progressCalls).toEqual([ + { progress: 0, message: 'Finding posts to update...' }, + { progress: 40, message: 'Updated 1/2 posts...' }, + { progress: 80, message: 'Updated 2/2 posts...' }, + { progress: 90, message: 'Updating tag record...' }, + { progress: 100, message: 'Complete' }, + ]); + }); }); describe('getTag', () => {