public-offering
/
cal.pub0.org
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Allow rescheduleReason to be marked required (#7729)

pull/7324/head^2
Hariom Balhara 2023-03-16 10:40:20 +05:30 committed by GitHub
parent 325dc9805f
commit 7179b09d7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 233 additions and 205 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
apps/web/components/booking/pages/BookingPage.tsx
View File

@ -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();

6
apps/web/lib/getBooking.tsx
View File

@ -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)),
};
};

2
apps/web/pages/booking/[uid].tsx
View File

@ -606,7 +606,7 @@ export default function Success(props: SuccessProps) {
{!props.recurringBookings && (
<span className="text-bookinglight inline text-gray-700">
<span className="underline">
<span className="underline" data-testid="reschedule-link">
<Link
href={`/reschedule/${seatReferenceUid || bookingInfo?.uid}`}
legacyBehavior>

322
apps/web/playwright/manage-booking-questions.e2e.ts
View File

@ -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<void>
callback: (page: Page) => Promise<void>,
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<typeof createUsersFixture> }) {
const user = await users.create(null, {
hasTeam: true,
@ -399,6 +343,16 @@ async function createAndLoginUserWithEventTypes({ users }: { users: ReturnType<t
return user;
}
async function rescheduleFromTheLinkOnPage({ page }: { page: Page }) {
await page.locator('[data-testid="reschedule-link"]').click();
await page.waitForLoadState();
await selectFirstAvailableTimeSlotNextMonth(page);
await page.waitForNavigation({
url: (url) => 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();

26
packages/features/bookings/lib/getBookingFields.ts
View File

@ -61,7 +61,7 @@ export const SystemFieldsEditability: Record<z.infer<typeof SystemField>, 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],
};
}
}

39
packages/features/bookings/lib/getBookingResponsesSchema.ts
View File

@ -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<typeof preprocess>[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<T extends z.ZodType>({
schema,
eventType,
isPartialSchema,
view: currentView,
}: {
schema: T;
isPartialSchema: boolean;
eventType: {
bookingFields: z.infer<typeof eventTypeBookingFields> & z.BRAND<"HAS_SYSTEM_FIELDS">;
};
view: View;
}): z.ZodType<z.infer<T>, z.infer<T>, z.infer<T>> {
const preprocessed = z.preprocess(
(responses) => {
@ -42,8 +54,15 @@ function preprocess<T extends z.ZodType>({
// 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<T extends z.ZodType>({
: 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;
}

5
packages/features/bookings/lib/handleNewBooking.ts
View File

@ -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",
}),
})
)

4
packages/features/form-builder/FormBuilder.tsx
View File

@ -335,6 +335,7 @@ export const FormBuilder = function FormBuilder({
}}
/>
<Button
data-testid="edit-field-action"
color="secondary"
onClick={() => {
editField(index, field);
@ -378,7 +379,7 @@ export const FormBuilder = function FormBuilder({
fieldIndex: -1,
})
}>
<DialogContent enableOverflow>
<DialogContent enableOverflow data-testid="edit-field-dialog">
<DialogHeader title={t("add_a_booking_question")} subtitle={t("form_builder_field_add_subtitle")} />
<div>
<Form
@ -472,6 +473,7 @@ export const FormBuilder = function FormBuilder({
render={({ field: { value, onChange } }) => {
return (
<BooleanToggleGroupField
data-testid="field-required"
disabled={fieldForm.getValues("editable") === "system"}
value={value}
onValueChange={(val) => {

11
packages/features/form-builder/FormBuilderFieldsSchema.ts
View File

@ -23,7 +23,7 @@ export const EditableSchema = z.enum([
"user", // Fully editable
"user-readonly", // All fields are readOnly.
]);
export type ALL_VIEWS = "ALL_VIEWS";
const fieldSchema = z.object({
name: z.string(),
// TODO: We should make at least one of `defaultPlaceholder` and `placeholder` required. Do the same for label.
@ -35,7 +35,14 @@ const fieldSchema = z.object({
*/
defaultLabel: z.string().optional(),
defaultPlaceholder: z.string().optional(),
views: z
.object({
label: z.string(),
id: z.string(),
description: z.string().optional(),
})
.array()
.optional(),
type: fieldTypeEnum,
options: z.array(z.object({ label: z.string(), value: z.string() })).optional(),
optionsInputs: z

4
packages/ui/components/form/toggleGroup/BooleanToggleGroup.tsx
View File

@ -16,6 +16,7 @@ export const BooleanToggleGroup = function BooleanToggleGroup({
disabled = false,
// eslint-disable-next-line @typescript-eslint/no-empty-function
onValueChange = () => {},
...passThrough
}: {
defaultValue?: boolean;
value?: boolean;
@ -44,7 +45,8 @@ export const BooleanToggleGroup = function BooleanToggleGroup({
onValueChange={(yesNoValue: "yes" | "no") => {
setYesNoValue(yesNoValue);
onValueChange(boolean(yesNoValue));
}}>
}}
{...passThrough}>
<ToggleGroupItemPrimitive
className={classNames(boolean(yesNoValue) ? selectedClass : unselectedClass)}
disabled={disabled}