-
Notifications
You must be signed in to change notification settings - Fork 10.5k
feat: add hourly booking charts on /insights #22619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add hourly booking charts on /insights #22619
Conversation
WalkthroughThis set of changes introduces a new "Bookings by Hour" analytics feature across both backend and frontend. On the backend, a new method Estimated code review effort3 (30–60 minutes) Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/features/insights/components/BookingsByHourChart.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/lib/server/service/insightsBooking.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ No security or compliance issues detected. Reviewed everything up to f62e301. Security Overview
Detected Code Changes
Reply to this PR with |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/21/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (07/21/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/page.tsx (1)
8-11
: Add trailing comma & localise titles
- A trailing comma after the last object keeps the array style consistent with the previous entry.
- Both link titles are user-visible strings; they should flow through our i18n layer instead of being hard-coded.
{ title: "Bookings by Hour", href: "/settings/admin/playground/bookings-by-hour", + },
Example localisation:
import { useTranslations } from "next-intl"; // … const t = useTranslations("admin.playground"); const LINKS = [ { title: t("routing_funnel"), href: "/settings/admin/playground/routing-funnel" }, { title: t("bookings_by_hour"), href: "/settings/admin/playground/bookings-by-hour" }, ];Ensure the corresponding keys exist in the locale files (
admin.playground.routing_funnel
,admin.playground.bookings_by_hour
).packages/features/insights/components/TotalBookingUsersTable.tsx (1)
25-26
: Padding override & potentially non-unique key
ChartCardItem
already setspy-3.5
; appendingpy-3
overrides it via Tailwind’s last-class-wins rule. That works, but it leaves duplicate utility classes in the markup and may confuse future readers. Consider either:-<ChartCardItem key={item.userId} count={item.count} className="py-3"> +<ChartCardItem key={item.userId ?? `${item.emailMd5}-${item.count}`}" count={item.count} className="py-3">and remove the hard-coded
py-3.5
insideChartCardItem
if finer control is needed across callers.Additionally,
key={item.userId}
can collapse to"null"
for guests, causing React key collisions. Safeguard with a fallback (e.g., email hash + count or array index).apps/web/public/static/locales/en/common.json (1)
2170-2170
: New key looks good – ensure parity across all locales
"bookings_by_hour"
follows the existing snake-case convention and the value reads well.
Just make sure the key is added (even if untranslated) to every other locale file (common.json
inde
,fr
, etc.) to avoid runtime fall-backs or missing-key warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/bookings-by-hour/page.tsx
(1 hunks)apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/page.tsx
(1 hunks)apps/web/modules/insights/insights-view.tsx
(2 hunks)apps/web/public/static/locales/en/common.json
(1 hunks)packages/features/insights/components/BookingsByHourChart.tsx
(1 hunks)packages/features/insights/components/ChartCard.tsx
(1 hunks)packages/features/insights/components/LoadingInsights.tsx
(1 hunks)packages/features/insights/components/TotalBookingUsersTable.tsx
(1 hunks)packages/features/insights/components/index.ts
(1 hunks)packages/features/insights/server/raw-data.schema.ts
(1 hunks)packages/features/insights/server/trpc-router.ts
(2 hunks)packages/lib/server/service/__tests__/insightsRouting.integration-test.ts
(1 hunks)packages/lib/server/service/insightsBooking.ts
(1 hunks)packages/lib/server/service/insightsRouting.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 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.341Z
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.
packages/features/insights/components/TotalBookingUsersTable.tsx (1)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
packages/features/insights/components/index.ts (2)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
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.
apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/page.tsx (2)
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the currentLink.maxUsageCount ?? 1
fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
packages/lib/server/service/__tests__/insightsRouting.integration-test.ts (3)
Learnt from: eunjae-lee
PR: #22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.359Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
packages/lib/server/service/insightsRouting.ts (3)
Learnt from: eunjae-lee
PR: #22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.359Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/bookings-by-hour/page.tsx (1)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
apps/web/public/static/locales/en/common.json (1)
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
packages/features/insights/server/trpc-router.ts (3)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.359Z
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.
apps/web/modules/insights/insights-view.tsx (2)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
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.
packages/features/insights/components/BookingsByHourChart.tsx (2)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.497Z
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.
packages/features/insights/server/raw-data.schema.ts (2)
Learnt from: eunjae-lee
PR: #22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.359Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
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.
🧬 Code Graph Analysis (3)
packages/features/insights/components/TotalBookingUsersTable.tsx (1)
packages/features/insights/components/ChartCard.tsx (1)
ChartCardItem
(54-73)
apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/bookings-by-hour/page.tsx (2)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard
(14-52)packages/features/insights/components/BookingsByHourChart.tsx (1)
BookingsByHourChartContent
(28-65)
packages/features/insights/server/trpc-router.ts (3)
packages/features/insights/server/raw-data.schema.ts (1)
bookingRepositoryBaseInputSchema
(73-81)packages/lib/server/service/insightsBooking.ts (1)
InsightsBookingService
(45-269)packages/platform/libraries/index.ts (1)
TRPCError
(56-56)
⏰ 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). (3)
- GitHub Check: Detect changes
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (16)
packages/features/insights/components/LoadingInsights.tsx (1)
2-2
: Import reorder looks fineNo functional impact; ordering change complies with our style-guides.
packages/features/insights/components/index.ts (1)
3-4
: LGTM! Clean export addition.The new
BookingsByHourChart
export is correctly positioned alphabetically and follows the consistent formatting pattern of the other exports.packages/lib/server/service/insightsRouting.ts (1)
172-176
: LGTM! Consistent SQL condition formatting.The addition of parentheses around SQL conditions improves logical grouping and maintains consistency across the codebase. This formatting change enhances readability without altering the underlying logic.
packages/lib/server/service/__tests__/insightsRouting.integration-test.ts (1)
557-557
: LGTM! Test updated to match implementation changes.The test expectation correctly reflects the new SQL condition formatting with additional parentheses wrapping, ensuring consistency between the implementation and test coverage.
packages/features/insights/components/ChartCard.tsx (1)
54-68
: LGTM! Good component enhancement for styling flexibility.The addition of the optional
className
prop properly extends the component's styling capabilities while maintaining backward compatibility. The implementation correctly uses theclassNames
utility to merge custom styles with existing base classes.apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/bookings-by-hour/page.tsx (1)
1-61
: LGTM! Well-structured playground page.This playground page effectively demonstrates the BookingsByHourChart component with:
- Comprehensive sample data covering all 24 hours
- Proper component composition using ChartCard and BookingsByHourChartContent
- Localization support with useLocale
- Clean layout with descriptive content and sample data display
- Good developer experience for testing and demonstration
The implementation follows established patterns and React best practices.
packages/features/insights/server/trpc-router.ts (2)
11-11
: LGTM!The import of
bookingRepositoryBaseInputSchema
is correctly added to support the new hourly bookings stats procedure.
13-13
: LGTM!The import of
InsightsBookingService
is correctly added to support the new procedure implementation.packages/features/insights/server/raw-data.schema.ts (1)
73-81
: Well-structured schema additionThe new
bookingRepositoryBaseInputSchema
follows established patterns and includes appropriate field types for hourly booking statistics. The requiredtimeZone
field and optional nullable filters foreventTypeId
andmemberUserId
provide good flexibility for the feature.apps/web/modules/insights/insights-view.tsx (3)
16-16
: LGTM!The import of
BookingsByHourChart
is correctly added to support the new hourly bookings feature.
75-82
: Excellent responsive layout improvementThe new grid structure effectively organizes the charts with proper responsive behavior. The
BookingsByHourChart
andAverageEventDurationChart
are logically grouped together, each spanning 2 columns on smaller screens and sharing the 4-column grid on larger screens.
84-102
: Well-organized component groupingThe reorganization of tables into separate grid containers improves visual separation and maintains consistent responsive behavior across different screen sizes. The layout structure is clean and logical.
packages/lib/server/service/insightsBooking.ts (2)
68-115
: Excellent implementation of hourly booking statisticsThe method is well-implemented with several strong points:
- Proper date format validation prevents SQL injection and ensures data integrity
- Raw SQL query with CTE provides efficient data aggregation
- Timezone handling using PostgreSQL's
AT TIME ZONE
is correct- Filtering by 'accepted' status ensures only confirmed bookings are counted
- Complete 24-hour data array prevents gaps in the chart visualization
The use of
Map
for result lookup andArray.from
for generating the complete hourly dataset is an efficient approach.
122-127
: Improved SQL condition formattingThe consistent wrapping of SQL conditions in parentheses ensures proper logical grouping when combining authorization and filter conditions. This prevents potential issues with operator precedence in complex WHERE clauses.
packages/features/insights/components/BookingsByHourChart.tsx (2)
28-65
: Well-implemented chart content componentThe
BookingsByHourChartContent
component is excellently structured with:
- Proper data transformation for hour formatting
- Appropriate empty state handling
- Good chart configuration with disabled decimals and clean styling
- Responsive design using ResponsiveContainer
- Proper accessibility with custom tooltip
The bar chart styling with rounded corners and hover effects provides a polished user experience.
100-132
: Excellent main chart component implementationThe main
BookingsByHourChart
component demonstrates excellent practices:
- Proper use of custom hooks for parameters and data table context
- Well-configured TRPC query with appropriate stale time and batching settings
- Proper loading and error state handling
- Good fallback to
CURRENT_TIMEZONE
when timeZone is not available- Clean integration with the ChartCard wrapper
The component follows established patterns in the codebase and provides a solid foundation for the hourly booking insights feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with this! Great job! Thanks for fixing feedback
E2E results are ready! |
* feat: add hourly booking charts on /insights * safe guard * rename * update styles * fix data * clean up * clean up * re-order charts * update style * apply feedback * rename * update query
What does this PR do?
This PR adds hourly booking charts on the /insights page.
In order for that, I have
added a new method
getHourlyBookingStats
inInsightsBookingService
to fetch the dataadded a trpc handler to run that method
implemented
BookingsByHourChart.tsx
update some styles here and there
Fixes CAL-6108
Visual Demo (For contributors especially)
Screenshot.2025-07-21.at.14.00.45.mp4
Screenshot.2025-07-21.at.14.00.19.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist