From 3ad79bc7eadc4b509e04dde01b64d64f8396e495 Mon Sep 17 00:00:00 2001 From: Piyush Gupta Date: Fri, 17 Oct 2025 22:02:09 +0530 Subject: [PATCH 1/3] fix: returns error on duplicate folder creation --- .../secret-folder/secret-folder-service.ts | 19 +------ .../src/hooks/api/secretFolders/index.tsx | 1 + .../src/hooks/api/secretFolders/queries.tsx | 56 +++++++++++++++++++ .../OverviewPage/OverviewPage.tsx | 40 +++++++++---- .../CreateSecretForm/CreateSecretForm.tsx | 11 +++- 5 files changed, 97 insertions(+), 30 deletions(-) diff --git a/backend/src/services/secret-folder/secret-folder-service.ts b/backend/src/services/secret-folder/secret-folder-service.ts index 7bacd9468..c216ea3a8 100644 --- a/backend/src/services/secret-folder/secret-folder-service.ts +++ b/backend/src/services/secret-folder/secret-folder-service.ts @@ -118,24 +118,11 @@ export const secretFolderServiceFactory = ({ }); } - // check if the exact folder already exists - const existingFolder = await folderDAL.findOne( - { - envId: env.id, - parentId: parentFolder.id, - name, - isReserved: false - }, - tx - ); - - if (existingFolder) { - return existingFolder; - } - // exact folder case if (parentFolder.path === pathWithFolder) { - return parentFolder; + throw new BadRequestError({ + message: `Folder with name '${name}' already exists in path '${secretPath}'` + }); } let currentParentId = parentFolder.id; diff --git a/frontend/src/hooks/api/secretFolders/index.tsx b/frontend/src/hooks/api/secretFolders/index.tsx index 0676f5d14..9d2e44621 100644 --- a/frontend/src/hooks/api/secretFolders/index.tsx +++ b/frontend/src/hooks/api/secretFolders/index.tsx @@ -2,6 +2,7 @@ export { useCreateFolder, useDeleteFolder, useGetFoldersByEnv, + useGetOrCreateFolder, useGetProjectFolders, useUpdateFolder } from "./queries"; diff --git a/frontend/src/hooks/api/secretFolders/queries.tsx b/frontend/src/hooks/api/secretFolders/queries.tsx index 694a33d13..56f7825c0 100644 --- a/frontend/src/hooks/api/secretFolders/queries.tsx +++ b/frontend/src/hooks/api/secretFolders/queries.tsx @@ -140,6 +140,59 @@ export const useGetFoldersByEnv = ({ return { folders, folderNames, isFolderPresentInEnv, getFolderByNameAndEnv }; }; +export const useGetOrCreateFolder = () => { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: async (dto) => { + const { data: existingFolder } = await apiRequest.get<{ folders: TSecretFolder[] }>( + "/api/v2/folders", + { + params: { + projectId: dto.projectId, + environment: dto.environment, + path: dto.path || "/" + } + } + ); + + const folder = existingFolder.folders.find((f) => f.name === dto.name); + + if (folder) return folder; + + const { data } = await apiRequest.post("/api/v2/folders", { + ...dto, + projectId: dto.projectId + }); + + return data; + }, + onSuccess: (_, { projectId, environment, path }) => { + queryClient.invalidateQueries({ + queryKey: dashboardKeys.getDashboardSecrets({ + projectId, + secretPath: path ?? "/" + }) + }); + queryClient.invalidateQueries({ + queryKey: folderQueryKeys.getSecretFolders({ projectId, environment, path }) + }); + queryClient.invalidateQueries({ + queryKey: secretSnapshotKeys.list({ projectId, environment, directory: path }) + }); + queryClient.invalidateQueries({ + queryKey: secretSnapshotKeys.count({ projectId, environment, directory: path }) + }); + queryClient.invalidateQueries({ + queryKey: commitKeys.count({ projectId, environment, directory: path }) + }); + queryClient.invalidateQueries({ + queryKey: commitKeys.history({ projectId, environment, directory: path }) + }); + } + }); +}; + export const useCreateFolder = () => { const queryClient = useQueryClient(); @@ -170,6 +223,9 @@ export const useCreateFolder = () => { queryClient.invalidateQueries({ queryKey: commitKeys.count({ projectId, environment, directory: path }) }); + queryClient.invalidateQueries({ + queryKey: commitKeys.history({ projectId, environment, directory: path }) + }); } }); }; diff --git a/frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx b/frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx index 022b33f88..5ec480298 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/OverviewPage.tsx @@ -86,6 +86,7 @@ import { useCreateSecretV3, useDeleteSecretV3, useGetImportedSecretsAllEnvs, + useGetOrCreateFolder, useGetWsTags, useUpdateSecretV3 } from "@app/hooks/api"; @@ -367,6 +368,7 @@ export const OverviewPage = () => { const { mutateAsync: updateSecretV3 } = useUpdateSecretV3(); const { mutateAsync: deleteSecretV3 } = useDeleteSecretV3(); const { mutateAsync: createFolder } = useCreateFolder(); + const { mutateAsync: getOrCreateFolder } = useGetOrCreateFolder(); const { mutateAsync: updateFolderBatch } = useUpdateFolderBatch(); const { handlePopUpOpen, handlePopUpToggle, handlePopUpClose, popUp } = usePopUp([ @@ -384,16 +386,32 @@ export const OverviewPage = () => { ] as const); const handleFolderCreate = async (folderName: string, description: string | null) => { - const promises = userAvailableEnvs.map((env) => { - const environment = env.slug; - return createFolder({ - name: folderName, - path: secretPath, - environment, - projectId, - description + const promises = userAvailableEnvs + .map((env) => { + const environment = env.slug; + const isFolderPresent = isFolderPresentInEnv(folderName, environment); + if (isFolderPresent) { + return undefined; + } + + return createFolder({ + name: folderName, + path: secretPath, + environment, + projectId, + description + }); + }) + .filter((promise) => promise !== undefined); + + if (promises.length === 0) { + handlePopUpClose("addFolder"); + createNotification({ + type: "info", + text: "Folder already exists in all environments" }); - }); + return; + } const results = await Promise.allSettled(promises); const isFoldersAdded = results.some((result) => result.status === "fulfilled"); @@ -481,7 +499,7 @@ export const OverviewPage = () => { }) ); if (folderName && parentPath && canCreateFolder) { - await createFolder({ + await getOrCreateFolder({ projectId, path: parentPath, environment: env, @@ -645,7 +663,7 @@ export const OverviewPage = () => { }) ); if (folderName && parentPath && canCreateFolder) { - await createFolder({ + await getOrCreateFolder({ projectId, environment: slug, path: parentPath, diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx index 8eb6c5035..fede4e44d 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/CreateSecretForm/CreateSecretForm.tsx @@ -25,7 +25,12 @@ import { } from "@app/context"; import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { getKeyValue } from "@app/helpers/parseEnvVar"; -import { useCreateFolder, useCreateSecretV3, useCreateWsTag, useGetWsTags } from "@app/hooks/api"; +import { + useCreateSecretV3, + useCreateWsTag, + useGetOrCreateFolder, + useGetWsTags +} from "@app/hooks/api"; import { SecretType } from "@app/hooks/api/types"; const typeSchema = z @@ -64,7 +69,7 @@ export const CreateSecretForm = ({ secretPath = "/", onClose }: Props) => { const environments = currentProject?.environments || []; const { mutateAsync: createSecretV3 } = useCreateSecretV3(); - const { mutateAsync: createFolder } = useCreateFolder(); + const { mutateAsync: getOrCreateFolder } = useGetOrCreateFolder(); const { data: projectTags, isPending: isTagsLoading } = useGetWsTags( canReadTags ? projectId : "" ); @@ -92,7 +97,7 @@ export const CreateSecretForm = ({ secretPath = "/", onClose }: Props) => { ); if (folderName && parentPath && canCreateFolder) { - await createFolder({ + await getOrCreateFolder({ projectId, path: parentPath, environment, From ba98419303dea7541bcd0e9f28cf9282a98a1bd1 Mon Sep 17 00:00:00 2001 From: Piyush Gupta Date: Sat, 18 Oct 2025 00:15:40 +0530 Subject: [PATCH 2/3] fix: e2e tests --- .../e2e-test/routes/v2/secret-folder.spec.ts | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/backend/e2e-test/routes/v2/secret-folder.spec.ts b/backend/e2e-test/routes/v2/secret-folder.spec.ts index 9cdad32fe..babc08385 100644 --- a/backend/e2e-test/routes/v2/secret-folder.spec.ts +++ b/backend/e2e-test/routes/v2/secret-folder.spec.ts @@ -41,7 +41,7 @@ describe("Secret Folder Router", async () => { { name: "folder1", path: "/" }, // one in root { name: "folder1", path: "/level1/level2" }, // then create a deep one creating intermediate ones { name: "folder2", path: "/" }, - { name: "folder1", path: "/level1/level2" } // this should not create folder return same thing + { name: "folder3", path: "/level1/level2" } ])("Create folder $name in $path", async ({ name, path }) => { const createdFolder = await createFolder({ path, name }); // check for default environments @@ -58,7 +58,7 @@ describe("Secret Folder Router", async () => { { path: "/", expected: { - folders: [{ name: "folder1" }, { name: "level1" }, { name: "folder2" }], + folders: [{ name: "folder4" }, { name: "level2" }, { name: "folder5" }], length: 3 } }, @@ -163,4 +163,25 @@ describe("Secret Folder Router", async () => { expect(updatedFolderList).toHaveProperty("folders"); expect(updatedFolderList.folders.length).toEqual(0); }); + + test("Creating a duplicate folder should return a 400 error", async () => { + await createFolder({ name: "folder-duplicate", path: "/level1/level2" }); + + const res = await testServer.inject({ + method: "POST", + url: `/api/v2/folders`, + headers: { + authorization: `Bearer ${jwtAuthToken}` + }, + body: { + projectId: seedData1.project.id, + environment: seedData1.environment.slug, + name: "folder-duplicate", + path: "/level1/level2" + } + }); + expect(res.statusCode).toBe(400); + const payload = JSON.parse(res.payload); + expect(payload).toHaveProperty("error"); + }); }); From c0d6d8c41d3c2a02de206887221ce68357b7cf84 Mon Sep 17 00:00:00 2001 From: Piyush Gupta Date: Sat, 18 Oct 2025 00:45:49 +0530 Subject: [PATCH 3/3] fix: update e2e tests for folder creation and handle duplicate folder error --- .../e2e-test/routes/v1/secret-folder.spec.ts | 25 +++++++++++++++++-- .../e2e-test/routes/v2/secret-folder.spec.ts | 3 ++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/backend/e2e-test/routes/v1/secret-folder.spec.ts b/backend/e2e-test/routes/v1/secret-folder.spec.ts index 4d4bd7ab4..e954179a7 100644 --- a/backend/e2e-test/routes/v1/secret-folder.spec.ts +++ b/backend/e2e-test/routes/v1/secret-folder.spec.ts @@ -40,7 +40,7 @@ describe("Secret Folder Router", async () => { { name: "folder1", path: "/" }, // one in root { name: "folder1", path: "/level1/level2" }, // then create a deep one creating intermediate ones { name: "folder2", path: "/" }, - { name: "folder1", path: "/level1/level2" } // this should not create folder return same thing + { name: "folder3", path: "/level1/level2" } ])("Create folder $name in $path", async ({ name, path }) => { const createdFolder = await createFolder({ path, name }); // check for default environments @@ -57,7 +57,7 @@ describe("Secret Folder Router", async () => { { path: "/", expected: { - folders: [{ name: "folder1" }, { name: "level1" }, { name: "folder2" }], + folders: [{ name: "folder4" }, { name: "level2" }, { name: "folder5" }], length: 3 } }, @@ -162,4 +162,25 @@ describe("Secret Folder Router", async () => { expect(updatedFolderList).toHaveProperty("folders"); expect(updatedFolderList.folders.length).toEqual(0); }); + test("Creating a duplicate folder should return a 400 error", async () => { + const newFolder = await createFolder({ name: "folder-duplicate", path: "/level1/level2" }); + + const res = await testServer.inject({ + method: "POST", + url: `/api/v1/folders`, + headers: { + authorization: `Bearer ${jwtAuthToken}` + }, + body: { + workspaceId: seedData1.project.id, + environment: seedData1.environment.slug, + name: "folder-duplicate", + path: "/level1/level2" + } + }); + expect(res.statusCode).toBe(400); + const payload = JSON.parse(res.payload); + expect(payload).toHaveProperty("error"); + await deleteFolder({ path: "/level1/level2", id: newFolder.id }); + }); }); diff --git a/backend/e2e-test/routes/v2/secret-folder.spec.ts b/backend/e2e-test/routes/v2/secret-folder.spec.ts index babc08385..92bcc92e5 100644 --- a/backend/e2e-test/routes/v2/secret-folder.spec.ts +++ b/backend/e2e-test/routes/v2/secret-folder.spec.ts @@ -165,7 +165,7 @@ describe("Secret Folder Router", async () => { }); test("Creating a duplicate folder should return a 400 error", async () => { - await createFolder({ name: "folder-duplicate", path: "/level1/level2" }); + const newFolder = await createFolder({ name: "folder-duplicate", path: "/level1/level2" }); const res = await testServer.inject({ method: "POST", @@ -183,5 +183,6 @@ describe("Secret Folder Router", async () => { expect(res.statusCode).toBe(400); const payload = JSON.parse(res.payload); expect(payload).toHaveProperty("error"); + await deleteFolder({ path: "/level1/level2", id: newFolder.id }); }); });