From 803a65999f3d3a55329bf3a9a45503b79049046d Mon Sep 17 00:00:00 2001 From: hugo Date: Sun, 15 Feb 2026 22:13:01 +0100 Subject: [PATCH] fix: deduplicate and focus code on projects --- src/main/engine/ProjectEngine.ts | 62 ++++++++++++------------------ tests/engine/ProjectEngine.test.ts | 47 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 38 deletions(-) diff --git a/src/main/engine/ProjectEngine.ts b/src/main/engine/ProjectEngine.ts index da04fb0..77bcbe0 100644 --- a/src/main/engine/ProjectEngine.ts +++ b/src/main/engine/ProjectEngine.ts @@ -23,6 +23,25 @@ export class ProjectEngine extends EventEmitter { super(); } + private assertDeletableProject(id: string): void { + if (id === 'default') { + throw new Error('Cannot delete the default project'); + } + } + + private mapDbProjectToProjectData(dbProject: Project): ProjectData { + return { + id: dbProject.id, + name: dbProject.name, + slug: dbProject.slug, + description: dbProject.description || undefined, + dataPath: dbProject.dataPath || undefined, + createdAt: dbProject.createdAt, + updatedAt: dbProject.updatedAt, + isActive: dbProject.isActive ?? false, + }; + } + private generateSlug(name: string): string { return name .toLowerCase() @@ -160,10 +179,7 @@ export class ProjectEngine extends EventEmitter { } async deleteProject(id: string): Promise { - // Prevent deleting the default project - if (id === 'default') { - throw new Error('Cannot delete the default project'); - } + this.assertDeletableProject(id); const db = getDatabase().getLocal(); const existing = await db.select().from(projects).where(eq(projects.id, id)).get(); @@ -182,10 +198,7 @@ export class ProjectEngine extends EventEmitter { } async deleteProjectWithData(id: string): Promise { - // Prevent deleting the default project - if (id === 'default') { - throw new Error('Cannot delete the default project'); - } + this.assertDeletableProject(id); const db = getDatabase().getLocal(); const existing = await db.select().from(projects).where(eq(projects.id, id)).get(); @@ -224,32 +237,14 @@ export class ProjectEngine extends EventEmitter { return null; } - return { - id: dbProject.id, - name: dbProject.name, - slug: dbProject.slug, - description: dbProject.description || undefined, - dataPath: dbProject.dataPath || undefined, - createdAt: dbProject.createdAt, - updatedAt: dbProject.updatedAt, - isActive: dbProject.isActive ?? false, - }; + return this.mapDbProjectToProjectData(dbProject); } async getAllProjects(): Promise { const db = getDatabase().getLocal(); const dbProjects = await db.select().from(projects).all(); - return dbProjects.map(p => ({ - id: p.id, - name: p.name, - slug: p.slug, - description: p.description || undefined, - dataPath: p.dataPath || undefined, - createdAt: p.createdAt, - updatedAt: p.updatedAt, - isActive: p.isActive ?? false, - })); + return dbProjects.map(p => this.mapDbProjectToProjectData(p)); } async getActiveProject(): Promise { @@ -261,16 +256,7 @@ export class ProjectEngine extends EventEmitter { return this.getProject('default'); } - return { - id: dbProject.id, - name: dbProject.name, - slug: dbProject.slug, - description: dbProject.description || undefined, - dataPath: dbProject.dataPath || undefined, - createdAt: dbProject.createdAt, - updatedAt: dbProject.updatedAt, - isActive: dbProject.isActive ?? false, - }; + return this.mapDbProjectToProjectData(dbProject); } async setActiveProject(id: string): Promise { diff --git a/tests/engine/ProjectEngine.test.ts b/tests/engine/ProjectEngine.test.ts index c974053..6cd0294 100644 --- a/tests/engine/ProjectEngine.test.ts +++ b/tests/engine/ProjectEngine.test.ts @@ -215,6 +215,15 @@ describe('ProjectEngine', () => { ); }); + it('should short-circuit before database access when deleting default project', async () => { + await expect(projectEngine.deleteProject('default')).rejects.toThrow( + 'Cannot delete the default project' + ); + + expect(mockLocalDb.select).not.toHaveBeenCalled(); + expect(mockLocalDb.delete).not.toHaveBeenCalled(); + }); + it('should return false for non-existent project', async () => { const result = await projectEngine.deleteProject('non-existent-id'); expect(result).toBe(false); @@ -260,6 +269,15 @@ describe('ProjectEngine', () => { ); }); + it('should short-circuit before database access when deleting default project', async () => { + await expect(projectEngine.deleteProjectWithData('default')).rejects.toThrow( + 'Cannot delete the default project' + ); + + expect(mockLocalDb.select).not.toHaveBeenCalled(); + expect(mockLocalDb.delete).not.toHaveBeenCalled(); + }); + it('should return false for non-existent project', async () => { const result = await projectEngine.deleteProjectWithData('non-existent-id'); expect(result).toBe(false); @@ -990,6 +1008,35 @@ describe('ProjectEngine', () => { expect(result?.id).toBe('active-project'); expect(result?.isActive).toBe(true); }); + + it('should normalize nullable fields in active project mapping', async () => { + const activeProject = { + id: 'active-nullable', + name: 'Active Nullable', + slug: 'active-nullable', + description: null, + dataPath: null, + createdAt: new Date(), + updatedAt: new Date(), + isActive: null, + }; + + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(activeProject), + }); + return chain; + }); + + const result = await projectEngine.getActiveProject(); + + expect(result).not.toBeNull(); + expect(result?.description).toBeUndefined(); + expect(result?.dataPath).toBeUndefined(); + expect(result?.isActive).toBe(false); + }); }); describe('setActiveProject', () => {