From c3d56f736a4960ef5ff98c314954adfad27bea57 Mon Sep 17 00:00:00 2001 From: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Date: Thu, 24 Oct 2024 23:16:28 +0530 Subject: [PATCH] fix: insights event timeline perf and accuracy (#17218) * aggregate multiple calls to one for events timeline * raw query * fix inside wrong val * fix timeline view entirely * fix avg event duration * fix timeline * type-fix * fixes * remove unnecessary comments --- apps/web/public/static/locales/en/common.json | 3 +- .../components/AverageEventDurationChart.tsx | 5 +- .../components/BookingStatusLineChart.tsx | 5 +- .../features/insights/filters/DateSelect.tsx | 2 +- packages/features/insights/server/events.ts | 190 ++++++++++++++++-- .../features/insights/server/trpc-router.ts | 167 +++++++-------- 6 files changed, 274 insertions(+), 98 deletions(-) diff --git a/apps/web/public/static/locales/en/common.json b/apps/web/public/static/locales/en/common.json index 472020d62d..ed28d58de8 100644 --- a/apps/web/public/static/locales/en/common.json +++ b/apps/web/public/static/locales/en/common.json @@ -2558,7 +2558,8 @@ "no_support_needed": "No Support Needed?", "hide_support": "Hide Support", "event_ratings": "Average Ratings", - "event_no_show": "Host No Show", + "event_no_show": "No-Show (Host)", + "event_no_show_guest": "No-Show (Guest)", "recent_ratings": "Recent ratings", "no_ratings": "No ratings submitted", "no_ratings_description": "Add a workflow with 'Rating' to collect ratings after meetings", diff --git a/packages/features/insights/components/AverageEventDurationChart.tsx b/packages/features/insights/components/AverageEventDurationChart.tsx index 86888a4ef8..c7fcf30b01 100644 --- a/packages/features/insights/components/AverageEventDurationChart.tsx +++ b/packages/features/insights/components/AverageEventDurationChart.tsx @@ -1,5 +1,6 @@ import { Title } from "@tremor/react"; +import dayjs from "@calcom/dayjs"; import { useLocale } from "@calcom/lib/hooks/useLocale"; import { trpc } from "@calcom/trpc"; @@ -18,8 +19,8 @@ export const AverageEventDurationChart = () => { const initialConfigIsReady = !!(initialConfig?.teamId || initialConfig?.userId || initialConfig?.isAll); const { data, isSuccess, isPending } = trpc.viewer.insights.averageEventDuration.useQuery( { - startDate: startDate.toISOString(), - endDate: endDate.toISOString(), + startDate: dayjs.utc(startDate).toISOString(), + endDate: dayjs.utc(endDate).toISOString(), teamId, eventTypeId: selectedEventTypeId ?? undefined, memberUserId: selectedMemberUserId ?? undefined, diff --git a/packages/features/insights/components/BookingStatusLineChart.tsx b/packages/features/insights/components/BookingStatusLineChart.tsx index 216337cefe..688a306c1b 100644 --- a/packages/features/insights/components/BookingStatusLineChart.tsx +++ b/packages/features/insights/components/BookingStatusLineChart.tsx @@ -1,5 +1,6 @@ import { Title } from "@tremor/react"; +import dayjs from "@calcom/dayjs"; import { useLocale } from "@calcom/lib/hooks/useLocale"; import { trpc } from "@calcom/trpc"; @@ -33,8 +34,8 @@ export const BookingStatusLineChart = () => { } = trpc.viewer.insights.eventsTimeline.useQuery( { timeView: selectedTimeView, - startDate: startDate.toISOString(), - endDate: endDate.toISOString(), + startDate: dayjs.utc(startDate).toISOString(), + endDate: dayjs.utc(endDate).toISOString(), teamId: selectedTeamId ?? undefined, eventTypeId: selectedEventTypeId ?? undefined, userId: selectedUserId ?? undefined, diff --git a/packages/features/insights/filters/DateSelect.tsx b/packages/features/insights/filters/DateSelect.tsx index 3f340dae74..5f3874e6fb 100644 --- a/packages/features/insights/filters/DateSelect.tsx +++ b/packages/features/insights/filters/DateSelect.tsx @@ -39,7 +39,7 @@ export const DateSelect = () => { endDate = dayjs().endOf("day"); break; case "w": // Last 7 days - startDate = dayjs().subtract(7, "day").startOf("day"); + startDate = dayjs().subtract(1, "week").startOf("day"); endDate = dayjs().endOf("day"); break; case "t": // Last 30 days diff --git a/packages/features/insights/server/events.ts b/packages/features/insights/server/events.ts index 1f699ec2b1..6930fca490 100644 --- a/packages/features/insights/server/events.ts +++ b/packages/features/insights/server/events.ts @@ -1,13 +1,180 @@ import type { Dayjs } from "@calcom/dayjs"; import dayjs from "@calcom/dayjs"; import { readonlyPrisma as prisma } from "@calcom/prisma"; -import type { Prisma } from "@calcom/prisma/client"; +import { Prisma } from "@calcom/prisma/client"; import type { RawDataInput } from "./raw-data.schema"; type TimeViewType = "week" | "month" | "year" | "day"; +type DateRange = { + startDate: string; // ISO string format + endDate: string; // ISO string format + formattedDate: string; +}; + +type StatusAggregate = { + completed: number; + rescheduled: number; + cancelled: number; + noShowHost: number; + _all: number; + uncompleted: number; +}; + +type AggregateResult = { + [date: string]: StatusAggregate; +}; + +// Recursive function to convert a JSON condition object into SQL +// Helper type guard function to check if value has 'in' property +function isInCondition(value: any): value is { in: any[] } { + return typeof value === "object" && value !== null && "in" in value && Array.isArray(value.in); +} + +// Helper type guard function to check if value has 'gte' property +function isGteCondition(value: any): value is { gte: any } { + return typeof value === "object" && value !== null && "gte" in value; +} + +// Helper type guard function to check if value has 'lte' property +function isLteCondition(value: any): value is { lte: any } { + return typeof value === "object" && value !== null && "lte" in value; +} + +function buildSqlCondition(condition: any): string { + if (Array.isArray(condition.OR)) { + return `(${condition.OR.map(buildSqlCondition).join(" OR ")})`; + } else if (Array.isArray(condition.AND)) { + return `(${condition.AND.map(buildSqlCondition).join(" AND ")})`; + } else { + const clauses: string[] = []; + for (const [key, value] of Object.entries(condition)) { + if (isInCondition(value)) { + const valuesList = value.in.map((v) => `'${v}'`).join(", "); + clauses.push(`"${key}" IN (${valuesList})`); + } else if (isGteCondition(value)) { + clauses.push(`"${key}" >= '${value.gte}'`); + } else if (isLteCondition(value)) { + clauses.push(`"${key}" <= '${value.lte}'`); + } else { + const formattedValue = typeof value === "string" ? `'${value}'` : value; + clauses.push(`"${key}" = ${formattedValue}`); + } + } + return clauses.join(" AND "); + } +} + class EventsInsights { + static countGroupedByStatusForRanges = async ( + whereConditional: Prisma.BookingTimeStatusWhereInput, + startDate: Dayjs, + endDate: Dayjs, + timeView: "week" | "month" | "year" | "day" + ): Promise => { + // Determine the date truncation and date range based on timeView + const formattedStartDate = dayjs(startDate).format("YYYY-MM-DD HH:mm:ss"); + const formattedEndDate = dayjs(endDate).format("YYYY-MM-DD HH:mm:ss"); + const whereClause = buildSqlCondition(whereConditional); + + const data = await prisma.$queryRaw< + { periodStart: Date; bookingsCount: number; timeStatus: string; noShowHost: boolean }[] + >` + SELECT + "periodStart", + CAST(COUNT(*) AS INTEGER) AS "bookingsCount", + "timeStatus", + "noShowHost" + FROM ( + SELECT + DATE_TRUNC(${timeView}, "createdAt") AS "periodStart", + "timeStatus", + "noShowHost" + FROM + "BookingTimeStatus" + WHERE + "createdAt" BETWEEN ${formattedStartDate}::timestamp AND ${formattedEndDate}::timestamp + AND ${Prisma.raw(whereClause)} + ) AS truncated_dates + GROUP BY + "periodStart", + "timeStatus", + "noShowHost" + ORDER BY + "periodStart"; + `; + + const aggregate: AggregateResult = {}; + data.forEach(({ periodStart, bookingsCount, timeStatus, noShowHost }) => { + const formattedDate = dayjs(periodStart).format("MMM D, YYYY"); + + if (dayjs(periodStart).isAfter(endDate)) { + return; + } + + // Ensure the date entry exists in the aggregate object + if (!aggregate[formattedDate]) { + aggregate[formattedDate] = { + completed: 0, + rescheduled: 0, + cancelled: 0, + noShowHost: 0, + _all: 0, + uncompleted: 0, + }; + } + + // Add to the specific status count + const statusKey = timeStatus as keyof StatusAggregate; + aggregate[formattedDate][statusKey] += Number(bookingsCount); + + // Always add to the total count (_all) + aggregate[formattedDate]["_all"] += Number(bookingsCount); + + // Track no-show host counts separately + if (noShowHost) { + aggregate[formattedDate]["noShowHost"] += Number(bookingsCount); + } + }); + + // Generate a complete list of expected date labels based on the timeline + let current = dayjs(startDate); + const expectedDates: string[] = []; + + while (current.isBefore(endDate) || current.isSame(endDate)) { + const formattedDate = current.format("MMM D, YYYY"); + expectedDates.push(formattedDate); + + // Increment based on the selected timeView + if (timeView === "day") { + current = current.add(1, "day"); + } else if (timeView === "week") { + current = current.add(1, "week"); + } else if (timeView === "month") { + current = current.add(1, "month"); + } else if (timeView === "year") { + current = current.add(1, "year"); + } + } + + // Fill in any missing dates with zero counts + expectedDates.forEach((label) => { + if (!aggregate[label]) { + aggregate[label] = { + completed: 0, + rescheduled: 0, + cancelled: 0, + noShowHost: 0, + _all: 0, + uncompleted: 0, + }; + } + }); + + return aggregate; + }; + static countGroupedByStatus = async (where: Prisma.BookingTimeStatusWhereInput) => { const data = await prisma.bookingTimeStatus.groupBy({ where, @@ -122,24 +289,23 @@ class EventsInsights { } static getWeekTimeline(startDate: Dayjs, endDate: Dayjs): string[] { - const now = dayjs(); - const endOfDay = now.endOf("day"); - let pivotDate = dayjs(startDate); + let pivotDate = dayjs(endDate); const dates: string[] = []; - while (pivotDate.isBefore(endDate) || pivotDate.isSame(endDate)) { - const pivotAdded = pivotDate.add(6, "day"); - const weekEndDate = pivotAdded.isBefore(endOfDay) ? pivotAdded : endOfDay; - dates.push(pivotDate.format("YYYY-MM-DD")); + // Add the endDate as the last date in the timeline + dates.push(pivotDate.format("YYYY-MM-DD")); - if (pivotDate.isSame(endDate)) { + // Move backwards in 6-day increments until reaching or passing the startDate + while (pivotDate.isAfter(startDate)) { + pivotDate = pivotDate.subtract(7, "day"); + if (pivotDate.isBefore(startDate)) { break; } - - pivotDate = weekEndDate.add(1, "day"); + dates.push(pivotDate.format("YYYY-MM-DD")); } - return dates; + // Reverse the array to have the timeline in ascending order + return dates.reverse(); } static getMonthTimeline(startDate: Dayjs, endDate: Dayjs) { diff --git a/packages/features/insights/server/trpc-router.ts b/packages/features/insights/server/trpc-router.ts index 304ce8f454..84df945667 100644 --- a/packages/features/insights/server/trpc-router.ts +++ b/packages/features/insights/server/trpc-router.ts @@ -288,8 +288,8 @@ export const insightsRouter = router({ const baseWhereCondition = { ...whereConditional, createdAt: { - gte: new Date(startDate), - lte: new Date(endDate), + gte: dayjs(startDate).startOf("day").toDate(), + lte: dayjs(endDate).endOf("day").toDate(), }, }; @@ -417,11 +417,11 @@ export const insightsRouter = router({ userId: selfUserId, } = input; - const startDate = dayjs(startDateString); - const endDate = dayjs(endDateString); - const user = ctx.user; + // Convert to UTC without shifting the time zone + let startDate = dayjs.utc(startDateString).startOf("day"); + let endDate = dayjs.utc(endDateString).endOf("day"); - if (selfUserId && user?.id !== selfUserId) { + if (selfUserId && ctx.user?.id !== selfUserId) { throw new TRPCError({ code: "UNAUTHORIZED" }); } @@ -431,12 +431,17 @@ export const insightsRouter = router({ let timeView = inputTimeView; - if (timeView === "week") { - // Difference between start and end date is less than 14 days use day view - if (endDate.diff(startDate, "day") < 14) { - timeView = "day"; - } + // Adjust the timeView if the range is less than 14 days + if (timeView === "week" && endDate.diff(startDate, "day") < 14) { + timeView = "day"; } + + // Align startDate to the Monday of the week if timeView is 'week' + if (timeView === "week") { + startDate = startDate.day(1); // Set startDate to Monday + endDate = endDate.day(1).add(6, "day").endOf("day"); // Set endDate to Sunday of the same week + } + const r = await buildBaseWhereCondition({ teamId, eventTypeId, @@ -450,53 +455,59 @@ export const insightsRouter = router({ }, }); - let { whereCondition: whereConditional } = r; + const { whereCondition: whereConditional } = r; + const timeline = await EventsInsights.getTimeLine(timeView, startDate, endDate); - // Get timeline data - const timeline = await EventsInsights.getTimeLine(timeView, dayjs(startDate), dayjs(endDate)); - - // iterate timeline and fetch data if (!timeline) { return []; } const dateFormat: string = timeView === "year" ? "YYYY" : timeView === "month" ? "MMM YYYY" : "ll"; - const result = []; - for (const date of timeline) { + // Align date ranges consistently with weeks starting on Monday + const dateRanges = timeline.map((date) => { + let startOfRange = dayjs.utc(date).startOf(timeView); + let endOfRange = dayjs.utc(date).endOf(timeView); + + if (timeView === "week") { + startOfRange = dayjs.utc(date).day(1); // Start at Monday in UTC + endOfRange = startOfRange.add(6, "day").endOf("day"); // End at Sunday in UTC + } + + return { + startDate: startOfRange.toISOString(), + endDate: endOfRange.toISOString(), + formattedDate: startOfRange.format(dateFormat), // Align formatted date for consistency + }; + }); + // Fetch counts grouped by status for the entire range + const countsByStatus = await EventsInsights.countGroupedByStatusForRanges( + whereConditional, + startDate, + endDate, + timeView + ); + const result = dateRanges.map(({ formattedDate }) => { const EventData = { - Month: dayjs(date).format(dateFormat), + Month: formattedDate, Created: 0, Completed: 0, Rescheduled: 0, Cancelled: 0, "No-Show (Host)": 0, }; - const startOfEndOf = timeView; - let startDate = dayjs(date).startOf(startOfEndOf); - let endDate = dayjs(date).endOf(startOfEndOf); - if (timeView === "week") { - startDate = dayjs(date).startOf("day"); - endDate = dayjs(date).add(6, "day").endOf("day"); + + const countsForDateRange = countsByStatus[formattedDate]; + + if (countsForDateRange) { + EventData["Created"] = countsForDateRange["_all"] || 0; + EventData["Completed"] = countsForDateRange["completed"] || 0; + EventData["Rescheduled"] = countsForDateRange["rescheduled"] || 0; + EventData["Cancelled"] = countsForDateRange["cancelled"] || 0; + EventData["No-Show (Host)"] = countsForDateRange["noShowHost"] || 0; } - - whereConditional = { - ...whereConditional, - createdAt: { - gte: startDate.toISOString(), - lte: endDate.toISOString(), - }, - }; - - const countsByStatus = await EventsInsights.countGroupedByStatus(whereConditional); - - EventData["Created"] = countsByStatus["_all"]; - EventData["Completed"] = countsByStatus["completed"]; - EventData["Rescheduled"] = countsByStatus["rescheduled"]; - EventData["Cancelled"] = countsByStatus["cancelled"]; - EventData["No-Show (Host)"] = countsByStatus["noShowHost"]; - result.push(EventData); - } + return EventData; + }); return result; }), @@ -672,8 +683,8 @@ export const insightsRouter = router({ return []; } - const startDate = dayjs(startDateString); - const endDate = dayjs(endDateString); + const startDate = dayjs.utc(startDateString).startOf("day"); + const endDate = dayjs.utc(endDateString).endOf("day"); const { whereCondition: whereConditional } = await buildBaseWhereCondition({ teamId, @@ -695,43 +706,42 @@ export const insightsRouter = router({ return []; } - const dateFormat = "ll"; + const startOfEndOf = timeView === "year" ? "year" : timeView === "month" ? "month" : "week"; - const result = []; + const allBookings = await ctx.insightsDb.bookingTimeStatus.findMany({ + select: { + eventLength: true, + createdAt: true, + }, + where: { + ...whereConditional, + createdAt: { + gte: startDate.toDate(), + lte: endDate.toDate(), + }, + }, + }); + + const resultMap = new Map(); for (const date of timeLine) { - const EventData = { - Date: dayjs(date).format(dateFormat), - Average: 0, - }; - const startOfEndOf = timeView === "year" ? "year" : timeView === "month" ? "month" : "week"; - - const startDate = dayjs(date).startOf(startOfEndOf); - const endDate = dayjs(date).endOf(startOfEndOf); - - const bookingsInTimeRange = await ctx.insightsDb.bookingTimeStatus.findMany({ - select: { - eventLength: true, - }, - where: { - ...whereConditional, - createdAt: { - gte: startDate.toDate(), - lte: endDate.toDate(), - }, - }, - }); - - const avgDuration = - bookingsInTimeRange.reduce((acc, booking) => { - const duration = booking.eventLength || 0; - return acc + duration; - }, 0) / bookingsInTimeRange.length; - - EventData["Average"] = Number(avgDuration) || 0; - result.push(EventData); + resultMap.set(dayjs(date).startOf(startOfEndOf).format("ll"), { totalDuration: 0, count: 0 }); } + for (const booking of allBookings) { + const periodStart = dayjs(booking.createdAt).startOf(startOfEndOf).format("ll"); + if (resultMap.has(periodStart)) { + const current = resultMap.get(periodStart)!; + current.totalDuration += booking.eventLength || 0; + current.count += 1; + } + } + + const result = Array.from(resultMap.entries()).map(([date, { totalDuration, count }]) => ({ + Date: date, + Average: count > 0 ? totalDuration / count : 0, + })); + return result; }), membersWithMostBookings: userBelongsToTeamProcedure @@ -771,7 +781,6 @@ export const insightsRouter = router({ bookingWhere = { ...bookingWhere, - teamId, createdAt: { gte: dayjs(startDate).startOf("day").toDate(), lte: dayjs(endDate).endOf("day").toDate(), @@ -863,8 +872,6 @@ export const insightsRouter = router({ bookingWhere = { ...bookingWhere, - teamId, - eventTypeId, createdAt: { gte: dayjs(startDate).startOf("day").toDate(), lte: dayjs(endDate).endOf("day").toDate(),