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
This commit is contained in:
Nico 2026-01-29 20:57:20 +01:00 committed by GitHub
parent 861f81f14e
commit 93aabebd51
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 38 additions and 65 deletions

View file

@ -2,7 +2,7 @@ import { arktypeResolver } from "@hookform/resolvers/arktype";
import { type } from "arktype"; import { type } from "arktype";
import { useEffect } from "react"; import { useEffect } from "react";
import { useForm } from "react-hook-form"; 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 { deepClean } from "~/utils/object";
import { import {
Form, Form,
@ -141,7 +141,6 @@ export const CreateNotificationForm = ({ onSubmit, mode = "create", initialValue
<Input <Input
{...field} {...field}
placeholder="My notification" placeholder="My notification"
onChange={(e) => field.onChange(slugify(e.target.value))}
max={32} max={32}
min={2} min={2}
/> />

View file

@ -4,7 +4,7 @@ import { type } from "arktype";
import { CheckCircle, Loader2, Plug, Save, XCircle } from "lucide-react"; import { CheckCircle, Loader2, Plug, Save, XCircle } from "lucide-react";
import { useEffect, useState } from "react"; import { useEffect, useState } from "react";
import { useForm } from "react-hook-form"; 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 { deepClean } from "~/utils/object";
import { Button } from "../../../components/ui/button"; import { Button } from "../../../components/ui/button";
import { import {
@ -122,7 +122,6 @@ export const CreateVolumeForm = ({ onSubmit, mode = "create", initialValues, for
<Input <Input
{...field} {...field}
placeholder="Volume name" placeholder="Volume name"
onChange={(e) => field.onChange(slugify(e.target.value))}
maxLength={32} maxLength={32}
minLength={2} minLength={2}
/> />

View file

@ -87,7 +87,7 @@ export const createApp = () => {
const { status, message } = handleServiceError(err); const { status, message } = handleServiceError(err);
return c.json({ message }, status); return c.json({ message }, status as 500);
}); });
return app; return app;

View file

@ -1,6 +1,5 @@
import { eq, and, ne, inArray } from "drizzle-orm"; import { eq, and, inArray } from "drizzle-orm";
import { ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced"; import { BadRequestError, ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced";
import slugify from "slugify";
import { db } from "../../db/db"; import { db } from "../../db/db";
import { import {
notificationDestinationsTable, notificationDestinationsTable,
@ -29,7 +28,10 @@ const listDestinations = async () => {
const getDestination = async (id: number) => { const getDestination = async (id: number) => {
const organizationId = getOrganizationId(); const organizationId = getOrganizationId();
const destination = await db.query.notificationDestinationsTable.findFirst({ 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) { if (!destination) {
@ -139,14 +141,10 @@ async function decryptSensitiveFields(config: NotificationConfig): Promise<Notif
const createDestination = async (name: string, config: NotificationConfig) => { const createDestination = async (name: string, config: NotificationConfig) => {
const organizationId = getOrganizationId(); const organizationId = getOrganizationId();
const slug = slugify(name, { lower: true, strict: true }); const trimmedName = name.trim();
const existing = await db.query.notificationDestinationsTable.findFirst({ if (trimmedName.length === 0) {
where: and(eq(notificationDestinationsTable.name, slug), eq(notificationDestinationsTable.organizationId, organizationId)), throw new BadRequestError("Name cannot be empty");
});
if (existing) {
throw new ConflictError("Notification destination with this name already exists");
} }
const encryptedConfig = await encryptSensitiveFields(config); const encryptedConfig = await encryptSensitiveFields(config);
@ -154,7 +152,7 @@ const createDestination = async (name: string, config: NotificationConfig) => {
const [created] = await db const [created] = await db
.insert(notificationDestinationsTable) .insert(notificationDestinationsTable)
.values({ .values({
name: slug, name: trimmedName,
type: config.type, type: config.type,
config: encryptedConfig, config: encryptedConfig,
organizationId, organizationId,
@ -184,16 +182,11 @@ const updateDestination = async (
}; };
if (updates.name !== undefined) { if (updates.name !== undefined) {
const slug = slugify(updates.name, { lower: true, strict: true }); const trimmedName = updates.name.trim();
if (trimmedName.length === 0) {
const conflict = await db.query.notificationDestinationsTable.findFirst({ throw new BadRequestError("Name cannot be empty");
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");
} }
updateData.name = slug; updateData.name = trimmedName;
} }
if (updates.enabled !== undefined) { if (updates.enabled !== undefined) {
@ -202,7 +195,7 @@ const updateDestination = async (
const newConfig = notificationConfigSchema(updates.config || existing.config); const newConfig = notificationConfigSchema(updates.config || existing.config);
if (newConfig instanceof type.errors) { if (newConfig instanceof type.errors) {
throw new InternalServerError("Invalid notification configuration"); throw new BadRequestError("Invalid notification configuration");
} }
const encryptedConfig = await encryptSensitiveFields(newConfig); const encryptedConfig = await encryptSensitiveFields(newConfig);
@ -212,7 +205,9 @@ const updateDestination = async (
const [updated] = await db const [updated] = await db
.update(notificationDestinationsTable) .update(notificationDestinationsTable)
.set(updateData) .set(updateData)
.where(eq(notificationDestinationsTable.id, id)) .where(
and(eq(notificationDestinationsTable.id, id), eq(notificationDestinationsTable.organizationId, organizationId)),
)
.returning(); .returning();
if (!updated) { if (!updated) {
@ -227,7 +222,9 @@ const deleteDestination = async (id: number) => {
await getDestination(id); await getDestination(id);
await db await db
.delete(notificationDestinationsTable) .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) => { const testDestination = async (id: number) => {

View file

@ -1,9 +1,8 @@
import * as fs from "node:fs/promises"; import * as fs from "node:fs/promises";
import * as os from "node:os"; import * as os from "node:os";
import * as path from "node:path"; import * as path from "node:path";
import { and, eq, ne } from "drizzle-orm"; import { and, eq } from "drizzle-orm";
import { ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced"; import { BadRequestError, InternalServerError, NotFoundError } from "http-errors-enhanced";
import slugify from "slugify";
import { db } from "../../db/db"; import { db } from "../../db/db";
import { volumesTable } from "../../db/schema"; import { volumesTable } from "../../db/schema";
import { cryptoUtils } from "../../utils/crypto"; import { cryptoUtils } from "../../utils/crypto";
@ -65,14 +64,10 @@ const findVolume = async (idOrShortId: string | number) => {
const createVolume = async (name: string, backendConfig: BackendConfig) => { const createVolume = async (name: string, backendConfig: BackendConfig) => {
const organizationId = getOrganizationId(); const organizationId = getOrganizationId();
const slug = slugify(name, { lower: true, strict: true }); const trimmedName = name.trim();
const existing = await db.query.volumesTable.findFirst({ if (trimmedName.length === 0) {
where: and(eq(volumesTable.name, slug), eq(volumesTable.organizationId, organizationId)), throw new BadRequestError("Volume name cannot be empty");
});
if (existing) {
throw new ConflictError("Volume already exists");
} }
const shortId = generateShortId(); const shortId = generateShortId();
@ -82,7 +77,7 @@ const createVolume = async (name: string, backendConfig: BackendConfig) => {
.insert(volumesTable) .insert(volumesTable)
.values({ .values({
shortId, shortId,
name: slug, name: trimmedName,
config: encryptedConfig, config: encryptedConfig,
type: backendConfig.backend, type: backendConfig.backend,
organizationId, organizationId,
@ -191,23 +186,10 @@ const updateVolume = async (idOrShortId: string | number, volumeData: UpdateVolu
throw new NotFoundError("Volume not found"); throw new NotFoundError("Volume not found");
} }
let newName = existing.name; const newName = volumeData.name !== undefined ? volumeData.name.trim() : existing.name;
if (volumeData.name !== undefined && volumeData.name !== existing.name) {
const newSlug = slugify(volumeData.name, { lower: true, strict: true });
const conflict = await db.query.volumesTable.findFirst({ if (newName.length === 0) {
where: and( throw new BadRequestError("Volume name cannot be empty");
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;
} }
const configChanged = const configChanged =
@ -221,7 +203,7 @@ const updateVolume = async (idOrShortId: string | number, volumeData: UpdateVolu
const newConfig = volumeConfigSchema(volumeData.config || existing.config); const newConfig = volumeConfigSchema(volumeData.config || existing.config);
if (newConfig instanceof type.errors) { if (newConfig instanceof type.errors) {
throw new InternalServerError("Invalid volume configuration"); throw new BadRequestError("Invalid volume configuration");
} }
const encryptedConfig = await encryptSensitiveFields(newConfig); const encryptedConfig = await encryptSensitiveFields(newConfig);
@ -332,7 +314,7 @@ const listFiles = async (idOrShortId: string | number, subPath?: string) => {
const relative = path.relative(volumePath, normalizedPath); const relative = path.relative(volumePath, normalizedPath);
if (relative.startsWith("..") || path.isAbsolute(relative)) { if (relative.startsWith("..") || path.isAbsolute(relative)) {
throw new InternalServerError("Invalid path"); throw new BadRequestError("Invalid path");
} }
try { try {

View file

@ -1,13 +1,9 @@
import { ConflictError, NotFoundError } from "http-errors-enhanced"; import { HttpError } from "http-errors-enhanced";
import { sanitizeSensitiveData } from "./sanitize"; import { sanitizeSensitiveData } from "./sanitize";
export const handleServiceError = (error: unknown) => { export const handleServiceError = (error: unknown) => {
if (error instanceof ConflictError) { if (error instanceof HttpError) {
return { message: sanitizeSensitiveData(error.message), status: 409 as const }; return { message: sanitizeSensitiveData(error.message), status: error.statusCode };
}
if (error instanceof NotFoundError) {
return { message: sanitizeSensitiveData(error.message), status: 404 as const };
} }
return { message: sanitizeSensitiveData(toMessage(error)), status: 500 as const }; return { message: sanitizeSensitiveData(toMessage(error)), status: 500 as const };