From beee5455685117b34646d5352e3ed465eac12630 Mon Sep 17 00:00:00 2001 From: Amit Sharma <74371312+Amit91848@users.noreply.github.com> Date: Fri, 5 Apr 2024 15:05:48 +0530 Subject: [PATCH] fix: "Checkbox" custom field throws error (#14339) * fix: "Checkbox" custom field throws error * Add e2e * Self review feedback * Fix typo --------- Co-authored-by: Hariom --- .../manage-booking-questions.e2e.ts | 55 ++++++++++++++++++- packages/features/form-builder/Components.tsx | 3 +- .../features/form-builder/FormBuilder.tsx | 7 +++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/apps/web/playwright/manage-booking-questions.e2e.ts b/apps/web/playwright/manage-booking-questions.e2e.ts index 9f0d4762ae..988f35a12d 100644 --- a/apps/web/playwright/manage-booking-questions.e2e.ts +++ b/apps/web/playwright/manage-booking-questions.e2e.ts @@ -10,8 +10,13 @@ import type { CalendarEvent } from "@calcom/types/Calendar"; import { test } from "./lib/fixtures"; import { createHttpServer, selectFirstAvailableTimeSlotNextMonth } from "./lib/testUtils"; +function getLabelLocator(field: Locator) { + // There are 2 labels right now. Will be one in future. The second one is hidden + return field.locator("label").first(); +} + async function getLabelText(field: Locator) { - return await field.locator("label").first().locator("span").first().innerText(); + return await getLabelLocator(field).locator("span").first().innerText(); } test.describe.configure({ mode: "parallel" }); @@ -21,7 +26,7 @@ test.describe("Manage Booking Questions", () => { }); test.describe("For User EventType", () => { - test("Do a booking with a user added question and verify a few thing in b/w", async ({ + test("Do a booking with a Address type question and verify a few thing in b/w", async ({ page, users, context, @@ -42,6 +47,51 @@ test.describe("Manage Booking Questions", () => { await runTestStepsCommonForTeamAndUserEventType(page, context, webhookReceiver); }); + test("Do a booking with Checkbox type question and verify a few thing in b/w", async ({ + page, + users, + context, + }, testInfo) => { + // Considering there are many steps in it, it would need more than default test timeout + test.setTimeout(testInfo.timeout * 2); + const user = await createAndLoginUserWithEventTypes({ users, page }); + + // const webhookReceiver = await addWebhook(user); + + await test.step("Go to EventType Advanced Page ", async () => { + const $eventTypes = page.locator("[data-testid=event-types] > li a"); + const firstEventTypeElement = $eventTypes.first(); + + await firstEventTypeElement.click(); + await page.click('[href$="tabName=advanced"]'); + }); + + await test.step("Add Question and see that it's shown on Booking Page at appropriate position", async () => { + await addQuestionAndSave({ + page, + question: { + name: "agree-to-terms", + type: "Checkbox", + label: "Agree to [terms](https://example.com/terms)", + required: true, + }, + }); + + await doOnFreshPreview(page, context, async (page) => { + const allFieldsLocator = await expectSystemFieldsToBeThereOnBookingPage({ page }); + const userFieldLocator = allFieldsLocator.nth(5); + + await expect(userFieldLocator.locator('[name="agree-to-terms"]')).toBeVisible(); + expect(await getLabelText(userFieldLocator)).toBe("Agree to terms"); + // Verify that markdown is working + expect(await getLabelLocator(userFieldLocator).locator("a").getAttribute("href")).toBe( + "https://example.com/terms" + ); + await expect(userFieldLocator.locator("input")).toBeVisible(); + }); + }); + }); + test("Split 'Full name' into 'First name' and 'Last name'", async ({ page, users, @@ -246,7 +296,6 @@ async function runTestStepsCommonForTeamAndUserEventType( const userFieldLocator = allFieldsLocator.nth(5); await expect(userFieldLocator.locator('[name="how-are-you"]')).toBeVisible(); - // There are 2 labels right now. Will be one in future. The second one is hidden expect(await getLabelText(userFieldLocator)).toBe("How are you?"); await expect(userFieldLocator.locator("input")).toBeVisible(); }); diff --git a/packages/features/form-builder/Components.tsx b/packages/features/form-builder/Components.tsx index 4949de5668..c6161bda1b 100644 --- a/packages/features/form-builder/Components.tsx +++ b/packages/features/form-builder/Components.tsx @@ -489,10 +489,11 @@ export const Components: Record = { }, boolean: { propsType: propsTypes.boolean, - factory: ({ readOnly, label, value, setValue }) => { + factory: ({ readOnly, name, label, value, setValue }) => { return (
{ if (e.target.checked) { setValue(true); diff --git a/packages/features/form-builder/FormBuilder.tsx b/packages/features/form-builder/FormBuilder.tsx index 33b8183f4e..200d93dcc6 100644 --- a/packages/features/form-builder/FormBuilder.tsx +++ b/packages/features/form-builder/FormBuilder.tsx @@ -120,6 +120,13 @@ export const FormBuilder = function FormBuilder({ : field.getOptionsAt ? dataStore.options[field.getOptionsAt as keyof typeof dataStore] : []; + + // Note: We recently started calling getAndUpdateNormalizedValues in the FormBuilder. It was supposed to be called only on booking pages earlier. + // Due to this we have to meet some strict requirements like of labelAsSafeHtml. + if (fieldsThatSupportLabelAsSafeHtml.includes(field.type)) { + field = { ...field, labelAsSafeHtml: markdownToSafeHTML(field.label ?? "") }; + } + const { hidden } = getAndUpdateNormalizedValues({ ...field, options }, t); if (field.hideWhenJustOneOption && (hidden || !options?.length)) { return null;