From 93aabebd513189ac54e137c9b2044920147cfff6 Mon Sep 17 00:00:00 2001 From: Nico <47644445+nicotsx@users.noreply.github.com> Date: Thu, 29 Jan 2026 20:57:20 +0100 Subject: [PATCH] refactor: remove slug constraint in volume name (#428) * refactor: remove slug constraint in volume name Closes #413 * chore: remove un-used imports * fix(notifications): validate name is not empty * refactor: deny empty names --- .../components/create-notification-form.tsx | 3 +- .../volumes/components/create-volume-form.tsx | 3 +- app/server/app.ts | 2 +- .../notifications/notifications.service.ts | 45 +++++++++---------- app/server/modules/volumes/volume.service.ts | 40 +++++------------ app/server/utils/errors.ts | 10 ++--- 6 files changed, 38 insertions(+), 65 deletions(-) diff --git a/app/client/modules/notifications/components/create-notification-form.tsx b/app/client/modules/notifications/components/create-notification-form.tsx index 6e669e4d..bea6e566 100644 --- a/app/client/modules/notifications/components/create-notification-form.tsx +++ b/app/client/modules/notifications/components/create-notification-form.tsx @@ -2,7 +2,7 @@ import { arktypeResolver } from "@hookform/resolvers/arktype"; import { type } from "arktype"; import { useEffect } from "react"; import { useForm } from "react-hook-form"; -import { cn, slugify } from "~/client/lib/utils"; +import { cn } from "~/client/lib/utils"; import { deepClean } from "~/utils/object"; import { Form, @@ -141,7 +141,6 @@ export const CreateNotificationForm = ({ onSubmit, mode = "create", initialValue field.onChange(slugify(e.target.value))} max={32} min={2} /> diff --git a/app/client/modules/volumes/components/create-volume-form.tsx b/app/client/modules/volumes/components/create-volume-form.tsx index 745f9eb3..eb630069 100644 --- a/app/client/modules/volumes/components/create-volume-form.tsx +++ b/app/client/modules/volumes/components/create-volume-form.tsx @@ -4,7 +4,7 @@ import { type } from "arktype"; import { CheckCircle, Loader2, Plug, Save, XCircle } from "lucide-react"; import { useEffect, useState } from "react"; import { useForm } from "react-hook-form"; -import { cn, slugify } from "~/client/lib/utils"; +import { cn } from "~/client/lib/utils"; import { deepClean } from "~/utils/object"; import { Button } from "../../../components/ui/button"; import { @@ -122,7 +122,6 @@ export const CreateVolumeForm = ({ onSubmit, mode = "create", initialValues, for field.onChange(slugify(e.target.value))} maxLength={32} minLength={2} /> diff --git a/app/server/app.ts b/app/server/app.ts index bbdad162..5df7f999 100644 --- a/app/server/app.ts +++ b/app/server/app.ts @@ -87,7 +87,7 @@ export const createApp = () => { const { status, message } = handleServiceError(err); - return c.json({ message }, status); + return c.json({ message }, status as 500); }); return app; diff --git a/app/server/modules/notifications/notifications.service.ts b/app/server/modules/notifications/notifications.service.ts index 342c91bf..f3af6b64 100644 --- a/app/server/modules/notifications/notifications.service.ts +++ b/app/server/modules/notifications/notifications.service.ts @@ -1,6 +1,5 @@ -import { eq, and, ne, inArray } from "drizzle-orm"; -import { ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced"; -import slugify from "slugify"; +import { eq, and, inArray } from "drizzle-orm"; +import { BadRequestError, ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced"; import { db } from "../../db/db"; import { notificationDestinationsTable, @@ -29,7 +28,10 @@ const listDestinations = async () => { const getDestination = async (id: number) => { const organizationId = getOrganizationId(); const destination = await db.query.notificationDestinationsTable.findFirst({ - where: and(eq(notificationDestinationsTable.id, id), eq(notificationDestinationsTable.organizationId, organizationId)), + where: and( + eq(notificationDestinationsTable.id, id), + eq(notificationDestinationsTable.organizationId, organizationId), + ), }); if (!destination) { @@ -139,14 +141,10 @@ async function decryptSensitiveFields(config: NotificationConfig): Promise { const organizationId = getOrganizationId(); - const slug = slugify(name, { lower: true, strict: true }); + const trimmedName = name.trim(); - const existing = await db.query.notificationDestinationsTable.findFirst({ - where: and(eq(notificationDestinationsTable.name, slug), eq(notificationDestinationsTable.organizationId, organizationId)), - }); - - if (existing) { - throw new ConflictError("Notification destination with this name already exists"); + if (trimmedName.length === 0) { + throw new BadRequestError("Name cannot be empty"); } const encryptedConfig = await encryptSensitiveFields(config); @@ -154,7 +152,7 @@ const createDestination = async (name: string, config: NotificationConfig) => { const [created] = await db .insert(notificationDestinationsTable) .values({ - name: slug, + name: trimmedName, type: config.type, config: encryptedConfig, organizationId, @@ -184,16 +182,11 @@ const updateDestination = async ( }; if (updates.name !== undefined) { - const slug = slugify(updates.name, { lower: true, strict: true }); - - const conflict = await db.query.notificationDestinationsTable.findFirst({ - where: and(eq(notificationDestinationsTable.name, slug), ne(notificationDestinationsTable.id, id), eq(notificationDestinationsTable.organizationId, organizationId)), - }); - - if (conflict) { - throw new ConflictError("Notification destination with this name already exists"); + const trimmedName = updates.name.trim(); + if (trimmedName.length === 0) { + throw new BadRequestError("Name cannot be empty"); } - updateData.name = slug; + updateData.name = trimmedName; } if (updates.enabled !== undefined) { @@ -202,7 +195,7 @@ const updateDestination = async ( const newConfig = notificationConfigSchema(updates.config || existing.config); if (newConfig instanceof type.errors) { - throw new InternalServerError("Invalid notification configuration"); + throw new BadRequestError("Invalid notification configuration"); } const encryptedConfig = await encryptSensitiveFields(newConfig); @@ -212,7 +205,9 @@ const updateDestination = async ( const [updated] = await db .update(notificationDestinationsTable) .set(updateData) - .where(eq(notificationDestinationsTable.id, id)) + .where( + and(eq(notificationDestinationsTable.id, id), eq(notificationDestinationsTable.organizationId, organizationId)), + ) .returning(); if (!updated) { @@ -227,7 +222,9 @@ const deleteDestination = async (id: number) => { await getDestination(id); await db .delete(notificationDestinationsTable) - .where(and(eq(notificationDestinationsTable.id, id), eq(notificationDestinationsTable.organizationId, organizationId))); + .where( + and(eq(notificationDestinationsTable.id, id), eq(notificationDestinationsTable.organizationId, organizationId)), + ); }; const testDestination = async (id: number) => { diff --git a/app/server/modules/volumes/volume.service.ts b/app/server/modules/volumes/volume.service.ts index a570f268..f150c9f5 100644 --- a/app/server/modules/volumes/volume.service.ts +++ b/app/server/modules/volumes/volume.service.ts @@ -1,9 +1,8 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; -import { and, eq, ne } from "drizzle-orm"; -import { ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced"; -import slugify from "slugify"; +import { and, eq } from "drizzle-orm"; +import { BadRequestError, InternalServerError, NotFoundError } from "http-errors-enhanced"; import { db } from "../../db/db"; import { volumesTable } from "../../db/schema"; import { cryptoUtils } from "../../utils/crypto"; @@ -65,14 +64,10 @@ const findVolume = async (idOrShortId: string | number) => { const createVolume = async (name: string, backendConfig: BackendConfig) => { const organizationId = getOrganizationId(); - const slug = slugify(name, { lower: true, strict: true }); + const trimmedName = name.trim(); - const existing = await db.query.volumesTable.findFirst({ - where: and(eq(volumesTable.name, slug), eq(volumesTable.organizationId, organizationId)), - }); - - if (existing) { - throw new ConflictError("Volume already exists"); + if (trimmedName.length === 0) { + throw new BadRequestError("Volume name cannot be empty"); } const shortId = generateShortId(); @@ -82,7 +77,7 @@ const createVolume = async (name: string, backendConfig: BackendConfig) => { .insert(volumesTable) .values({ shortId, - name: slug, + name: trimmedName, config: encryptedConfig, type: backendConfig.backend, organizationId, @@ -191,23 +186,10 @@ const updateVolume = async (idOrShortId: string | number, volumeData: UpdateVolu throw new NotFoundError("Volume not found"); } - let newName = existing.name; - if (volumeData.name !== undefined && volumeData.name !== existing.name) { - const newSlug = slugify(volumeData.name, { lower: true, strict: true }); + const newName = volumeData.name !== undefined ? volumeData.name.trim() : existing.name; - const conflict = await db.query.volumesTable.findFirst({ - where: and( - eq(volumesTable.name, newSlug), - ne(volumesTable.id, existing.id), - eq(volumesTable.organizationId, organizationId), - ), - }); - - if (conflict) { - throw new ConflictError("A volume with this name already exists"); - } - - newName = newSlug; + if (newName.length === 0) { + throw new BadRequestError("Volume name cannot be empty"); } const configChanged = @@ -221,7 +203,7 @@ const updateVolume = async (idOrShortId: string | number, volumeData: UpdateVolu const newConfig = volumeConfigSchema(volumeData.config || existing.config); if (newConfig instanceof type.errors) { - throw new InternalServerError("Invalid volume configuration"); + throw new BadRequestError("Invalid volume configuration"); } const encryptedConfig = await encryptSensitiveFields(newConfig); @@ -332,7 +314,7 @@ const listFiles = async (idOrShortId: string | number, subPath?: string) => { const relative = path.relative(volumePath, normalizedPath); if (relative.startsWith("..") || path.isAbsolute(relative)) { - throw new InternalServerError("Invalid path"); + throw new BadRequestError("Invalid path"); } try { diff --git a/app/server/utils/errors.ts b/app/server/utils/errors.ts index 22379a36..3024d570 100644 --- a/app/server/utils/errors.ts +++ b/app/server/utils/errors.ts @@ -1,13 +1,9 @@ -import { ConflictError, NotFoundError } from "http-errors-enhanced"; +import { HttpError } from "http-errors-enhanced"; import { sanitizeSensitiveData } from "./sanitize"; export const handleServiceError = (error: unknown) => { - if (error instanceof ConflictError) { - return { message: sanitizeSensitiveData(error.message), status: 409 as const }; - } - - if (error instanceof NotFoundError) { - return { message: sanitizeSensitiveData(error.message), status: 404 as const }; + if (error instanceof HttpError) { + return { message: sanitizeSensitiveData(error.message), status: error.statusCode }; } return { message: sanitizeSensitiveData(toMessage(error)), status: 500 as const };