Skip to content

Commit 6479adf

Browse files
fix: integer to text comparison in routing insights query (calcom#25019)
* fix: integer to text comparison in routing insights query Add explicit integer array cast to prevent PostgreSQL type mismatch error when comparing bookingUserId (integer) with user_id array values in getRoutedToPerPeriodData query * add e2e tests * refactor: remove LoadingInsight component and handle loading in ChartCard - Enhanced ChartCard to render default loading UI when isPending is true - Replaced all LoadingInsight usages with ChartCard that accepts isPending/isError props - Removed LoadingInsight component and updated exports - Updated 16 chart components to use the new pattern - ChartCard now shows spinner and skeleton title during loading state Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix: make children prop optional in ChartCard when isPending is true Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * refactor: remove unused loadingState prop from ChartCard - Remove loadingState prop and ChartLoadingState type export - Simplify computedLoadingState to derive state only from isPending/isError - No functional changes - loadingState was not being used by any components - data-loading-state attribute behavior remains unchanged for E2E tests Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * refactor: remove duplicate isPending early returns from chart components - Update all 16 chart components to use single ChartCard return pattern - Gate children rendering with !isPending && isSuccess && data checks - Prevents data processing code from executing during loading state - Improves code consistency and maintainability across all charts Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * refactor: remove redundant !isPending check from chart conditionals - Simplify conditional rendering to use just 'isSuccess && data' or 'isSuccess' - In TanStack Query, isSuccess and isPending are mutually exclusive - The !isPending check was redundant since isSuccess already implies !isPending - Applied to all 16 chart components for consistency - Components with safe defaults (data ?? []) use just 'isSuccess' - Components requiring data check use 'isSuccess && data' Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * revert the mistake * apply feedback * apply feedback * fix e2e --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 3c46c35 commit 6479adf

30 files changed

Lines changed: 794 additions & 404 deletions
Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
import { expect } from "@playwright/test";
2+
import { v4 as uuidv4 } from "uuid";
3+
4+
import { BookingStatus } from "@calcom/prisma/enums";
5+
6+
import { expectAllChartsToLoad, getAllChartIds } from "./lib/chart-helpers";
7+
import { test } from "./lib/fixtures";
8+
9+
test.afterEach(({ users }) => users.deleteAll());
10+
11+
test.describe("Insights > Charts Loading", () => {
12+
test.describe("Routing Insights", () => {
13+
test("all charts should load successfully with data", async ({ page, users, routingForms, prisma }) => {
14+
// Setup: Create routing form with complete data
15+
const owner = await users.create(
16+
{ name: "Chart Test Owner" },
17+
{
18+
hasTeam: true,
19+
isUnpublished: true,
20+
isOrg: true,
21+
hasSubteam: true,
22+
}
23+
);
24+
await owner.apiLogin();
25+
26+
const membership = await owner.getFirstTeamMembership();
27+
28+
// Create a routing form with fields
29+
const formName = "Chart Loading Test Form";
30+
const form = await routingForms.create({
31+
name: formName,
32+
userId: owner.id,
33+
teamId: membership.teamId,
34+
fields: [
35+
{
36+
type: "text",
37+
label: "Name",
38+
identifier: "name",
39+
required: true,
40+
},
41+
{
42+
type: "email",
43+
label: "Email",
44+
identifier: "email",
45+
required: true,
46+
},
47+
{
48+
type: "select",
49+
label: "Department",
50+
identifier: "department",
51+
required: true,
52+
options: [
53+
{
54+
id: uuidv4(),
55+
label: "Sales",
56+
},
57+
{
58+
id: uuidv4(),
59+
label: "Support",
60+
},
61+
],
62+
},
63+
],
64+
});
65+
66+
const fieldIds = (form.fields as Array<{ id: string }>).map((f) => f.id);
67+
68+
// Create users for routing
69+
const user1 = await users.create({ name: "Sales User" });
70+
const user2 = await users.create({ name: "Support User" });
71+
72+
// Add users to the team
73+
await prisma.membership.createMany({
74+
data: [
75+
{
76+
teamId: membership.teamId,
77+
userId: user1.id,
78+
role: "MEMBER",
79+
accepted: true,
80+
},
81+
{
82+
teamId: membership.teamId,
83+
userId: user2.id,
84+
role: "MEMBER",
85+
accepted: true,
86+
},
87+
],
88+
});
89+
90+
// Create bookings for both users
91+
const booking1 = await prisma.booking.create({
92+
data: {
93+
uid: `booking-${uuidv4()}`,
94+
title: "Sales Booking",
95+
startTime: new Date(),
96+
endTime: new Date(Date.now() + 60 * 60 * 1000),
97+
userId: user1.id,
98+
status: BookingStatus.ACCEPTED,
99+
},
100+
});
101+
102+
const booking2 = await prisma.booking.create({
103+
data: {
104+
uid: `booking-${uuidv4()}`,
105+
title: "Support Booking",
106+
startTime: new Date(),
107+
endTime: new Date(Date.now() + 60 * 60 * 1000),
108+
userId: user2.id,
109+
status: BookingStatus.ACCEPTED,
110+
},
111+
});
112+
113+
// Create form responses with bookings
114+
await prisma.app_RoutingForms_FormResponse.create({
115+
data: {
116+
formFillerId: "filler-1",
117+
formId: form.id,
118+
response: {
119+
[fieldIds[0]]: { label: "Name", value: "John Doe" },
120+
[fieldIds[1]]: { label: "Email", value: "john@example.com" },
121+
[fieldIds[2]]: { label: "Department", value: "Sales" },
122+
},
123+
routedToBookingUid: booking1.uid,
124+
},
125+
});
126+
127+
await prisma.app_RoutingForms_FormResponse.create({
128+
data: {
129+
formFillerId: "filler-2",
130+
formId: form.id,
131+
response: {
132+
[fieldIds[0]]: { label: "Name", value: "Jane Smith" },
133+
[fieldIds[1]]: { label: "Email", value: "jane@example.com" },
134+
[fieldIds[2]]: { label: "Department", value: "Support" },
135+
},
136+
routedToBookingUid: booking2.uid,
137+
},
138+
});
139+
140+
// Create a response without booking (for "Failed Bookings" chart)
141+
await prisma.app_RoutingForms_FormResponse.create({
142+
data: {
143+
formFillerId: "filler-3",
144+
formId: form.id,
145+
response: {
146+
[fieldIds[0]]: { label: "Name", value: "Bob Wilson" },
147+
[fieldIds[1]]: { label: "Email", value: "bob@example.com" },
148+
[fieldIds[2]]: { label: "Department", value: "Sales" },
149+
},
150+
routedToBookingUid: null,
151+
},
152+
});
153+
154+
// Navigate to insights routing page
155+
await page.goto("/insights/routing");
156+
await expect(page).toHaveURL(/\/insights\/routing/);
157+
158+
// Wait for the page to be fully loaded
159+
await page.waitForLoadState("networkidle");
160+
161+
// Get all chart IDs before checking
162+
const chartIds = await getAllChartIds(page);
163+
console.log(`[Routing] Found ${chartIds.length} charts: ${chartIds.join(", ")}`);
164+
165+
// Assert all charts load successfully
166+
await expectAllChartsToLoad(page, 20000); // 20 second timeout for all charts
167+
168+
// Verify specific expected charts are present and loaded
169+
const expectedCharts = ["stats", "routing-funnel", "routed-to-per-period", "failed-bookings-by-field"];
170+
171+
for (const chartId of expectedCharts) {
172+
const chart = page.locator(`[data-testid="chart-card"][data-chart-id="${chartId}"]`);
173+
await expect(chart).toBeVisible();
174+
await expect(chart).toHaveAttribute("data-loading-state", "loaded");
175+
}
176+
177+
// Cleanup
178+
await prisma.booking.delete({ where: { id: booking1.id } });
179+
await prisma.booking.delete({ where: { id: booking2.id } });
180+
});
181+
});
182+
183+
test.describe("Bookings Insights", () => {
184+
test("all charts should load successfully with booking data", async ({
185+
page,
186+
users,
187+
bookings,
188+
prisma,
189+
}) => {
190+
// Setup: Create users with team membership
191+
const owner = await users.create({ name: "Chart Test Owner" });
192+
const member = await users.create({ name: "Chart Test Member" });
193+
194+
// Create team and add both users
195+
const team = await prisma.team.create({
196+
data: {
197+
name: "test-insights-bookings",
198+
slug: `test-insights-bookings-${Date.now()}`,
199+
},
200+
});
201+
202+
await prisma.membership.createMany({
203+
data: [
204+
{
205+
userId: owner.id,
206+
teamId: team.id,
207+
accepted: true,
208+
role: "ADMIN",
209+
},
210+
{
211+
userId: member.id,
212+
teamId: team.id,
213+
accepted: true,
214+
role: "MEMBER",
215+
},
216+
],
217+
});
218+
219+
await owner.apiLogin();
220+
221+
// Create some bookings for the user
222+
const now = new Date();
223+
const tomorrow = new Date(now);
224+
tomorrow.setDate(tomorrow.getDate() + 1);
225+
226+
const dayAfterTomorrow = new Date(tomorrow);
227+
dayAfterTomorrow.setDate(dayAfterTomorrow.getDate() + 1);
228+
229+
const threeDaysFromNow = new Date(dayAfterTomorrow);
230+
threeDaysFromNow.setDate(threeDaysFromNow.getDate() + 1);
231+
232+
// Create bookings at different times to ensure unique idempotency keys
233+
await bookings.create(owner.id, owner.username, owner.eventTypes[0].id, {
234+
status: BookingStatus.ACCEPTED,
235+
startTime: now,
236+
endTime: tomorrow,
237+
});
238+
239+
await bookings.create(owner.id, owner.username, owner.eventTypes[0].id, {
240+
status: BookingStatus.ACCEPTED,
241+
startTime: tomorrow,
242+
endTime: dayAfterTomorrow,
243+
});
244+
245+
await bookings.create(owner.id, owner.username, owner.eventTypes[0].id, {
246+
status: BookingStatus.CANCELLED,
247+
startTime: dayAfterTomorrow,
248+
endTime: threeDaysFromNow,
249+
});
250+
251+
// Navigate to insights bookings page
252+
await page.goto("/insights");
253+
await expect(page).toHaveURL(/\/insights/);
254+
255+
// Wait for the page to be fully loaded
256+
await page.waitForLoadState("networkidle");
257+
258+
// Get all chart IDs before checking
259+
const chartIds = await getAllChartIds(page);
260+
console.log(`[Bookings] Found ${chartIds.length} charts: ${chartIds.join(", ")}`);
261+
262+
// Assert all charts load successfully
263+
await expectAllChartsToLoad(page, 20000); // 20 second timeout for all charts
264+
265+
// Verify specific expected charts are present and loaded
266+
const expectedCharts = [
267+
"events",
268+
"performance",
269+
"event-trends",
270+
"no-show-hosts-over-time",
271+
"csat-over-time",
272+
"bookings-by-hour",
273+
"average-event-duration",
274+
];
275+
276+
for (const chartId of expectedCharts) {
277+
const chart = page.locator(`[data-testid="chart-card"][data-chart-id="${chartId}"]`);
278+
// Chart should exist and be loaded
279+
await expect(chart).toHaveCount(1);
280+
await expect(chart).toHaveAttribute("data-loading-state", "loaded");
281+
}
282+
283+
// Verify no charts are in error state
284+
const errorCharts = page.locator('[data-testid="chart-card"][data-loading-state="error"]');
285+
await expect(errorCharts).toHaveCount(0);
286+
});
287+
});
288+
});

apps/web/playwright/insights.e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ test.describe("Insights", async () => {
272272

273273
for (const title of expectedChartTitles) {
274274
const chartCard = page
275-
.locator("[data-testid='panel-card'] h2")
275+
.locator("[data-testid='chart-card'] h2")
276276
.filter({ hasText: new RegExp(`^${title}$`) });
277277
await expect(chartCard).toBeVisible();
278278
}

0 commit comments

Comments
 (0)