refactor: replace tremor with recharts#22791
Conversation
WalkthroughThis set of changes removes the dependency on the Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Complexity label: Complex Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 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
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (4)
apps/web/public/static/locales/en/common.json (1)
3422-3422: Avoid overly-generic localisation keys
"stats"is very broad; generic keys tend to collide or get (re)used in unintended contexts down the line. Recommend renaming to something scoped, e.g.:-"stats": "Stats" +"insights_stats_tab": "Stats"and replicating that new key wherever it’s referenced. This keeps the namespace unambiguous and future-proofs translations.
packages/features/insights/components/booking/AverageEventDurationChart.tsx (1)
19-34: Consider improving the duration formatting logic.The
formatDurationfunction handles most cases well, but there are a few potential improvements:const formatDuration = (minutes: number) => { - if (!minutes) return "0m"; + if (!minutes || minutes < 0) return "0m"; const hours = Math.floor(minutes / 60); const remainingMinutes = minutes % 60; - const remainingMinutesStr = remainingMinutes.toFixed(1); + const remainingMinutesStr = remainingMinutes < 1 && remainingMinutes > 0 + ? remainingMinutes.toFixed(1) + : Math.round(remainingMinutes).toString(); if (hours > 0 && remainingMinutes > 0) { return `${hours}h ${remainingMinutesStr}m`; } else if (hours > 0) { return `${hours}h`; } else { return `${remainingMinutesStr}m`; } };This handles negative values and avoids unnecessary decimals for whole minutes.
packages/ui/components/progress-bar/ProgressBar.tsx (1)
6-19: Consider using theme-aware color classes.The current implementation uses fixed color classes that may not integrate well with the application's theme system. Consider using theme-aware color utilities if available in the design system.
For example, if your design system has semantic color tokens:
color: { - green: "bg-green-500", - blue: "bg-blue-500", - red: "bg-red-500", - yellow: "bg-yellow-500", - gray: "bg-gray-500", + green: "bg-success", + blue: "bg-primary", + red: "bg-error", + yellow: "bg-warning", + gray: "bg-muted", },packages/features/insights/components/routing/FailedBookingsByField.tsx (1)
121-124: Consider removing debug information from production code.The JSON.stringify debug output could expose sensitive data structure information in production and may not be useful to end users.
Consider removing or conditionalizing the debug information:
- No data available for selected field - <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div> + {t("insights_no_data_found_for_filter")}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
apps/web/modules/insights/insights-view.tsx(0 hunks)apps/web/modules/settings/billing/components/BillingCredits.tsx(1 hunks)apps/web/package.json(0 hunks)apps/web/public/static/locales/en/common.json(2 hunks)apps/web/tailwind.config.js(1 hunks)packages/features/insights/components/Card.tsx(0 hunks)packages/features/insights/components/ChartCard.tsx(3 hunks)packages/features/insights/components/KPICard.tsx(2 hunks)packages/features/insights/components/LineChart.tsx(0 hunks)packages/features/insights/components/LoadingInsights.tsx(1 hunks)packages/features/insights/components/UserStatsTable.tsx(1 hunks)packages/features/insights/components/booking/AverageEventDurationChart.tsx(2 hunks)packages/features/insights/components/booking/BookingKPICards.tsx(0 hunks)packages/features/insights/components/booking/BookingsByHourChart.tsx(1 hunks)packages/features/insights/components/booking/EventTrendsChart.tsx(2 hunks)packages/features/insights/components/booking/RecentFeedbackTable.tsx(2 hunks)packages/features/insights/components/booking/RecentFeedbackTableContent.tsx(1 hunks)packages/features/insights/components/index.ts(0 hunks)packages/features/insights/components/routing/FailedBookingsByField.tsx(4 hunks)packages/features/insights/components/routing/RoutedToPerPeriod.tsx(2 hunks)packages/features/insights/components/routing/RoutingFormResponsesTable.tsx(1 hunks)packages/features/insights/components/routing/RoutingKPICards.tsx(2 hunks)packages/features/insights/components/tremor.css(0 hunks)packages/features/insights/components/tremor.types.ts(0 hunks)packages/features/insights/components/tremor/BarList.tsx(0 hunks)packages/features/insights/lib/colors.ts(0 hunks)packages/features/insights/lib/index.ts(0 hunks)packages/lib/server/service/insightsBooking.ts(1 hunks)packages/platform/atoms/tailwind.config.cjs(1 hunks)packages/ui/components/progress-bar/ProgressBar.tsx(1 hunks)packages/ui/components/progress-bar/index.ts(1 hunks)packages/ui/package.json(1 hunks)
💤 Files with no reviewable changes (11)
- packages/features/insights/components/index.ts
- packages/features/insights/components/booking/BookingKPICards.tsx
- packages/features/insights/components/Card.tsx
- apps/web/package.json
- packages/features/insights/components/tremor.css
- apps/web/modules/insights/insights-view.tsx
- packages/features/insights/lib/colors.ts
- packages/features/insights/lib/index.ts
- packages/features/insights/components/tremor/BarList.tsx
- packages/features/insights/components/LineChart.tsx
- packages/features/insights/components/tremor.types.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/insights/components/ChartCard.tsxapps/web/modules/settings/billing/components/BillingCredits.tsxpackages/features/insights/components/routing/FailedBookingsByField.tsxpackages/features/insights/components/booking/EventTrendsChart.tsxpackages/features/insights/components/KPICard.tsxpackages/features/insights/components/booking/RecentFeedbackTable.tsxpackages/features/insights/components/UserStatsTable.tsxpackages/features/insights/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/components/booking/BookingsByHourChart.tsxpackages/features/insights/components/routing/RoutedToPerPeriod.tsxpackages/features/insights/components/LoadingInsights.tsxpackages/ui/components/progress-bar/ProgressBar.tsxpackages/features/insights/components/booking/RecentFeedbackTableContent.tsxpackages/features/insights/components/booking/AverageEventDurationChart.tsxpackages/features/insights/components/routing/RoutingKPICards.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/ChartCard.tsxapps/web/modules/settings/billing/components/BillingCredits.tsxpackages/features/insights/components/routing/FailedBookingsByField.tsxpackages/features/insights/components/booking/EventTrendsChart.tsxpackages/features/insights/components/KPICard.tsxpackages/features/insights/components/booking/RecentFeedbackTable.tsxpackages/lib/server/service/insightsBooking.tspackages/features/insights/components/UserStatsTable.tsxpackages/features/insights/components/routing/RoutingFormResponsesTable.tsxpackages/features/insights/components/booking/BookingsByHourChart.tsxpackages/features/insights/components/routing/RoutedToPerPeriod.tsxpackages/features/insights/components/LoadingInsights.tsxpackages/ui/components/progress-bar/index.tspackages/ui/components/progress-bar/ProgressBar.tsxpackages/features/insights/components/booking/RecentFeedbackTableContent.tsxpackages/features/insights/components/booking/AverageEventDurationChart.tsxpackages/features/insights/components/routing/RoutingKPICards.tsx
**/*.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/insightsBooking.tspackages/ui/components/progress-bar/index.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
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 **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
packages/features/insights/components/routing/FailedBookingsByField.tsx (3)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
Learnt from: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
packages/features/insights/components/booking/EventTrendsChart.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/booking/RecentFeedbackTable.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/lib/server/service/insightsBooking.ts (3)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/UserStatsTable.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/ui/package.json (3)
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 **/*.tsx : Always use t() for text localization in frontend code; direct text embedding should trigger a warning
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 **/*.{service,repository}.ts : Avoid dot-suffixes like .service.ts or .repository.ts for new files; reserve .test.ts, .spec.ts, .types.ts for their specific purposes
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
packages/features/insights/components/routing/RoutingFormResponsesTable.tsx (4)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
Learnt from: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/booking/BookingsByHourChart.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/routing/RoutedToPerPeriod.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/booking/RecentFeedbackTableContent.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/booking/AverageEventDurationChart.tsx (3)
Learnt from: eunjae-lee
PR: #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.
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
Learnt from: eunjae-lee
PR: #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.
packages/features/insights/components/routing/RoutingKPICards.tsx (2)
Learnt from: eunjae-lee
PR: #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.
Learnt from: eunjae-lee
PR: #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.
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.233Z
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.
🧬 Code Graph Analysis (2)
packages/features/insights/components/KPICard.tsx (2)
packages/features/insights/components/index.ts (1)
KPICard(6-6)packages/features/insights/lib/index.ts (2)
calculateDeltaType(1-1)valueFormatter(2-2)
packages/features/insights/components/LoadingInsights.tsx (1)
packages/features/insights/components/ChartCard.tsx (1)
ChartCard(16-58)
🔇 Additional comments (39)
packages/features/insights/components/booking/RecentFeedbackTable.tsx (1)
9-9: LGTM! Clean component rename refactor.The import and usage have been consistently updated from
FeedbackTabletoRecentFeedbackTableContent. The component interface remains unchanged, maintaining backward compatibility.Also applies to: 29-29
packages/features/insights/components/booking/RecentFeedbackTableContent.tsx (3)
5-5: LGTM! Improved type safety with RouterOutputs.Using
RouterOutputs["viewer"]["insights"]["recentRatings"]instead of inline types provides better type safety and consistency with the TRPC API contract.Also applies to: 11-11
13-13: LGTM! Component rename is consistent.The component has been properly renamed from
FeedbackTabletoRecentFeedbackTableContentwith the updated type usage.
17-18: LGTM! Simplified conditional logic.The removal of optional chaining suggests the type system now guarantees
datais properly defined, which aligns with the stronger typing fromRouterOutputs.packages/features/insights/components/booking/BookingsByHourChart.tsx (2)
16-16: LGTM: Added RouterOutputs import for centralized typing.Adding the RouterOutputs import enables the use of backend-derived types, improving type consistency between frontend and backend.
22-22: Excellent type centralization improvement.Replacing the inline type definition with
RouterOutputs["viewer"]["insights"]["bookingsByHourStats"][number]ensures type consistency with the backend API schema. This eliminates potential drift between manually defined types and actual API responses.packages/features/insights/components/UserStatsTable.tsx (2)
5-5: LGTM: RouterOutputs import for type centralization.Adding the RouterOutputs import supports the type alias refactoring and ensures consistency with backend schema.
10-12: Excellent type alias implementation.The
UserStatsDatatype alias derived fromRouterOutputs["viewer"]["insights"]["membersWithMostBookings"]centralizes type definitions and ensures frontend-backend type consistency. The updated function parameter type maintains the same interface while leveraging the new centralized type.packages/features/insights/components/routing/RoutedToPerPeriod.tsx (2)
12-12: LGTM: RouterOutputs import added for centralized typing.Adding the RouterOutputs import enables the use of backend-derived types, supporting the type alias refactoring approach.
169-169: Excellent type centralization with RouterOutputs.Replacing the manually defined
RoutedToTableRowtype withRouterOutputs["viewer"]["insights"]["routedToPerPeriod"]["users"]["data"][number]ensures type consistency between the frontend component and backend API schema. This eliminates potential type drift and improves maintainability.packages/ui/package.json (1)
46-46: Path looks correct – verify naming convention alignment
progress-baris kebab-cased whereas several nearby component export keys (e.g.buttonGroup,createButton) use camel-case. The mixed style already exists, but if we are converging on one convention for new components it’s worth double-checking before publishing.
No functional issue, just consistency.apps/web/public/static/locales/en/common.json (1)
3394-3394: Key name is fine; confirm pluralisation & translations later
"failed_bookings"complements the existing"failed_bookings_by_field"key and is self-explanatory. No issues from an English-source standpoint, just make sure any non-English locale files receive the same addition to avoid runtime fallback.packages/platform/atoms/tailwind.config.cjs (1)
5-5: LGTM! Tremor paths correctly removed from Tailwind content.The removal of tremor package paths from the content array aligns perfectly with the PR objective to replace tremor with recharts.
packages/features/insights/components/KPICard.tsx (4)
4-8: Good migration from tremor to Cal.com UI components.The import changes properly replace tremor dependencies with Cal.com's internal UI components.
27-54: Well-structured helper functions for UI mapping.The
getBadgeVariantandgetDeltaIconfunctions provide clean separation of concerns and good mapping between delta types and UI elements.
59-84: UI structure properly refactored with good localization.The component correctly uses
t()for localization and the new structure using native divs with Cal.com styling classes is appropriate.
24-25: Confirm Delta Calculation in KPICard.tsx (packages/features/insights/components/KPICard.tsx:24–25)
The code currently does:const deltaType = calculateDeltaType(previousMetricData.deltaPrevious - previousMetricData.count); const deltaValue = Number(previousMetricData.deltaPrevious).toFixed(0);– Typically, a delta is computed as
current – previous, but here it’sdeltaPrevious – count.
– I couldn’t locate wherepreviousMetricData(or itsdeltaPreviousfield) is defined/passed in the repo.Please manually verify that:
deltaPreviousindeed represents the “current” metric value (or otherwise swap the subtraction order),- and that this logic matches the intended UI behavior.
apps/web/modules/settings/billing/components/BillingCredits.tsx (1)
18-18: Correct migration from tremor to Cal.com ProgressBar component.The import change properly replaces the tremor dependency while maintaining the same API, ensuring no functional changes are needed.
apps/web/tailwind.config.js (1)
5-5: Consistent tremor removal from Tailwind configuration.The removal of tremor package paths from the content array is consistent with the overall migration strategy and correctly maintains only the necessary routing forms path.
packages/lib/server/service/insightsBooking.ts (1)
660-662: Good enhancement to support detailed date formatting.The addition of
formattedDateFullproperty provides more comprehensive date information for the recharts components while maintaining backwards compatibility with existingformattedDateusage.packages/ui/components/progress-bar/index.ts (1)
1-2: LGTM! Clean module exports.The re-export pattern follows standard conventions for component libraries, providing a clean public API for the ProgressBar component and its types.
packages/features/insights/components/LoadingInsights.tsx (2)
3-3: Good migration to ChartCard!The replacement of
CardInsightswithChartCardaligns well with the broader Tremor removal. TheChartCardcomponent's support forReact.ReactNodeas title enables passing theSkeletonTextcomponent directly, which is a cleaner approach.Also applies to: 7-7, 23-23
9-21: Simplified loading spinner implementation looks good.The SVG spinner has been appropriately simplified by removing conditional styling and using consistent stroke properties. The explicit dimensions and viewBox make it more predictable.
packages/features/insights/components/ChartCard.tsx (2)
24-24: Excellent enhancement for title flexibility!Expanding the
titleprop to acceptReact.ReactNodegreatly improves the component's flexibility, enabling rich content like skeleton loaders or formatted text. The conditional rendering logic is clean and type-safe.Also applies to: 34-38
87-87: Good refinement of legend spacing.The padding adjustments on legend items (
py-1→py-0.5) and the addition of vertical padding on tooltip paragraphs improve the visual hierarchy and spacing consistency.Also applies to: 93-93
packages/features/insights/components/booking/AverageEventDurationChart.tsx (1)
3-3: Excellent migration to recharts!The direct implementation using recharts components provides much better control and customization compared to wrapper components. The chart configuration is comprehensive with proper responsive container, styling, and animation settings.
Also applies to: 95-120
packages/ui/components/progress-bar/ProgressBar.tsx (1)
27-38: Well-implemented progress bar component!The component design is solid with proper percentage clamping, clean styling using cva variants, and flexible label support. The use of
class-variance-authorityfor color variants is a good architectural choice.packages/features/insights/components/routing/RoutingKPICards.tsx (3)
5-6: LGTM: Import additions align with the Tremor replacement.The new imports for
classNames,SkeletonText, andChartCardproperly support the refactor from Tremor components to custom implementations.Also applies to: 10-10
63-78: Well-structured refactor from Tremor Grid to CSS Grid.The implementation successfully replaces Tremor's
Gridcomponent with native CSS Grid using Tailwind classes, maintaining responsive behavior. The card styling with borders and padding provides consistent visual hierarchy.
83-103: Good addition of localization to LoadingKPICards.The addition of
useLocalehook ensures consistent title localization across both main and loading components. The skeleton loading structure matches the main component layout well.packages/features/insights/components/routing/FailedBookingsByField.tsx (4)
4-13: LGTM: Comprehensive recharts imports for vertical bar chart.The imports include all necessary recharts components for implementing a detailed vertical bar chart with custom styling and interactions.
22-53: Well-implemented custom tooltip with proper localization.The tooltip component correctly uses the
t()function for localization and provides good null-safety checks. The styling matches the design system with CSS variables.
74-83: Good empty state handling with proper localization.The empty data detection logic is sound, and the no-data message uses proper localization with
t().
94-119: Excellent recharts implementation with proper theming.The BarChart configuration is comprehensive with good responsive design, proper margins, and CSS variable integration for theming. The use of
ResponsiveContainerensures proper scaling.packages/features/insights/components/booking/EventTrendsChart.tsx (5)
3-3: LGTM: Complete recharts import for line chart implementation.The import includes all necessary recharts components for a fully-featured line chart with custom styling and interactions.
14-30: Well-structured color palette and legend configuration.The color constants use consistent hex values and the exported legend array enables reuse across components. The structure is clean and maintainable.
32-32: Good use of RouterOutputs for type safety.The type definition ensures type safety and consistency with the API contract.
34-64: Excellent custom tooltip with proper typing and localization.The tooltip implementation includes comprehensive TypeScript typing, proper null-safety checks, and uses the
valueFormatterfor consistent data presentation. The styling integrates well with the design system.
86-134: Comprehensive recharts implementation with excellent configuration.The LineChart implementation includes:
- Proper responsive container setup
- Good margin configuration
- CSS class integration for styling
- Consistent use of valueFormatter
- Dynamic line generation with proper color mapping
- Animation configuration for enhanced UX
The implementation maintains all the functionality while moving away from Tremor dependencies.
| } | ||
|
|
||
| return ( | ||
| <div className="bg-default text-inverted border-subtle rounded-lg border p-3 shadow-lg"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix tooltip styling classes.
The tooltip has conflicting text color classes that may cause styling issues.
- <div className="bg-default text-inverted border-subtle rounded-lg border p-3 shadow-lg">
+ <div className="bg-default border-subtle rounded-lg border p-3 shadow-lg">Remove text-inverted since text-default is used on child elements, or use consistent color classes throughout.
📝 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.
| <div className="bg-default text-inverted border-subtle rounded-lg border p-3 shadow-lg"> | |
| <div className="bg-default border-subtle rounded-lg border p-3 shadow-lg"> |
🤖 Prompt for AI Agents
In packages/features/insights/components/booking/AverageEventDurationChart.tsx
at line 57, the div has conflicting text color classes with both text-inverted
and child elements using text-default. To fix this, remove the text-inverted
class from the div to ensure consistent text color styling throughout the
tooltip.
| const { data: headers, isSuccess: isHeadersSuccess } = | ||
| trpc.viewer.insights.routingFormResponsesHeaders.useQuery({ | ||
| userId, | ||
| teamId, | ||
| isAll, | ||
| routingFormId, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider the UX impact of removing loading state handling.
The removal of isHeadersLoading eliminates loading feedback while headers are being fetched. This could result in a brief flash of empty or incorrectly configured table content during initial load, since both processedData (line 68) and useInsightsColumns (line 72) depend on isHeadersSuccess.
Consider adding alternative loading feedback or ensuring the table gracefully handles the loading state without showing incomplete content.
If loading skeleton removal is intentional, ensure the DataTableWrapper handles the pending state appropriately:
const { data: headers, isSuccess: isHeadersSuccess } =
trpc.viewer.insights.routingFormResponsesHeaders.useQuery({
userId,
teamId,
isAll,
routingFormId,
});Alternatively, if loading feedback is needed, consider showing a loading state when headers are not yet successful:
+const { data: headers, isSuccess: isHeadersSuccess, isLoading: isHeadersLoading } =
trpc.viewer.insights.routingFormResponsesHeaders.useQuery({
userId,
teamId,
isAll,
routingFormId,
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/features/insights/components/routing/RoutingFormResponsesTable.tsx
around lines 41 to 47, the loading state handling for headers was removed, which
can cause the table to briefly display incomplete or empty content during data
fetching. To fix this, reintroduce a loading state check using isHeadersLoading
or a similar flag from the query, and conditionally render a loading indicator
or skeleton UI while headers are loading. Alternatively, ensure the
DataTableWrapper component gracefully handles the loading state by not rendering
incomplete data or by showing a placeholder until headers are fully loaded.
| export function ProgressBar({ color, percentageValue, label, className }: ProgressBarProps) { | ||
| const clampedPercentage = Math.min(100, Math.max(0, percentageValue)); | ||
|
|
||
| return ( | ||
| <div className={classNames("flex w-full items-center gap-3", className)}> | ||
| <div className="h-2 flex-1 rounded-full bg-gray-200"> | ||
| <div className={progressBarStyles({ color })} style={{ width: `${clampedPercentage}%` }} /> | ||
| </div> | ||
| {label && <span className="text-sm font-medium text-gray-700">{label}</span>} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes for better screen reader support.
Consider adding ARIA attributes to improve accessibility for users with screen readers.
export function ProgressBar({ color, percentageValue, label, className }: ProgressBarProps) {
const clampedPercentage = Math.min(100, Math.max(0, percentageValue));
return (
<div className={classNames("flex w-full items-center gap-3", className)}>
- <div className="h-2 flex-1 rounded-full bg-gray-200">
+ <div
+ className="h-2 flex-1 rounded-full bg-gray-200"
+ role="progressbar"
+ aria-valuenow={clampedPercentage}
+ aria-valuemin={0}
+ aria-valuemax={100}
+ aria-label={label || `Progress: ${clampedPercentage}%`}>
<div className={progressBarStyles({ color })} style={{ width: `${clampedPercentage}%` }} />
</div>
{label && <span className="text-sm font-medium text-gray-700">{label}</span>}
</div>
);
}📝 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.
| export function ProgressBar({ color, percentageValue, label, className }: ProgressBarProps) { | |
| const clampedPercentage = Math.min(100, Math.max(0, percentageValue)); | |
| return ( | |
| <div className={classNames("flex w-full items-center gap-3", className)}> | |
| <div className="h-2 flex-1 rounded-full bg-gray-200"> | |
| <div className={progressBarStyles({ color })} style={{ width: `${clampedPercentage}%` }} /> | |
| </div> | |
| {label && <span className="text-sm font-medium text-gray-700">{label}</span>} | |
| </div> | |
| ); | |
| } | |
| export function ProgressBar({ color, percentageValue, label, className }: ProgressBarProps) { | |
| const clampedPercentage = Math.min(100, Math.max(0, percentageValue)); | |
| return ( | |
| <div className={classNames("flex w-full items-center gap-3", className)}> | |
| <div | |
| className="h-2 flex-1 rounded-full bg-gray-200" | |
| role="progressbar" | |
| aria-valuenow={clampedPercentage} | |
| aria-valuemin={0} | |
| aria-valuemax={100} | |
| aria-label={label || `Progress: ${clampedPercentage}%`}> | |
| <div | |
| className={progressBarStyles({ color })} | |
| style={{ width: `${clampedPercentage}%` }} | |
| /> | |
| </div> | |
| {label && <span className="text-sm font-medium text-gray-700">{label}</span>} | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/ui/components/progress-bar/ProgressBar.tsx around lines 27 to 38,
the ProgressBar component lacks ARIA attributes for accessibility. Add
appropriate ARIA attributes such as role="progressbar", aria-valuemin="0",
aria-valuemax="100", and aria-valuenow set to the clamped percentage value on
the progress bar element to improve screen reader support.
| const categories = [ | ||
| "Created", | ||
| "Completed", | ||
| "Rescheduled", | ||
| "Cancelled", | ||
| "No-Show (Host)", | ||
| "No-Show (Guest)", | ||
| ]; | ||
| const colors = [ | ||
| COLOR.CREATED, | ||
| COLOR.COMPLETED, | ||
| COLOR.RESCHEDULED, | ||
| COLOR.CANCELLED, | ||
| COLOR.NO_SHOW_HOST, | ||
| COLOR.NO_SHOW_GUEST, | ||
| ]; |
There was a problem hiding this comment.
maybe we should do something like:
const categories = legend.map((x) => x.label) and same for colors for consistency
hbjORbj
left a comment
hbjORbj
left a comment
There was a problem hiding this comment.
just 1 non-blocking nit + you have merge conflicts in yarn.lock
E2E results are ready! |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/features/insights/components/UserStatsTable.tsx(2 hunks)packages/lib/server/service/insightsBooking.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/insights/components/UserStatsTable.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/service/insightsBooking.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/insightsBooking.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
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 **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
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/insightsBooking.ts
📚 Learning: in the insights routing funnel component (packages/features/insights/components/routingfunnel.tsx), ...
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/insightsBooking.ts
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/lib/server/service/insightsBooking.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (1)
packages/lib/server/service/insightsBooking.ts (1)
660-662: LGTM! Good additive enhancement for recharts support.The addition of
formattedDateFullproperty provides more detailed date information for the new recharts implementation while preserving backward compatibility with the existingMonthfield.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/features/insights/components/routing/FailedBookingsByField.tsx(4 hunks)packages/features/insights/components/routing/RoutedToPerPeriod.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/insights/components/routing/RoutedToPerPeriod.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/FailedBookingsByField.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/FailedBookingsByField.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
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 **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
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/FailedBookingsByField.tsx
📚 Learning: in the insights routing funnel component (packages/features/insights/components/routingfunnel.tsx), ...
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/FailedBookingsByField.tsx
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
packages/features/insights/components/routing/FailedBookingsByField.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Production builds / Build Web App
🔇 Additional comments (5)
packages/features/insights/components/routing/FailedBookingsByField.tsx (5)
4-20: LGTM! Comprehensive recharts imports.All necessary recharts components are properly imported for implementing the vertical bar chart functionality.
22-53: LGTM! Well-implemented custom tooltip with proper localization.The CustomTooltip component correctly uses
t()for localization, has proper TypeScript typing, and implements appropriate tooltip functionality for the recharts implementation.
78-84: LGTM! Good empty state handling.The empty data check and fallback message provide a good user experience when no data is available.
95-120: LGTM! Excellent recharts implementation.The vertical bar chart implementation is well-configured with:
- Proper responsive container
- Good margin settings and styling
- Appropriate grid, axes, and tooltip configuration
- Nice visual touches like rounded corners and active state highlighting
132-152: LGTM! Clean main component implementation.The main component properly uses localization for the chart title and maintains clean structure while integrating the new recharts-based FormCard component.
| </div> | ||
| ) : ( | ||
| <div className="mt-4 p-4 text-center text-gray-500"> | ||
| No data available for selected field |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use localization for hardcoded text.
The hardcoded English text should be localized using the t() function according to the coding guidelines.
- No data available for selected field
+ {t("no_data_available_for_selected_field")}🤖 Prompt for AI Agents
In packages/features/insights/components/routing/FailedBookingsByField.tsx at
line 123, the hardcoded text "No data available for selected field" should be
wrapped with the localization function t() to support internationalization.
Replace the plain string with t('No data available for selected field') or the
appropriate translation key to ensure the text is localized.
| ) : ( | ||
| <div className="mt-4 p-4 text-center text-gray-500"> | ||
| No data available for selected field | ||
| <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug JSON display for production.
The JSON.stringify debug information should be removed or made conditional for development only, as it's not appropriate for production UI.
- <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div>Or make it conditional:
- <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div>
+ {process.env.NODE_ENV === 'development' && (
+ <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div>
+ )}📝 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.
| <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div> | |
| {process.env.NODE_ENV === 'development' && ( | |
| <div className="mt-2 text-xs">Data: {JSON.stringify(selectedFieldData, null, 2)}</div> | |
| )} |
🤖 Prompt for AI Agents
In packages/features/insights/components/routing/FailedBookingsByField.tsx at
line 124, the JSON.stringify debug output displaying selectedFieldData should be
removed or wrapped in a condition that only renders it in development mode. Use
a check like process.env.NODE_ENV === 'development' to conditionally render this
debug information so it does not appear in production builds.
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
LGTM! Nice refactor @eunjae-lee 🚀
4 participants
What does this PR do?
This PR replaces
tremorwithrecharts. It's easier to compose and customize the design in this project./insights
Before
Screenshot.2025-07-29.at.15.36.09.mp4
After
Screenshot.2025-07-29.at.15.33.48.mp4
/insights/routing
Before
Screenshot.2025-07-29.at.15.36.28.mp4
After
Screenshot.2025-07-29.at.15.34.07.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/insights and /insights/routing are almost the same (slight design difference)