From 90443d117da7758a47d5bc3f5034355f340ace22 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Mon, 20 Mar 2023 18:33:49 +0530 Subject: [PATCH] Improve validation and types --- apps/web/lib/getBooking.tsx | 28 ++++--------------- apps/web/pages/booking/[uid].tsx | 3 +- .../bookings/lib/getBookingResponsesSchema.ts | 21 +++++++++++++- .../bookings/lib/getCalEventResponses.ts | 6 ++-- packages/lib/getLabelValueMapFromResponses.ts | 8 +++++- .../trpc/server/routers/viewer/bookings.tsx | 13 ++++----- packages/types/Calendar.d.ts | 26 ++++++++--------- 7 files changed, 54 insertions(+), 51 deletions(-) diff --git a/apps/web/lib/getBooking.tsx b/apps/web/lib/getBooking.tsx index 8659dc25f4077c..77654a54d08772 100644 --- a/apps/web/lib/getBooking.tsx +++ b/apps/web/lib/getBooking.tsx @@ -1,9 +1,8 @@ import type { Prisma, PrismaClient } from "@prisma/client"; import type { z } from "zod"; -import { getBookingResponsesPartialSchema } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; +import { bookingResponsesDbSchema } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; import slugify from "@calcom/lib/slugify"; -import type { eventTypeBookingFields } from "@calcom/prisma/zod-utils"; type BookingSelect = { description: true; @@ -45,11 +44,7 @@ function getResponsesFromOldBooking( }; } -async function getBooking( - prisma: PrismaClient, - uid: string, - bookingFields: z.infer & z.BRAND<"HAS_SYSTEM_FIELDS"> -) { +async function getBooking(prisma: PrismaClient, uid: string) { const rawBooking = await prisma.booking.findFirst({ where: { uid, @@ -82,9 +77,7 @@ async function getBooking( return rawBooking; } - const booking = getBookingWithResponses(rawBooking, { - bookingFields, - }); + const booking = getBookingWithResponses(rawBooking); if (booking) { // @NOTE: had to do this because Server side cant return [Object objects] @@ -104,20 +97,11 @@ export const getBookingWithResponses = < }; }> >( - booking: T, - eventType: { - bookingFields: z.infer & z.BRAND<"HAS_SYSTEM_FIELDS">; - } + booking: T ) => { return { ...booking, - responses: getBookingResponsesPartialSchema({ - 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)), - }; + responses: bookingResponsesDbSchema.parse(booking.responses || getResponsesFromOldBooking(booking)), + } as Omit & { responses: z.infer }; }; export default getBooking; diff --git a/apps/web/pages/booking/[uid].tsx b/apps/web/pages/booking/[uid].tsx index 18d002d1ba3f1d..290c7865fc571e 100644 --- a/apps/web/pages/booking/[uid].tsx +++ b/apps/web/pages/booking/[uid].tsx @@ -1087,8 +1087,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) { }; } - const bookingInfo = getBookingWithResponses(bookingInfoRaw, eventTypeRaw); - + const bookingInfo = getBookingWithResponses(bookingInfoRaw); // @NOTE: had to do this because Server side cant return [Object objects] // probably fixable with json.stringify -> json.parse bookingInfo["startTime"] = (bookingInfo?.startTime as Date)?.toISOString() as unknown as Date; diff --git a/packages/features/bookings/lib/getBookingResponsesSchema.ts b/packages/features/bookings/lib/getBookingResponsesSchema.ts index a1734fe228f7fa..95789243ca5e81 100644 --- a/packages/features/bookings/lib/getBookingResponsesSchema.ts +++ b/packages/features/bookings/lib/getBookingResponsesSchema.ts @@ -9,6 +9,20 @@ type EventType = Parameters[0]["eventType"]; // eslint-disable-next-line @typescript-eslint/ban-types type View = ALL_VIEWS | (string & {}); +export const bookingResponse = z.union([ + z.string(), + z.boolean(), + z.string().array(), + z.object({ + optionValue: z.string(), + value: z.string(), + }), +]); + +export const bookingResponsesDbSchema = z.record(bookingResponse); + +const catchAllSchema = bookingResponsesDbSchema; + export const getBookingResponsesPartialSchema = ({ eventType, view, @@ -16,7 +30,7 @@ export const getBookingResponsesPartialSchema = ({ eventType: EventType; view: View; }) => { - const schema = bookingResponses.unwrap().partial().and(z.record(z.any())); + const schema = bookingResponses.unwrap().partial().and(catchAllSchema); return preprocess({ schema, eventType, isPartialSchema: true, view }); }; @@ -38,6 +52,9 @@ function preprocess({ view: currentView, }: { schema: T; + // It is useful when we want to prefill the responses with the partial values. Partial can be in 2 ways + // - Not all required fields are need to be provided for prefill. + // - Even a field response itself can be partial so the content isn't validated e.g. a field with type="phone" can be given a partial phone number(e.g. Specifying the country code like +91) isPartialSchema: boolean; eventType: { bookingFields: (z.infer & z.BRAND<"HAS_SYSTEM_FIELDS">) | null; @@ -48,6 +65,7 @@ function preprocess({ (responses) => { const parsedResponses = z.record(z.any()).nullable().parse(responses) || {}; const newResponses = {} as typeof parsedResponses; + // if eventType has been deleted, we won't have bookingFields and thus we can't preprocess or validate them. if (!eventType.bookingFields) return parsedResponses; eventType.bookingFields.forEach((field) => { const value = parsedResponses[field.name]; @@ -88,6 +106,7 @@ function preprocess({ }, schema.superRefine((responses, ctx) => { if (!eventType.bookingFields) { + // if eventType has been deleted, we won't have bookingFields and thus we can't validate the responses. return; } eventType.bookingFields.forEach((bookingField) => { diff --git a/packages/features/bookings/lib/getCalEventResponses.ts b/packages/features/bookings/lib/getCalEventResponses.ts index 882c7cb83e1f82..50c92b60f9c175 100644 --- a/packages/features/bookings/lib/getCalEventResponses.ts +++ b/packages/features/bookings/lib/getCalEventResponses.ts @@ -1,7 +1,7 @@ import type z from "zod"; import { SystemField } from "@calcom/features/bookings/lib/getBookingFields"; -import type { getBookingResponsesPartialSchema } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; +import type { bookingResponsesDbSchema } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; import type { eventTypeBookingFields } from "@calcom/prisma/zod-utils"; import type { CalendarEvent } from "@calcom/types/Calendar"; @@ -12,7 +12,7 @@ export const getCalEventResponses = ({ // If the eventType has been deleted and a booking is Accepted later on, then bookingFields will be null and we can't know the label of fields. So, we should store the label as well in the DB // Also, it is no longer straightforward to identify if a field is system field or not bookingFields: z.infer | null; - responses: z.infer>; + responses: z.infer; }) => { const calEventUserFieldsResponses = {} as NonNullable; const calEventResponses = {} as NonNullable; @@ -39,7 +39,7 @@ export const getCalEventResponses = ({ for (const [name, value] of Object.entries(responses)) { const isSystemField = SystemField.safeParse(name); - // Use name for Label because we don't have access to the label. + // Use name for Label because we don't have access to the label. This will not be needed once we start storing the label along with the response const label = name; if (!isSystemField.success) { diff --git a/packages/lib/getLabelValueMapFromResponses.ts b/packages/lib/getLabelValueMapFromResponses.ts index 124c8af35c9704..5897fc077c213e 100644 --- a/packages/lib/getLabelValueMapFromResponses.ts +++ b/packages/lib/getLabelValueMapFromResponses.ts @@ -1,11 +1,17 @@ +import type z from "zod"; + +import type { bookingResponse } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; import type { CalendarEvent } from "@calcom/types/Calendar"; export default function getLabelValueMapFromResponses(calEvent: CalendarEvent) { const { customInputs, userFieldsResponses } = calEvent; - let labelValueMap: Record = {}; + let labelValueMap: Record> = {}; if (userFieldsResponses) { for (const [, value] of Object.entries(userFieldsResponses)) { + if (!value.label) { + continue; + } labelValueMap[value.label] = value.value; } } else { diff --git a/packages/trpc/server/routers/viewer/bookings.tsx b/packages/trpc/server/routers/viewer/bookings.tsx index ea135b65de4900..ed8bd8d5e0549b 100644 --- a/packages/trpc/server/routers/viewer/bookings.tsx +++ b/packages/trpc/server/routers/viewer/bookings.tsx @@ -16,7 +16,7 @@ import { deleteScheduledEmailReminder } from "@calcom/ee/workflows/lib/reminders import { deleteScheduledSMSReminder } from "@calcom/ee/workflows/lib/reminders/smsReminderManager"; import { sendDeclinedEmails, sendLocationChangeEmails, sendRequestRescheduleEmail } from "@calcom/emails"; import { getBookingFieldsWithSystemFields } from "@calcom/features/bookings/lib/getBookingFields"; -import { getBookingResponsesPartialSchema } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; +import { bookingResponsesDbSchema } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; import { getCalEventResponses } from "@calcom/features/bookings/lib/getCalEventResponses"; import { handleConfirmation } from "@calcom/features/bookings/lib/handleConfirmation"; import getWebhooks from "@calcom/features/webhooks/lib/getWebhooks"; @@ -773,18 +773,14 @@ export const bookingsRouter = router({ scheduledJobs: true, }, }); + const bookingFields = bookingRaw.eventType ? getBookingFieldsWithSystemFields(bookingRaw.eventType) : null; + const booking = { ...bookingRaw, - responses: getBookingResponsesPartialSchema({ - eventType: { - bookingFields, - }, - // An existing booking can have data from any number of views, so the schema should consider ALL_VIEWS - view: "ALL_VIEWS", - }), + responses: bookingResponsesDbSchema.parse(bookingRaw.responses), eventType: bookingRaw.eventType ? { ...bookingRaw.eventType, @@ -850,6 +846,7 @@ export const bookingsRouter = router({ const attendeesList = await Promise.all(attendeesListPromises); + // TODO: Remove the usage of `bookingFields` in computing responses. We can do that by storing `label` with the response. Also, this would allow us to correctly show the label for a field even after the Event Type has been deleted. const { calEventUserFieldsResponses, calEventResponses } = getCalEventResponses({ bookingFields: booking.eventType?.bookingFields ?? null, responses: booking.responses, diff --git a/packages/types/Calendar.d.ts b/packages/types/Calendar.d.ts index 1e20cf0e8cbc79..f97079faa8bb07 100644 --- a/packages/types/Calendar.d.ts +++ b/packages/types/Calendar.d.ts @@ -3,7 +3,9 @@ import type { Dayjs } from "dayjs"; import type { calendar_v3 } from "googleapis"; import type { Time } from "ical.js"; import type { TFunction } from "next-i18next"; +import type z from "zod"; +import type { bookingResponse } from "@calcom/features/bookings/lib/getBookingResponsesSchema"; import type { Calendar } from "@calcom/features/calendars/weeklyview"; import type { TimeFormat } from "@calcom/lib/timeFormat"; import type { Frequency } from "@calcom/prisma/zod-utils"; @@ -129,6 +131,14 @@ export type AppsStatus = { warnings?: string[]; }; +type CalEventResponses = Record< + string, + { + label: string; + value: z.infer; + } +>; + // If modifying this interface, probably should update builders/calendarEvent files export interface CalendarEvent { type: string; @@ -164,22 +174,10 @@ export interface CalendarEvent { seatsPerTimeSlot?: number | null; // It has responses to all the fields(system + user) - responses?: Record< - string, - { - value: string | string[]; - label: string; - } - > | null; + responses?: CalEventResponses | null; // It just has responses to only the user fields. It allows to easily iterate over to show only user fields - userFieldsResponses?: Record< - string, - { - value: string | string[]; - label: string; - } - > | null; + userFieldsResponses?: CalEventResponses | null; } export interface EntryPoint {