feat: add attendee filters to /insights/routing (+ use RoutingFormResponseDenormalized) #21297
Conversation
eunjae-lee
force-pushed
the
eunjae/cal-5755-use-routingformresponsedenormalized-on-insightsrouting
branch
from
7d9028f to
7f0e10c
Compare
eunjae-lee
changed the title
|
This PR is being marked as stale due to inactivity. |
E2E results are ready! |
|
This PR is being marked as stale due to inactivity. |
eunjae-lee
force-pushed
the
eunjae/cal-5755-use-routingformresponsedenormalized-on-insightsrouting
branch
from
2ee1249 to
5fd1f41
Compare
eunjae-lee
changed the base branch from
main
to
eunjae/cal-5986-drop-off-funnel-chart-for-insightsrouting
|
✅ No security or compliance issues detected. Reviewed everything up to ea2be2e. Security Overview
Detected Code Changes
Reply to this PR with |
eunjae-lee
force-pushed
the
eunjae/cal-5755-use-routingformresponsedenormalized-on-insightsrouting
branch
from
60b066e to
6f306ed
Compare
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/19/25)1 reviewer was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/insights/server/routing-events.ts (1)
241-258: Risk of SQL injection via string-concatenated WHERE clauses — parameterize conditions.
routingFormIdis interpolated into raw SQL strings and then injected withPrisma.raw(...). Use parameterizedPrisma.sqlfragments andPrisma.jointo safely compose conditions.Apply these patches to the three occurrences:
@@ - const teamConditions = []; + const teamConditions: Prisma.Sql[] = []; @@ - // @ts-expect-error it doesn't exist but TS isn't smart enough when it's a number or int filter - if (formsWhereCondition.teamId?.in) { - // @ts-expect-error it doesn't exist but TS isn't smart enough when it's a number or int filter - teamConditions.push(`f."teamId" IN (${formsWhereCondition.teamId.in.join(",")})`); - } - // @ts-expect-error it doesn't exist but TS isn't smart enough when it's a number or int filter - if (!formsWhereCondition.teamId?.in && userId) { - teamConditions.push(`f."userId" = ${userId}`); - } - if (routingFormId) { - teamConditions.push(`f.id = '${routingFormId}'`); - } + if (formsWhereCondition.teamId && "in" in formsWhereCondition.teamId && Array.isArray(formsWhereCondition.teamId.in)) { + teamConditions.push(Prisma.sql`f."teamId" IN (${Prisma.join(formsWhereCondition.teamId.in)})`); + } else if (userId) { + teamConditions.push(Prisma.sql`f."userId" = ${userId}`); + } + if (routingFormId) { + teamConditions.push(Prisma.sql`f.id = ${routingFormId}`); + } @@ - const whereClause = teamConditions.length - ? Prisma.sql`AND ${Prisma.raw(teamConditions.join(" AND "))}` - : Prisma.sql``; + const whereClause = teamConditions.length + ? Prisma.sql`AND ${Prisma.join(teamConditions, Prisma.sql` AND `)}` + : Prisma.empty;Repeat the same pattern in routedToPerPeriod() and routedToPerPeriodCsv() blocks (lines indicated in this comment).
This removes string concatenation and ensures proper value binding.
Also applies to: 467-486, 720-741
♻️ Duplicate comments (2)
packages/features/insights/server/routing-events.ts (1)
180-188: Filter-then-reduce for attendee indexing — resolved as per prior review.Filtering before reduce ensures contiguous "Attendee N" numbering. This addresses the earlier suggestion to avoid double iteration while keeping correct indices; correct choice here given the requirements.
packages/features/insights/components/routing/RoutingFormResponsesTable.tsx (1)
87-90: Verify backend filter support for attendeeName/attendeeEmail/attendeePhoneNew hidden columns are perfect for filter-only fields. Ensure the service maps these IDs to the appropriate EXISTS subqueries (including phone).
See my earlier verification comment in the integration tests with scripts to confirm service support.
🧹 Nitpick comments (17)
packages/features/insights/components/BookedByCell.tsx (1)
23-47: Harden key generation and label fallback; drop redundant tooltip nulling.
- Keys can collide if attendee.email is missing/undefined; include a stable fallback (name or index).
- Badge can render empty if name is missing; fallback to email or a translated anonymous label.
content={tooltipContent || undefined}is redundant sincetooltipContentis always a ReactNode.Apply this diff:
- {attendees.map((attendee) => { + {attendees.map((attendee, index) => { const tooltipContent = ( <div className="space-y-1"> {attendee.email && ( <div> {t("email")}: {attendee.email} </div> )} {attendee.phoneNumber && ( <div> {t("phone")}: {attendee.phoneNumber} </div> )} </div> ); return ( - <CellWithOverflowX key={`${cellId}-${attendee.email}-${rowId}`} className="w-[200px]"> - <Tooltip content={tooltipContent || undefined}> + <CellWithOverflowX key={`${cellId}-${attendee.email ?? attendee.name ?? index}-${rowId}`} className="w-[200px]"> + <Tooltip content={tooltipContent}> <Badge variant="gray" className="whitespace-nowrap"> - {attendee.name} + {attendee.name || attendee.email || t("anonymous_user")} </Badge> </Tooltip> </CellWithOverflowX> ); })}Optional: to avoid empty tooltips for attendees without email/phone, conditionally render Tooltip only when either is present.
packages/features/insights/server/routing-events.ts (4)
126-131: Type ReturnType for class method: prefer InstanceType pattern for accuracy.
ReturnType<typeof InsightsRoutingService.prototype.getTableData>can be brittle. Use the method type off an instance to avoid prototype pitfalls.- }: { - headersPromise: ReturnType<typeof RoutingEventsInsights.getRoutingFormHeaders>; - dataPromise: ReturnType<typeof InsightsRoutingService.prototype.getTableData>; - }) { + }: { + headersPromise: ReturnType<typeof RoutingEventsInsights.getRoutingFormHeaders>; + dataPromise: ReturnType<InstanceType<typeof InsightsRoutingService>["getTableData"]>; + }) {
134-159: Minor cleanup: remove redundant fallbacks.
bookingAttendeesis already defaulted to[]; the subsequent|| []is redundant..reduce(..., {})always returns an object; the final|| {}is redundant.- const bookingAttendees = item.bookingAttendees || []; + const bookingAttendees = item.bookingAttendees || []; @@ - ...((bookingAttendees || []) + ...(bookingAttendees .filter( (attendee): attendee is { name: string; email: string; timeZone: string | null } => typeof attendee.name === "string" && typeof attendee.email === "string" ) .reduce((acc, attendee, index) => { acc[`Attendee ${index + 1}`] = `${attendee.name} (${attendee.email})`; return acc; - }, {} as Record<string, string>) || {}), + }, {} as Record<string, string>)),Also applies to: 180-189
13-16: Remove unused types/imports.
RoutingFormStatsInputandRoutingFormStatsFilterappear unused after the refactor. Clean them up to avoid confusion.-import type { RoutingFormStatsInput } from "@calcom/features/insights/server/raw-data.schema"; @@ -type RoutingFormStatsFilter = RoutingFormStatsInput & { - organizationId: number | null; -};Also applies to: 26-28
160-189: CSV attendee details: consider including phone/timezone.Optional: In addition to "Attendee N" name/email, consider adding phone and/or timezone columns if available to match the UI tooltip richness and support downstream analysis.
packages/features/insights/server/raw-data.schema.ts (4)
42-49: Scoped insights input: good direction; consider tightening invariants between scope and selectedTeamIdThe new, scoped input surface is clear and aligns with the PR goals. To prevent invalid combinations from reaching the service, add schema-level invariants (e.g., require selectedTeamId when scope === "team", and disallow it when scope === "user"). This avoids pushing avoidable branching into the service layer.
69-76: Enforce scope/selectedTeamId invariants at validation timeAdd a superRefine to ensure only valid combinations are accepted.
export const insightsRoutingServiceInputSchema = z.object({ scope: z.union([z.literal("user"), z.literal("team"), z.literal("org")]), selectedTeamId: z.number().optional(), columnFilters: z.array(ZColumnFilter).optional(), startDate: z.string(), endDate: z.string(), -}) satisfies z.ZodType<InsightsRoutingServiceInput>; +}).superRefine((val, ctx) => { + if (val.scope === "team" && (val.selectedTeamId === undefined || val.selectedTeamId === null)) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: "selectedTeamId is required when scope is 'team'" }); + } + if (val.scope === "user" && val.selectedTeamId != null) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: "selectedTeamId must be omitted when scope is 'user'" }); + } +}) satisfies z.ZodType<InsightsRoutingServiceInput>;
77-83: Add integer and lower-bound constraints to paginationMake offset/limit strictly non-negative integers and enforce a minimum page size of 1.
export const insightsRoutingServicePaginatedInputSchema = z.object({ ...insightsRoutingServiceInputSchema.shape, - offset: z.number(), - limit: z.number().max(100), + offset: z.number().int().min(0), + limit: z.number().int().min(1).max(100), sorting: z.array(ZSorting).optional(), }) satisfies z.ZodType<InsightsRoutingServicePaginatedInput>;
69-76: Reduce duplication with routingRepositoryBaseInputSchemainsightsRoutingServiceInputSchema and routingRepositoryBaseInputSchema share the same shape. Consider extracting a single base schema and reusing it to DRY things up.
Example:
const BaseRoutingScopeSchema = z.object({ scope: z.union([z.literal("user"), z.literal("team"), z.literal("org")]), selectedTeamId: z.number().optional(), startDate: z.string(), endDate: z.string(), columnFilters: z.array(ZColumnFilter).optional(), }); export const insightsRoutingServiceInputSchema = BaseRoutingScopeSchema; export const routingRepositoryBaseInputSchema = BaseRoutingScopeSchema;packages/features/insights/components/routing/RoutingFormResponsesTable.tsx (1)
64-67: Avoid UI flicker on pagination/sorting with keepPreviousDataConsider enabling keepPreviousData on the query to reduce table flicker when paging/sorting.
You can pass options as the second argument:
const { data, isPending } = trpc.viewer.insights.routingFormResponses.useQuery( { startDate, endDate, sorting, limit, offset, scope, selectedTeamId, columnFilters, }, { keepPreviousData: true } );packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx (1)
47-50: De-dupe pagination limit with a shared constant to avoid driftBATCH_SIZE=100 matches the server’s zod max(100). To prevent future drift, consider hoisting a shared constant (or importing from a shared config) used by both client and server.
Example:
// shared/insights/constants.ts export const INSIGHTS_ROUTING_MAX_PAGE_SIZE = 100;Then:
- Use INSIGHTS_ROUTING_MAX_PAGE_SIZE in the zod schema’s .max(...) and on the client for BATCH_SIZE.
packages/features/insights/hooks/useInsightsColumns.tsx (2)
111-151: Invisible attendee filters: server alignment looks good; consider operator constraints (optional)These hidden columns correctly expose client-side filter UI and map to server-side filter IDs (attendeeName, attendeeEmail, attendeePhone). If you want to reduce accidental “equals” usage for free-text, you could restrict allowed operators to contains/startsWith.
Example tweak (if desired):
- filter: { - type: ColumnFilterType.TEXT, - }, + filter: { + type: ColumnFilterType.TEXT, + textOptions: { allowedOperators: ["contains", "startsWith"] }, + },
187-195: Minor: avoid reassigningvalues(readability)You can simplify by deriving
valuesin a single expression or small helper to reduce branching.- let values; - - if (isMultiSelect) { - values = info.getValue()?.valueStringArray; - } else if (isNumber) { - values = info.getValue()?.valueNumber; - } else { - values = info.getValue()?.valueString; - } + const field = info.getValue(); + const values = isMultiSelect + ? field?.valueStringArray + : isNumber + ? field?.valueNumber + : field?.valueString;packages/features/insights/server/trpc-router.ts (1)
846-869: Normalize nullable filters for download endpointFor consistency with the paginated list call and to harden against future filter-shape changes, coalesce your optional fields to
nulland guard theformIdextraction with the existing single-select type guard:• File packages/features/insights/server/trpc-router.ts (lines 846–869)
• Import your project’sisSingleSelectFilterValuehelper from the data-table filter utilitiesProposed diff:
const headersPromise = RoutingEventsInsights.getRoutingFormHeaders({ - userId: ctx.user.id, - teamId: input.selectedTeamId, + userId: ctx.user.id, + teamId: input.selectedTeamId ?? null, isAll: input.scope === "org", - organizationId: ctx.user.organizationId, - routingFormId: (input.columnFilters || []) - .find((f) => f.id === "formId") - ?.value?.data as string | undefined, + organizationId: ctx.user.organizationId ?? null, + routingFormId: + isSingleSelectFilterValue( + (input.columnFilters || []).find((f) => f.id === "formId")?.value + )?.data ?? null, });These changes are optional style/refactors to keep your calls aligned and safe.
packages/lib/server/service/InsightsRoutingBaseService.ts (3)
379-399: Stabilize pagination ordering with deterministic tiebreaker (optional)Offset pagination can produce duplicates/skips if the primary sort has many ties. Consider appending a secondary ORDER BY "id" DESC to stabilize results.
- if (orderByParts.length === 0) { - return Prisma.sql`ORDER BY "createdAt" DESC`; - } + if (orderByParts.length === 0) { + return Prisma.sql`ORDER BY "createdAt" DESC, "id" DESC`; + } @@ - return Prisma.sql`ORDER BY ${orderByParts.reduce((acc, part, index) => { + const combined = orderByParts.reduce((acc, part, index) => { if (index === 0) return part; return Prisma.sql`${acc}, ${part}`; - })}`; + }); + // Append deterministic tiebreaker + return Prisma.sql`ORDER BY ${combined}, "id" DESC`;
617-655: Use explicit text[] cast for multi-select filterThe
valueStringArraycolumn is a Postgrestext[], so the RHS should be an array literal cast totext[]to ensure the planner infers the correct type. For example, in yourbuildFormFieldSqlCondition:- AND rrf."valueStringArray" @> ${filterValue.data.map(String)} + AND rrf."valueStringArray" @> ARRAY[${Prisma.join(filterValue.data.map(String))}]::text[]This ensures the parameter is treated as a
text[]array.
241-342: Preserve original bookingStatus casingTo avoid mismatches with the lowercase Prisma enum (
cancelled,accepted, etc.) and downstream UI/filters, drop the UPPER() call so we return the raw DB value:--- a/packages/lib/server/service/InsightsRoutingBaseService.ts +++ b/packages/lib/server/service/InsightsRoutingBaseService.ts @@ lines 241-342 - UPPER(rfrd."bookingStatus"::text) as "bookingStatus", + rfrd."bookingStatus" as "bookingStatus",
- File: packages/lib/server/service/InsightsRoutingBaseService.ts
- Method: getTableData → SELECT projection
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/web/public/static/locales/en/common.json(1 hunks)packages/features/insights/components/BookedByCell.tsx(2 hunks)packages/features/insights/components/routing/RoutingFormResponsesTable.tsx(3 hunks)packages/features/insights/components/routing/RoutingKPICards.tsx(1 hunks)packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx(2 hunks)packages/features/insights/hooks/useInsightsColumns.tsx(3 hunks)packages/features/insights/hooks/useInsightsParameters.ts(1 hunks)packages/features/insights/lib/types.ts(1 hunks)packages/features/insights/server/raw-data.schema.ts(2 hunks)packages/features/insights/server/routing-events.ts(3 hunks)packages/features/insights/server/trpc-router.ts(2 hunks)packages/lib/server/service/InsightsRoutingBaseService.ts(10 hunks)packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
📚 Learning: 2025-07-15T12:58:40.539Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.539Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
Applied to files:
packages/features/insights/hooks/useInsightsParameters.tspackages/features/insights/components/routing/RoutingKPICards.tsxpackages/features/insights/filters/Download/RoutingFormResponsesDownload.tsxpackages/features/insights/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/hooks/useInsightsColumns.tsxpackages/features/insights/server/raw-data.schema.tspackages/lib/server/service/InsightsRoutingBaseService.tspackages/features/insights/server/routing-events.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/features/insights/components/routing/RoutingKPICards.tsxpackages/features/insights/filters/Download/RoutingFormResponsesDownload.tsxpackages/features/insights/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/components/BookedByCell.tsxpackages/features/insights/server/raw-data.schema.tspackages/lib/server/service/InsightsRoutingBaseService.tspackages/features/insights/server/trpc-router.tspackages/features/insights/server/routing-events.ts
📚 Learning: 2025-07-15T13:02:17.403Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.403Z
Learning: In the InsightsRoutingService (packages/lib/server/service/insightsRouting.ts), multi-select filter data is already validated by zod before reaching the buildFormFieldSqlCondition method, so null/undefined values are not present in filterValue.data arrays and don't need to be filtered out.
Applied to files:
packages/lib/server/service/InsightsRoutingBaseService.tspackages/features/insights/server/routing-events.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (17)
apps/web/public/static/locales/en/common.json (1)
3475-3475: Added locale key "phone" — LGTMThe new translation key is consistent with existing keys (e.g., "email") and supports the attendee tooltip addition.
packages/features/insights/hooks/useInsightsParameters.ts (1)
54-55: No duplicate filter application detected
All dedicated parameters (bookingUserId,eventTypeId,formId) are consumed only via the explicituseFilterValuehooks in each component/service. ThecolumnFiltersarray produced byuseColumnFiltersis entirely driven by the DataTable machinery and is never read by the InsightsRoutingService or any downstream query builder. In other words:
useInsightsParametersreturns both dedicated values (e.g.eventTypeId,routingFormId) andcolumnFilters.- Each component passes its dedicated values directly into its service calls.
- DataTable consumes only the
columnFiltersfor its own UI‐driven queries.- The backend service (
InsightsRoutingService) does not referencecolumnFiltersat all.You can safely expose those additional columns without risking double‐filtering.
packages/features/insights/lib/types.ts (1)
9-16: ZResponse switched to primitives — verify all consumers (.value/.label → primitive)*ZResponseMultipleValues/Single/Text/Numeric now return raw primitives (string | string[] | number). I scanned packages/features/insights and did not find direct response[fieldId].value/.label usages inside insights, but the repo contains multiple remaining accesses that will break and need updating.
Files to review (definitive matches from the scan):
- packages/lib/server/service/routingForm/responseData/findFieldValueByIdentifier.ts — uses data.response[field.id]?.value
- packages/lib/server/service/routingForm/responseData/parseRoutingFormResponse.test.ts — tests assert parsed.response[...].value / .label
- packages/lib/server/repository/booking.ts — const responseValue = response[fieldId].value
- packages/lib/server/getLuckyUser.ts — uses fieldResponse.value for selectedOptionIds
- packages/lib/getLabelValueMapFromResponses.ts — uses value.label and value.value to build maps
- packages/prisma/zod-utils.ts — .transform((foo) => (foo as any).value ...)
- packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts — branches that read field.value
- (Inspect) packages/features/insights/server/routing-events.ts — uses field.valueString / field.valueStringArray and header.options?.find(...).label — verify these mappings align with the new primitive shape
Actionable next steps:
- Replace occurrences of resp[fieldId].value → resp[fieldId] (and remove any .label use), update download mappers/renderers/filters and tests accordingly.
- Run full test suite after changes.
If you want, I can draft a simple codemod (jscodeshift / sed) to update straightforward patterns (e.g., resp[key].value → resp[key], label/value pair mappings) — tell me which tools you prefer.
packages/features/insights/server/raw-data.schema.ts (1)
51-55: LGTM: clear separation for paginated inputAdding an explicit paginated variant keeps concerns tidy and aligns with the download/table paths.
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts (1)
1083-1083: LGTM: alias consistency with rfrd in attendee join conditionSwitching the WHERE join to use rfrd."bookingUid" matches the service’s SQL generation and keeps tests aligned.
packages/features/insights/components/routing/RoutingKPICards.tsx (1)
14-14: LGTM: KPIs now use scoped inputs (scope/selectedTeamId)Payload changes match the new server input shape and keep stats in sync with table filters.
Also applies to: 20-22
packages/features/insights/components/routing/RoutingFormResponsesTable.tsx (1)
38-39: LGTM: parameters now include scope/selectedTeamId for table dataKeeping legacy parameters for headers while switching the main data query to scoped inputs is a pragmatic, low-risk transition.
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx (1)
30-31: LGTM: download payload aligned to scoped inputsUsing scope/selectedTeamId keeps the download path consistent with table queries and server schemas.
packages/features/insights/hooks/useInsightsColumns.tsx (4)
106-109: BookedByCell wiring looks correctPassing attendees and rowId is consistent with the new table shape. No issues.
182-182: Accessor refactor to row.fields.find is correctSwitching to lookup by fieldId matches the new denormalized row shape. The cell handles undefined gracefully via zod safeParse.
203-204: Good normalization for select valuesNormalizing result.data to an array keeps ResponseValueCell simple. LGTM.
213-213: Title/content update for text/number fields is appropriateUsing String(result.data) avoids previous nested value access; aligns with new schema.
packages/features/insights/server/trpc-router.ts (2)
829-834: Service + input schema swap for routingFormsByStatus is correctUsing insightsRoutingServiceInputSchema and getRoutingFormStats() aligns with the new service interface.
836-844: Pagination endpoint correctly switched to service.getTableDataThe wiring of sorting/limit/offset into the service is clear and consistent with the new schema.
packages/lib/server/service/InsightsRoutingBaseService.ts (3)
55-96: New InsightsRoutingTableItem type looks comprehensive and alignedCovers all columns used by the table, attendees, and fields payloads. Good foundation for strongly typed consumers.
99-129: Allowlist for sortable columns is a solid guardPrevents ORDER BY injection and keeps sorting bounded to known fields.
475-500: Attendee filters: good isolation and SQL safetySwitching on the column avoids dynamic column interpolation; EXISTS with a proper join on booking uid is correct. LGTM.
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
Show resolved
Hide resolved
eunjae-lee
changed the title
There was a problem hiding this comment.
All the removed lines here are refactored and moved to insightsRoutingBaseService.ts
| private buildAttendeeSqlCondition( | ||
| filterValue: TextFilterValue, | ||
| attendeeColumn: "name" | "email" | "phone" | ||
| ): Prisma.Sql | null { |
There was a problem hiding this comment.
We used to have one single attendee condition that checks both name and email. Now we have split it into explicit three filters (attendee name, attendee email, attendee phone number)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/features/insights/server/routing-events.ts (3)
233-251: Parameterize raw SQL to avoid injection (routingFormId and team conditions)routingFormId is interpolated into a raw string: teamConditions.push(
f.id = '${routingFormId}'). This is vulnerable if routingFormId is not fully trusted. Use Prisma.sql parameters for all dynamic parts (routingFormId, userId, teamId list).Proposed refactor:
- const teamConditions = []; - - // @ts-expect-error it doesn't exist but TS isn't smart enough when it's a number or int filter - if (formsWhereCondition.teamId?.in) { - // @ts-expect-error it doesn't exist but TS isn't smart enough when it's a number or int filter - teamConditions.push(`f."teamId" IN (${formsWhereCondition.teamId.in.join(",")})`); - } - // @ts-expect-error it doesn't exist but TS isn't smart enough when it's a number or int filter - if (!formsWhereCondition.teamId?.in && userId) { - teamConditions.push(`f."userId" = ${userId}`); - } - if (routingFormId) { - teamConditions.push(`f.id = '${routingFormId}'`); - } - - const whereClause = teamConditions.length - ? Prisma.sql`AND ${Prisma.raw(teamConditions.join(" AND "))}` - : Prisma.sql``; + const whereFragments: Prisma.Sql[] = []; + if (Array.isArray((formsWhereCondition as any).teamId?.in)) { + whereFragments.push( + Prisma.sql`f."teamId" IN (${Prisma.join((formsWhereCondition as any).teamId.in)})` + ); + } else if (userId) { + whereFragments.push(Prisma.sql`f."userId" = ${userId}`); + } + if (routingFormId) { + whereFragments.push(Prisma.sql`f.id = ${routingFormId}`); + } + const whereClause = + whereFragments.length > 0 + ? whereFragments.slice(1).reduce( + (acc, frag) => Prisma.sql`${acc} AND ${frag}`, + Prisma.sql`AND ${whereFragments[0]}` + ) + : Prisma.empty;Repeat similar hardening wherever dynamic WHERE fragments are built (see below).
457-471: Same injection concern in routedToPerPeriod team/routingFormId conditionsThe same string-based condition building appears here. Parameterize with Prisma.sql/Prisma.join as suggested above to avoid injection and improve safety.
Patch pattern identical to the previous comment: replace the string array and join with a Prisma.Sql[] accumulation and reduction into a single whereClause.
712-726: Same injection concern in routedToPerPeriodCsv team/routingFormId conditionsApply the same parameterization fix here. Avoid interpolating identifiers/values directly into raw SQL strings.
♻️ Duplicate comments (1)
packages/features/insights/server/routing-events.ts (1)
175-181: Keeping attendee indices contiguous is intentional and correctFilter-first then reduce ensures Attendee 1..N without gaps even when some entries have missing name/email. This addresses the earlier indexing concern discussed in prior reviews.
🧹 Nitpick comments (1)
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx (1)
72-78: Optional UX: show “no data to download” instead of generating empty CSVWhen total is 0, we can surface a friendly toast and exit early rather than downloading an empty file.
Apply this diff:
- // Continue fetching remaining batches + // Continue fetching remaining batches while (totalRecords > 0 && allData.length < totalRecords) { offset += BATCH_SIZE; const result = await fetchBatch(offset); allData = [...allData, ...result.data]; const currentProgress = Math.min(Math.round((allData.length / totalRecords) * 100), 99); showProgressToast(currentProgress); } - if (allData.length >= totalRecords) { + if (totalRecords === 0) { + showToast(t("no_data_to_download"), "info"); + return; + } + if (allData.length >= totalRecords) { showProgressToast(100); // Set to 100% before actual download const filename = `RoutingFormResponses-${dayjs(startDate).format("YYYY-MM-DD")}-${dayjs( endDate ).format("YYYY-MM-DD")}.csv`; downloadAsCsv(allData as Record<string, unknown>[], filename); }Note: ensure the "no_data_to_download" translation key exists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/features/insights/components/BookedByCell.tsx(2 hunks)packages/features/insights/components/routing/RoutingFormResponsesTable.tsx(4 hunks)packages/features/insights/components/routing/RoutingKPICards.tsx(1 hunks)packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx(3 hunks)packages/features/insights/server/routing-events.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/features/insights/components/BookedByCell.tsx
- packages/features/insights/components/routing/RoutingKPICards.tsx
- packages/features/insights/components/routing/RoutingFormResponsesTable.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsxpackages/features/insights/server/routing-events.ts
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/insights/server/routing-events.ts
🧠 Learnings (3)
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsxpackages/features/insights/server/routing-events.ts
📚 Learning: 2025-07-15T12:58:40.539Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.539Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
Applied to files:
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsxpackages/features/insights/server/routing-events.ts
📚 Learning: 2025-07-15T13:02:17.403Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.403Z
Learning: In the InsightsRoutingService (packages/lib/server/service/insightsRouting.ts), multi-select filter data is already validated by zod before reaching the buildFormFieldSqlCondition method, so null/undefined values are not present in filterValue.data arrays and don't need to be filtered out.
Applied to files:
packages/features/insights/server/routing-events.ts
🧬 Code Graph Analysis (2)
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx (1)
packages/features/insights/hooks/useInsightsRoutingParameters.ts (1)
useInsightsRoutingParameters(10-43)
packages/features/insights/server/routing-events.ts (1)
packages/lib/server/service/InsightsRoutingBaseService.ts (1)
InsightsRoutingBaseService(130-696)
🔇 Additional comments (5)
packages/features/insights/filters/Download/RoutingFormResponsesDownload.tsx (2)
30-32: Good move to consolidated routing parametersSwitching to useInsightsRoutingParameters centralizes param derivation (incl. attendee filters) and keeps this component aligned with the new server input schemas.
41-46: TRPC input schema aligns withuseInsightsRoutingParametersoutput
Verified thatroutingFormResponsesForDownloadusesinsightsRoutingServicePaginatedInputSchema, which spreadsinsightsRoutingServiceInputSchema(scope, selectedTeamId, columnFilters, startDate, endDate) and adds offset, limit, sorting—exactly matching the fields returned by the hook. No mismatches or silent drops.packages/features/insights/server/routing-events.ts (3)
121-126: Nice decoupling via headerPromise/dataPromiseAccepting dependencies (headersPromise, dataPromise) makes this download path composable and aligns with the new InsightsRoutingBaseService. Good separation of concerns.
355-414: Header construction is solid; minor nit on deduping across multiple formsGood use of zod parsing and de-duplication via ids Set. If labels differ for the same id across forms, the first encountered will win, which is acceptable here. No action needed.
897-917: CSV escaping logic looks correctEscapes quotes and wraps cells containing commas/newlines/quotes. This aligns with RFC 4180 CSV expectations.
| const field = item.fields.find((field) => field.fieldId === id); | ||
| if (!field) { | ||
| acc[header.label] = ""; | ||
| return acc; | ||
| } | ||
| if (header.type === "select") { | ||
| acc[header.label] = header.options?.find((option) => option.id === response[id].value)?.label; | ||
| } else if (header.type === "multiselect" && Array.isArray(response[id].value)) { | ||
| acc[header.label] = (response[id].value as string[]) | ||
| acc[header.label] = header.options?.find((option) => option.id === field.valueString)?.label; | ||
| } else if (header.type === "multiselect" && Array.isArray(field.valueStringArray)) { | ||
| acc[header.label] = field.valueStringArray | ||
| .map((value) => header.options?.find((option) => option.id === value)?.label) | ||
| .filter((label): label is string => label !== undefined) | ||
| .sort() | ||
| .join(", "); | ||
| } else if (header.type === "number") { | ||
| acc[header.label] = field.valueNumber?.toString() || ""; | ||
| } else { | ||
| acc[header.label] = response[id].value as string; | ||
| acc[header.label] = field.valueString || ""; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Field type checks likely mismatched with enum; normalize to RoutingFormFieldType values
Headers are built from zod-parsed routing form fields and earlier logic filters by RoutingFormFieldType.SINGLE_SELECT and .MULTI_SELECT. Here you compare header.type against "select" | "multiselect" | "number", which may not match the enum values (e.g., SINGLE_SELECT, MULTI_SELECT, NUMBER). This can break option label resolution and number formatting in CSV.
Apply this diff to use the enum consistently:
- if (header.type === "select") {
+ if (header.type === RoutingFormFieldType.SINGLE_SELECT) {
acc[header.label] = header.options?.find((option) => option.id === field.valueString)?.label;
- } else if (header.type === "multiselect" && Array.isArray(field.valueStringArray)) {
+ } else if (header.type === RoutingFormFieldType.MULTI_SELECT && Array.isArray(field.valueStringArray)) {
acc[header.label] = field.valueStringArray
.map((value) => header.options?.find((option) => option.id === value)?.label)
.filter((label): label is string => label !== undefined)
.sort()
.join(", ");
- } else if (header.type === "number") {
+ } else if (header.type === RoutingFormFieldType.NUMBER) {
acc[header.label] = field.valueNumber?.toString() || "";
} else {
acc[header.label] = field.valueString || "";
}If the enum’s string values differ (e.g., "single_select"), alternatively normalize with a small helper:
function toLowerKeyedType(t: string) {
return t.toLowerCase().replace("-", "_");
}and branch on "single_select" | "multi_select" | "number".
🏁 Script executed:
#!/bin/bash
# Display the top portion of the file to locate imports, type definitions, and header construction
sed -n '1,300p' packages/features/insights/server/routing-events.ts
# Search specifically for the RoutingFormFieldType enum in this file
rg -n "RoutingFormFieldType" packages/features/insights/server/routing-events.tsLength of output: 9976
🏁 Script executed:
#!/bin/bash
# Locate the getRoutingFormHeaders method to inspect how headers are constructed
rg -n -C3 "static async getRoutingFormHeaders" packages/features/insights/server/routing-events.tsLength of output: 247
🏁 Script executed:
#!/bin/bash
# Display the code around getRoutingFormHeaders to see header construction
sed -n '300,500p' packages/features/insights/server/routing-events.tsLength of output: 6232
Use the RoutingFormFieldType enum for CSV-export type checks
The CSV-flattening logic in getRoutingFormPaginatedResponsesForDownload compares header.type against raw strings ("select", "multiselect", "number"), but header.type is drawn directly from the RoutingFormFieldType enum. Those branches never fire, so select/multiselect fields fall back to the default string case (showing IDs instead of labels).
• File: packages/features/insights/server/routing-events.ts
• Method: RoutingEventsInsights.getRoutingFormPaginatedResponsesForDownload
• Block: the fields.reduce callback around the if (header.type === …) checks
Apply this diff to consistently use the enum:
--- a/packages/features/insights/server/routing-events.ts
+++ b/packages/features/insights/server/routing-events.ts
@@ getRoutingFormPaginatedResponsesForDownload
- if (header.type === "select") {
+ if (header.type === RoutingFormFieldType.SINGLE_SELECT) {
acc[header.label] = header.options
?.find((option) => option.id === field.valueString)
?.label;
- } else if (header.type === "multiselect" && Array.isArray(field.valueStringArray)) {
+ } else if (
+ header.type === RoutingFormFieldType.MULTI_SELECT &&
+ Array.isArray(field.valueStringArray)
+ ) {
acc[header.label] = field.valueStringArray
.map((value) => header.options?.find((option) => option.id === value)?.label)
.filter((label): label is string => label !== undefined)
.sort()
.join(", ");
- } else if (header.type === "number") {
+ } else if (header.type === RoutingFormFieldType.NUMBER) {
acc[header.label] = field.valueNumber?.toString() || "";
} else {
acc[header.label] = field.valueString || "";
}This ensures your select/multiselect and number branches execute correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const field = item.fields.find((field) => field.fieldId === id); | |
| if (!field) { | |
| acc[header.label] = ""; | |
| return acc; | |
| } | |
| if (header.type === "select") { | |
| acc[header.label] = header.options?.find((option) => option.id === response[id].value)?.label; | |
| } else if (header.type === "multiselect" && Array.isArray(response[id].value)) { | |
| acc[header.label] = (response[id].value as string[]) | |
| acc[header.label] = header.options?.find((option) => option.id === field.valueString)?.label; | |
| } else if (header.type === "multiselect" && Array.isArray(field.valueStringArray)) { | |
| acc[header.label] = field.valueStringArray | |
| .map((value) => header.options?.find((option) => option.id === value)?.label) | |
| .filter((label): label is string => label !== undefined) | |
| .sort() | |
| .join(", "); | |
| } else if (header.type === "number") { | |
| acc[header.label] = field.valueNumber?.toString() || ""; | |
| } else { | |
| acc[header.label] = response[id].value as string; | |
| acc[header.label] = field.valueString || ""; | |
| } | |
| const field = item.fields.find((field) => field.fieldId === id); | |
| if (!field) { | |
| acc[header.label] = ""; | |
| return acc; | |
| } | |
| if (header.type === RoutingFormFieldType.SINGLE_SELECT) { | |
| acc[header.label] = header.options | |
| ?.find((option) => option.id === field.valueString) | |
| ?.label; | |
| } else if ( | |
| header.type === RoutingFormFieldType.MULTI_SELECT && | |
| Array.isArray(field.valueStringArray) | |
| ) { | |
| acc[header.label] = field.valueStringArray | |
| .map((value) => header.options?.find((option) => option.id === value)?.label) | |
| .filter((label): label is string => label !== undefined) | |
| .sort() | |
| .join(", "); | |
| } else if (header.type === RoutingFormFieldType.NUMBER) { | |
| acc[header.label] = field.valueNumber?.toString() || ""; | |
| } else { | |
| acc[header.label] = field.valueString || ""; | |
| } |
🤖 Prompt for AI Agents
In packages/features/insights/server/routing-events.ts around lines 134 to 151,
the CSV-flattening reduce is comparing header.type to raw strings ("select",
"multiselect", "number") which never match because header.type is a
RoutingFormFieldType enum; replace those string comparisons with the
corresponding RoutingFormFieldType enum members (import the enum if not already
imported) so the select, multiselect and number branches execute correctly (e.g.
header.type === RoutingFormFieldType.<appropriateMember>), keeping the existing
branch bodies intact.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts (5)
1056-1108: LGTM: Solid coverage for attendeeName contains filter with rfrd aliasCovers the EXISTS subquery with a.name ILIKE and aligns the join/aliasing to rfrd."bookingUid". Matches the previous ask to split discrete attendee filters.
1110-1162: LGTM: Adds attendeeEmail endsWith filter with correct wildcard placementEXISTS subquery with a.email ILIKE '%@gmail.com' is correct for endsWith, and alias/reference usage is consistent with service SQL.
1270-1270: Same note on random UUID source as aboveUse randomUUID() to avoid the extra uuid dep.
- const customFieldId = uuid(); + const customFieldId = randomUUID();
1310-1310: Same note on random UUID source as aboveUse randomUUID() here as well.
- const customFieldId = uuid(); + const customFieldId = randomUUID();
1465-1465: Same note on random UUID source as aboveUse randomUUID() consistently.
- const customFieldId = uuid(); + const customFieldId = randomUUID();
🧹 Nitpick comments (2)
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts (2)
4-5: Prefer Node's built-in randomUUID over the uuid package in testsYou’re already importing randomUUID from crypto. Dropping the extra uuid dependency keeps dev deps slimmer and avoids mixed sources of randomness.
Apply this diff to remove the extra import:
-import { v4 as uuid } from "uuid";
1219-1219: Use crypto.randomUUID() consistently for customFieldIdSince randomUUID is already imported, prefer it over uuid() for consistency and one source of randomness.
Apply this diff:
- const customFieldId = uuid(); + const customFieldId = randomUUID();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts(15 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
🧬 Code Graph Analysis (1)
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts (2)
packages/platform/libraries/index.ts (1)
MembershipRole(98-98)packages/lib/server/service/InsightsRoutingDIService.ts (1)
InsightsRoutingService(13-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (5)
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts (5)
1239-1259: LGTM: Correct rfrd aliasing in EXISTS subquery for custom field (text)The WHERE rrf."responseId" = rfrd."id" and fieldId/value checks match the denormalized table usage and align with service SQL.
1284-1304: LGTM: Correct rfrd aliasing in EXISTS subquery for custom field (multi-select)Uses valueStringArray @> and the correct rfrd-bound responseId; aligns with expected denormalized shape.
1343-1365: LGTM: Combined filters AND-chained with custom field EXISTS subqueryComposition and parameterization look correct; rfrd aliasing continues to be consistent.
1486-1506: LGTM: System filters excluded from form field processing with proper EXISTS subqueryCorrectly validates that system filters are kept separate while custom field filters use rfrd-bound EXISTS.
1164-1216: Attendee phoneNumber column correctly referencedVerified that:
- Prisma schema defines
Attendee.phoneNumber(packages/prisma/schema.prisma:736).buildAttendeeSqlCondition(..., "phone")maps toa."phoneNumber"(InsightsRoutingBaseService.ts).- Integration test asserts
a."phoneNumber" ILIKE, matching the schema and service logic.No changes required.
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
Outdated
Show resolved
Hide resolved
…malized-on-insightsrouting
…ntegration-test.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
CarinaWolli
left a comment
CarinaWolli
left a comment
There was a problem hiding this comment.
How should this be tested?
Visit /insights/routing andtest if the table is loaded with correct data (you could switch to main branch to compare if you want)
test attendee filters (name, email, phone number)
click "Download" button on the top right corner
Tested and everything works as expected 🙏
eunjae-lee
deleted the
eunjae/cal-5755-use-routingformresponsedenormalized-on-insightsrouting
branch
What does this PR do?
This PR replaces
RoutingFormReponseview withRoutingFormResponseDenormalizedtable.In order to do that, I'm replacing the view usage and the filter logic with
InsightsRoutingService. Now we the service is the unified interface to get data for both the table and the metrics for charts.This PR also adds "attendee" filter to the /insights/routing table.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Visit /insights/routing and
mainbranch to compare if you want)