From 464bd701ba27868f79a3ab752825daebe26d1902 Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Fri, 24 Apr 2026 02:34:38 +0100 Subject: [PATCH] feat(base): enforce unique property names per base --- .../components/formula/formula-editor.tsx | 4 ++- .../property/create-property-popover.tsx | 18 ++++++++++--- .../base/services/base-property.service.ts | 27 +++++++++++++++++++ ...0424T120000-base-properties-name-unique.ts | 20 ++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/database/migrations/20260424T120000-base-properties-name-unique.ts diff --git a/apps/client/src/features/base/components/formula/formula-editor.tsx b/apps/client/src/features/base/components/formula/formula-editor.tsx index 07d9544c5..002b40397 100644 --- a/apps/client/src/features/base/components/formula/formula-editor.tsx +++ b/apps/client/src/features/base/components/formula/formula-editor.tsx @@ -25,6 +25,7 @@ type Props = { editingPropertyId: string | null; initialSource?: string; name?: string; + disabled?: boolean; onSave: ( source: string, ast: unknown, @@ -39,6 +40,7 @@ export function FormulaEditor({ editingPropertyId, initialSource = "", name, + disabled = false, onSave, onCancel, }: Props) { @@ -49,7 +51,7 @@ export function FormulaEditor({ editingPropertyId, registry, ); - const canSave = parseState.state === "ok"; + const canSave = parseState.state === "ok" && !disabled; const insertAtEnd = (snippet: string) => setSource((s) => `${s}${s ? " " : ""}${snippet}`); diff --git a/apps/client/src/features/base/components/property/create-property-popover.tsx b/apps/client/src/features/base/components/property/create-property-popover.tsx index 76ecf54ff..8acba9bd1 100644 --- a/apps/client/src/features/base/components/property/create-property-popover.tsx +++ b/apps/client/src/features/base/components/property/create-property-popover.tsx @@ -66,6 +66,14 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }: return name.trim().length > 0 || Object.keys(typeOptions).length > 0; }, [name, typeOptions]); + const nameTaken = useMemo(() => { + const trimmed = name.trim().toLowerCase(); + if (!trimmed) return false; + return (properties ?? []).some( + (p) => p.name.trim().toLowerCase() === trimmed, + ); + }, [name, properties]); + const resetState = useCallback(() => { setPanel("typePicker"); setSelectedType(null); @@ -112,7 +120,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }: }, [panel]); const handleCreate = useCallback(() => { - if (!selectedType) return; + if (!selectedType || nameTaken) return; const finalName = name.trim() || selectedTypeLabel; createPropertyMutation.mutate( { @@ -130,7 +138,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }: }, ); handleClose(); - }, [selectedType, name, selectedTypeLabel, typeOptions, baseId, createPropertyMutation, handleClose, onPropertyCreated]); + }, [selectedType, nameTaken, name, selectedTypeLabel, typeOptions, baseId, createPropertyMutation, handleClose, onPropertyCreated]); const handleBackToTypePicker = useCallback(() => { setPanel("typePicker"); @@ -242,13 +250,16 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }: placeholder={selectedTypeLabel} value={name} onChange={(e) => setName(e.currentTarget.value)} + error={nameTaken ? t("A property with this name already exists") : undefined} /> { + if (nameTaken) return; createPropertyMutation.mutate( { baseId, @@ -279,6 +290,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }: value={name} onChange={(e) => setName(e.currentTarget.value)} onKeyDown={handleNameKeyDown} + error={nameTaken ? t("A property with this name already exists") : undefined} mb="xs" /> {t("Cancel")} - diff --git a/apps/server/src/core/base/services/base-property.service.ts b/apps/server/src/core/base/services/base-property.service.ts index 461caae56..3a4342563 100644 --- a/apps/server/src/core/base/services/base-property.service.ts +++ b/apps/server/src/core/base/services/base-property.service.ts @@ -88,6 +88,8 @@ export class BasePropertyService { async create(workspaceId: string, dto: CreatePropertyDto, actorId?: string) { const type = dto.type as BasePropertyTypeValue; + await this.ensureNameUnique(dto.baseId, dto.name); + let validatedTypeOptions: unknown; if (type === 'formula') { const sourceCandidate = (dto.typeOptions as any)?.source; @@ -192,6 +194,10 @@ export class BasePropertyService { throw new BadRequestException('Property does not belong to this base'); } + if (dto.name !== undefined) { + await this.ensureNameUnique(dto.baseId, dto.name, dto.propertyId); + } + // Block concurrent type changes — the worker still owns the previous // conversion, and letting a second one through would race on `type`. if (property.pendingType) { @@ -418,6 +424,27 @@ export class BasePropertyService { * has to happen after the outer transaction commits so socket consumers * never race ahead of visibility. */ + private async ensureNameUnique( + baseId: string, + candidate: string, + excludePropertyId?: string, + ): Promise { + const trimmed = candidate.trim(); + if (!trimmed) return; + const existing = await this.basePropertyRepo.findByBaseId(baseId); + const lower = trimmed.toLowerCase(); + const clash = existing.find( + (p) => + p.id !== excludePropertyId && + p.name.trim().toLowerCase() === lower, + ); + if (clash) { + throw new BadRequestException( + `A property named "${trimmed}" already exists in this base`, + ); + } + } + private async loadAndEmit( dto: UpdatePropertyDto, workspaceId: string, diff --git a/apps/server/src/database/migrations/20260424T120000-base-properties-name-unique.ts b/apps/server/src/database/migrations/20260424T120000-base-properties-name-unique.ts new file mode 100644 index 000000000..6fd87b9e2 --- /dev/null +++ b/apps/server/src/database/migrations/20260424T120000-base-properties-name-unique.ts @@ -0,0 +1,20 @@ +import { type Kysely, sql } from "kysely"; + +/* + * Enforce one property name per base (case-insensitive, excluding soft-deleted). + * Formulas reference properties by name via `prop("Name")`, and the resolver + * builds a `Map` — duplicates would silently clobber and make + * references non-deterministic. Belt-and-suspenders against races that slip + * past service-layer validation. + */ +export async function up(db: Kysely): Promise { + await sql` + CREATE UNIQUE INDEX base_properties_name_unique + ON base_properties (base_id, lower(name)) + WHERE deleted_at IS NULL + `.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`DROP INDEX IF EXISTS base_properties_name_unique`.execute(db); +}