From 27f39fa04b383442610c860928499c2aed8fca90 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 16 Jun 2025 23:33:00 -0700 Subject: [PATCH 1/3] modified folder deletion to delete subfolders & workflows in it instead of moving to root --- apps/sim/app/api/__test-utils__/utils.ts | 174 +++++++ apps/sim/app/api/folders/[id]/route.test.ts | 291 ++++++++++++ apps/sim/app/api/folders/[id]/route.ts | 98 ++-- apps/sim/app/api/folders/route.test.ts | 435 ++++++++++++++++++ .../folder-tree/components/folder-item.tsx | 10 +- apps/sim/stores/folders/store.ts | 78 +++- apps/sim/stores/workflows/registry/store.ts | 15 +- 7 files changed, 1027 insertions(+), 74 deletions(-) create mode 100644 apps/sim/app/api/folders/[id]/route.test.ts create mode 100644 apps/sim/app/api/folders/route.test.ts diff --git a/apps/sim/app/api/__test-utils__/utils.ts b/apps/sim/app/api/__test-utils__/utils.ts index 1c235da5f4c..289e1c3551c 100644 --- a/apps/sim/app/api/__test-utils__/utils.ts +++ b/apps/sim/app/api/__test-utils__/utils.ts @@ -427,3 +427,177 @@ export function mockScheduleExecuteDb({ return { db: { select, update } } }) } + +/** + * Mock authentication for API tests + */ +export function mockAuth(user: any = mockUser) { + const mockGetSession = vi.fn() + + vi.doMock('@/lib/auth', () => ({ + getSession: mockGetSession, + })) + + return { + mockGetSession, + mockAuthenticatedUser: () => mockGetSession.mockResolvedValueOnce({ user }), + mockUnauthenticated: () => mockGetSession.mockResolvedValueOnce(null), + } +} + +/** + * Create a flexible query builder mock for database operations + */ +export function createQueryBuilderMock(data: any[] = []) { + return { + select: vi.fn().mockReturnThis(), + from: vi.fn().mockReturnThis(), + where: vi.fn().mockReturnThis(), + orderBy: vi.fn().mockReturnThis(), + limit: vi.fn().mockReturnThis(), + offset: vi.fn().mockReturnThis(), + then: vi.fn().mockResolvedValue(data), + } +} + +/** + * Mock common schema patterns + */ +export function mockCommonSchemas() { + vi.doMock('@/db/schema', () => ({ + workflowFolder: { + id: 'id', + userId: 'userId', + parentId: 'parentId', + updatedAt: 'updatedAt', + workspaceId: 'workspaceId', + sortOrder: 'sortOrder', + createdAt: 'createdAt', + }, + workflow: { + id: 'id', + folderId: 'folderId', + userId: 'userId', + updatedAt: 'updatedAt', + }, + account: { + userId: 'userId', + providerId: 'providerId', + }, + user: { + email: 'email', + id: 'id', + }, + })) +} + +/** + * Mock drizzle-orm operators + */ +export function mockDrizzleOrm() { + vi.doMock('drizzle-orm', () => ({ + and: vi.fn((...conditions) => ({ conditions, type: 'and' })), + eq: vi.fn((field, value) => ({ field, value, type: 'eq' })), + or: vi.fn((...conditions) => ({ type: 'or', conditions })), + gte: vi.fn((field, value) => ({ type: 'gte', field, value })), + lte: vi.fn((field, value) => ({ type: 'lte', field, value })), + asc: vi.fn((field) => ({ field, type: 'asc' })), + desc: vi.fn((field) => ({ field, type: 'desc' })), + isNull: vi.fn((field) => ({ field, type: 'isNull' })), + sql: vi.fn((strings, ...values) => ({ + type: 'sql', + sql: strings, + values, + })), + })) +} + +/** + * Mock console logger + */ +export function mockConsoleLogger() { + vi.doMock('@/lib/logs/console-logger', () => ({ + createLogger: vi.fn().mockReturnValue(mockLogger), + })) +} + +/** + * Setup common API test mocks (auth, logger, schema, drizzle) + */ +export function setupCommonApiMocks() { + mockCommonSchemas() + mockDrizzleOrm() + mockConsoleLogger() +} + +/** + * Create mock database with CRUD operations + */ +export function createMockDatabase( + options: { + selectData?: any[] + insertResult?: any[] + updateResult?: any[] + deleteResult?: any[] + throwError?: boolean + } = {} +) { + const { + selectData = [], + insertResult = [], + updateResult = [], + deleteResult = [], + throwError = false, + } = options + + if (throwError) { + const throwingMock = vi.fn().mockImplementation(() => { + throw new Error('Database error') + }) + + return { + db: { + select: throwingMock, + insert: throwingMock, + update: throwingMock, + delete: throwingMock, + }, + } + } + + return { + db: { + select: vi.fn().mockImplementation(() => createQueryBuilderMock(selectData)), + insert: vi.fn().mockImplementation(() => ({ + values: vi.fn().mockImplementation(() => ({ + returning: vi.fn().mockReturnValue(insertResult), + onConflictDoUpdate: vi.fn().mockResolvedValue({}), + })), + })), + update: vi.fn().mockImplementation(() => ({ + set: vi.fn().mockImplementation(() => ({ + where: vi.fn().mockResolvedValue(updateResult), + })), + })), + delete: vi.fn().mockImplementation(() => ({ + where: vi.fn().mockResolvedValue(deleteResult), + })), + transaction: vi.fn().mockImplementation(async (callback) => { + return await callback({ + select: vi.fn().mockImplementation(() => createQueryBuilderMock(selectData)), + insert: vi.fn().mockImplementation(() => ({ + values: vi.fn().mockResolvedValue(insertResult), + })), + update: vi.fn().mockImplementation(() => ({ + set: vi.fn().mockImplementation(() => ({ + where: vi.fn().mockResolvedValue(updateResult), + })), + })), + delete: vi.fn().mockImplementation(() => ({ + where: vi.fn().mockResolvedValue(deleteResult), + })), + }) + }), + }, + } +} diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts new file mode 100644 index 00000000000..c9721e13917 --- /dev/null +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -0,0 +1,291 @@ +/** + * Tests for individual folder API route (/api/folders/[id]) + * + * @vitest-environment node + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createMockRequest, + mockAuth, + mockLogger, + setupCommonApiMocks, +} from '@/app/api/__test-utils__/utils' + +describe('Individual Folder API Route', () => { + const mockFolder = { + id: 'folder-1', + name: 'Test Folder', + userId: 'user-123', + workspaceId: 'workspace-123', + parentId: null, + color: '#6B7280', + sortOrder: 1, + createdAt: new Date(), + updatedAt: new Date(), + } + + const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth() + + function createFolderDbMock( + options: { + folderLookupResult?: any + updateResult?: any[] + throwError?: boolean + circularCheckResults?: any[] + } = {} + ) { + const { + folderLookupResult = mockFolder, + updateResult = [{ ...mockFolder, name: 'Updated Folder' }], + throwError = false, + circularCheckResults = [], + } = options + + let callCount = 0 + + const mockSelect = vi.fn().mockImplementation(() => ({ + from: vi.fn().mockImplementation(() => ({ + where: vi.fn().mockImplementation(() => ({ + then: vi.fn().mockImplementation((callback) => { + if (throwError) { + throw new Error('Database error') + } + + callCount++ + // First call: folder lookup + if (callCount === 1) { + // The route code does .then((rows) => rows[0]) + // So we need to return an array for folderLookupResult + const result = folderLookupResult === undefined ? [] : [folderLookupResult] + return Promise.resolve(callback(result)) + } + // Subsequent calls: circular reference checks + if (callCount > 1 && circularCheckResults.length > 0) { + const index = callCount - 2 + const result = circularCheckResults[index] ? [circularCheckResults[index]] : [] + return Promise.resolve(callback(result)) + } + return Promise.resolve(callback([])) + }), + })), + })), + })) + + const mockUpdate = vi.fn().mockImplementation(() => ({ + set: vi.fn().mockImplementation(() => ({ + where: vi.fn().mockImplementation(() => ({ + returning: vi.fn().mockReturnValue(updateResult), + })), + })), + })) + + const mockDelete = vi.fn().mockImplementation(() => ({ + where: vi.fn().mockImplementation(() => Promise.resolve()), + })) + + return { + db: { + select: mockSelect, + update: mockUpdate, + delete: mockDelete, + }, + mocks: { + select: mockSelect, + update: mockUpdate, + delete: mockDelete, + }, + } + } + + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + setupCommonApiMocks() + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe('PUT /api/folders/[id]', () => { + it('should update folder successfully', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: 'Updated Folder Name', + color: '#FF0000', + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + expect(response.status).toBe(200) + + const data = await response.json() + expect(data).toHaveProperty('folder') + expect(data.folder).toMatchObject({ + name: 'Updated Folder', + }) + }) + + it('should update parent folder successfully', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: 'Updated Folder', + parentId: 'parent-folder-1', + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + expect(response.status).toBe(200) + }) + + it('should return 401 for unauthenticated requests', async () => { + mockUnauthenticated() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: 'Updated Folder', + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + expect(response.status).toBe(401) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Unauthorized') + }) + + it('should return 400 when trying to set folder as its own parent', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: 'Updated Folder', + parentId: 'folder-1', // Same as the folder ID + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + expect(response.status).toBe(400) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Folder cannot be its own parent') + }) + + it('should trim folder name when updating', async () => { + mockAuthenticatedUser() + + let capturedUpdates: any = null + const dbMock = createFolderDbMock({ + updateResult: [{ ...mockFolder, name: 'Folder With Spaces' }], + }) + + // Override the set implementation to capture updates + const originalSet = dbMock.mocks.update().set + dbMock.mocks.update.mockReturnValue({ + set: vi.fn().mockImplementation((updates) => { + capturedUpdates = updates + return originalSet(updates) + }), + }) + + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: ' Folder With Spaces ', + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + await PUT(req, { params }) + + expect(capturedUpdates.name).toBe('Folder With Spaces') + }) + + it('should handle database errors gracefully', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock({ + throwError: true, + }) + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: 'Updated Folder', + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + expect(response.status).toBe(500) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Internal server error') + expect(mockLogger.error).toHaveBeenCalledWith('Error updating folder:', { + error: expect.any(Error), + }) + }) + }) + + describe('Circular Reference Prevention', () => { + it('should prevent circular references when updating parent', async () => { + mockAuthenticatedUser() + + // Mock the circular reference scenario + // folder-3 trying to set folder-1 as parent, + // but folder-1 -> folder-2 -> folder-3 (would create cycle) + const circularCheckResults = [ + { parentId: 'folder-2' }, // folder-1 has parent folder-2 + { parentId: 'folder-3' }, // folder-2 has parent folder-3 (creates cycle!) + ] + + const dbMock = createFolderDbMock({ + folderLookupResult: { id: 'folder-3', parentId: null, name: 'Folder 3' }, + circularCheckResults, + }) + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: 'Updated Folder 3', + parentId: 'folder-1', // This would create a circular reference + }) + const params = Promise.resolve({ id: 'folder-3' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + // Should return 400 due to circular reference + expect(response.status).toBe(400) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Cannot create circular folder reference') + }) + }) +}) diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index 341c5b9d6ee..4cdca3fdc98 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/apps/sim/app/api/folders/[id]/route.ts @@ -68,7 +68,7 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ } } -// DELETE - Delete a folder +// DELETE - Delete a folder and all its contents export async function DELETE( request: NextRequest, { params }: { params: Promise<{ id: string }> } @@ -80,8 +80,6 @@ export async function DELETE( } const { id } = await params - const { searchParams } = new URL(request.url) - const moveWorkflowsTo = searchParams.get('moveWorkflowsTo') // Optional: move workflows to another folder // Verify the folder exists and belongs to the user const existingFolder = await db @@ -94,57 +92,17 @@ export async function DELETE( return NextResponse.json({ error: 'Folder not found' }, { status: 404 }) } - // Check if folder has child folders - const childFolders = await db - .select({ id: workflowFolder.id }) - .from(workflowFolder) - .where(eq(workflowFolder.parentId, id)) - - // Check if folder has workflows - const workflowsInFolder = await db - .select({ id: workflow.id }) - .from(workflow) - .where(eq(workflow.folderId, id)) - - // Handle child folders - move them to parent or root - if (childFolders.length > 0) { - await db - .update(workflowFolder) - .set({ - parentId: existingFolder.parentId, // Move to the parent of the deleted folder - updatedAt: new Date(), - }) - .where(eq(workflowFolder.parentId, id)) - } - - // Handle workflows in the folder - if (workflowsInFolder.length > 0) { - const newFolderId = moveWorkflowsTo || null // Move to specified folder or root - await db - .update(workflow) - .set({ - folderId: newFolderId, - updatedAt: new Date(), - }) - .where(eq(workflow.folderId, id)) - } + // Recursively delete folder and all its contents + const deletionStats = await deleteFolderRecursively(id, session.user.id) - // Delete the folder - await db.delete(workflowFolder).where(eq(workflowFolder.id, id)) - - logger.info('Deleted folder:', { + logger.info('Deleted folder and all contents:', { id, - childFoldersCount: childFolders.length, - workflowsCount: workflowsInFolder.length, - movedWorkflowsTo: moveWorkflowsTo, + deletionStats, }) return NextResponse.json({ success: true, - movedItems: { - childFolders: childFolders.length, - workflows: workflowsInFolder.length, - }, + deletedItems: deletionStats, }) } catch (error) { logger.error('Error deleting folder:', { error }) @@ -152,6 +110,50 @@ export async function DELETE( } } +// Helper function to recursively delete a folder and all its contents +async function deleteFolderRecursively( + folderId: string, + userId: string +): Promise<{ folders: number; workflows: number }> { + const stats = { folders: 0, workflows: 0 } + + // Get all child folders first + const childFolders = await db + .select({ id: workflowFolder.id }) + .from(workflowFolder) + .where(and(eq(workflowFolder.parentId, folderId), eq(workflowFolder.userId, userId))) + + // Recursively delete child folders + for (const childFolder of childFolders) { + const childStats = await deleteFolderRecursively(childFolder.id, userId) + stats.folders += childStats.folders + stats.workflows += childStats.workflows + } + + // Delete all workflows in this folder + const workflowsInFolder = await db + .select({ id: workflow.id }) + .from(workflow) + .where(and(eq(workflow.folderId, folderId), eq(workflow.userId, userId))) + + if (workflowsInFolder.length > 0) { + await db + .delete(workflow) + .where(and(eq(workflow.folderId, folderId), eq(workflow.userId, userId))) + + stats.workflows += workflowsInFolder.length + } + + // Delete this folder + await db + .delete(workflowFolder) + .where(and(eq(workflowFolder.id, folderId), eq(workflowFolder.userId, userId))) + + stats.folders += 1 + + return stats +} + // Helper function to check for circular references async function checkForCircularReference(folderId: string, parentId: string): Promise { let currentParentId: string | null = parentId diff --git a/apps/sim/app/api/folders/route.test.ts b/apps/sim/app/api/folders/route.test.ts new file mode 100644 index 00000000000..59083c4aa48 --- /dev/null +++ b/apps/sim/app/api/folders/route.test.ts @@ -0,0 +1,435 @@ +/** + * Tests for folders API route + * + * @vitest-environment node + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createMockRequest, + mockAuth, + mockLogger, + setupCommonApiMocks, +} from '@/app/api/__test-utils__/utils' + +describe('Folders API Route', () => { + const mockFolders = [ + { + id: 'folder-1', + name: 'Test Folder 1', + userId: 'user-123', + workspaceId: 'workspace-123', + parentId: null, + color: '#6B7280', + isExpanded: true, + sortOrder: 0, + createdAt: new Date('2023-01-01T00:00:00.000Z'), + updatedAt: new Date('2023-01-01T00:00:00.000Z'), + }, + { + id: 'folder-2', + name: 'Test Folder 2', + userId: 'user-123', + workspaceId: 'workspace-123', + parentId: 'folder-1', + color: '#EF4444', + isExpanded: false, + sortOrder: 1, + createdAt: new Date('2023-01-02T00:00:00.000Z'), + updatedAt: new Date('2023-01-02T00:00:00.000Z'), + }, + ] + + const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth() + const mockUUID = 'mock-uuid-12345678-90ab-cdef-1234-567890abcdef' + + const mockSelect = vi.fn() + const mockFrom = vi.fn() + const mockWhere = vi.fn() + const mockOrderBy = vi.fn() + const mockInsert = vi.fn() + const mockValues = vi.fn() + const mockReturning = vi.fn() + const mockTransaction = vi.fn() + + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + + vi.stubGlobal('crypto', { + randomUUID: vi.fn().mockReturnValue(mockUUID), + }) + + setupCommonApiMocks() + + mockSelect.mockReturnValue({ from: mockFrom }) + mockFrom.mockReturnValue({ where: mockWhere }) + mockWhere.mockReturnValue({ orderBy: mockOrderBy }) + mockOrderBy.mockReturnValue(mockFolders) + + mockInsert.mockReturnValue({ values: mockValues }) + mockValues.mockReturnValue({ returning: mockReturning }) + mockReturning.mockReturnValue([mockFolders[0]]) + + vi.doMock('@/db', () => ({ + db: { + select: mockSelect, + insert: mockInsert, + transaction: mockTransaction, + }, + })) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe('GET /api/folders', () => { + it('should return folders for a valid workspace', async () => { + mockAuthenticatedUser() + + const mockRequest = createMockRequest('GET') + Object.defineProperty(mockRequest, 'url', { + value: 'http://localhost:3000/api/folders?workspaceId=workspace-123', + }) + + const { GET } = await import('./route') + const response = await GET(mockRequest) + + expect(response.status).toBe(200) + + const data = await response.json() + expect(data).toHaveProperty('folders') + expect(data.folders).toHaveLength(2) + expect(data.folders[0]).toMatchObject({ + id: 'folder-1', + name: 'Test Folder 1', + workspaceId: 'workspace-123', + }) + }) + + it('should return 401 for unauthenticated requests', async () => { + mockUnauthenticated() + + const mockRequest = createMockRequest('GET') + Object.defineProperty(mockRequest, 'url', { + value: 'http://localhost:3000/api/folders?workspaceId=workspace-123', + }) + + const { GET } = await import('./route') + const response = await GET(mockRequest) + + expect(response.status).toBe(401) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Unauthorized') + }) + + it('should return 400 when workspaceId is missing', async () => { + mockAuthenticatedUser() + + const mockRequest = createMockRequest('GET') + Object.defineProperty(mockRequest, 'url', { + value: 'http://localhost:3000/api/folders', + }) + + const { GET } = await import('./route') + const response = await GET(mockRequest) + + expect(response.status).toBe(400) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Workspace ID is required') + }) + + it('should handle database errors gracefully', async () => { + mockAuthenticatedUser() + + mockSelect.mockImplementationOnce(() => { + throw new Error('Database connection failed') + }) + + const mockRequest = createMockRequest('GET') + Object.defineProperty(mockRequest, 'url', { + value: 'http://localhost:3000/api/folders?workspaceId=workspace-123', + }) + + const { GET } = await import('./route') + const response = await GET(mockRequest) + + expect(response.status).toBe(500) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Internal server error') + expect(mockLogger.error).toHaveBeenCalledWith('Error fetching folders:', { + error: expect.any(Error), + }) + }) + }) + + describe('POST /api/folders', () => { + it('should create a new folder successfully', async () => { + mockAuthenticatedUser() + + mockTransaction.mockImplementationOnce(async (callback: any) => { + const tx = { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockReturnValue([]), // No existing folders + }), + }), + }), + }), + insert: vi.fn().mockReturnValue({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockReturnValue([mockFolders[0]]), + }), + }), + } + return await callback(tx) + }) + + const req = createMockRequest('POST', { + name: 'New Test Folder', + workspaceId: 'workspace-123', + color: '#6B7280', + }) + + const { POST } = await import('./route') + const response = await POST(req) + + expect(response.status).toBe(200) + + const data = await response.json() + expect(data).toHaveProperty('folder') + expect(data.folder).toMatchObject({ + id: 'folder-1', + name: 'Test Folder 1', + workspaceId: 'workspace-123', + }) + }) + + it('should create folder with correct sort order', async () => { + mockAuthenticatedUser() + + mockTransaction.mockImplementationOnce(async (callback: any) => { + const tx = { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockReturnValue([{ sortOrder: 5 }]), // Existing folder with sort order 5 + }), + }), + }), + }), + insert: vi.fn().mockReturnValue({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockReturnValue([{ ...mockFolders[0], sortOrder: 6 }]), + }), + }), + } + return await callback(tx) + }) + + const req = createMockRequest('POST', { + name: 'New Test Folder', + workspaceId: 'workspace-123', + }) + + const { POST } = await import('./route') + const response = await POST(req) + + expect(response.status).toBe(200) + + const data = await response.json() + expect(data.folder).toMatchObject({ + sortOrder: 6, + }) + }) + + it('should create subfolder with parent reference', async () => { + mockAuthenticatedUser() + + mockTransaction.mockImplementationOnce(async (callback: any) => { + const tx = { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockReturnValue([]), + }), + }), + }), + }), + insert: vi.fn().mockReturnValue({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockReturnValue([{ ...mockFolders[1] }]), + }), + }), + } + return await callback(tx) + }) + + const req = createMockRequest('POST', { + name: 'Subfolder', + workspaceId: 'workspace-123', + parentId: 'folder-1', + }) + + const { POST } = await import('./route') + const response = await POST(req) + + expect(response.status).toBe(200) + + const data = await response.json() + expect(data.folder).toMatchObject({ + parentId: 'folder-1', + }) + }) + + it('should return 401 for unauthenticated requests', async () => { + mockUnauthenticated() + + const req = createMockRequest('POST', { + name: 'Test Folder', + workspaceId: 'workspace-123', + }) + + const { POST } = await import('./route') + const response = await POST(req) + + expect(response.status).toBe(401) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Unauthorized') + }) + + it('should return 400 when required fields are missing', async () => { + const testCases = [ + { name: '', workspaceId: 'workspace-123' }, // Missing name + { name: 'Test Folder', workspaceId: '' }, // Missing workspaceId + { workspaceId: 'workspace-123' }, // Missing name entirely + { name: 'Test Folder' }, // Missing workspaceId entirely + ] + + for (const body of testCases) { + mockAuthenticatedUser() + + const req = createMockRequest('POST', body) + + const { POST } = await import('./route') + const response = await POST(req) + + expect(response.status).toBe(400) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Name and workspace ID are required') + } + }) + + it('should handle database errors gracefully', async () => { + mockAuthenticatedUser() + + // Make transaction throw an error + mockTransaction.mockImplementationOnce(() => { + throw new Error('Database transaction failed') + }) + + const req = createMockRequest('POST', { + name: 'Test Folder', + workspaceId: 'workspace-123', + }) + + const { POST } = await import('./route') + const response = await POST(req) + + expect(response.status).toBe(500) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Internal server error') + expect(mockLogger.error).toHaveBeenCalledWith('Error creating folder:', { + error: expect.any(Error), + }) + }) + + it('should trim folder name when creating', async () => { + mockAuthenticatedUser() + + let capturedValues: any = null + + mockTransaction.mockImplementationOnce(async (callback: any) => { + const tx = { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockReturnValue([]), + }), + }), + }), + }), + insert: vi.fn().mockReturnValue({ + values: vi.fn().mockImplementation((values) => { + capturedValues = values + return { + returning: vi.fn().mockReturnValue([mockFolders[0]]), + } + }), + }), + } + return await callback(tx) + }) + + const req = createMockRequest('POST', { + name: ' Test Folder With Spaces ', + workspaceId: 'workspace-123', + }) + + const { POST } = await import('./route') + await POST(req) + + expect(capturedValues.name).toBe('Test Folder With Spaces') + }) + + it('should use default color when not provided', async () => { + mockAuthenticatedUser() + + let capturedValues: any = null + + mockTransaction.mockImplementationOnce(async (callback: any) => { + const tx = { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockReturnValue([]), + }), + }), + }), + }), + insert: vi.fn().mockReturnValue({ + values: vi.fn().mockImplementation((values) => { + capturedValues = values + return { + returning: vi.fn().mockReturnValue([mockFolders[0]]), + } + }), + }), + } + return await callback(tx) + }) + + const req = createMockRequest('POST', { + name: 'Test Folder', + workspaceId: 'workspace-123', + }) + + const { POST } = await import('./route') + await POST(req) + + expect(capturedValues.color).toBe('#6B7280') + }) + }) +}) diff --git a/apps/sim/app/w/components/sidebar/components/folder-tree/components/folder-item.tsx b/apps/sim/app/w/components/sidebar/components/folder-tree/components/folder-item.tsx index 4f02c73497d..db9ffe073a2 100644 --- a/apps/sim/app/w/components/sidebar/components/folder-tree/components/folder-item.tsx +++ b/apps/sim/app/w/components/sidebar/components/folder-tree/components/folder-item.tsx @@ -133,7 +133,8 @@ export function FolderItem({ Are you sure you want to delete "{folder.name}"? - Child folders and workflows will be moved to the parent folder. + This will permanently delete the folder and all its contents, including subfolders + and workflows. This action cannot be undone. @@ -143,7 +144,7 @@ export function FolderItem({ disabled={isDeleting} className='bg-destructive text-destructive-foreground hover:bg-destructive/90' > - {isDeleting ? 'Deleting...' : 'Delete'} + {isDeleting ? 'Deleting...' : 'Delete Forever'} @@ -197,7 +198,8 @@ export function FolderItem({ Are you sure you want to delete "{folder.name}"? - Child folders and workflows will be moved to the parent folder. + This will permanently delete the folder and all its contents, including subfolders and + workflows. This action cannot be undone. @@ -207,7 +209,7 @@ export function FolderItem({ disabled={isDeleting} className='bg-destructive text-destructive-foreground hover:bg-destructive/90' > - {isDeleting ? 'Deleting...' : 'Delete'} + {isDeleting ? 'Deleting...' : 'Delete Forever'} diff --git a/apps/sim/stores/folders/store.ts b/apps/sim/stores/folders/store.ts index 1c2cedbf236..68220eb57db 100644 --- a/apps/sim/stores/folders/store.ts +++ b/apps/sim/stores/folders/store.ts @@ -1,5 +1,9 @@ import { create } from 'zustand' import { devtools } from 'zustand/middleware' +import { createLogger } from '@/lib/logs/console-logger' +import { useWorkflowRegistry } from '../workflows/registry/store' + +const logger = createLogger('FoldersStore') export interface WorkflowFolder { id: string @@ -57,7 +61,11 @@ interface FolderState { color?: string }) => Promise updateFolderAPI: (id: string, updates: Partial) => Promise - deleteFolder: (id: string, moveWorkflowsTo?: string) => Promise + deleteFolder: (id: string) => Promise + + // Helper functions + isWorkflowInDeletedSubfolder: (workflow: any, deletedFolderId: string) => boolean + removeSubfoldersRecursively: (parentFolderId: string) => void } export const useFolderStore = create()( @@ -233,7 +241,7 @@ export const useFolderStore = create()( }) set({ expandedFolders: expandedSet }) } catch (error) { - console.error('Error fetching folders:', error) + logger.error('Error fetching folders:', error) } finally { set({ isLoading: false }) } @@ -286,18 +294,16 @@ export const useFolderStore = create()( return processedFolder }, - deleteFolder: async (id, moveWorkflowsTo) => { - const url = moveWorkflowsTo - ? `/api/folders/${id}?moveWorkflowsTo=${moveWorkflowsTo}` - : `/api/folders/${id}` - - const response = await fetch(url, { method: 'DELETE' }) + deleteFolder: async (id: string) => { + const response = await fetch(`/api/folders/${id}`, { method: 'DELETE' }) if (!response.ok) { const error = await response.json() throw new Error(error.error || 'Failed to delete folder') } + const responseData = await response.json() + get().removeFolder(id) // Remove from expanded state @@ -306,6 +312,62 @@ export const useFolderStore = create()( newExpanded.delete(id) return { expandedFolders: newExpanded } }) + + const workflowRegistry = useWorkflowRegistry.getState() + if (responseData.deletedItems) { + try { + const workflows = Object.values(workflowRegistry.workflows) + const workflowsToDelete = workflows.filter( + (workflow: any) => + workflow.folderId === id || get().isWorkflowInDeletedSubfolder(workflow, id) + ) + + workflowsToDelete.forEach((workflow: any) => { + workflowRegistry.removeWorkflow(workflow.id) + }) + + get().removeSubfoldersRecursively(id) + + logger.info( + `Deleted ${responseData.deletedItems.workflows} workflow(s) and ${responseData.deletedItems.folders} folder(s)` + ) + } catch (error) { + logger.error('Error updating local state after folder deletion:', error) + } + } + + if (workflowRegistry.activeWorkspaceId) { + const { fetchWorkflowsFromDB } = await import('../workflows/sync') + await fetchWorkflowsFromDB() + } + }, + + isWorkflowInDeletedSubfolder: (workflow: any, deletedFolderId: string) => { + if (!workflow.folderId) return false + + const folders = get().folders + let currentFolderId = workflow.folderId + + while (currentFolderId && folders[currentFolderId]) { + if (currentFolderId === deletedFolderId) { + return true + } + currentFolderId = folders[currentFolderId].parentId + } + + return false + }, + + removeSubfoldersRecursively: (parentFolderId: string) => { + const folders = get().folders + const childFolderIds = Object.keys(folders).filter( + (id) => folders[id].parentId === parentFolderId + ) + + childFolderIds.forEach((childId) => { + get().removeSubfoldersRecursively(childId) + get().removeFolder(childId) + }) }, }), { name: 'folder-store' } diff --git a/apps/sim/stores/workflows/registry/store.ts b/apps/sim/stores/workflows/registry/store.ts index 70d7cf13291..4cd1ae74b68 100644 --- a/apps/sim/stores/workflows/registry/store.ts +++ b/apps/sim/stores/workflows/registry/store.ts @@ -2,7 +2,7 @@ import { create } from 'zustand' import { devtools } from 'zustand/middleware' import { createLogger } from '@/lib/logs/console-logger' import { clearWorkflowVariablesTracking } from '@/stores/panel/variables/store' -import { API_ENDPOINTS, STORAGE_KEYS } from '../../constants' +import { STORAGE_KEYS } from '../../constants' import { loadWorkflowState, removeFromStorage, @@ -1084,19 +1084,6 @@ export const useWorkflowRegistry = create()( removeFromStorage(STORAGE_KEYS.SUBBLOCK(id)) saveRegistry(newWorkflows) - // Ensure any schedule for this workflow is cancelled - // The API will handle the deletion of the schedule - fetch(API_ENDPOINTS.SCHEDULE, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - workflowId: id, - state: { blocks: {} }, // Empty blocks will signal to cancel the schedule - }), - }).catch((error) => { - logger.error(`Error cancelling schedule for deleted workflow ${id}:`, { error }) - }) - // Mark as dirty to ensure sync useWorkflowStore.getState().sync.markDirty() From 0b03b3506f7042252924131f9c61090624776fb1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 16 Jun 2025 23:38:22 -0700 Subject: [PATCH 2/3] added additional testing utils --- apps/sim/app/api/__test-utils__/utils.ts | 172 ++++++++++++++++++-- apps/sim/app/api/folders/[id]/route.test.ts | 130 ++++++++++++++- apps/sim/lib/variables/variable-manager.ts | 1 - biome.json | 3 +- 4 files changed, 288 insertions(+), 18 deletions(-) diff --git a/apps/sim/app/api/__test-utils__/utils.ts b/apps/sim/app/api/__test-utils__/utils.ts index 289e1c3551c..844b875fee0 100644 --- a/apps/sim/app/api/__test-utils__/utils.ts +++ b/apps/sim/app/api/__test-utils__/utils.ts @@ -1,6 +1,29 @@ import { NextRequest } from 'next/server' import { vi } from 'vitest' +// Add type definitions for better type safety +export interface MockUser { + id: string + email: string + name?: string +} + +export interface MockAuthResult { + mockGetSession: ReturnType + mockAuthenticatedUser: (user?: MockUser) => void + mockUnauthenticated: () => void +} + +export interface MockDatabaseOptions { + selectData?: any[] + insertResult?: any[] + updateResult?: any[] + deleteResult?: any[] + throwError?: boolean + errorType?: 'connection' | 'constraint' | 'timeout' | 'generic' + errorMessage?: string +} + export const sampleWorkflowState = { blocks: { 'starter-id': { @@ -430,8 +453,10 @@ export function mockScheduleExecuteDb({ /** * Mock authentication for API tests + * @param user - Optional user object to use for authenticated requests + * @returns Object with authentication helper functions */ -export function mockAuth(user: any = mockUser) { +export function mockAuth(user: MockUser = mockUser): MockAuthResult { const mockGetSession = vi.fn() vi.doMock('@/lib/auth', () => ({ @@ -440,7 +465,8 @@ export function mockAuth(user: any = mockUser) { return { mockGetSession, - mockAuthenticatedUser: () => mockGetSession.mockResolvedValueOnce({ user }), + mockAuthenticatedUser: (customUser?: MockUser) => + mockGetSession.mockResolvedValueOnce({ user: customUser || user }), mockUnauthenticated: () => mockGetSession.mockResolvedValueOnce(null), } } @@ -532,27 +558,45 @@ export function setupCommonApiMocks() { /** * Create mock database with CRUD operations + * @param options - Configuration options for the mock database + * @returns Mock database object with all necessary methods */ -export function createMockDatabase( - options: { - selectData?: any[] - insertResult?: any[] - updateResult?: any[] - deleteResult?: any[] - throwError?: boolean - } = {} -) { +export function createMockDatabase(options: MockDatabaseOptions = {}) { const { selectData = [], insertResult = [], updateResult = [], deleteResult = [], throwError = false, + errorType = 'generic', + errorMessage = 'Database error', } = options if (throwError) { + const createError = () => { + switch (errorType) { + case 'connection': { + const connError = new Error(`Connection failed: ${errorMessage}`) + ;(connError as any).code = 'ECONNREFUSED' + return connError + } + case 'constraint': { + const constraintError = new Error(`Constraint violation: ${errorMessage}`) + ;(constraintError as any).code = '23505' // PostgreSQL unique violation + return constraintError + } + case 'timeout': { + const timeoutError = new Error(`Query timeout: ${errorMessage}`) + ;(timeoutError as any).code = 'ETIMEDOUT' + return timeoutError + } + default: + return new Error(errorMessage) + } + } + const throwingMock = vi.fn().mockImplementation(() => { - throw new Error('Database error') + throw createError() }) return { @@ -561,6 +605,7 @@ export function createMockDatabase( insert: throwingMock, update: throwingMock, delete: throwingMock, + transaction: throwingMock, }, } } @@ -601,3 +646,106 @@ export function createMockDatabase( }, } } + +/** + * Measure execution time of an async function + * @param fn - The async function to measure + * @param label - Optional label for logging + * @returns Promise with result and execution time + */ +export async function measureExecutionTime( + fn: () => Promise, + label?: string +): Promise<{ result: T; executionTime: number }> { + const startTime = performance.now() + const result = await fn() + const executionTime = performance.now() - startTime + + if (label) { + console.log(`${label} executed in ${executionTime.toFixed(2)}ms`) + } + + return { result, executionTime } +} + +/** + * Assert that a function executes within a time limit + * @param fn - The async function to test + * @param maxTime - Maximum allowed execution time in milliseconds + * @param label - Optional label for better error messages + */ +export async function expectWithinTimeLimit( + fn: () => Promise, + maxTime: number, + label = 'Function' +): Promise { + const { result, executionTime } = await measureExecutionTime(fn, label) + + if (executionTime > maxTime) { + throw new Error( + `${label} took ${executionTime.toFixed(2)}ms, which exceeds the limit of ${maxTime}ms` + ) + } + + return result +} + +/** + * Test data factory for creating consistent test objects + */ +export class TestDataFactory { + /** + * Create a mock user with optional overrides + */ + static createUser(overrides: Partial = {}): MockUser { + return { + id: `user-${Date.now()}`, + email: `test-${Date.now()}@example.com`, + name: 'Test User', + ...overrides, + } + } + + /** + * Create a mock folder with optional overrides + */ + static createFolder(overrides: Record = {}) { + const now = new Date() + return { + id: `folder-${Date.now()}`, + name: `Test Folder ${Date.now()}`, + userId: 'user-123', + workspaceId: 'workspace-123', + parentId: null, + color: '#6B7280', + sortOrder: 1, + createdAt: now, + updatedAt: now, + ...overrides, + } + } + + /** + * Create a mock workflow with optional overrides + */ + static createWorkflow(overrides: Record = {}) { + const now = new Date() + return { + id: `workflow-${Date.now()}`, + name: `Test Workflow ${Date.now()}`, + userId: 'user-123', + workspaceId: 'workspace-123', + folderId: null, + description: 'Test workflow description', + color: '#3B82F6', + state: { + blocks: {}, + edges: [], + loops: {}, + }, + createdAt: now, + updatedAt: now, + ...overrides, + } + } +} diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts index c9721e13917..b9578c048de 100644 --- a/apps/sim/app/api/folders/[id]/route.test.ts +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -6,25 +6,33 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createMockRequest, + type MockUser, mockAuth, mockLogger, setupCommonApiMocks, } from '@/app/api/__test-utils__/utils' describe('Individual Folder API Route', () => { + // Test data constants + const TEST_USER: MockUser = { + id: 'user-123', + email: 'test@example.com', + name: 'Test User', + } + const mockFolder = { id: 'folder-1', name: 'Test Folder', - userId: 'user-123', + userId: TEST_USER.id, workspaceId: 'workspace-123', parentId: null, color: '#6B7280', sortOrder: 1, - createdAt: new Date(), - updatedAt: new Date(), + createdAt: new Date('2024-01-01T00:00:00Z'), + updatedAt: new Date('2024-01-01T00:00:00Z'), } - const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth() + const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth(TEST_USER) function createFolderDbMock( options: { @@ -253,6 +261,51 @@ describe('Individual Folder API Route', () => { }) }) + describe('Input Validation', () => { + it('should handle empty folder name', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('PUT', { + name: '', // Empty name + }) + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + // Should still work as the API doesn't validate empty names + expect(response.status).toBe(200) + }) + + it('should handle invalid JSON payload', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + // Create a request with invalid JSON + const req = new Request('http://localhost:3000/api/folders/folder-1', { + method: 'PUT', + headers: { + 'Content-Type': 'application/json', + }, + body: 'invalid-json', + }) as any + + const params = Promise.resolve({ id: 'folder-1' }) + + const { PUT } = await import('./route') + + const response = await PUT(req, { params }) + + expect(response.status).toBe(500) // Should handle JSON parse error gracefully + }) + }) + describe('Circular Reference Prevention', () => { it('should prevent circular references when updating parent', async () => { mockAuthenticatedUser() @@ -288,4 +341,73 @@ describe('Individual Folder API Route', () => { expect(data).toHaveProperty('error', 'Cannot create circular folder reference') }) }) + + describe('DELETE /api/folders/[id]', () => { + it('should delete folder and all contents successfully', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock({ + folderLookupResult: mockFolder, + }) + + // Mock the recursive deletion function + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('DELETE') + const params = Promise.resolve({ id: 'folder-1' }) + + const { DELETE } = await import('./route') + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(200) + + const data = await response.json() + expect(data).toHaveProperty('success', true) + expect(data).toHaveProperty('deletedItems') + }) + + it('should return 401 for unauthenticated delete requests', async () => { + mockUnauthenticated() + + const dbMock = createFolderDbMock() + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('DELETE') + const params = Promise.resolve({ id: 'folder-1' }) + + const { DELETE } = await import('./route') + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(401) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Unauthorized') + }) + + it('should handle database errors during deletion', async () => { + mockAuthenticatedUser() + + const dbMock = createFolderDbMock({ + throwError: true, + }) + vi.doMock('@/db', () => dbMock) + + const req = createMockRequest('DELETE') + const params = Promise.resolve({ id: 'folder-1' }) + + const { DELETE } = await import('./route') + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(500) + + const data = await response.json() + expect(data).toHaveProperty('error', 'Internal server error') + expect(mockLogger.error).toHaveBeenCalledWith('Error deleting folder:', { + error: expect.any(Error), + }) + }) + }) }) diff --git a/apps/sim/lib/variables/variable-manager.ts b/apps/sim/lib/variables/variable-manager.ts index 3afdbb2b539..e3a21005709 100644 --- a/apps/sim/lib/variables/variable-manager.ts +++ b/apps/sim/lib/variables/variable-manager.ts @@ -6,7 +6,6 @@ import type { VariableType } from '@/stores/panel/variables/types' * to minimize type conversion issues and ensure predictable behavior. */ -// biome-ignore lint: false positive export class VariableManager { /** * Core method to convert any value to its appropriate native JavaScript type diff --git a/biome.json b/biome.json index 8f491388d16..25d183a9b4e 100644 --- a/biome.json +++ b/biome.json @@ -133,7 +133,8 @@ }, "complexity": { "noForEach": "off", - "noUselessFragments": "off" + "noUselessFragments": "off", + "noStaticOnlyClass": "off" }, "performance": { "noAccumulatingSpread": "off", From 6620f3626ae58673896a63503fcbe492fed4c3a6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 17 Jun 2025 10:20:24 -0700 Subject: [PATCH 3/3] ack PR comments --- apps/sim/app/api/__test-utils__/utils.ts | 112 ++++++++++++++++---- apps/sim/app/api/folders/[id]/route.test.ts | 23 ++-- apps/sim/app/api/folders/route.test.ts | 37 +++---- apps/sim/stores/folders/store.ts | 20 +++- 4 files changed, 134 insertions(+), 58 deletions(-) diff --git a/apps/sim/app/api/__test-utils__/utils.ts b/apps/sim/app/api/__test-utils__/utils.ts index 844b875fee0..f6e867ad78b 100644 --- a/apps/sim/app/api/__test-utils__/utils.ts +++ b/apps/sim/app/api/__test-utils__/utils.ts @@ -14,16 +14,56 @@ export interface MockAuthResult { mockUnauthenticated: () => void } +// Database result types +export interface DatabaseSelectResult { + id: string + [key: string]: any +} + +export interface DatabaseInsertResult { + id: string + [key: string]: any +} + +export interface DatabaseUpdateResult { + id: string + updatedAt?: Date + [key: string]: any +} + +export interface DatabaseDeleteResult { + id: string + [key: string]: any +} + export interface MockDatabaseOptions { - selectData?: any[] - insertResult?: any[] - updateResult?: any[] - deleteResult?: any[] + selectData?: DatabaseSelectResult[] + insertResult?: DatabaseInsertResult[] + updateResult?: DatabaseUpdateResult[] + deleteResult?: DatabaseDeleteResult[] throwError?: boolean errorType?: 'connection' | 'constraint' | 'timeout' | 'generic' errorMessage?: string } +export interface CapturedFolderValues { + name?: string + color?: string + parentId?: string | null + isExpanded?: boolean + sortOrder?: number + updatedAt?: Date +} + +export interface CapturedWorkflowValues { + name?: string + description?: string + color?: string + folderId?: string | null + state?: any + updatedAt?: Date +} + export const sampleWorkflowState = { blocks: { 'starter-id': { @@ -627,21 +667,11 @@ export function createMockDatabase(options: MockDatabaseOptions = {}) { delete: vi.fn().mockImplementation(() => ({ where: vi.fn().mockResolvedValue(deleteResult), })), - transaction: vi.fn().mockImplementation(async (callback) => { - return await callback({ - select: vi.fn().mockImplementation(() => createQueryBuilderMock(selectData)), - insert: vi.fn().mockImplementation(() => ({ - values: vi.fn().mockResolvedValue(insertResult), - })), - update: vi.fn().mockImplementation(() => ({ - set: vi.fn().mockImplementation(() => ({ - where: vi.fn().mockResolvedValue(updateResult), - })), - })), - delete: vi.fn().mockImplementation(() => ({ - where: vi.fn().mockResolvedValue(deleteResult), - })), - }) + transaction: createMockTransaction({ + selectData, + insertResult, + updateResult, + deleteResult, }), }, } @@ -749,3 +779,47 @@ export class TestDataFactory { } } } + +/** + * Create a mock transaction function for database testing + * @param mockData - Data to return from transaction operations + * @returns Mock transaction function + */ +export function createMockTransaction( + mockData: { + selectData?: DatabaseSelectResult[] + insertResult?: DatabaseInsertResult[] + updateResult?: DatabaseUpdateResult[] + deleteResult?: DatabaseDeleteResult[] + } = {} +) { + const { selectData = [], insertResult = [], updateResult = [], deleteResult = [] } = mockData + + return vi.fn().mockImplementation(async (callback: any) => { + const tx = { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + orderBy: vi.fn().mockReturnValue({ + limit: vi.fn().mockReturnValue(selectData), + }), + }), + }), + }), + insert: vi.fn().mockReturnValue({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockReturnValue(insertResult), + }), + }), + update: vi.fn().mockReturnValue({ + set: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue(updateResult), + }), + }), + delete: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue(deleteResult), + }), + } + return await callback(tx) + }) +} diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts index b9578c048de..41fc2fb743f 100644 --- a/apps/sim/app/api/folders/[id]/route.test.ts +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -5,6 +5,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + type CapturedFolderValues, createMockRequest, type MockUser, mockAuth, @@ -12,8 +13,14 @@ import { setupCommonApiMocks, } from '@/app/api/__test-utils__/utils' +interface FolderDbMockOptions { + folderLookupResult?: any + updateResult?: any[] + throwError?: boolean + circularCheckResults?: any[] +} + describe('Individual Folder API Route', () => { - // Test data constants const TEST_USER: MockUser = { id: 'user-123', email: 'test@example.com', @@ -34,14 +41,7 @@ describe('Individual Folder API Route', () => { const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth(TEST_USER) - function createFolderDbMock( - options: { - folderLookupResult?: any - updateResult?: any[] - throwError?: boolean - circularCheckResults?: any[] - } = {} - ) { + function createFolderDbMock(options: FolderDbMockOptions = {}) { const { folderLookupResult = mockFolder, updateResult = [{ ...mockFolder, name: 'Updated Folder' }], @@ -206,7 +206,7 @@ describe('Individual Folder API Route', () => { it('should trim folder name when updating', async () => { mockAuthenticatedUser() - let capturedUpdates: any = null + let capturedUpdates: CapturedFolderValues | null = null const dbMock = createFolderDbMock({ updateResult: [{ ...mockFolder, name: 'Folder With Spaces' }], }) @@ -231,7 +231,8 @@ describe('Individual Folder API Route', () => { await PUT(req, { params }) - expect(capturedUpdates.name).toBe('Folder With Spaces') + expect(capturedUpdates).not.toBeNull() + expect(capturedUpdates!.name).toBe('Folder With Spaces') }) it('should handle database errors gracefully', async () => { diff --git a/apps/sim/app/api/folders/route.test.ts b/apps/sim/app/api/folders/route.test.ts index 59083c4aa48..ddfad3f3b3b 100644 --- a/apps/sim/app/api/folders/route.test.ts +++ b/apps/sim/app/api/folders/route.test.ts @@ -5,7 +5,9 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + type CapturedFolderValues, createMockRequest, + createMockTransaction, mockAuth, mockLogger, setupCommonApiMocks, @@ -252,25 +254,12 @@ describe('Folders API Route', () => { it('should create subfolder with parent reference', async () => { mockAuthenticatedUser() - mockTransaction.mockImplementationOnce(async (callback: any) => { - const tx = { - select: vi.fn().mockReturnValue({ - from: vi.fn().mockReturnValue({ - where: vi.fn().mockReturnValue({ - orderBy: vi.fn().mockReturnValue({ - limit: vi.fn().mockReturnValue([]), - }), - }), - }), - }), - insert: vi.fn().mockReturnValue({ - values: vi.fn().mockReturnValue({ - returning: vi.fn().mockReturnValue([{ ...mockFolders[1] }]), - }), - }), - } - return await callback(tx) - }) + mockTransaction.mockImplementationOnce( + createMockTransaction({ + selectData: [], // No existing folders + insertResult: [{ ...mockFolders[1] }], + }) + ) const req = createMockRequest('POST', { name: 'Subfolder', @@ -357,7 +346,7 @@ describe('Folders API Route', () => { it('should trim folder name when creating', async () => { mockAuthenticatedUser() - let capturedValues: any = null + let capturedValues: CapturedFolderValues | null = null mockTransaction.mockImplementationOnce(async (callback: any) => { const tx = { @@ -390,13 +379,14 @@ describe('Folders API Route', () => { const { POST } = await import('./route') await POST(req) - expect(capturedValues.name).toBe('Test Folder With Spaces') + expect(capturedValues).not.toBeNull() + expect(capturedValues!.name).toBe('Test Folder With Spaces') }) it('should use default color when not provided', async () => { mockAuthenticatedUser() - let capturedValues: any = null + let capturedValues: CapturedFolderValues | null = null mockTransaction.mockImplementationOnce(async (callback: any) => { const tx = { @@ -429,7 +419,8 @@ describe('Folders API Route', () => { const { POST } = await import('./route') await POST(req) - expect(capturedValues.color).toBe('#6B7280') + expect(capturedValues).not.toBeNull() + expect(capturedValues!.color).toBe('#6B7280') }) }) }) diff --git a/apps/sim/stores/folders/store.ts b/apps/sim/stores/folders/store.ts index 68220eb57db..2287c8c60ea 100644 --- a/apps/sim/stores/folders/store.ts +++ b/apps/sim/stores/folders/store.ts @@ -5,6 +5,16 @@ import { useWorkflowRegistry } from '../workflows/registry/store' const logger = createLogger('FoldersStore') +export interface Workflow { + id: string + folderId?: string | null + name?: string + description?: string + userId?: string + workspaceId?: string + [key: string]: any // For additional properties +} + export interface WorkflowFolder { id: string name: string @@ -64,7 +74,7 @@ interface FolderState { deleteFolder: (id: string) => Promise // Helper functions - isWorkflowInDeletedSubfolder: (workflow: any, deletedFolderId: string) => boolean + isWorkflowInDeletedSubfolder: (workflow: Workflow, deletedFolderId: string) => boolean removeSubfoldersRecursively: (parentFolderId: string) => void } @@ -318,11 +328,11 @@ export const useFolderStore = create()( try { const workflows = Object.values(workflowRegistry.workflows) const workflowsToDelete = workflows.filter( - (workflow: any) => + (workflow) => workflow.folderId === id || get().isWorkflowInDeletedSubfolder(workflow, id) ) - workflowsToDelete.forEach((workflow: any) => { + workflowsToDelete.forEach((workflow) => { workflowRegistry.removeWorkflow(workflow.id) }) @@ -342,11 +352,11 @@ export const useFolderStore = create()( } }, - isWorkflowInDeletedSubfolder: (workflow: any, deletedFolderId: string) => { + isWorkflowInDeletedSubfolder: (workflow: Workflow, deletedFolderId: string) => { if (!workflow.folderId) return false const folders = get().folders - let currentFolderId = workflow.folderId + let currentFolderId: string | null = workflow.folderId while (currentFolderId && folders[currentFolderId]) { if (currentFolderId === deletedFolderId) {