diff --git a/apps/web/app/(use-page-wrapper)/apps/routing-forms/[...pages]/FormEdit.tsx b/apps/web/app/(use-page-wrapper)/apps/routing-forms/[...pages]/FormEdit.tsx index 2939aedd26..c193242b71 100644 --- a/apps/web/app/(use-page-wrapper)/apps/routing-forms/[...pages]/FormEdit.tsx +++ b/apps/web/app/(use-page-wrapper)/apps/routing-forms/[...pages]/FormEdit.tsx @@ -8,6 +8,7 @@ import { v4 as uuidv4 } from "uuid"; import { FieldTypes } from "@calcom/app-store/routing-forms/lib/FieldTypes"; import type { RoutingFormWithResponseCount } from "@calcom/app-store/routing-forms/types/types"; +import { getFieldIdentifier } from "@calcom/features/form-builder/utils/getFieldIdentifier"; import { useLocale } from "@calcom/lib/hooks/useLocale"; import classNames from "@calcom/ui/classNames"; import { Button } from "@calcom/ui/components/button"; @@ -102,14 +103,25 @@ function Field({ required {...hookForm.register(`${hookFieldNamespace}.label`)} onChange={(e) => { - hookForm.setValue(`${hookFieldNamespace}.label`, e.target.value, { shouldDirty: true }); + const newLabel = e.target.value; + // Use label from useWatch which is guaranteed to be the previous value + // since useWatch updates reactively (after re-render), not synchronously + const previousLabel = label || ""; + hookForm.setValue(`${hookFieldNamespace}.label`, newLabel, { shouldDirty: true }); + const currentIdentifier = hookForm.getValues(`${hookFieldNamespace}.identifier`); + // Only auto-update identifier if it was auto-generated from the previous label + // This preserves manual identifier changes + const isIdentifierGeneratedFromPreviousLabel = currentIdentifier === getFieldIdentifier(previousLabel); + if (!currentIdentifier || isIdentifierGeneratedFromPreviousLabel) { + hookForm.setValue(`${hookFieldNamespace}.identifier`, getFieldIdentifier(newLabel), { shouldDirty: true }); + } }} />
{ label: "Test Field", }); const queryString = - "firstField=456&Test Field Number=456&Test Field Single Selection=456&Test Field Multiple Selection=456&Test Field Multiple Selection=789&Test Field Phone=456&Test Field Email=456@example.com"; + "firstField=456&Test-Field-Number=456&Test-Field-Single-Selection=456&Test-Field-Multiple-Selection=456&Test-Field-Multiple-Selection=789&Test-Field-Phone=456&Test-Field-Email=456@example.com"; await gotoRoutingLink({ page, queryString }); - await page.fill('[data-testid="form-field-Test Field Long Text"]', "manual-fill"); + await page.fill('[data-testid="form-field-Test-Field-Long-Text"]', "manual-fill"); await expect(page.locator('[data-testid="form-field-firstField"]')).toHaveValue("456"); - await expect(page.locator('[data-testid="form-field-Test Field Number"]')).toHaveValue("456"); + await expect(page.locator('[data-testid="form-field-Test-Field-Number"]')).toHaveValue("456"); // TODO: Verify select and multiselect has prefilled values. - // expect(await page.locator(`[data-testid="form-field-Test Field Select"]`).inputValue()).toBe("456"); - // expect(await page.locator(`[data-testid="form-field-Test Field MultiSelect"]`).inputValue()).toBe("456"); + // expect(await page.locator(`[data-testid="form-field-Test-Field-Select"]`).inputValue()).toBe("456"); + // expect(await page.locator(`[data-testid="form-field-Test-Field-MultiSelect"]`).inputValue()).toBe("456"); - await expect(page.locator('[data-testid="form-field-Test Field Phone"]')).toHaveValue("456"); - await expect(page.locator('[data-testid="form-field-Test Field Email"]')).toHaveValue( + await expect(page.locator('[data-testid="form-field-Test-Field-Phone"]')).toHaveValue("456"); + await expect(page.locator('[data-testid="form-field-Test-Field-Email"]')).toHaveValue( "456@example.com" ); @@ -300,12 +300,12 @@ test.describe("Routing Forms", () => { expect(url.searchParams.get("firstField")).toBe("456"); // All other params come from prefill URL - expect(url.searchParams.get("Test Field Number")).toBe("456"); - expect(url.searchParams.get("Test Field Long Text")).toBe("manual-fill"); - expect(url.searchParams.get("Test Field Multiple Selection")).toBe("456"); - expect(url.searchParams.getAll("Test Field Multiple Selection")).toMatchObject(["456", "789"]); - expect(url.searchParams.get("Test Field Phone")).toBe("456"); - expect(url.searchParams.get("Test Field Email")).toBe("456@example.com"); + expect(url.searchParams.get("Test-Field-Number")).toBe("456"); + expect(url.searchParams.get("Test-Field-Long-Text")).toBe("manual-fill"); + expect(url.searchParams.get("Test-Field-Multiple-Selection")).toBe("456"); + expect(url.searchParams.getAll("Test-Field-Multiple-Selection")).toMatchObject(["456", "789"]); + expect(url.searchParams.get("Test-Field-Phone")).toBe("456"); + expect(url.searchParams.get("Test-Field-Email")).toBe("456@example.com"); }); // TODO: How to install the app just once? diff --git a/packages/lib/getValidRhfFieldName.test.ts b/packages/lib/getValidRhfFieldName.test.ts index 77319cea24..01c6b113fc 100644 --- a/packages/lib/getValidRhfFieldName.test.ts +++ b/packages/lib/getValidRhfFieldName.test.ts @@ -16,11 +16,11 @@ describe("getValidRhfFieldName", () => { }); // So that user can freely add spaces and any other character iteratively and it gets converted to - and he can add more characters. - // We don't really care about a hyphen in the end - it("should not remove dashes from start and end.", () => { - expect(getValidRhfFieldName("hello-there-")).toEqual("hello-there-"); + // Trailing dashes are removed to ensure valid field names + it("should remove trailing dashes but keep leading dashes.", () => { + expect(getValidRhfFieldName("hello-there-")).toEqual("hello-there"); expect(getValidRhfFieldName("-hello-there")).toEqual("-hello-there"); - expect(getValidRhfFieldName("$hello-there-")).toEqual("-hello-there-"); + expect(getValidRhfFieldName("$hello-there-")).toEqual("-hello-there"); }); it("should not remove underscore from start and end.", () => { @@ -31,7 +31,8 @@ describe("getValidRhfFieldName", () => { it("should remove unicode and emoji characters", () => { expect(getValidRhfFieldName("Hello 📚🕯️®️ There")).toEqual("Hello---------There"); - expect(getValidRhfFieldName("📚🕯️®️")).toEqual("-------"); + // When all characters are emojis/special chars, they become dashes which are then removed + expect(getValidRhfFieldName("📚🕯️®️")).toEqual(""); }); it("should keep numbers as is", () => { @@ -39,4 +40,18 @@ describe("getValidRhfFieldName", () => { expect(getValidRhfFieldName("321hello there123")).toEqual("321hello-there123"); expect(getValidRhfFieldName("hello$there")).toEqual("hello-there"); }); + + it("should not modify system field names", () => { + // System fields should remain unchanged as they don't contain invalid characters or trailing dashes + expect(getValidRhfFieldName("name")).toEqual("name"); + expect(getValidRhfFieldName("email")).toEqual("email"); + expect(getValidRhfFieldName("location")).toEqual("location"); + expect(getValidRhfFieldName("title")).toEqual("title"); + expect(getValidRhfFieldName("notes")).toEqual("notes"); + expect(getValidRhfFieldName("guests")).toEqual("guests"); + expect(getValidRhfFieldName("rescheduleReason")).toEqual("rescheduleReason"); + expect(getValidRhfFieldName("smsReminderNumber")).toEqual("smsReminderNumber"); + expect(getValidRhfFieldName("attendeePhoneNumber")).toEqual("attendeePhoneNumber"); + expect(getValidRhfFieldName("aiAgentCallPhoneNumber")).toEqual("aiAgentCallPhoneNumber"); + }); }); diff --git a/packages/lib/getValidRhfFieldName.ts b/packages/lib/getValidRhfFieldName.ts index bb2a38fe2c..0749441a64 100644 --- a/packages/lib/getValidRhfFieldName.ts +++ b/packages/lib/getValidRhfFieldName.ts @@ -1,5 +1,5 @@ export const getValidRhfFieldName = (fieldName: string) => { // Remember that any transformation that you do here would run on System Field names as well. So, be careful and avoiding doing anything here that would modify the SystemField names. // e.g. SystemField name currently have uppercases in them. So, no need to lowercase unless absolutely needed. - return fieldName.replace(/[^a-zA-Z0-9-_]/g, "-"); + return fieldName.replace(/[^a-zA-Z0-9-_]/g, "-").replace(/-+$/, ""); };