feat(base): enforce unique property names per base

This commit is contained in:
Philipinho
2026-04-24 02:34:38 +01:00
parent 8c0071ee23
commit 464bd701ba
4 changed files with 65 additions and 4 deletions
@@ -25,6 +25,7 @@ type Props = {
editingPropertyId: string | null; editingPropertyId: string | null;
initialSource?: string; initialSource?: string;
name?: string; name?: string;
disabled?: boolean;
onSave: ( onSave: (
source: string, source: string,
ast: unknown, ast: unknown,
@@ -39,6 +40,7 @@ export function FormulaEditor({
editingPropertyId, editingPropertyId,
initialSource = "", initialSource = "",
name, name,
disabled = false,
onSave, onSave,
onCancel, onCancel,
}: Props) { }: Props) {
@@ -49,7 +51,7 @@ export function FormulaEditor({
editingPropertyId, editingPropertyId,
registry, registry,
); );
const canSave = parseState.state === "ok"; const canSave = parseState.state === "ok" && !disabled;
const insertAtEnd = (snippet: string) => const insertAtEnd = (snippet: string) =>
setSource((s) => `${s}${s ? " " : ""}${snippet}`); setSource((s) => `${s}${s ? " " : ""}${snippet}`);
@@ -66,6 +66,14 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }:
return name.trim().length > 0 || Object.keys(typeOptions).length > 0; return name.trim().length > 0 || Object.keys(typeOptions).length > 0;
}, [name, typeOptions]); }, [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(() => { const resetState = useCallback(() => {
setPanel("typePicker"); setPanel("typePicker");
setSelectedType(null); setSelectedType(null);
@@ -112,7 +120,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }:
}, [panel]); }, [panel]);
const handleCreate = useCallback(() => { const handleCreate = useCallback(() => {
if (!selectedType) return; if (!selectedType || nameTaken) return;
const finalName = name.trim() || selectedTypeLabel; const finalName = name.trim() || selectedTypeLabel;
createPropertyMutation.mutate( createPropertyMutation.mutate(
{ {
@@ -130,7 +138,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }:
}, },
); );
handleClose(); handleClose();
}, [selectedType, name, selectedTypeLabel, typeOptions, baseId, createPropertyMutation, handleClose, onPropertyCreated]); }, [selectedType, nameTaken, name, selectedTypeLabel, typeOptions, baseId, createPropertyMutation, handleClose, onPropertyCreated]);
const handleBackToTypePicker = useCallback(() => { const handleBackToTypePicker = useCallback(() => {
setPanel("typePicker"); setPanel("typePicker");
@@ -242,13 +250,16 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }:
placeholder={selectedTypeLabel} placeholder={selectedTypeLabel}
value={name} value={name}
onChange={(e) => setName(e.currentTarget.value)} onChange={(e) => setName(e.currentTarget.value)}
error={nameTaken ? t("A property with this name already exists") : undefined}
/> />
<FormulaEditor <FormulaEditor
properties={properties ?? []} properties={properties ?? []}
editingPropertyId={null} editingPropertyId={null}
name={name.trim() || undefined} name={name.trim() || undefined}
onCancel={handleBackToTypePicker} onCancel={handleBackToTypePicker}
disabled={nameTaken}
onSave={(source, ast, resultType, dependencies) => { onSave={(source, ast, resultType, dependencies) => {
if (nameTaken) return;
createPropertyMutation.mutate( createPropertyMutation.mutate(
{ {
baseId, baseId,
@@ -279,6 +290,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }:
value={name} value={name}
onChange={(e) => setName(e.currentTarget.value)} onChange={(e) => setName(e.currentTarget.value)}
onKeyDown={handleNameKeyDown} onKeyDown={handleNameKeyDown}
error={nameTaken ? t("A property with this name already exists") : undefined}
mb="xs" mb="xs"
/> />
<UnstyledButton <UnstyledButton
@@ -316,7 +328,7 @@ export function CreatePropertyPopover({ baseId, properties, onPropertyCreated }:
<Button variant="default" size="xs" onClick={attemptClose}> <Button variant="default" size="xs" onClick={attemptClose}>
{t("Cancel")} {t("Cancel")}
</Button> </Button>
<Button size="xs" onClick={handleCreate}> <Button size="xs" onClick={handleCreate} disabled={nameTaken}>
{t("Create field")} {t("Create field")}
</Button> </Button>
</Group> </Group>
@@ -88,6 +88,8 @@ export class BasePropertyService {
async create(workspaceId: string, dto: CreatePropertyDto, actorId?: string) { async create(workspaceId: string, dto: CreatePropertyDto, actorId?: string) {
const type = dto.type as BasePropertyTypeValue; const type = dto.type as BasePropertyTypeValue;
await this.ensureNameUnique(dto.baseId, dto.name);
let validatedTypeOptions: unknown; let validatedTypeOptions: unknown;
if (type === 'formula') { if (type === 'formula') {
const sourceCandidate = (dto.typeOptions as any)?.source; const sourceCandidate = (dto.typeOptions as any)?.source;
@@ -192,6 +194,10 @@ export class BasePropertyService {
throw new BadRequestException('Property does not belong to this base'); 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 // Block concurrent type changes — the worker still owns the previous
// conversion, and letting a second one through would race on `type`. // conversion, and letting a second one through would race on `type`.
if (property.pendingType) { if (property.pendingType) {
@@ -418,6 +424,27 @@ export class BasePropertyService {
* has to happen after the outer transaction commits so socket consumers * has to happen after the outer transaction commits so socket consumers
* never race ahead of visibility. * never race ahead of visibility.
*/ */
private async ensureNameUnique(
baseId: string,
candidate: string,
excludePropertyId?: string,
): Promise<void> {
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( private async loadAndEmit(
dto: UpdatePropertyDto, dto: UpdatePropertyDto,
workspaceId: string, workspaceId: string,
@@ -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<name, id>` — 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<any>): Promise<void> {
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<any>): Promise<void> {
await sql`DROP INDEX IF EXISTS base_properties_name_unique`.execute(db);
}