feat: allow to choose dateTarget for /insights (startTime by default) #23752
Conversation
WalkthroughMoves date-range filtering into columnFilters (removing top-level startDate/endDate), adds dual date-target support (startTime | createdAt) with a DateTargetSelector and i18n keys, updates DateRangeFilter to optionally convert timestamps between timezones and adjusts end-of-day handling to the picker, introduces bookingUtils helpers (extract/replace DATE_RANGE) with tests, refactors insights hooks/services/router to drive date logic from columnFilters (including changing getMembersStatsWithCount signature), and removes forced endDate overrides from several components. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
bot
added
insights
- Add TimestampFilter component with Start Time/Created At options - Extend useInsightsBookingParameters hook with timestamp selection - Update all insight components to use dateTarget parameter - Add i18n translations for new UI strings - Position selector next to DateRangeFilter as requested Addresses user request to add select box next to date range filter allowing users to choose between startTime (default) and createdAt for displaying booking metrics on the Insights page. Co-Authored-By: [email protected] <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
…state - Rename TimestampFilter component to DateTargetSelector - Implement nuqs hook in InsightsPageContent for URL state management - Update useInsightsBookingParameters to return dateTarget from URL state - Add dateTarget field to insightsRoutingServiceInputSchema and related types - Simplify individual insight components to use insightsBookingParams directly - Remove manual timestampTarget destructuring from all components - Update all tRPC routing service calls to include dateTarget parameter - All TypeScript checks now pass successfully Co-Authored-By: [email protected] <[email protected]>
- Replace Select with Command + Popover for compact width and wider dropdown - Add descriptive option labels with translations - Change filter ID from 'createdAt' to 'timestamp' across all components - Maintain URL state management with nuqs - Fix ESLint warning for missing dependency in useEffect Co-Authored-By: [email protected] <[email protected]>
- Keep timestamp filter ID change scoped only to main insights page - Routing components should continue using createdAt as filter ID - Only insights-view.tsx and related booking hooks use timestamp Co-Authored-By: [email protected] <[email protected]>
| type: ColumnFilterType.DATE_RANGE, | ||
| dateRangeOptions: { | ||
| endOfDay: true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
this endOfDay param was unnecessary. confirmed working well without it.
![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/trpc-router.ts (1)
501-515: averageEventDuration buckets by createdAt only; ignores selected DateTarget & time zone.When DateTarget is “startTime”, bucketing by
createdAtyields incorrect series. Also grouping should use the requesttimeZoneto align withgetDateRanges.- const { columnFilters, timeZone } = input; - const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); + const { columnFilters, timeZone } = input; + const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); + const dateTargetId = + (columnFilters ?? []).find( + (f) => (f.id === "startTime" || f.id === "createdAt") && isDateRangeFilterValue(f.value) + )?.id ?? "startTime"; @@ - const allBookings = await insightsBookingService.findAll({ - select: { - eventLength: true, - createdAt: true, - }, - }); + const select = + dateTargetId === "createdAt" + ? { eventLength: true, createdAt: true } + : { eventLength: true, startTime: true }; + const allBookings = await insightsBookingService.findAll({ select }); @@ - for (const range of dateRanges) { - resultMap.set(dayjs(range.startDate).format("ll"), { totalDuration: 0, count: 0 }); - } + for (const range of dateRanges) { + resultMap.set(dayjs(range.startDate).tz(timeZone).format("ll"), { totalDuration: 0, count: 0 }); + } @@ - for (const booking of allBookings) { - const periodStart = dayjs(booking.createdAt).startOf(startOfEndOf).format("ll"); + for (const booking of allBookings) { + const dateValue = dateTargetId === "createdAt" ? booking.createdAt : booking.startTime; + const periodStart = dayjs(dateValue).tz(timeZone).startOf(startOfEndOf).format("ll"); if (resultMap.has(periodStart)) { const current = resultMap.get(periodStart); if (!current) continue; current.totalDuration += booking.eventLength || 0; current.count += 1; } }Follow‑up: add an assertion to ensure exactly one DATE_RANGE filter exists (either
startTimeorcreatedAt) to avoid conflicting filters. I can wire a small helper if you want.Also applies to: 522-527, 532-544
🧹 Nitpick comments (20)
packages/features/insights/hooks/useInsightsBookingParameters.ts (3)
21-23: Narrow the “date-range exists” check to the DATE_RANGE type.
Right now any filter with id "startTime" or "createdAt" will short‑circuit, even if it’s not a DATE_RANGE. Guard by type to avoid false positives.Apply this diff:
- const hasDateRangeFilter = columnFilters.find( - (filter) => filter.id === "startTime" || filter.id === "createdAt" - ); + const hasDateRangeFilter = columnFilters.some( + (f) => + (f.id === "startTime" || f.id === "createdAt") && + f.value?.type === ColumnFilterType.DATE_RANGE + );
34-36: Remove redundant Day.js re-parsing and start/end-of-day calls.
getDefaultStartDate/EndDate already return start/end of day. Re-wrapping and re-applying startOf/endOf is unnecessary and risks subtle TZ drift.Apply this diff:
- startDate: dayjs(getDefaultStartDate().toISOString()).startOf("day").toISOString(), - endDate: dayjs(getDefaultEndDate().toISOString()).endOf("day").toISOString(), + startDate: getDefaultStartDate().toISOString(), + endDate: getDefaultEndDate().toISOString(),
19-42: Optional: defensive de-dupe if both date-range filters appear.
If both "startTime" and "createdAt" DATE_RANGE filters are present, consider keeping the most recent or the active DateTarget only to avoid server ambiguity.packages/features/insights/components/booking/EventTrendsChart.tsx (1)
36-41: Minor: drop unused prop destructure or keep underscore for clarityUsing
label: _labelis fine to silence unused-prop linting. If preferred, omitlabelfrom destructuring to reduce noise.-const CustomTooltip = ({ - active, - payload, - label: _label, -}: { +const CustomTooltip = ({ + active, + payload, +}: {packages/features/insights/components/booking/TimezoneBadge.tsx (1)
41-45: A11y: make the tooltip trigger focusable and labeledWithout a focusable trigger, keyboard and SR users may miss this info. Wrap the icon in a focusable element and expose an accessible name.
- <Tooltip content={timezoneData.tooltipContent}> - <Icon name="info" data-testid="timezone-mismatch-badge" className="text-subtle" /> - </Tooltip> + <Tooltip content={timezoneData.tooltipContent}> + <span tabIndex={0} aria-label={timezoneData.tooltipContent}> + <Icon name="info" data-testid="timezone-mismatch-badge" className="text-subtle" /> + </span> + </Tooltip>packages/features/data-table/lib/preserveLocalTime.ts (1)
13-19: Docs polishSmall grammar/clarity tweaks.
- * - * For example, the profile timezone is Asia/Seoul, - * but the current user is in Europe/Paris. - * `Date` pickers will normally emit timestamps in the user's local timezone. (00:00:00 ~ 23:59:59 in Paris time) - * but what we really want is to fetch the data based on the user's profile timezone. (00:00:00 ~ 23:59:59 in Seoul time) - * That's why we need to convert the timestamp to the user's profile timezone. + * + * Example: the profile timezone is Asia/Seoul, but the current user is in Europe/Paris. + * Date pickers emit timestamps in the user's local timezone (00:00–23:59 in Paris), + * but data should be fetched using the profile timezone (00:00–23:59 in Seoul), + * so we convert the timestamp to the profile timezone.packages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.ts (1)
3-4: Switch to CURRENT_TIMEZONE is sensible; minor doc typo + sanity checkGreat move away from runtime guessing. Fix tiny typo and confirm
CURRENT_TIMEZONEreflects the browser/local TZ (not the profile TZ), otherwise conversion becomes a no-op.- * Fore more info, read packages/features/data-table/lib/preserveLocalTime.ts + * For more info, read packages/features/data-table/lib/preserveLocalTime.tsVerification: how is
CURRENT_TIMEZONEderived? If it’s the profile TZ, original→target will match and skip needed conversions.Also applies to: 10-11, 15-19
packages/features/insights/filters/DateTargetSelector.tsx (2)
68-70: Use value equality for selection check (labels can collide across locales).Compare by
valueinstead oflabelto avoid mismatches if translations change.- {selectedOption.label === option.label && ( + {selectedOption.value === option.value && ( <Icon name="check" className="text-primary-foreground h-4 w-4" /> )}
44-51: Add an explicit aria-label to the trigger.You already have an SR-only span, but adding
aria-labelon the Button improves AT support for icon-only controls.- <Button + <Button variant="icon" color="secondary" StartIcon="sliders-horizontal" role="combobox" - aria-expanded={open}> + aria-expanded={open} + aria-label={selectedOption.label}>apps/web/modules/insights/insights-view.tsx (3)
57-61: Localize column titles (frontend text should use t()).
title: "startTime"/"createdAt"are user-facing in some contexts; prefer i18n.-const startTimeColumn: Extract<FilterableColumn, { type: ColumnFilterType.DATE_RANGE }> = { - id: "startTime", - title: "startTime", +const startTimeColumn: Extract<FilterableColumn, { type: ColumnFilterType.DATE_RANGE }> = { + id: "startTime", + title: "start_time", // use an i18n key; render with t() when shown type: ColumnFilterType.DATE_RANGE, };-const createdAtColumn: Extract<FilterableColumn, { type: ColumnFilterType.DATE_RANGE }> = { - id: "createdAt", - title: "createdAt", +const createdAtColumn: Extract<FilterableColumn, { type: ColumnFilterType.DATE_RANGE }> = { + id: "createdAt", + title: "created_at", // use an i18n key; render with t() when shown type: ColumnFilterType.DATE_RANGE, };
68-76: Type with shared DateTarget and preserve date range when toggling.
- Minor: use the exported
DateTargetinstead of re-declaring the union.- UX: when switching targets, transfer the existing range instead of clearing it.
- const { removeFilter } = useDataTable(); - const [dateTarget, _setDateTarget] = useState<"startTime" | "createdAt">("startTime"); + const { removeFilter, updateFilter } = useDataTable(); + const [dateTarget, _setDateTarget] = useState<DateTarget>("startTime");Example approach (sketch):
- Read current filter (e.g., via
useFilterValuefor the active id).- When toggling, write the same
{ startDate, endDate, preset }under the new id viaupdateFilter, then remove the old id.
91-96: Avoid unnecessary type cast and pass a stable options object.
- No need for
as DateTargetonce the state is typed.- Consider memoizing
{ convertToTimeZone: true }to avoid re-renders in children.- <DateRangeFilter - column={dateTarget === "startTime" ? startTimeColumn : createdAtColumn} - options={{ convertToTimeZone: true }} - /> - <DateTargetSelector value={dateTarget as DateTarget} onChange={setDateTarget} /> + <DateRangeFilter + column={dateTarget === "startTime" ? startTimeColumn : createdAtColumn} + options={useMemo(() => ({ convertToTimeZone: true }), [])} + /> + <DateTargetSelector value={dateTarget} onChange={setDateTarget} />packages/features/insights/lib/bookingUtils.ts (2)
5-7: Be defensive: don’t throw when no date filter is present.Throwing here can tear down server calls if a client omits filters. Return
nullor a typedResultinstead and handle upstream.-export function extractDateRangeFromColumnFilters(columnFilters?: ColumnFilter[]) { - if (!columnFilters) throw new Error("No date range filter found"); +export function extractDateRangeFromColumnFilters(columnFilters?: ColumnFilter[]) { + if (!columnFilters) return null; ... - throw new Error("No date range filter found"); + return null; }Upstream (e.g.,
calculatePreviousPeriodDates) should handlenullgracefully.Also applies to: 23-24
39-55: If both date filters are present, define precedence.Today you update whichever appears; consider preferring
startTimeovercreatedAt(or vice‑versa) explicitly to avoid order‑dependent behavior.packages/lib/server/service/InsightsBookingBaseService.ts (2)
1241-1246: Handle missing date filters defensively.
extractDateRangeFromColumnFiltersthrows when absent; either handlenullhere or default to a safe last‑period window.- calculatePreviousPeriodDates() { - const result = extractDateRangeFromColumnFilters(this.filters?.columnFilters); + calculatePreviousPeriodDates() { + const result = extractDateRangeFromColumnFilters(this.filters?.columnFilters); + if (!result) { + // fallback: previous 7 days + const end = dayjs(); + const start = end.subtract(6, "day"); + return { + startDate: start.toISOString(), + endDate: end.toISOString(), + formattedStartDate: start.format("YYYY-MM-DD"), + formattedEndDate: end.format("YYYY-MM-DD"), + }; + }
660-704: Event trends group by createdAt only; make it respect selected date target.When the active filter is
startTime, aggregate byDATE("startTime" AT TIME ZONE ...)instead ofcreatedAt.Sketch:
- Detect active date target:
const groupByColumn = this.filters?.columnFilters?.some(f => f.id === "startTime" && isDateRangeFilterValue(f.value)) ? Prisma.sql"startTime": Prisma.sql"createdAt";- Use
groupByColumnin both CTEs and adjust ORDER BY accordingly.packages/features/insights/server/trpc-router.ts (4)
4-9: Clean up unused imports.
ColumnFilterTypeandColumnFilterare not used in this file. Drop them to satisfy linters and keep the diff tight.-import { ColumnFilterType, type ColumnFilter } from "@calcom/features/data-table/lib/types";
368-373: Guard for missing columnFilters in previous-period calc.If
input.columnFiltersis undefined,previousPeriodColumnFiltersbecomesundefined, leading to an unbounded previous-period fetch and misleading deltas. Either makecolumnFiltersrequired at the schema or fail fast here.Would you like me to hard-require
columnFiltersinbookingRepositoryBaseInputSchemaor add a runtime check here?
464-466: Fail fast with BAD_REQUEST when date range is missing/invalid.
extractDateRangeFromColumnFiltersthrows; currently that becomes a 500. Return a 400 instead.- const { columnFilters, timeZone } = input; - const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); + const { columnFilters, timeZone } = input; + let startDate: string, endDate: string; + try { + ({ startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters)); + } catch { + throw new TRPCError({ code: "BAD_REQUEST", message: "Missing or invalid date range filter." }); + }
358-462: Optional: unify error handling/logging for booking insights endpoints.Some handlers wrap service calls in try/catch but pre-processing errors (like date extraction) still bubble as 500. Consider consistent BAD_REQUEST for input issues and log unexpected errors.
Also applies to: 463-486
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
apps/web/modules/insights/insights-view.tsx(4 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/features/data-table/components/filters/DateRangeFilter.tsx(6 hunks)packages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.ts(1 hunks)packages/features/data-table/lib/preserveLocalTime.ts(1 hunks)packages/features/data-table/lib/types.ts(1 hunks)packages/features/insights/components/booking/EventTrendsChart.tsx(1 hunks)packages/features/insights/components/booking/LeastCompletedBookings.tsx(0 hunks)packages/features/insights/components/booking/MostCompletedBookings.tsx(0 hunks)packages/features/insights/components/booking/TimezoneBadge.tsx(1 hunks)packages/features/insights/components/routing/RoutingFormResponsesTable.tsx(1 hunks)packages/features/insights/filters/DateTargetSelector.tsx(1 hunks)packages/features/insights/filters/Download/Download.tsx(2 hunks)packages/features/insights/hooks/useInsightsBookingParameters.ts(1 hunks)packages/features/insights/lib/bookingUtils.ts(1 hunks)packages/features/insights/server/raw-data.schema.ts(0 hunks)packages/features/insights/server/trpc-router.ts(11 hunks)packages/features/users/components/UserTable/UserListTable.tsx(0 hunks)packages/lib/server/service/InsightsBookingBaseService.ts(4 hunks)
💤 Files with no reviewable changes (4)
- packages/features/insights/server/raw-data.schema.ts
- packages/features/users/components/UserTable/UserListTable.tsx
- packages/features/insights/components/booking/MostCompletedBookings.tsx
- packages/features/insights/components/booking/LeastCompletedBookings.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/filters/Download/Download.tsxapps/web/modules/insights/insights-view.tsxpackages/features/insights/components/booking/EventTrendsChart.tsxpackages/features/insights/components/booking/TimezoneBadge.tsxpackages/features/data-table/components/filters/DateRangeFilter.tsxpackages/features/insights/filters/DateTargetSelector.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/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/filters/Download/Download.tsxpackages/features/insights/lib/bookingUtils.tsapps/web/modules/insights/insights-view.tsxpackages/features/data-table/lib/preserveLocalTime.tspackages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.tspackages/features/insights/components/booking/EventTrendsChart.tsxpackages/features/data-table/lib/types.tspackages/features/insights/components/booking/TimezoneBadge.tsxpackages/features/data-table/components/filters/DateRangeFilter.tsxpackages/features/insights/filters/DateTargetSelector.tsxpackages/features/insights/hooks/useInsightsBookingParameters.tspackages/features/insights/server/trpc-router.tspackages/lib/server/service/InsightsBookingBaseService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/insights/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/filters/Download/Download.tsxpackages/features/insights/lib/bookingUtils.tsapps/web/modules/insights/insights-view.tsxpackages/features/data-table/lib/preserveLocalTime.tspackages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.tspackages/features/insights/components/booking/EventTrendsChart.tsxpackages/features/data-table/lib/types.tspackages/features/insights/components/booking/TimezoneBadge.tsxpackages/features/data-table/components/filters/DateRangeFilter.tsxpackages/features/insights/filters/DateTargetSelector.tsxpackages/features/insights/hooks/useInsightsBookingParameters.tspackages/features/insights/server/trpc-router.tspackages/lib/server/service/InsightsBookingBaseService.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/lib/bookingUtils.tspackages/features/data-table/lib/preserveLocalTime.tspackages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.tspackages/features/data-table/lib/types.tspackages/features/insights/hooks/useInsightsBookingParameters.tspackages/features/insights/server/trpc-router.tspackages/lib/server/service/InsightsBookingBaseService.ts
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/lib/server/service/InsightsBookingBaseService.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
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.
📚 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/RoutingFormResponsesTable.tsxpackages/features/insights/filters/Download/Download.tsx
📚 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/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/filters/Download/Download.tsxpackages/features/insights/lib/bookingUtils.tsapps/web/modules/insights/insights-view.tsxpackages/features/insights/hooks/useInsightsBookingParameters.tspackages/features/insights/server/trpc-router.tspackages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-08-22T16:38:00.225Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
Applied to files:
packages/features/insights/lib/bookingUtils.tspackages/features/insights/hooks/useInsightsBookingParameters.tspackages/features/insights/server/trpc-router.tspackages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
Applied to files:
packages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.tspackages/features/data-table/components/filters/DateRangeFilter.tsx
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/insights/components/booking/EventTrendsChart.tsx
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/features/insights/server/trpc-router.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/trpc-router.tspackages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-07-24T08:39:06.185Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
🧬 Code graph analysis (10)
packages/features/insights/filters/Download/Download.tsx (1)
packages/features/insights/lib/bookingUtils.ts (1)
extractDateRangeFromColumnFilters(5-24)
packages/features/insights/lib/bookingUtils.ts (1)
packages/features/data-table/lib/types.ts (1)
ColumnFilter(169-172)
apps/web/modules/insights/insights-view.tsx (4)
packages/features/data-table/lib/types.ts (1)
FilterableColumn(143-167)packages/features/insights/hooks/useInsightsBookings.ts (1)
useInsightsBookings(23-138)packages/features/data-table/components/filters/DateRangeFilter.tsx (1)
DateRangeFilter(45-256)packages/features/insights/filters/DateTargetSelector.tsx (2)
DateTargetSelector(22-78)DateTarget(9-9)
packages/features/data-table/hooks/useChangeTimeZoneWithPreservedLocalTime.ts (1)
packages/features/data-table/lib/preserveLocalTime.ts (1)
preserveLocalTime(20-37)
packages/features/insights/components/booking/TimezoneBadge.tsx (1)
apps/ui-playground/app/components/icon-swatch.tsx (1)
Icon(6-19)
packages/features/data-table/components/filters/DateRangeFilter.tsx (2)
packages/features/data-table/lib/preserveLocalTime.ts (1)
preserveLocalTime(20-37)packages/features/data-table/lib/dateRange.ts (2)
CUSTOM_PRESET(20-20)getDefaultStartDate(31-31)
packages/features/insights/filters/DateTargetSelector.tsx (1)
packages/ui/components/button/Button.tsx (1)
Button(221-349)
packages/features/insights/hooks/useInsightsBookingParameters.ts (2)
packages/features/insights/hooks/useInsightsOrgTeams.ts (1)
useInsightsOrgTeams(6-32)packages/features/data-table/lib/dateRange.ts (3)
getDefaultStartDate(31-31)getDefaultEndDate(33-33)DEFAULT_PRESET(15-19)
packages/features/insights/server/trpc-router.ts (2)
packages/features/insights/server/raw-data.schema.ts (1)
bookingRepositoryBaseInputSchema(103-108)packages/features/insights/lib/bookingUtils.ts (2)
replaceDateRangeColumnFilter(26-56)extractDateRangeFromColumnFilters(5-24)
packages/lib/server/service/InsightsBookingBaseService.ts (1)
packages/features/insights/lib/bookingUtils.ts (1)
extractDateRangeFromColumnFilters(5-24)
⏰ Context from checks skipped due to timeout of 180000ms. 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 (12)
packages/features/insights/hooks/useInsightsBookingParameters.ts (3)
6-11: LGTM: shared date-range helpers and types are correctly imported.
Using the central dateRange helpers keeps defaults consistent across the app.
27-40: ```shell
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "=== Search for ColumnFilterType.DATE_RANGE occurrences ==="
rg -n --hidden --no-ignore '\bColumnFilterType.DATE_RANGE\b' -S -C3 || trueecho
echo "=== Search for ColumnFilter objects with id startTime or createdAt ==="
rg -n --hidden --no-ignore '{\sid\s:\s*"(startTime|createdAt)"' -S -C3 || trueecho
echo "=== Search for literal startTime / createdAt tokens (context) ==="
rg -n --hidden --no-ignore '\b(startTime|createdAt)\b' -S -C3 | sed -n '1,200p' || trueecho
echo "=== Search for DateTarget-related identifiers ==="
rg -n --hidden --no-ignore 'DateTargetSelector|DateTarget|dateTarget|dateTargetSelector' -S -C3 || trueecho
echo "=== Show file: packages/features/insights/hooks/useInsightsBookingParameters.ts (if present) ==="
if [ -f packages/features/insights/hooks/useInsightsBookingParameters.ts ]; then
nl -ba packages/features/insights/hooks/useInsightsBookingParameters.ts | sed -n '1,240p'
else
echo "NOT FOUND: packages/features/insights/hooks/useInsightsBookingParameters.ts"
fiecho
echo "=== Search for code that updates/replaces filters when date target changes ==="
rg -n --hidden --no-ignore '(replace.(startTime|createdAt)|remove.(startTime|createdAt)|ensure.*Date.*Range|normalize.*filters|set.*ColumnFilters|update.*ColumnFilters|select.*DateTarget)' -S -C3 || true--- `48-48`: **Resolved — startDate/endDate removal verified; consumers use columnFilters.** No direct destructuring of startDate/endDate from useInsightsBookingParameters was found; consumers read dates via insightsBookingParams.columnFilters and extractDateRangeFromColumnFilters (e.g. packages/features/insights/filters/Download/Download.tsx). </blockquote></details> <details> <summary>packages/features/insights/components/routing/RoutingFormResponsesTable.tsx (1)</summary><blockquote> `97-109`: **Good: effect deps updated to include `updateFilter`** Prevents stale-closure bugs if the updater identity changes. Also guarded by `routingFormId`. LGTM. If `updateFilter` is not stable (memoized), effect could re-run unnecessarily. Confirm `useDataTable()` returns a stable `updateFilter`. </blockquote></details> <details> <summary>packages/features/insights/filters/Download/Download.tsx (1)</summary><blockquote> `17-28`: **Aligned with columnFilters; add defensive guard or verify invariant** Using `extractDateRangeFromColumnFilters` matches the new contract. Ensure a date-range filter is always present; otherwise this throws and breaks download. If this invariant isn’t guaranteed, wrap in a memoized try/catch and fallback to the currently selected preset or today. ```diff - const { startDate, endDate } = extractDateRangeFromColumnFilters(insightsBookingParams.columnFilters); + const { startDate, endDate } = extractDateRangeFromColumnFilters(insightsBookingParams.columnFilters);packages/features/data-table/lib/types.ts (1)
117-121: Type rename verified — no stale 'endOfDay' option usages remain.
- Call-sites updated: packages/features/data-table/lib/types.ts:119; packages/features/data-table/components/filters/DateRangeFilter.tsx (uses options?.convertToTimeZone); apps/web/modules/insights/insights-view.tsx:93 (options={{ convertToTimeZone: true }}).
- 'endOfDay' identifiers found in packages/features/schedules/components/Schedule.tsx and packages/lib/server/getLuckyUser.ts are unrelated to the filter option.
apps/web/public/static/locales/en/common.json (1)
3692-3696: Add translations for the new i18n keys and confirm call-site usage
- Findings: the keys are present only in apps/web/public/static/locales/en/common.json (lines ~3692–3695); search returned no literal usages outside the locale files. The keys are missing in 43 of 44 locale common.json files under apps/web/public/static/locales/*/common.json.
- Action: add these keys (booking_time_option, booking_time_option_description, created_at_option, created_at_option_description) to each locale file at apps/web/public/static/locales//common.json (or document an explicit English-fallback decision), and verify the UI uses these exact keys (or update call sites accordingly) to avoid untranslated fallbacks.
⛔ Skipped due to learnings
Learnt from: Udit-takkar PR: calcom/cal.com#23761 File: packages/features/shell/navigation/Navigation.tsx:125-130 Timestamp: 2025-09-12T14:46:29.697Z Learning: In calcom/cal.com, translations for other locales are added later in the development process, so don't flag missing translation keys in non-English locale files when only the English locale has been updated.packages/features/data-table/components/filters/DateRangeFilter.tsx (3)
74-85: Timezone-preserving conversion looks good.Conditionally converting via
preserveLocalTimeonly whenconvertToTimeZoneand TZ differ is correct.Please confirm there isn’t another end‑of‑day normalization downstream to avoid double-adjusting the end timestamp when
convertToTimeZoneis enabled.
93-105: Store converted ISO strings in filter value.Using
convertTimestamp(startDate.toISOString())ensures consistent server semantics. LGTM.
143-149: End-of-day handling moved to picker path is appropriate.Adjusting
endDateto 23:59:59.999 avoids half‑open range mistakes from the UI. Good change.packages/features/insights/server/trpc-router.ts (2)
562-565: LGTM: updated calls to new service surface.The
getMembersStatsWithCountinvocations and other service calls align with the new object-arg API.Please confirm InsightsBookingBaseService avoids adding
endTime <= now()for accepted-bookings filtering, per prior learning, since completion is now driven by date filters.Also applies to: 576-580, 591-595, 606-607, 617-617, 813-813, 1022-1022, 1035-1035
1-1209: Contract check — require a single DATE_RANGE columnFilter on createdAt OR startTime for booking insights endpointsRouter assumes a single date-target filter; verify all client call sites pass columnFilters containing exactly one DATE_RANGE with id "createdAt" or "startTime". Run the following from the repo root to find call sites and any missing/misconstructed filters:
#!/bin/bash set -euo pipefail PATTERN='insights\.(bookingKPIStats|eventTrends|averageEventDuration|membersWithMostCancelledBookings|membersWithMostCompletedBookings|membersWithLeastCompletedBookings|membersWithMostBookings|membersWithLeastBookings|membersWithMostNoShow|bookingsByHourStats|recentNoShowGuests)\.(useQuery|fetch|mutate)\(' echo "All call sites for booking insights endpoints:" rg -n --hidden -g '!node_modules/**' -C3 "$PATTERN" || true echo echo "Call sites missing 'columnFilters' within the next 12 lines:" rg -n --hidden --no-heading "$PATTERN" | while IFS=: read -r file line _; do echo "---- $file:$line ----" sed -n "${line},$((line+12))p" "$file" if ! sed -n "${line},$((line+12))p" "$file" | rg -q 'columnFilters'; then echo ">>> MISSING columnFilters" fi done echo echo "Locations where DATE_RANGE filters are constructed (inspect for both createdAt + startTime):" rg -n --hidden -C3 'ColumnFilterType\.DATE_RANGE' || trueIf any call site is missing columnFilters or constructs multiple DATE_RANGE filters (both createdAt and startTime), update the client to pass a single DATE_RANGE for the correct field.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/features/insights/lib/__tests__/bookingUtils.test.ts (8)
8-30: Reduce mock duplication with a small factory.A tiny helper will DRY up repeated DATE_RANGE/Text/SingleSelect literals and ease future changes.
Apply this diff to introduce a factory after the imports:
@@ import { extractDateRangeFromColumnFilters, replaceDateRangeColumnFilter } from "../bookingUtils"; +// Test helpers +const makeDateRangeFilter = ( + id: "startTime" | "createdAt", + startDate: string, + endDate: string, + preset: string = "thisMonth" +): ColumnFilter => ({ + id, + value: { + type: ColumnFilterType.DATE_RANGE, + data: { startDate, endDate, preset }, + }, +}); + +const makeTextFilter = (id = "status", operand = "confirmed"): ColumnFilter => ({ + id, + value: { type: ColumnFilterType.TEXT, data: { operator: "equals" as const, operand } }, +}); + +const makeSingleSelectFilter = (id = "eventType", data: string = "meeting"): ColumnFilter => ({ + id, + value: { type: ColumnFilterType.SINGLE_SELECT, data }, +});Also applies to: 32-49
51-95: Add a test for “both date targets present” to lock ordering semantics.Function currently returns the first matching filter by order. Add a test to codify this or adjust if a different precedence is intended.
@@ describe("successful extraction", () => { it("should ignore non-date-range filters", () => { @@ }); + + it("should return the first matching date-range in order when both startTime and createdAt exist", () => { + const result = extractDateRangeFromColumnFilters([mockCreatedAtFilter, mockStartTimeFilter]); + expect(result).toEqual({ + startDate: "2024-02-01T00:00:00.000Z", + endDate: "2024-02-29T23:59:59.999Z", + }); + });Please confirm that “first in order” is the desired behavior for callers that might accidentally pass both.
97-105: Deduplicate undefined/no-args error tests.Both cases exercise the same error path; keep one.
describe("error cases", () => { it("should throw error when no column filters provided", () => { - expect(() => extractDateRangeFromColumnFilters()).toThrow("No date range filter found"); + expect(() => extractDateRangeFromColumnFilters()).toThrow(/No date range filter found/i); }); - - it("should throw error when column filters is undefined", () => { - expect(() => extractDateRangeFromColumnFilters(undefined)).toThrow("No date range filter found"); - });
106-186: Make error assertions resilient.Prefer regex over exact string matches to avoid brittle tests if wording changes.
Example:
-expect(() => extractDateRangeFromColumnFilters([invalidStartTimeFilter])).toThrow( - "No date range filter found" -); +expect(() => extractDateRangeFromColumnFilters([invalidStartTimeFilter])).toThrow( + /No date range filter found/i +);
271-295: Also assert referential identity for unchanged filters.Strengthens guarantees: unchanged filters keep object identity.
expect(result).toEqual([ @@ mockTextFilter, // Non-date filters remain unchanged ]); + // identity checks for unchanged items + expect(result?.[1]).toBe(mockTextFilter);
322-346: Good preset‑preservation check. Consider extra fields too.Nice. Add a companion test to ensure additional DATE_RANGE data keys (e.g., time zone flags) are preserved on replace.
@@ it("should preserve preset value when replacing filter", () => { @@ }); + + it("should preserve additional DATE_RANGE data fields on replace", () => { + const withExtra: ColumnFilter = { + id: "startTime", + value: { + type: ColumnFilterType.DATE_RANGE, + data: { + startDate: "2024-01-01T00:00:00.000Z", + endDate: "2024-01-31T23:59:59.999Z", + preset: "thisMonth", + // @ts-ignore — extra key used to assert runtime preservation + convertToTimeZone: "America/Los_Angeles", + }, + }, + }; + const result = replaceDateRangeColumnFilter({ + columnFilters: [withExtra], + newStartDate, + newEndDate, + })!; + // @ts-ignore + expect(result[0].value.data.convertToTimeZone).toBe("America/Los_Angeles"); + });Note: This will pass only if implementation preserves unknown keys (see next comment).
348-372: Strengthen immutability assertions for “unchanged” items and array.Verify unchanged entries are the same references and the returned array is a new instance.
@@ expect(result).toEqual([ @@ ]); + expect(result?.[0]).toBe(mockTextFilter); + expect(result?.[1]).toBe(mockSingleSelectFilter); + @@ expect(result).toEqual([ @@ mockSingleSelectFilter, // Unchanged ]); + expect(result?.[0]).toBe(mockTextFilter); + expect(result?.[2]).toBe(mockSingleSelectFilter);And for the “no date range filters exist” case:
@@ - expect(result).toEqual(columnFilters); + expect(result).toEqual(columnFilters); + expect(result).not.toBe(columnFilters); // new array instanceAlso applies to: 374-398
417-448: Coverage suggestion: both date targets present during replace.If both startTime and createdAt exist, current impl updates both. Confirm that’s desired; if not, we should gate by selected dateTarget in callers or in this helper.
@@ describe("edge cases", () => { + it("should replace both startTime and createdAt if both are present", () => { + const columnFilters = [mockStartTimeFilter, mockCreatedAtFilter, mockTextFilter]; + const result = replaceDateRangeColumnFilter({ columnFilters, newStartDate, newEndDate })!; + expect(result.filter((f) => f.id === "startTime")[0].value).toMatchObject({ + data: expect.objectContaining({ startDate: newStartDate, endDate: newEndDate }), + }); + expect(result.filter((f) => f.id === "createdAt")[0].value).toMatchObject({ + data: expect.objectContaining({ startDate: newStartDate, endDate: newEndDate }), + }); + });If only one should change, we need to pass in the active dateTarget and conditionally replace.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/insights/lib/__tests__/bookingUtils.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/lib/__tests__/bookingUtils.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/features/insights/lib/__tests__/bookingUtils.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/insights/lib/__tests__/bookingUtils.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
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.
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/insights/lib/__tests__/bookingUtils.test.ts
🧬 Code graph analysis (1)
packages/features/insights/lib/__tests__/bookingUtils.test.ts (2)
packages/features/data-table/lib/types.ts (1)
ColumnFilter(169-172)packages/features/insights/lib/bookingUtils.ts (2)
extractDateRangeFromColumnFilters(5-24)replaceDateRangeColumnFilter(26-56)
⏰ Context from checks skipped due to timeout of 180000ms. 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 (1)
packages/features/insights/lib/__tests__/bookingUtils.test.ts (1)
1-5: Solid test scaffold and clear imports.Vitest setup and type imports look good.
| it("should maintain filter structure when replacing", () => { | ||
| const result = replaceDateRangeColumnFilter({ | ||
| columnFilters: [mockStartTimeFilter], | ||
| newStartDate, | ||
| newEndDate, | ||
| }); | ||
|
|
||
| expect(result?.[0]).toHaveProperty("id"); | ||
| expect(result?.[0]).toHaveProperty("value"); | ||
| expect(result?.[0].value).toHaveProperty("type", ColumnFilterType.DATE_RANGE); | ||
| expect(result?.[0].value).toHaveProperty("data"); | ||
| expect(result?.[0].value.data).toHaveProperty("startDate"); | ||
| expect(result?.[0].value.data).toHaveProperty("endDate"); | ||
| expect(result?.[0].value.data).toHaveProperty("preset"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Bug: replacement drops extra DATE_RANGE data keys (e.g., timezone flags).
bookingUtils.ts overwrites data with only startDate/endDate/preset, potentially losing fields introduced by DateRangeFilter’s timezone option. Preserve all existing keys.
Apply this implementation fix in packages/features/insights/lib/bookingUtils.ts:
@@ export function replaceDateRangeColumnFilter({
- return columnFilters.map((filter) => {
- if ((filter.id === "startTime" || filter.id === "createdAt") && isDateRangeFilterValue(filter.value)) {
- return {
- id: filter.id,
- value: {
- type: ColumnFilterType.DATE_RANGE,
- data: {
- startDate: newStartDate,
- endDate: newEndDate,
- preset: filter.value.data.preset,
- },
- },
- } satisfies ColumnFilter;
- } else {
- return filter;
- }
- });
+ return columnFilters.map((filter) => {
+ if ((filter.id === "startTime" || filter.id === "createdAt") && isDateRangeFilterValue(filter.value)) {
+ return {
+ id: filter.id,
+ value: {
+ type: ColumnFilterType.DATE_RANGE,
+ data: {
+ // preserve all existing keys (e.g., convertToTimeZone/timeZone)
+ ...(filter.value as any).data,
+ startDate: newStartDate,
+ endDate: newEndDate,
+ },
+ },
+ } as ColumnFilter;
+ }
+ return filter;
+ });Then keep the added test (previous comment) to prevent regression.
📝 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.
| it("should maintain filter structure when replacing", () => { | |
| const result = replaceDateRangeColumnFilter({ | |
| columnFilters: [mockStartTimeFilter], | |
| newStartDate, | |
| newEndDate, | |
| }); | |
| expect(result?.[0]).toHaveProperty("id"); | |
| expect(result?.[0]).toHaveProperty("value"); | |
| expect(result?.[0].value).toHaveProperty("type", ColumnFilterType.DATE_RANGE); | |
| expect(result?.[0].value).toHaveProperty("data"); | |
| expect(result?.[0].value.data).toHaveProperty("startDate"); | |
| expect(result?.[0].value.data).toHaveProperty("endDate"); | |
| expect(result?.[0].value.data).toHaveProperty("preset"); | |
| }); | |
| }); | |
| return columnFilters.map((filter) => { | |
| if ((filter.id === "startTime" || filter.id === "createdAt") && isDateRangeFilterValue(filter.value)) { | |
| return { | |
| id: filter.id, | |
| value: { | |
| type: ColumnFilterType.DATE_RANGE, | |
| data: { | |
| // preserve all existing keys (e.g., convertToTimeZone/timeZone) | |
| ...(filter.value as any).data, | |
| startDate: newStartDate, | |
| endDate: newEndDate, | |
| }, | |
| }, | |
| } as ColumnFilter; | |
| } | |
| return filter; | |
| }); |
🤖 Prompt for AI Agents
In packages/features/insights/lib/bookingUtils.ts around the
replaceDateRangeColumnFilter function, the current implementation replaces the
filter.value.data object with only startDate/endDate/preset which drops any
extra keys (e.g., timezone flags); change the logic to merge the existing
filter.value.data with the new startDate/endDate/preset values (shallow merge)
so all other keys are preserved when replacing, and keep the added test to
prevent regressions.
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/trpc-router.ts (1)
462-474: Guard for missing/invalid date-range filter.extractDateRangeFromColumnFilters throws if the filter is absent; since columnFilters is optional in the schema, respond with BAD_REQUEST or inject a default before proceeding.
Apply this diff:
- const { columnFilters, timeZone } = input; - const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); + const { columnFilters, timeZone } = input; + const hasDateRange = + Array.isArray(columnFilters) && + columnFilters.some((f) => (f.id === "startTime" || f.id === "createdAt")); + if (!hasDateRange) { + throw new TRPCError({ code: "BAD_REQUEST", message: "Missing date range filter" }); + } + const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters);
🧹 Nitpick comments (1)
packages/features/insights/hooks/useInsightsBookingParameters.ts (1)
19-42: Avoid redundant dayjs serialize/parse; use helpers’ ISO directly.You’re converting to ISO, re-parsing with dayjs, and then startOf/endOf again. The helpers already return boundaries; calling toISOString() once is enough.
Apply this diff:
- startDate: dayjs(getDefaultStartDate().toISOString()).startOf("day").toISOString(), - endDate: dayjs(getDefaultEndDate().toISOString()).endOf("day").toISOString(), + startDate: getDefaultStartDate().toISOString(), + endDate: getDefaultEndDate().toISOString(),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/public/static/locales/en/common.json(1 hunks)packages/features/insights/hooks/useInsightsBookingParameters.ts(1 hunks)packages/features/insights/server/trpc-router.ts(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/public/static/locales/en/common.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/trpc-router.tspackages/features/insights/hooks/useInsightsBookingParameters.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/features/insights/server/trpc-router.tspackages/features/insights/hooks/useInsightsBookingParameters.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/insights/server/trpc-router.tspackages/features/insights/hooks/useInsightsBookingParameters.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
📚 Learning: 2025-08-22T16:38:00.225Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
Applied to files:
packages/features/insights/server/trpc-router.tspackages/features/insights/hooks/useInsightsBookingParameters.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/server/trpc-router.tspackages/features/insights/hooks/useInsightsBookingParameters.ts
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/features/insights/server/trpc-router.ts
🧬 Code graph analysis (2)
packages/features/insights/server/trpc-router.ts (2)
packages/features/insights/server/raw-data.schema.ts (1)
bookingRepositoryBaseInputSchema(103-108)packages/features/insights/lib/bookingUtils.ts (2)
replaceDateRangeColumnFilter(26-56)extractDateRangeFromColumnFilters(5-24)
packages/features/insights/hooks/useInsightsBookingParameters.ts (3)
packages/features/insights/hooks/useInsightsOrgTeams.ts (1)
useInsightsOrgTeams(6-32)packages/features/data-table/lib/dateRange.ts (3)
getDefaultStartDate(31-31)getDefaultEndDate(33-33)DEFAULT_PRESET(15-19)packages/features/data-table/lib/types.ts (1)
ColumnFilter(169-172)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
- GitHub Check: Tests / Unit
🔇 Additional comments (6)
packages/features/insights/hooks/useInsightsBookingParameters.ts (2)
6-12: LGTM on imports and defaults alignment.Importing DEFAULT_PRESET and default date helpers matches the new columnFilters-based flow and defaulting to startTime.
19-42: Incorrect — convertToTimeZone is a column-level option, not part of the DATE_RANGE filter value.DateRangeFilterOptions.convertToTimeZone (packages/features/data-table/lib/types.ts) is consumed by DateRangeFilter.tsx via options?.convertToTimeZone; the ColumnFilter.value for DATE_RANGE only contains { startDate, endDate, preset } so useInsightsBookingParameters cannot set that flag on the filter value. To enforce timezone conversion for the default, set dateRangeOptions.convertToTimeZone on the FilterableColumn for "startTime" (or pre-convert the timestamps before creating the default filter).
Likely an incorrect or invalid review comment.
packages/features/insights/server/trpc-router.ts (4)
4-7: LGTM on bookingUtils import.Brings in the right helpers for columnFilters-based date handling.
320-334: Service factory simplification looks good.Passing columnFilters through filters is the right direction; avoids split start/end handling.
1019-1026: LGTM on hourly stats.timeZone is forwarded; date targeting should be enforced by service filters.
1033-1040: LGTM on recent no-show guests.Factory usage is consistent with the new filters model.
| const previousPeriodColumnFilters = replaceDateRangeColumnFilter({ | ||
| columnFilters: input.columnFilters, | ||
| newStartDate: previousPeriodDates.startDate, | ||
| newEndDate: previousPeriodDates.endDate, | ||
| }); | ||
|
|
||
| const previousPeriodService = createInsightsBookingService(ctx, { | ||
| ...input, | ||
| startDate: previousPeriodDates.startDate, | ||
| endDate: previousPeriodDates.endDate, | ||
| columnFilters: previousPeriodColumnFilters, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Edge case: input.columnFilters can be undefined.
replaceDateRangeColumnFilter returns undefined when columnFilters is missing; ensure the service still computes previous period correctly or add a guard/fallback here.
If needed, inject a default date-range filter before replacement:
- const previousPeriodColumnFilters = replaceDateRangeColumnFilter({
- columnFilters: input.columnFilters,
+ const previousPeriodColumnFilters = replaceDateRangeColumnFilter({
+ columnFilters: input.columnFilters /* consider defaulting here if undefined */,
newStartDate: previousPeriodDates.startDate,
newEndDate: previousPeriodDates.endDate,
});Run to verify service-side defaults:
🏁 Script executed:
#!/bin/bash
rg -n -C3 'class InsightsBooking.*|constructor|calculatePreviousPeriodDates|columnFilters' packages/lib/server/service/InsightsBookingBaseService.tsLength of output: 1425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the utilities that handle column filters / date-range
rg -n --hidden -S "replaceDateRangeColumnFilter|extractDateRangeFromColumnFilters|extractDateRangeFrom" -g '!*node_modules/*' -C3 || true
# If files were found, show their full definitions (limit to reasonable size)
for f in $(rg -n --hidden -S "replaceDateRangeColumnFilter|extractDateRangeFromColumnFilters|extractDateRangeFrom" -g '!*node_modules/*' -l || true); do
echo "----- FILE: $f -----"
sed -n '1,240p' "$f"
echo "----- end $f -----"
doneLength of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "rg version:"
rg --version || true
echo
echo "Searching for helpers and usages..."
rg -n "replaceDateRangeColumnFilter" -S || true
rg -n "extractDateRangeFromColumnFilters" -S || true
rg -n "extractDateRangeFrom" -S || true
rg -n "replaceDateRange" -S || true
rg -n "ZColumnFilter" -S || true
echo
files=$(rg -n "replaceDateRangeColumnFilter|extractDateRangeFromColumnFilters|replaceDateRange|extractDateRangeFrom" -S -l || true)
if [ -n "$files" ]; then
echo "----- Found helper files -----"
for f in $files; do
echo "----- FILE: $f -----"
sed -n '1,240p' "$f" || true
echo "----- end $f -----"
done
else
echo "No exact helper files found; searching for general date-range helpers and column filter utilities..."
rg -n "dateRange|date range|columnFilter|ColumnFilter|columnFilters" -S || true
fiLength of output: 38526
Guard previous-period columnFilters to avoid a runtime throw
- replaceDateRangeColumnFilter returns undefined when columnFilters is undefined (packages/features/insights/lib/bookingUtils.ts); InsightsBookingBaseService.calculatePreviousPeriodDates calls extractDateRangeFromColumnFilters(this.filters?.columnFilters) and will throw if filters are missing (packages/lib/server/service/InsightsBookingBaseService.ts:1240).
- Update packages/features/insights/server/trpc-router.ts (~lines 366–375) to only call replaceDateRangeColumnFilter / create the previous-period service when input.columnFilters contains a date-range (or check the replaceDateRangeColumnFilter result and skip previous-period computation).
🤖 Prompt for AI Agents
In packages/features/insights/server/trpc-router.ts around lines 366 to 375,
calling replaceDateRangeColumnFilter when input.columnFilters is undefined leads
to replaceDateRangeColumnFilter returning undefined and later causing
InsightsBookingBaseService.calculatePreviousPeriodDates to throw; update the
code to guard this path by first checking that input.columnFilters includes a
date-range filter (or that replaceDateRangeColumnFilter returned a non-undefined
result) before creating previousPeriodColumnFilters and instantiating
previousPeriodService — if no date-range exists, skip previous-period
computation entirely (or pass a safe empty filters object) so
calculatePreviousPeriodDates never receives undefined.
| const { columnFilters, timeZone } = input; | ||
| const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); | ||
|
|
There was a problem hiding this comment.
Respect selected date target; don’t hard-code createdAt for grouping.
averageEventDuration groups by createdAt regardless of whether the active date target is startTime or createdAt, which will misrepresent results when startTime is selected.
Apply this diff:
- const { columnFilters, timeZone } = input;
- const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters);
+ const { columnFilters, timeZone } = input;
+ const hasDateRange =
+ Array.isArray(columnFilters) &&
+ columnFilters.some((f) => (f.id === "startTime" || f.id === "createdAt"));
+ if (!hasDateRange) {
+ throw new TRPCError({ code: "BAD_REQUEST", message: "Missing date range filter" });
+ }
+ const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters);
+ const dateTarget =
+ (columnFilters!.find((f) => f.id === "startTime" || f.id === "createdAt")?.id as
+ | "startTime"
+ | "createdAt") ?? "startTime";
@@
- const allBookings = await insightsBookingService.findAll({
- select: {
- eventLength: true,
- createdAt: true,
- },
- });
+ const allBookings = await insightsBookingService.findAll({
+ select: {
+ eventLength: true,
+ createdAt: true,
+ startTime: true,
+ },
+ });
@@
- for (const booking of allBookings) {
- const periodStart = dayjs(booking.createdAt).startOf(startOfEndOf).format("ll");
+ for (const booking of allBookings) {
+ const baseDate = dateTarget === "startTime" ? booking.startTime : booking.createdAt;
+ const periodStart = dayjs(baseDate).startOf(startOfEndOf).format("ll");Optional perf tweak per guidelines: replace repeated dayjs() in the loop with Date/Intl or dayjs.utc() if large datasets.
Also applies to: 520-525, 534-547
🤖 Prompt for AI Agents
In packages/features/insights/server/trpc-router.ts around lines 499-501 (and
similarly at 520-525 and 534-547), the grouping key is hard-coded to "createdAt"
which ignores the user's selected date target (e.g., "startTime"); update the
code to read the active date target from the request input (use the existing
input field that represents the date target or derive it from columnFilters) and
use that value for all grouping/aggregation references instead of the literal
"createdAt". Ensure the same change is applied to the other mentioned ranges
(520-525, 534-547) and, if iterating many dates, replace repeated dayjs() calls
in the loop with a single UTC-based dayjs instance or native Date/Intl
operations for better performance.
sean-brydon
left a comment
sean-brydon
left a comment
There was a problem hiding this comment.
This NIT is all i have :')
Great job working flawlessly for me.
| <div className="font-medium">{option.label}</div> | ||
| <div className="text-muted-foreground text-sm">{option.description}</div> | ||
| </div> | ||
| {selectedOption.label === option.label && ( |
There was a problem hiding this comment.
Would value comparision make more sense here?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/lib/server/service/InsightsBookingBaseService.ts (2)
340-371: Add a defensive startDate > endDate guard to avoid confusing empty results.If both bounds are present but inverted, short‑circuit to a false condition instead of returning inconsistent ANDs.
Apply this diff:
if ((id === "startTime" || id === "createdAt") && isDateRangeFilterValue(value)) { const conditions: Prisma.Sql[] = []; // if `startTime` filter -> x <= "startTime" AND "endTime" <= y // if `createdAt` filter -> x <= "createdAt" AND "createdAt" <= y + // Guard: inverted range + if (value.data.startDate && value.data.endDate) { + const s = Date.parse(value.data.startDate); + const e = Date.parse(value.data.endDate); + if (isNaN(s) || isNaN(e)) { + throw new Error(`Invalid date range: ${value.data.startDate} ~ ${value.data.endDate}`); + } + if (s > e) { + return NOTHING_CONDITION; + } + }
1240-1244: Potential throw when columnFilters are missing.extractDateRangeFromColumnFilters throws if no date‑range filter exists. calculatePreviousPeriodDates will then throw for consumers that don’t pass columnFilters (API callers, tests).
Two options:
- Service-level: return a descriptive error early or a documented default.
- Router-level: guard before calling this method (see router comment).
If you prefer a service guard, minimal change:
- const result = extractDateRangeFromColumnFilters(this.filters?.columnFilters); + if (!this.filters?.columnFilters) { + throw new Error("No date range filter found in columnFilters"); + } + const result = extractDateRangeFromColumnFilters(this.filters.columnFilters);packages/features/insights/server/trpc-router.ts (3)
366-371: Guard previous‑period replacement to handle missing columnFilters.replaceDateRangeColumnFilter returns undefined when input.columnFilters is undefined. More importantly, calculatePreviousPeriodDates right above (Line 365) will throw if there’s no date‑range filter.
Suggested guarded flow (requires moving the calculate call inside the guard):
- const previousPeriodDates = currentPeriodService.calculatePreviousPeriodDates(); - const previousPeriodColumnFilters = replaceDateRangeColumnFilter({ - columnFilters: input.columnFilters, - newStartDate: previousPeriodDates.startDate, - newEndDate: previousPeriodDates.endDate, - }); + let previousPeriodColumnFilters = input.columnFilters; + try { + const previousPeriodDates = currentPeriodService.calculatePreviousPeriodDates(); + previousPeriodColumnFilters = replaceDateRangeColumnFilter({ + columnFilters: input.columnFilters, + newStartDate: previousPeriodDates.startDate, + newEndDate: previousPeriodDates.endDate, + }); + } catch { + // No date range: skip previous-period comparison + previousPeriodColumnFilters = undefined; + }Note: You’ll also need to move Line 365 inside this try/catch.
462-464: Return BAD_REQUEST when date range filter is missing.extractDateRangeFromColumnFilters throws; currently this bubbles into a 500. Make it explicit.
Apply this diff:
- const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); + if (!Array.isArray(columnFilters)) { + throw new TRPCError({ code: "BAD_REQUEST", message: "Missing date range filter" }); + } + const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters);
499-525: AverageEventDuration: good switch to dateTarget; align timezone for bucketing.Now that grouping respects dateTarget, ensure buckets use the requested timeZone to avoid local‑TZ drift.
Apply this diff to anchor grouping to the input timeZone:
- for (const booking of allBookings) { - const periodStart = dayjs(dateTarget === "startTime" ? booking.startTime : booking.createdAt) - .startOf(startOfEndOf) - .format("ll"); + for (const booking of allBookings) { + const baseDate = dateTarget === "startTime" ? booking.startTime : booking.createdAt; + const periodStart = dayjs.tz(baseDate, timeZone).startOf(startOfEndOf).format("ll"); if (resultMap.has(periodStart)) {Optional: also initialize resultMap keys with dayjs.tz(range.startDate, timeZone) for full consistency.
Also applies to: 536-539
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/features/insights/lib/__tests__/bookingUtils.test.ts(1 hunks)packages/features/insights/lib/bookingUtils.ts(1 hunks)packages/features/insights/server/trpc-router.ts(13 hunks)packages/lib/server/service/InsightsBookingBaseService.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/features/insights/lib/tests/bookingUtils.test.ts
- packages/features/insights/lib/bookingUtils.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/lib/server/service/InsightsBookingBaseService.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/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.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/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: eunjae-lee
PR: calcom/cal.com#23752
File: packages/lib/server/service/InsightsBookingBaseService.ts:340-371
Timestamp: 2025-09-18T08:50:49.906Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), the startTime date range filter intentionally uses different fields for the lower and upper bounds: `startDate <= "startTime" AND "endTime" <= endDate`. This ensures bookings start within the selected range but don't extend beyond the end date, which is the intended business logic and not an inconsistency.
📚 Learning: 2025-09-18T08:50:49.906Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23752
File: packages/lib/server/service/InsightsBookingBaseService.ts:340-371
Timestamp: 2025-09-18T08:50:49.906Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), the startTime date range filter intentionally uses different fields for the lower and upper bounds: `startDate <= "startTime" AND "endTime" <= endDate`. This ensures bookings start within the selected range but don't extend beyond the end date, which is the intended business logic and not an inconsistency.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.ts
📚 Learning: 2025-07-24T08:39:06.185Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.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/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.ts
📚 Learning: 2025-09-18T08:42:14.954Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23752
File: packages/features/insights/server/trpc-router.ts:560-563
Timestamp: 2025-09-18T08:42:14.954Z
Learning: In getMembersStatsWithCount() with completed: true, the SQL condition `"endTime" <= NOW()` is intentionally added to ensure only completed bookings (those in the past) are included in the member statistics.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.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/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/features/insights/server/trpc-router.ts
🧬 Code graph analysis (2)
packages/lib/server/service/InsightsBookingBaseService.ts (1)
packages/features/insights/lib/bookingUtils.ts (1)
extractDateRangeFromColumnFilters(5-25)
packages/features/insights/server/trpc-router.ts (2)
packages/features/insights/server/raw-data.schema.ts (1)
bookingRepositoryBaseInputSchema(103-108)packages/features/insights/lib/bookingUtils.ts (2)
replaceDateRangeColumnFilter(27-57)extractDateRangeFromColumnFilters(5-25)
🔇 Additional comments (10)
packages/lib/server/service/InsightsBookingBaseService.ts (6)
7-9: Typed imports look good.Explicit ColumnFilter imports improve clarity and TS help.
16-19: Good: reuse bookingUtils helpers.Keeps date‑range parsing centralized and consistent with the UI.
340-371: Date‑range logic for startTime vs createdAt is correct (per business rule).Lower bound on startTime and upper bound on endTime is intentional; thanks for the inline comment.
881-889: API shape improvement LGTM.The new options object for getMembersStatsWithCount is clearer and extensible.
892-904: Timezone check for “completed” filter.Confirm BookingTimeStatusDenormalized."endTime" is timestamptz. If it’s timestamp without time zone, comparing to NOW() (timestamptz) can be implicit-cast tricky across DB timezones.
If not timestamptz, prefer one of:
- Cast both sides to UTC:
"endTime" AT TIME ZONE 'UTC' <= NOW() AT TIME ZONE 'UTC'- Or store/compare in timestamptz.
Would you like a patch once column type is confirmed?
906-909: AND composer fix is solid.The reducer no longer introduces a leading AND; safe parenthesization prevents precedence bugs.
packages/features/insights/server/trpc-router.ts (4)
4-7: Good: centralized date‑range helpers imported.Keeps router lean and consistent with server service.
320-333: Factory now forwards columnFilters — LGTM.Matches the new backend contract; keeps DI surface stable.
563-566: Call sites updated to new getMembersStatsWithCount signature — looks correct.Types and completed flags match the intended behavior.
Also applies to: 574-582, 592-596, 607-607, 618-618
1024-1024: Service invocations wired correctly.Bookings by hour and recent no‑shows now respect baseConditions built from columnFilters.
Also applies to: 1036-1036
E2E results are ready! |
What does this PR do?
Visual Demo (For contributors especially)
Screenshot.2025-09-11.at.17.04.36.mp4
Code review
Screenshot.2025-09-12.at.17.19.24.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?