From 7179b09d7bb060c04193feaf686e40d722e0c951 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Thu, 16 Mar 2023 10:40:20 +0530 Subject: [PATCH] Allow rescheduleReason to be marked required (#7729) --- .../components/booking/pages/BookingPage.tsx | 19 +- apps/web/lib/getBooking.tsx | 6 +- apps/web/pages/booking/[uid].tsx | 2 +- .../manage-booking-questions.e2e.ts | 322 ++++++++---------- .../features/bookings/lib/getBookingFields.ts | 26 +- .../bookings/lib/getBookingResponsesSchema.ts | 39 ++- .../features/bookings/lib/handleNewBooking.ts | 5 +- .../features/form-builder/FormBuilder.tsx | 4 +- .../form-builder/FormBuilderFieldsSchema.ts | 11 +- .../form/toggleGroup/BooleanToggleGroup.tsx | 4 +- 10 files changed, 233 insertions(+), 205 deletions(-) diff --git a/apps/web/components/booking/pages/BookingPage.tsx b/apps/web/components/booking/pages/BookingPage.tsx index 2789581a66..54ddf661ac 100644 --- a/apps/web/components/booking/pages/BookingPage.tsx +++ b/apps/web/components/booking/pages/BookingPage.tsx @@ -78,6 +78,7 @@ const BookingFields = ({ const { t } = useLocale(); const { watch, setValue } = useFormContext(); const locationResponse = watch("responses.location"); + const currentView = rescheduleUid ? "reschedule" : ""; return ( // TODO: It might make sense to extract this logic into BookingFields config, that would allow to quickly configure system fields and their editability in fresh booking and reschedule booking view @@ -87,12 +88,16 @@ const BookingFields = ({ // Allowing a system field to be edited might require sending emails to attendees, so we need to be careful let readOnly = (field.editable === "system" || field.editable === "system-but-optional") && !!rescheduleUid; + let noLabel = false; let hidden = !!field.hidden; + const fieldViews = field.views; + + if (fieldViews && !fieldViews.find((view) => view.id === currentView)) { + return null; + } + if (field.name === SystemField.Enum.rescheduleReason) { - if (!rescheduleUid) { - return null; - } // rescheduleReason is a reschedule specific field and thus should be editable during reschedule readOnly = false; } @@ -292,7 +297,10 @@ const BookingPage = ({ useTheme(profile.theme); const date = asStringOrNull(router.query.date); const querySchema = getBookingResponsesPartialSchema({ - bookingFields: getBookingFieldsWithSystemFields(eventType), + eventType: { + bookingFields: getBookingFieldsWithSystemFields(eventType), + }, + view: rescheduleUid ? "reschedule" : "booking", }); const parsedQuery = querySchema.parse({ @@ -372,7 +380,8 @@ const BookingPage = ({ const bookingFormSchema = z .object({ responses: getBookingResponsesSchema({ - bookingFields: getBookingFieldsWithSystemFields(eventType), + eventType: { bookingFields: getBookingFieldsWithSystemFields(eventType) }, + view: rescheduleUid ? "reschedule" : "booking", }), }) .passthrough(); diff --git a/apps/web/lib/getBooking.tsx b/apps/web/lib/getBooking.tsx index 1b952d0a4a..8659dc25f4 100644 --- a/apps/web/lib/getBooking.tsx +++ b/apps/web/lib/getBooking.tsx @@ -112,7 +112,11 @@ export const getBookingWithResponses = < return { ...booking, responses: getBookingResponsesPartialSchema({ - bookingFields: eventType.bookingFields, + eventType: { + bookingFields: eventType.bookingFields, + }, + // An existing booking can have data from any number of views, so the schema should consider ALL_VIEWS + view: "ALL_VIEWS", }).parse(booking.responses || getResponsesFromOldBooking(booking)), }; }; diff --git a/apps/web/pages/booking/[uid].tsx b/apps/web/pages/booking/[uid].tsx index 9f8f8d0c0f..68278c1a32 100644 --- a/apps/web/pages/booking/[uid].tsx +++ b/apps/web/pages/booking/[uid].tsx @@ -606,7 +606,7 @@ export default function Success(props: SuccessProps) { {!props.recurringBookings && ( - + diff --git a/apps/web/playwright/manage-booking-questions.e2e.ts b/apps/web/playwright/manage-booking-questions.e2e.ts index ba55fefdfb..4357c97f2c 100644 --- a/apps/web/playwright/manage-booking-questions.e2e.ts +++ b/apps/web/playwright/manage-booking-questions.e2e.ts @@ -33,56 +33,108 @@ test.describe("Manage Booking Questions", () => { await firstEventTypeElement.click(); }); - await test.step("Add Question and see that it's shown on Booking Page at appropriate position", async () => { - await addQuestionAndSave({ - page, - question: { - name: "how_are_you", - type: "Name", - label: "How are you?", - placeholder: "I'm fine, thanks", - required: true, - }, - }); + await runTestStepsCommonForTeamAndUserEventType(page, context, webhookReceiver); + }); + }); - await doOnFreshPreview(page, context, async (page) => { - const allFieldsLocator = await expectSystemFieldsToBeThere(page); - const userFieldLocator = allFieldsLocator.nth(5); + test.describe("For Team EventType", () => { + test("Do a booking with a user added 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 * 3); + const user = await createAndLoginUserWithEventTypes({ users }); - 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 userFieldLocator.locator("label").nth(0).innerText()).toBe("How are you?"); - await expect(userFieldLocator.locator("input[type=text]")).toBeVisible(); - }); + const webhookReceiver = await addWebhook(user); + + await test.step("Go to First Team Event", async () => { + const $eventTypes = page.locator("[data-testid=event-types]").nth(1).locator("li a"); + const firstEventTypeElement = $eventTypes.first(); + + await firstEventTypeElement.click(); }); - await test.step("Hide Question and see that it's not shown on Booking Page", async () => { - await toggleQuestionAndSave({ - name: "how_are_you", - page, - }); - await doOnFreshPreview(page, context, async (page) => { - const formBuilderFieldLocator = page.locator('[data-fob-field-name="how_are_you"]'); - await expect(formBuilderFieldLocator).toBeHidden(); - }); - }); + await runTestStepsCommonForTeamAndUserEventType(page, context, webhookReceiver); + }); + }); +}); - await test.step("Show Question Again", async () => { - await toggleQuestionAndSave({ - name: "how_are_you", - page, - }); - }); +async function runTestStepsCommonForTeamAndUserEventType( + page: Page, + context: PlaywrightTestArgs["context"], + webhookReceiver: { + port: number; + close: () => import("http").Server; + requestList: (import("http").IncomingMessage & { body?: unknown })[]; + url: string; + } +) { + await page.click('[href$="tabName=advanced"]'); - await test.step('Try to book without providing "How are you?" response', async () => { - await doOnFreshPreview(page, context, async (page) => { - await bookTimeSlot({ page, name: "Booker", email: "booker@example.com" }); - await expectErrorToBeThereFor({ page, name: "how_are_you" }); - }); - }); + await test.step("Add Question and see that it's shown on Booking Page at appropriate position", async () => { + await addQuestionAndSave({ + page, + question: { + name: "how_are_you", + type: "Name", + label: "How are you?", + placeholder: "I'm fine, thanks", + required: true, + }, + }); - await test.step("Do a booking", async () => { - await doOnFreshPreview(page, context, async (page) => { + await doOnFreshPreview(page, context, async (page) => { + const allFieldsLocator = await expectSystemFieldsToBeThere(page); + 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 userFieldLocator.locator("label").nth(0).innerText()).toBe("How are you?"); + await expect(userFieldLocator.locator("input[type=text]")).toBeVisible(); + }); + }); + + await test.step("Hide Question and see that it's not shown on Booking Page", async () => { + await toggleQuestionAndSave({ + name: "how_are_you", + page, + }); + await doOnFreshPreview(page, context, async (page) => { + const formBuilderFieldLocator = page.locator('[data-fob-field-name="how_are_you"]'); + await expect(formBuilderFieldLocator).toBeHidden(); + }); + }); + + await test.step("Show Question Again", async () => { + await toggleQuestionAndSave({ + name: "how_are_you", + page, + }); + }); + + await test.step('Try to book without providing "How are you?" response', async () => { + await doOnFreshPreview(page, context, async (page) => { + await bookTimeSlot({ page, name: "Booker", email: "booker@example.com" }); + await expectErrorToBeThereFor({ page, name: "how_are_you" }); + }); + }); + + await test.step("Make rescheduleReason required - It won't be required for a fresh booking", async () => { + await toggleQuestionRequireStatusAndSave({ + required: true, + name: "rescheduleReason", + page, + }); + }); + + const previewTabPage = + await test.step("Do a booking and notice that we can book without giving a value for rescheduleReason", async () => { + return await doOnFreshPreview( + page, + context, + async (page) => { const formBuilderFieldLocator = page.locator('[data-fob-field-name="how_are_you"]'); await expect(formBuilderFieldLocator).toBeVisible(); expect( @@ -133,146 +185,16 @@ test.describe("Manage Booking Questions", () => { value: "I am great!", }, }); - }); - }); - }); - }); - - test.describe("For Team EventType", () => { - test("Do a booking with a user added 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 * 3); - const user = await createAndLoginUserWithEventTypes({ users }); - - const webhookReceiver = await addWebhook(user); - - await test.step("Go to First Team Event", async () => { - const $eventTypes = page.locator("[data-testid=event-types]").nth(1).locator("li a"); - const firstEventTypeElement = $eventTypes.first(); - - await firstEventTypeElement.click(); - }); - - await runTestStepsCommonForTeamAndUserEventType(page, context, webhookReceiver); - }); - }); -}); - -async function runTestStepsCommonForTeamAndUserEventType( - page: Page, - context: PlaywrightTestArgs["context"], - webhookReceiver: { - port: number; - close: () => import("http").Server; - requestList: (import("http").IncomingMessage & { body?: unknown })[]; - url: string; - } -) { - await test.step("Add Question and see that it's shown on Booking Page at appropriate position", async () => { - await addQuestionAndSave({ - page, - question: { - name: "how_are_you", - type: "Name", - label: "How are you?", - placeholder: "I'm fine, thanks", - required: true, - }, - }); - - await doOnFreshPreview(page, context, async (page) => { - const allFieldsLocator = await expectSystemFieldsToBeThere(page); - 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 userFieldLocator.locator("label").nth(0).innerText()).toBe("How are you?"); - await expect(userFieldLocator.locator("input[type=text]")).toBeVisible(); - }); - }); - - await test.step("Hide Question and see that it's not shown on Booking Page", async () => { - await toggleQuestionAndSave({ - name: "how_are_you", - page, - }); - await doOnFreshPreview(page, context, async (page) => { - const formBuilderFieldLocator = page.locator('[data-fob-field-name="how_are_you"]'); - await expect(formBuilderFieldLocator).toBeHidden(); - }); - }); - - await test.step("Show Question Again", async () => { - await toggleQuestionAndSave({ - name: "how_are_you", - page, - }); - }); - - await test.step('Try to book without providing "How are you?" response', async () => { - await doOnFreshPreview(page, context, async (page) => { - await bookTimeSlot({ page, name: "Booker", email: "booker@example.com" }); - await expectErrorToBeThereFor({ page, name: "how_are_you" }); - }); - }); - - await test.step("Do a booking", async () => { - await doOnFreshPreview(page, context, async (page) => { - const formBuilderFieldLocator = page.locator('[data-fob-field-name="how_are_you"]'); - await expect(formBuilderFieldLocator).toBeVisible(); - expect(await formBuilderFieldLocator.locator('[name="how_are_you"]').getAttribute("placeholder")).toBe( - "I'm fine, thanks" + }, + true ); - expect(await formBuilderFieldLocator.locator("label").nth(0).innerText()).toBe("How are you?"); - await formBuilderFieldLocator.locator('[name="how_are_you"]').fill("I am great!"); - await bookTimeSlot({ page, name: "Booker", email: "booker@example.com" }); - await expect(page.locator("[data-testid=success-page]")).toBeVisible(); - - expect( - await page.locator('[data-testid="field-response"][data-fob-field="how_are_you"]').innerText() - ).toBe("I am great!"); - - await waitFor(() => { - expect(webhookReceiver.requestList.length).toBe(1); - }); - - const [request] = webhookReceiver.requestList; - - const payload = (request.body as any).payload as any; - - expect(payload.responses).toMatchObject({ - email: { - label: "email_address", - value: "booker@example.com", - }, - how_are_you: { - label: "How are you?", - value: "I am great!", - }, - name: { - label: "your_name", - value: "Booker", - }, - }); - - expect(payload.location).toBe("integrations:daily"); - - expect(payload.attendees[0]).toMatchObject({ - name: "Booker", - email: "booker@example.com", - }); - - expect(payload.userFieldsResponses).toMatchObject({ - how_are_you: { - label: "How are you?", - value: "I am great!", - }, - }); }); + + await test.step("Do a reschedule and notice that we can't book without giving a value for rescheduleReason", async () => { + const page = previewTabPage; + await rescheduleFromTheLinkOnPage({ page }); + await page.pause(); + await expectErrorToBeThereFor({ page, name: "rescheduleReason" }); }); } @@ -333,7 +255,6 @@ async function addQuestionAndSave({ required?: boolean; }; }) { - await page.click('[href$="tabName=advanced"]'); await page.click('[data-testid="add-field"]'); if (question.type !== undefined) { @@ -379,11 +300,15 @@ async function expectErrorToBeThereFor({ page, name }: { page: Page; name: strin async function doOnFreshPreview( page: Page, context: PlaywrightTestArgs["context"], - callback: (page: Page) => Promise + callback: (page: Page) => Promise, + persistTab = false ) { const previewTabPage = await openBookingFormInPreviewTab(context, page); await callback(previewTabPage); - await previewTabPage.close(); + if (!persistTab) { + await previewTabPage.close(); + } + return previewTabPage; } async function toggleQuestionAndSave({ name, page }: { name: string; page: Page }) { @@ -391,6 +316,25 @@ async function toggleQuestionAndSave({ name, page }: { name: string; page: Page await saveEventType(page); } +async function toggleQuestionRequireStatusAndSave({ + required, + name, + page, +}: { + required: boolean; + name: string; + page: Page; +}) { + await page.locator(`[data-testid="field-${name}"]`).locator('[data-testid="edit-field-action"]').click(); + await page + .locator('[data-testid="edit-field-dialog"]') + .locator('[data-testid="field-required"] button') + .locator(`text=${required ? "Yes" : "No"}`) + .click(); + await page.locator('[data-testid="field-add-save"]').click(); + await saveEventType(page); +} + async function createAndLoginUserWithEventTypes({ users }: { users: ReturnType }) { const user = await users.create(null, { hasTeam: true, @@ -399,6 +343,16 @@ async function createAndLoginUserWithEventTypes({ users }: { users: ReturnType url.pathname.endsWith("/book"), + }); + await page.click('[data-testid="confirm-reschedule-button"]'); +} + async function openBookingFormInPreviewTab(context: PlaywrightTestArgs["context"], page: Page) { const previewTabPromise = context.waitForEvent("page"); await page.locator('[data-testid="preview-button"]').click(); diff --git a/packages/features/bookings/lib/getBookingFields.ts b/packages/features/bookings/lib/getBookingFields.ts index e5e0365c69..3667fa64fa 100644 --- a/packages/features/bookings/lib/getBookingFields.ts +++ b/packages/features/bookings/lib/getBookingFields.ts @@ -61,7 +61,7 @@ export const SystemFieldsEditability: Record, Fields location: "system", notes: "system-but-optional", guests: "system-but-optional", - rescheduleReason: "system", + rescheduleReason: "system-but-optional", smsReminderNumber: "system", }; @@ -252,6 +252,12 @@ export const ensureBookingInputsHaveSystemFields = ({ name: "rescheduleReason", defaultPlaceholder: "reschedule_placeholder", required: false, + views: [ + { + id: "reschedule", + label: "Reschedule View", + }, + ], sources: [ { label: "Default", @@ -264,9 +270,16 @@ export const ensureBookingInputsHaveSystemFields = ({ const missingSystemBeforeFields = []; for (const field of systemBeforeFields) { + const existingBookingFieldIndex = bookingFields.findIndex((f) => f.name === field.name); // Only do a push, we must not update existing system fields as user could have modified any property in it, - if (!bookingFields.find((f) => f.name === field.name)) { + if (existingBookingFieldIndex === -1) { missingSystemBeforeFields.push(field); + } else { + // Adding the fields from Code first and then fields from DB. Allows, the code to push new properties to the field + bookingFields[existingBookingFieldIndex] = { + ...field, + ...bookingFields[existingBookingFieldIndex], + }; } } @@ -310,9 +323,16 @@ export const ensureBookingInputsHaveSystemFields = ({ const missingSystemAfterFields = []; for (const field of systemAfterFields) { + const existingBookingFieldIndex = bookingFields.findIndex((f) => f.name === field.name); // Only do a push, we must not update existing system fields as user could have modified any property in it, - if (!bookingFields.find((f) => f.name === field.name)) { + if (existingBookingFieldIndex === -1) { missingSystemAfterFields.push(field); + } else { + bookingFields[existingBookingFieldIndex] = { + // Adding the fields from Code first and then fields from DB. Allows, the code to push new properties to the field + ...field, + ...bookingFields[existingBookingFieldIndex], + }; } } diff --git a/packages/features/bookings/lib/getBookingResponsesSchema.ts b/packages/features/bookings/lib/getBookingResponsesSchema.ts index c7fce8a68f..2f84d87ecc 100644 --- a/packages/features/bookings/lib/getBookingResponsesSchema.ts +++ b/packages/features/bookings/lib/getBookingResponsesSchema.ts @@ -1,22 +1,32 @@ import { isValidPhoneNumber } from "libphonenumber-js"; import z from "zod"; +import type { ALL_VIEWS } from "@calcom/features/form-builder/FormBuilderFieldsSchema"; import type { eventTypeBookingFields } from "@calcom/prisma/zod-utils"; import { bookingResponses } from "@calcom/prisma/zod-utils"; type EventType = Parameters[0]["eventType"]; -export const getBookingResponsesPartialSchema = (eventType: EventType) => { +// eslint-disable-next-line @typescript-eslint/ban-types +type View = ALL_VIEWS | (string & {}); + +export const getBookingResponsesPartialSchema = ({ + eventType, + view, +}: { + eventType: EventType; + view: View; +}) => { const schema = bookingResponses.unwrap().partial().and(z.record(z.any())); - return preprocess({ schema, eventType, isPartialSchema: true }); + return preprocess({ schema, eventType, isPartialSchema: true, view }); }; // Should be used when we know that not all fields responses are present // - Can happen when we are parsing the prefill query string // - Can happen when we are parsing a booking's responses (which was created before we added a new required field) -export default function getBookingResponsesSchema(eventType: EventType) { +export default function getBookingResponsesSchema({ eventType, view }: { eventType: EventType; view: View }) { const schema = bookingResponses.and(z.record(z.any())); - return preprocess({ schema, eventType, isPartialSchema: false }); + return preprocess({ schema, eventType, isPartialSchema: false, view }); } // TODO: Move preprocess of `booking.responses` to FormBuilder schema as that is going to parse the fields supported by FormBuilder @@ -25,12 +35,14 @@ function preprocess({ schema, eventType, isPartialSchema, + view: currentView, }: { schema: T; isPartialSchema: boolean; eventType: { bookingFields: z.infer & z.BRAND<"HAS_SYSTEM_FIELDS">; }; + view: View; }): z.ZodType, z.infer, z.infer> { const preprocessed = z.preprocess( (responses) => { @@ -42,8 +54,15 @@ function preprocess({ // If there is no response for the field, then we don't need to do any processing return; } - // Turn a boolean in string to a real boolean + const views = field.views; + const isFieldApplicableToCurrentView = + currentView === "ALL_VIEWS" ? true : views ? views.find((view) => view.id === currentView) : true; + if (!isFieldApplicableToCurrentView) { + // If the field is not applicable in the current view, then we don't need to do any processing + return; + } if (field.type === "boolean") { + // Turn a boolean in string to a real boolean newResponses[field.name] = value === "true" || value === true; } // Make sure that the value is an array @@ -76,8 +95,16 @@ function preprocess({ : z.string().refine((val) => isValidPhoneNumber(val)); // Tag the message with the input name so that the message can be shown at appropriate place const m = (message: string) => `{${bookingField.name}}${message}`; + const views = bookingField.views; + const isFieldApplicableToCurrentView = + currentView === "ALL_VIEWS" ? true : views ? views.find((view) => view.id === currentView) : true; // If the field is hidden, then it can never be required - const isRequired = bookingField.hidden ? false : bookingField.required; + const isRequired = bookingField.hidden + ? false + : isFieldApplicableToCurrentView + ? bookingField.required + : false; + if ((isPartialSchema || !isRequired) && value === undefined) { return; } diff --git a/packages/features/bookings/lib/handleNewBooking.ts b/packages/features/bookings/lib/handleNewBooking.ts index fed02a7ca3..0331a1a766 100644 --- a/packages/features/bookings/lib/handleNewBooking.ts +++ b/packages/features/bookings/lib/handleNewBooking.ts @@ -377,7 +377,10 @@ function getBookingData({ ? extendedBookingCreateBody.merge( z.object({ responses: getBookingResponsesSchema({ - bookingFields: eventType.bookingFields, + eventType: { + bookingFields: eventType.bookingFields, + }, + view: req.body.rescheduleUid ? "reschedule" : "booking", }), }) ) diff --git a/packages/features/form-builder/FormBuilder.tsx b/packages/features/form-builder/FormBuilder.tsx index 04157b6d5e..bf805d4c4d 100644 --- a/packages/features/form-builder/FormBuilder.tsx +++ b/packages/features/form-builder/FormBuilder.tsx @@ -335,6 +335,7 @@ export const FormBuilder = function FormBuilder({ }} />