From f1ce2cd2c6b845cfa041b28480baf73c2d84dede Mon Sep 17 00:00:00 2001 From: Ivan Vasilov Date: Thu, 7 Aug 2025 11:55:35 +0200 Subject: [PATCH] Handle a case when moving a snippet to a location where a snippet with that name already exists. --- apps/studio/lib/api/snippets.utils.test.ts | 41 +++++++++++++++++++++- apps/studio/lib/api/snippets.utils.ts | 15 ++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/apps/studio/lib/api/snippets.utils.test.ts b/apps/studio/lib/api/snippets.utils.test.ts index fcadeeb3e55..267dab35e5a 100644 --- a/apps/studio/lib/api/snippets.utils.test.ts +++ b/apps/studio/lib/api/snippets.utils.test.ts @@ -1,6 +1,7 @@ import fs from 'fs/promises' import path from 'path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { generateDeterministicUuid } from './snippets.browser' import { createFolder, deleteFolder, @@ -13,7 +14,6 @@ import { updateSnippet, type Snippet, } from './snippets.utils' -import { generateDeterministicUuid } from './snippets.browser' // Mock fs/promises vi.mock('fs/promises') @@ -904,6 +904,45 @@ describe('snippets.utils', () => { expect(result.name).toBe('new-name') expect(result.content.sql).toBe('SELECT * FROM old;') }) + + it('should throw error when moving snippet to folder that already contains snippet with same name', async () => { + const snippetName = 'existing-snippet' + const existingSnippetId = generateDeterministicUuid([snippetName]) + const targetFolderId = generateDeterministicUuid(['target-folder']) + const conflictingSnippetId = generateDeterministicUuid([targetFolderId, snippetName]) + + const createMockDirent = (name: string, isDirectory: boolean) => ({ + name, + isDirectory: () => isDirectory, + isFile: () => !isDirectory, + }) + + mockedFS.access.mockResolvedValue(undefined) + + // Mock filesystem structure with: + // - existing-snippet.sql in root + // - target-folder/ with existing-snippet.sql inside + mockedFS.readdir.mockImplementation((dirPath: any) => { + if (dirPath === MOCK_SNIPPETS_DIR) { + return Promise.resolve([ + createMockDirent('existing-snippet.sql', false), + createMockDirent('target-folder', true), + ] as any) + } else if (dirPath === path.join(MOCK_SNIPPETS_DIR, 'target-folder')) { + return Promise.resolve([createMockDirent(`${snippetName}.sql`, false)] as any) + } + return Promise.resolve([]) + }) + + mockedFS.readFile.mockResolvedValue('SELECT * FROM table;') + mockedFS.unlink.mockResolvedValue(undefined) + + const updates = { folder_id: targetFolderId } + + await expect(updateSnippet(existingSnippetId, updates)).rejects.toThrow( + `Snippet named "${snippetName}" already exists in the specified folder` + ) + }) }) describe('getFolders', () => { diff --git a/apps/studio/lib/api/snippets.utils.ts b/apps/studio/lib/api/snippets.utils.ts index 5b81403b847..c9767c94eb5 100644 --- a/apps/studio/lib/api/snippets.utils.ts +++ b/apps/studio/lib/api/snippets.utils.ts @@ -262,6 +262,21 @@ export async function updateSnippet(id: string, updates: DeepPartial): throw new Error(`Snippet with id ${id} not found`) } + const newId = generateDeterministicUuid([ + updates.folder_id !== undefined ? updates.folder_id : foundSnippet.folderId, + updates.name ?? foundSnippet.name, + ]) + + const snippetAtTargetLocation = entries.find( + (entry) => entry.id === newId && entry.type === 'file' + ) + + if (snippetAtTargetLocation && snippetAtTargetLocation.id !== foundSnippet.id) { + throw new Error( + `Snippet named "${updates.name ?? foundSnippet.name}" already exists in the specified folder` + ) + } + const snippet = buildSnippet(foundSnippet.name, foundSnippet.content || '', foundSnippet.folderId) // it's easier to delete the old file first and then recreate a new one