feat(insights): add booking analytics dashboard#29397
Closed
regisstedile wants to merge 25 commits into
Closed
Conversation
Contributor
|
Welcome to Cal.diy, @regisstedile! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (24)
apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsx-45-45 (1)
45-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
relfor the external_blanklink.Please set
rel="noopener noreferrer"on the external “learn more” button to avoid opener access.🔧 Suggested patch
- <Button color="minimal" href="https://go.cal.com/insights" target="_blank"> + <Button + color="minimal" + href="https://go.cal.com/insights" + target="_blank" + rel="noopener noreferrer"> {t("learn_more")} </Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/insights/UpgradeTipWrapper.tsx at line 45, The external "learn more" Button in UpgradeTipWrapper should include rel="noopener noreferrer" when using target="_blank" to prevent opener access; update the Button element in UpgradeTipWrapper.tsx (the Button with props color="minimal" href="https://go.cal.com/insights" target="_blank") to add rel="noopener noreferrer" so the external link is opened safely.apps/web/modules/insights/components/ChartCard.tsx-139-139 (1)
139-139:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the legend toggle aria-label.
aria-label={Toggle ${item.label}}is hard-coded English and should go through translations.As per coding guidelines: "Add translations to
packages/i18n/locales/en/common.jsonfor all UI strings".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/ChartCard.tsx` at line 139, Replace the hard-coded aria-label in ChartCard (aria-label={`Toggle ${item.label}`}) with a localized string: add a new key (e.g. "chart.legendToggle") to the common.json i18n file and use the i18n translator in the ChartCard component (e.g. const { t } = useTranslation('common')) then set aria-label={t('chart.legendToggle', { label: item.label })} so the label is translated while preserving the dynamic item.label parameter.apps/web/modules/insights/components/booking/BookingsByHourChart.tsx-60-60 (1)
60-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop the “what” comment above
CustomTooltip.It repeats the code intent and doesn’t add reasoning.
As per coding guidelines: "Only add code comments that explain why, not what" and "Never add comments that simply restate what the code does."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/BookingsByHourChart.tsx` at line 60, Remove the redundant “// Custom Tooltip component” comment that sits above the CustomTooltip declaration; locate the CustomTooltip component (function or const named CustomTooltip) and delete that single-line comment so only meaningful comments that explain why remain.apps/web/modules/insights/components/booking/BookingKPICards.tsx-166-172 (1)
166-172:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove comments that only restate the code.
These comments describe what the functions do and add noise without intent/context.
As per coding guidelines: "Only add code comments that explain why, not what" and "Never add comments that simply restate what the code does."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/BookingKPICards.tsx` around lines 166 - 172, Remove the redundant "StatContainer: wraps the grid" and "StatItem: handles border logic" comments that merely restate the function names and behavior; edit the BookingKPICards.tsx snippet to delete those two comment lines above the StatContainer and StatItem function declarations so only code (or meaningful "why" comments if needed) remains.apps/web/modules/insights/components/booking/EventTrendsChart.tsx-32-32 (1)
32-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the tooltip “what” comment.
It repeats what the code already makes obvious.
As per coding guidelines: "Only add code comments that explain why, not what" and "Never add comments that simply restate what the code does."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/EventTrendsChart.tsx` at line 32, Remove the redundant "what" comment that restates the code: delete the line containing "// Custom Tooltip component" in EventTrendsChart.tsx (the comment above the CustomTooltip implementation) so only meaningful "why" comments remain and the code stays clean and adheres to the guideline.apps/web/modules/insights/components/booking/CSATOverTimeChart.tsx-16-16 (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the redundant tooltip comment.
This comment restates the following declaration and doesn’t capture rationale.
As per coding guidelines: "Only add code comments that explain why, not what" and "Never add comments that simply restate what the code does."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/CSATOverTimeChart.tsx` at line 16, Remove the redundant comment "// Custom Tooltip component" which merely restates the declaration; delete this line near the CustomTooltip (or tooltip) component declaration in CSATOverTimeChart.tsx so only meaningful "why" comments remain and avoid duplicative "what" comments.apps/web/modules/insights/components/booking/EventTrendsChart.tsx-21-28 (1)
21-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize legend labels instead of hardcoding English strings.
The labels are user-facing and currently bypass translation, so non-English locales will still see English legend/tooltips. Keep a stable
keyfordataKeyand derive a translatedlabelfor display.
As per coding guidelines: "Add translations topackages/i18n/locales/en/common.jsonfor all UI strings."Suggested direction
-export const legend = [ - { label: "Created", color: COLOR.CREATED }, - { label: "Completed", color: COLOR.COMPLETED }, - { label: "Rescheduled", color: COLOR.RESCHEDULED }, - { label: "Cancelled", color: COLOR.CANCELLED }, - { label: "No-Show (Host)", color: COLOR.NO_SHOW_HOST }, - { label: "No-Show (Guest)", color: COLOR.NO_SHOW_GUEST }, -]; +export const legend = [ + { key: "Created", labelKey: "events_created", color: COLOR.CREATED }, + { key: "Completed", labelKey: "events_completed", color: COLOR.COMPLETED }, + { key: "Rescheduled", labelKey: "events_rescheduled", color: COLOR.RESCHEDULED }, + { key: "Cancelled", labelKey: "events_cancelled", color: COLOR.CANCELLED }, + { key: "No-Show (Host)", labelKey: "event_no_show", color: COLOR.NO_SHOW_HOST }, + { key: "No-Show (Guest)", labelKey: "event_no_show_guest", color: COLOR.NO_SHOW_GUEST }, +];Also applies to: 104-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/EventTrendsChart.tsx` around lines 21 - 28, The legend array currently hardcodes English labels — replace the static label strings in the exported legend with stable keys (keep the existing legend variable name and dataKey values) and derive display labels via the app i18n translator (e.g., call translation function for each key when rendering tooltips/legend) so UI text is localized; also add corresponding entries for each legend key to packages/i18n/locales/en/common.json (e.g., created, completed, rescheduled, cancelled, no_show_host, no_show_guest) to satisfy the guideline and ensure translations are available.apps/web/modules/insights/components/booking/BookingsByHourChart.tsx-29-30 (1)
29-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix empty-data detection for the no-data state.
When
datais empty,Math.max(...[])becomes-Infinity, soisEmptyis false and the empty-state message never renders.Suggested fix
- const maxBookings = Math.max(...data.map((item) => item.count)); - const isEmpty = maxBookings === 0; + const isEmpty = data.length === 0 || data.every((item) => item.count === 0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/BookingsByHourChart.tsx` around lines 29 - 30, The empty-data detection is broken because Math.max(...data.map(...)) yields -Infinity when data is empty; in BookingsByHourChart update the calculation of maxBookings to account for empty arrays (e.g., use a fallback or conditional) so that when data.length === 0 maxBookings becomes 0 and isEmpty correctly becomes true; change the expression that computes maxBookings (referencing maxBookings, isEmpty, and data) to return 0 for an empty data set before computing Math.max over mapped counts.apps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsx-18-21 (1)
18-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a unique key per feedback row, not
userIdalone.
userIdcan repeat in recent ratings, which can produce duplicate keys and unstable row reconciliation.Suggested change
- data.map((item) => ( + data.map((item, index) => ( <div - key={item.userId} + key={`${item.userId}-${index}`} className="border-subtle flex items-center justify-between border-b px-4 py-3 last:border-b-0">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsx` around lines 18 - 21, In RecentFeedbackTableContent, the JSX map uses item.userId as the element key which can be duplicated; change the key in the data.map callback to a unique, stable identifier (e.g. item.id or a concatenation like `${item.userId}-${item.id}` or `${item.userId}-${item.createdAt}`) and only fall back to the map index if no unique id exists so each feedback row rendered by data.map has a truly unique key instead of userId alone.apps/web/modules/insights/components/booking/NoShowHostsOverTimeChart.tsx-17-17 (1)
17-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove non-informative comment text.
This comment only restates the code and adds noise; please drop it or replace with rationale if there is one.
As per coding guidelines, "Only add code comments that explain why, not what — follow code comment guidelines for quality comments."Suggested change
-// Custom Tooltip component const CustomTooltip = ({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/NoShowHostsOverTimeChart.tsx` at line 17, Remove the non-informative inline comment "// Custom Tooltip component" inside the NoShowHostsOverTimeChart component file; either delete it or replace it with a brief rationale describing why a custom Tooltip is needed (for example: "Custom Tooltip to format date and metric for chart hover") and reference the Tooltip implementation so reviewers understand its purpose.apps/web/modules/insights/components/booking/PopularEventsTable.tsx-28-40 (1)
28-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmpty-state condition uses the wrong dataset.
You render from a filtered list, but the empty-state check uses unfiltered
data.length. When all items are filtered out, the card appears blank.Suggested change
return ( <ChartCard title={t("popular_events")} isPending={isPending} isError={isError}> {isSuccess && data ? ( <> + {(() => { + const visibleItems = data.filter((item) => item.eventTypeId); + return ( + <> <div> - {data - .filter((item) => item.eventTypeId) - .map((item) => ( + {visibleItems.map((item) => ( <ChartCardItem key={item.eventTypeId} count={item.count}> <div className="flex"> <div className="bg-subtle mr-1.5 h-5 w-[2px] shrink-0 rounded-sm" /> <p>{item.eventTypeName}</p> </div> </ChartCardItem> ))} </div> - {data.length === 0 && ( + {visibleItems.length === 0 && ( <div className="flex h-60 text-center"> <p className="m-auto text-sm font-light">{t("insights_no_data_found_for_filter")}</p> </div> )} + </> + ); + })()} </> ) : null} </ChartCard> );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/PopularEventsTable.tsx` around lines 28 - 40, The empty-state check in PopularEventsTable uses the original `data.length` while the UI renders `data.filter((item) => item.eventTypeId)`, causing a blank card when all entries are filtered out; fix by computing and using a filtered array (e.g., `const filtered = data.filter(item => item.eventTypeId)`) and render `filtered.map(...)` and check `filtered.length === 0` for the empty state instead of `data.length`. Ensure all references to mapping/empty checks (the map that renders `ChartCardItem` and the empty-state conditional) use this `filtered` variable.apps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsx-28-31 (1)
28-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrack copied state per row instead of using one global copied flag.
isCopiedis shared for the whole component, so copying one email flips all row buttons to the copied state.💡 Suggested fix
import { useCopy } from "`@calcom/lib/hooks/useCopy`"; @@ -import { ChartCard, ChartCardItem } from "../ChartCard"; +import { ChartCard, ChartCardItem } from "../ChartCard"; +import { useState } from "react"; @@ const { copyToClipboard, isCopied } = useCopy(); + const [copiedEmail, setCopiedEmail] = useState<string | null>(null); @@ const handleCopyEmail = (email: string) => { copyToClipboard(email); + setCopiedEmail(email); showToast(t("email_copied"), "success"); }; @@ - StartIcon={isCopied ? "clipboard-check" : "clipboard"} + StartIcon={isCopied && copiedEmail === item.guestEmail ? "clipboard-check" : "clipboard"} onClick={() => handleCopyEmail(item.guestEmail)}> - {!isCopied ? t("email") : t("copied")} + {isCopied && copiedEmail === item.guestEmail ? t("copied") : t("email")} </Button>Also applies to: 65-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsx` around lines 28 - 31, The component currently uses a single shared isCopied flag so clicking any "copy" toggles every row; change to track copied state per row by replacing the single boolean with a map keyed by a unique row identifier (email or guest id) in the component state, update handleCopyEmail(email) (and the other copy handlers referenced around the lines 65-67) to set only that key to true and reset it after a timeout, and pass the per-row value (e.g., copiedMap[email]) into the button/renderer so only the clicked row shows the copied state. Ensure you clear/reset the specific key rather than mutating a global flag.apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx-25-29 (1)
25-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize legend labels instead of hard-coding English text.
These labels are user-facing and should come from translation keys like the rest of the chart copy.
As per coding guidelines: "Add translations to
packages/i18n/locales/en/common.jsonfor all UI strings".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx` around lines 25 - 29, The legend array currently hard-codes English labels; replace these with translated strings by using the app's i18n mechanism (e.g., call to t('...') from useTranslation) when constructing legend in RoutingFunnelContent.tsx: update the three labels for "Total Submissions", "Successful Routings", and "Accepted Bookings" to translation keys (add corresponding entries to packages/i18n/locales/en/common.json) and import/use the translation hook so legend = [{ label: t('insights.totalSubmissions'), color: COLOR.TOTAL }, { label: t('insights.successfulRoutings'), color: COLOR.SUCCESFUL }, { label: t('insights.acceptedBookings'), color: COLOR.ACCEPTED }].apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx-141-142 (1)
141-142:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a stable unique checkbox id for each team row.
Using
team.name || ""can create duplicate/emptyidvalues, which breaks label targeting and keyboard accessibility.💡 Suggested fix
- id={team.name || ""} + id={`team-${team.id}`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx` around lines 141 - 142, The checkbox id/label currently use team.name which can be empty or duplicate; update OrgTeamsFilter so each checkbox uses a stable unique identifier (e.g., use team.id or team.slug) instead of team.name, and ensure the label prop still displays the human-readable team.name; replace the id and label assignments in the checkbox rendering (the JSX that sets id={team.name || ""} and label={team.name || ""}) to id={`team-${team.id || team.slug}`} (or directly id={team.id} if present) and label={team.name} so ids are unique and labels remain correct.apps/web/modules/insights/components/routing/FailedBookingsByField.tsx-120-123 (1)
120-123:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove debug JSON output and localize this empty-state message.
This branch currently exposes debug payload text to end users and bypasses localization.
💡 Suggested fix
- <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> - </div> + <div className="mt-4 p-4 text-center text-gray-500"> + {t("insights_no_data_found_for_filter")} + </div>As per coding guidelines: "Add translations to
packages/i18n/locales/en/common.jsonfor all UI strings".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/FailedBookingsByField.tsx` around lines 120 - 123, Remove the debug JSON output and replace the hard-coded empty-state string with a localized translation: delete the inner <div> that outputs Data: {JSON.stringify(selectedFieldData, null, 2)} and replace the text "No data available for selected field" in the FailedBookingsByField component with a translation lookup (e.g. useTranslation hook and t('insights.noDataForField') or the project's i18n helper). Add the new key "insights.noDataForField": "No data available for selected field" to packages/i18n/locales/en/common.json so the string is localized.apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx-37-45 (1)
37-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid render-time randomness for skeleton bar heights.
Math.random()in render makes the skeleton jump on re-renders. Use fixed/mock heights for stable UI.💡 Suggested fix
+const BAR_HEIGHTS = [ + { a: "62%", b: "41%", c: "24%" }, + { a: "58%", b: "39%", c: "21%" }, + { a: "64%", b: "44%", c: "26%" }, + { a: "55%", b: "36%", c: "20%" }, + { a: "61%", b: "40%", c: "23%" }, + { a: "57%", b: "37%", c: "22%" }, + { a: "63%", b: "42%", c: "25%" }, +]; ... - {Array.from({ length: 7 }).map((_, i) => ( + {BAR_HEIGHTS.map((h, i) => ( <div key={i} className="flex w-8 flex-col items-center"> <div className="w-full rounded-t bg-[`#8884d8`] opacity-20" - style={{ height: `${Math.random() * 40 + 30}%` }} + style={{ height: h.a }} /> <div className="w-full bg-[`#83a6ed`] opacity-20" - style={{ height: `${Math.random() * 30 + 20}%` }} + style={{ height: h.b }} /> <div className="w-full rounded-b bg-[`#82ca9d`] opacity-20" - style={{ height: `${Math.random() * 20 + 10}%` }} + style={{ height: h.c }} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx` around lines 37 - 45, RoutingFunnelSkeleton uses Math.random() directly in render for the three skeleton bar heights which causes visual jumps on re-renders; replace the render-time randomness by using fixed/mock heights (e.g., a constant array of percentages or heights) or derive deterministic values once (outside the component or via a useMemo) and reference those values in the three divs whose style currently uses Math.random(); update the three style={{ height: ... }} occurrences in the RoutingFunnelSkeleton component to use the fixed/deterministic heights instead of Math.random().apps/web/modules/insights/hooks/useInsightsColumns.tsx-345-345 (1)
345-345:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude
tin theuseMemodependency array.The memoized columns use translated headers; without
tin deps, headers can stay stale after locale changes.Proposed change
- }, [isHeadersSuccess, headers]); + }, [isHeadersSuccess, headers, t]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useInsightsColumns.tsx` at line 345, The memoized columns in useInsightsColumns (the useMemo call that depends on isHeadersSuccess and headers) uses the translation function t but does not include t in its dependency array; update the useMemo dependency array to include t so translated header labels refresh on locale changes, keeping isHeadersSuccess and headers as well.apps/web/modules/insights/hooks/useDefaultRoutingForm.ts-22-22 (1)
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse stricter route matching for insights routing pages.
includes("/insights/routing")can produce false positives on unrelated paths containing that substring.Proposed change
- const isRoutingInsights = pathname?.includes("/insights/routing"); + const isRoutingInsights = pathname?.startsWith("/insights/routing") ?? false;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useDefaultRoutingForm.ts` at line 22, The current boolean isRoutingInsights uses pathname?.includes("/insights/routing") which can match unintended paths; update the check in useDefaultRoutingForm (isRoutingInsights) to perform strict route matching instead—e.g., use pathname?.startsWith("/insights/routing") or a regex anchor like ^/insights/routing(/|$) so only the exact routing pages (and their subpaths) return true.apps/web/modules/insights/hooks/useInsightsColumns.tsx-278-321 (1)
278-321:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize UTM header labels instead of hardcoded strings.
These are user-facing table headers and should go through
t(...).Proposed change
- header: "utm_medium", + header: t("utm_medium"), ... - header: "utm_term", + header: t("utm_term"), ... - header: "utm_content", + header: t("utm_content"), ... - header: "utm_campaign", + header: t("utm_campaign"),As per coding guidelines "Add translations to
packages/i18n/locales/en/common.jsonfor all UI strings".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useInsightsColumns.tsx` around lines 278 - 321, The headers for UTM columns in useInsightsColumns (the columnHelper.accessor entries for "utm_medium", "utm_term", "utm_content", and "utm_campaign") are hardcoded strings; change each header value to call the i18n translator (t("...")) with new keys (e.g., insights.utm_medium, insights.utm_term, insights.utm_content, insights.utm_campaign) and add those keys with English values to packages/i18n/locales/en/common.json so the table headers are localized.apps/web/modules/insights/views/insights-virtual-queues-view.tsx-38-38 (1)
38-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the select placeholder.
Line 38 uses a hardcoded UI string (
"Select project"). Please replace it with a translation key.As per coding guidelines: "Add translations to
packages/i18n/locales/en/common.jsonfor all UI strings".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/views/insights-virtual-queues-view.tsx` at line 38, Replace the hardcoded placeholder="Select project" in the InsightsVirtualQueuesView component with a translation key (e.g., placeholder={t('insights.selectProject')}) using the existing i18n hook or useTranslation import in that file, and add the corresponding key/value ("insights.selectProject": "Select project") to packages/i18n/locales/en/common.json; ensure the component imports and calls the translation function (t) where needed so the placeholder uses the localized string.apps/web/modules/insights/hooks/useToggleableLegend.ts-4-6 (1)
4-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync
enabledSerieswithlegendchanges to prevent stale selections.If
legendupdates after mount,enabledSeriescan contain removed labels, potentially resulting in an emptyenabledLegendwhen the new legend has no matching items. Whilelegendis currently static in all usages, this pattern is brittle and will break if legend becomes dynamic.Suggested change
-import { useCallback, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; @@ const [enabledSeries, setEnabledSeries] = useState<string[]>( initialEnabled ?? legend.map((item) => item.label) ); + + useEffect(() => { + const allowed = new Set(legend.map((item) => item.label)); + setEnabledSeries((prev) => { + const next = prev.filter((label) => allowed.has(label)); + if (next.length > 0) return next; + return (initialEnabled ?? legend.map((item) => item.label)).filter((label) => allowed.has(label)); + }); + }, [legend, initialEnabled]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useToggleableLegend.ts` around lines 4 - 6, The hook useToggleableLegend currently initializes enabledSeries from initialEnabled or legend but never updates it when legend changes; add a useEffect in useToggleableLegend that runs on changes to legend (and optionally initialEnabled) that computes legendLabels = legend.map(item => item.label), filters the current enabledSeries to only labels present in legendLabels, and then calls setEnabledSeries(filtered) — but if the filtered array is empty, fall back to legendLabels to avoid producing an empty enabledLegend; reference enabledSeries, setEnabledSeries, legend and initialEnabled when making this change.packages/features/insights/server/insightsDateUtils.ts-151-153 (1)
151-153:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove extra whitespace in weekly label formatting.
"${startFormat} , YYYY"renders with an extra space before the comma in user-facing labels.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/insights/server/insightsDateUtils.ts` around lines 151 - 153, The weekly label includes an extra space before the comma because the template uses `${startFormat} , YYYY` and `${endFormat}, YYYY`; update the formatting in the branch where start.format("YYYY") !== end.format("YYYY") to remove the stray space so both use `${startFormat}, YYYY` and `${endFormat}, YYYY` (adjust the string passed to start.format and end.format accordingly) to eliminate the unintended whitespace in user-facing labels.packages/features/insights/server/raw-data.schema.ts-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign with codebase pattern: use named import for Zod.
The majority of the codebase uses
import { z } from "zod";(named import). Switch to this pattern for consistency.Suggested fix
-import z from "zod"; +import { z } from "zod";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/insights/server/raw-data.schema.ts` at line 3, Replace the default import of Zod with the codebase-standard named import: change the import statement `import z from "zod";` to `import { z } from "zod";` so that all references to `z` (the Zod identifier used in this module) follow the project's consistent import pattern.packages/features/insights/services/csvDataTransformer.ts-71-74 (1)
71-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize array elements before joining extracted values
extractFieldValuejoins raw array entries directly, so object entries serialize as"[object Object]"and corrupt CSV output.💡 Proposed fix
if (Array.isArray(value)) { - const filtered = value.filter((v) => v != null && v !== ""); - return filtered.length > 0 ? filtered.join(", ") : null; + const normalized = value + .map((v) => extractFieldValue(v)) + .filter((v): v is string => v != null && v !== ""); + return normalized.length > 0 ? normalized.join(", ") : null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/insights/services/csvDataTransformer.ts` around lines 71 - 74, extractFieldValue currently joins array elements raw which yields "[object Object]" for objects; update the Array.isArray(value) branch to first map/normalize each element (e.g., for each v: if v == null return null/skip, else if typeof v === "object" use JSON.stringify(v), else if v instanceof Date use v.toISOString(), else use String(v)), then filter out null/empty strings and join with ", " as before so object/complex entries serialize correctly; keep this logic inside the extractFieldValue function where value is handled as an array.
🧹 Nitpick comments (13)
apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx (1)
1-1: ⚡ Quick winUse native
Dateordate-fnsfor filename date formatting.Day.js isn’t necessary for this non-timezone formatting path.
As per coding guidelines: Use `date-fns` or native `Date` instead of Day.js when timezone awareness isn't needed.💡 Suggested fix
-import dayjs from "`@calcom/dayjs`"; @@ +const formatDateForFile = (value: Date | string) => new Date(value).toISOString().slice(0, 10); @@ - const filename = `RoutingFormResponses-${dayjs(startDate).format("YYYY-MM-DD")}-${dayjs( - endDate - ).format("YYYY-MM-DD")}.csv`; + const filename = `RoutingFormResponses-${formatDateForFile(startDate)}-${formatDateForFile( + endDate + )}.csv`;Also applies to: 73-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx` at line 1, Replace the Day.js dependency usage in RoutingFormResponsesDownload (remove the import "dayjs" and any calls to dayjs(...).format(...)) with either native Date or date-fns formatting for the filename generation logic; locate the filename construction inside the RoutingFormResponsesDownload component (and the date formatting calls around lines ~73-75) and change those calls to use new Date() with toISOString()/toLocaleString or date-fns/format(date, pattern) so timezone-aware libraries aren’t pulled in for simple non-timezone formatting, and update imports accordingly.apps/web/modules/insights/components/filters/Download/Download.tsx (1)
1-1: ⚡ Quick winPrefer native
Dateordate-fnshere instead of Day.js.This filename formatting path doesn’t need timezone-aware Day.js behavior.
As per coding guidelines: Use `date-fns` or native `Date` instead of Day.js when timezone awareness isn't needed.💡 Suggested fix
-import dayjs from "`@calcom/dayjs`"; @@ +const formatDateForFile = (value: Date | string) => new Date(value).toISOString().slice(0, 10); @@ - const filename = `Insights-${dayjs(startDate).format("YYYY-MM-DD")}-${dayjs(endDate).format( - "YYYY-MM-DD" - )}.csv`; + const filename = `Insights-${formatDateForFile(startDate)}-${formatDateForFile(endDate)}.csv`;Also applies to: 80-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/Download/Download.tsx` at line 1, The file Download.tsx imports Day.js via the identifier dayjs; replace that import and all usages of the dayjs identifier (search for "dayjs" and usages around the Download component and the block referenced at lines ~80-82) with either native Date or date-fns utilities (e.g., parse/format functions) so timezone-aware Day.js is not used; update any formatting/parsing calls to the chosen API, remove the dayjs import, and adjust tests/types if needed to match date-fns/native Date function signatures.apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx (1)
10-10: ⚡ Quick winImport
FilterSearchFieldfrom its direct source file instead of the form barrel.This keeps import boundaries aligned with repo standards and avoids barrel coupling.
As per coding guidelines: "Import directly from source files, not barrel files (e.g.,
@calcom/ui/components/buttonnot@calcom/ui)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx` at line 10, Replace the barrel import of FilterSearchField with a direct import from its source module: change the import statement that currently reads "import { FilterSearchField } from '`@calcom/ui/components/form`';" to import FilterSearchField directly from its component file (for example "import FilterSearchField from '`@calcom/ui/components/form/FilterSearchField`'"), updating the import path to the correct direct source file so the OrgTeamsFilter component uses the direct module export.apps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsx (1)
23-23: ⚡ Quick winImport
TimezoneBadgefrom its concrete source file, not the booking barrel.This keeps dependency direction explicit and avoids growing barrel coupling.
As per coding guidelines: "Import directly from source files, not barrel files (e.g.,
@calcom/ui/components/buttonnot@calcom/ui)"; "Never use barrel imports from index.ts files".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsx` at line 23, The import currently pulls TimezoneBadge from the booking barrel; replace that barrel import with a direct import from the concrete source file that actually defines TimezoneBadge (update the import in RoutingFormResponsesTable.tsx to point to the TimezoneBadge implementation file instead of "`@calcom/web/modules/insights/components/booking`") so dependency direction remains explicit and avoids barrel coupling.apps/web/modules/insights/hooks/useInsightsColumns.tsx (1)
1-1: ⚡ Quick winReplace Day.js usage here with native
Date/date-fns.This formatting path does not need Day.js-specific timezone behavior.
As per coding guidelines "Use
date-fnsor nativeDateinstead of Day.js when timezone awareness isn't needed".Also applies to: 340-340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useInsightsColumns.tsx` at line 1, Replace Day.js usage in the useInsightsColumns hook by removing the import of dayjs and switching any dayjs-based parsing/formatting to native Date or date-fns calls; locate the hook named useInsightsColumns and any occurrences around the noted area (including the call near line ~340) and update those Date operations (parsing, formatting, comparisons) to use Date or date-fns equivalents, then remove the import line `import dayjs from "`@calcom/dayjs`";` and any Day.js-specific APIs so timezone-aware behavior is not used.apps/web/modules/insights/hooks/useDefaultRoutingForm.ts (1)
48-48: ⚡ Quick winUse
router.replacewhen auto-applying defaultroutingFormId.This is an automatic normalization step, so
replaceavoids unnecessary back-stack entries.Proposed change
- router.push(`${pathname}?${newSearchParams.toString()}`); + router.replace(`${pathname}?${newSearchParams.toString()}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useDefaultRoutingForm.ts` at line 48, The code currently uses router.push to update the URL when auto-applying the default routingFormId; replace that call with router.replace to avoid adding a new history entry. Locate the router.push(`${pathname}?${newSearchParams.toString()}`) invocation in useDefaultRoutingForm (or the hook body) and change it to router.replace(`${pathname}?${newSearchParams.toString()}`) so the normalization updates the URL without creating a back-stack entry.apps/web/modules/insights/hooks/useInsightsBookingParameters.ts (1)
1-1: ⚡ Quick winPrefer native
Date/date-fnsover Day.js for this date-range defaulting logic.This path only normalizes day boundaries and does not require timezone-aware Day.js behavior.
As per coding guidelines "Use
date-fnsor nativeDateinstead of Day.js when timezone awareness isn't needed".Also applies to: 32-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useInsightsBookingParameters.ts` at line 1, The file imports dayjs and uses it to normalize date-range boundaries in the useInsightsBookingParameters hook; replace Day.js with native Date or date-fns: remove the import of dayjs, and change any dayjs(...).startOf('day') / dayjs(...).endOf('day') uses (notably around the defaulting logic referenced on lines 32-33) to either date-fns startOfDay()/endOfDay() or native Date normalization (new Date(date).setHours(0,0,0,0) and setHours(23,59,59,999)), updating the relevant variables and any tests/types accordingly so the hook still returns the same start/end-day values without timezone-aware Day.js.apps/web/modules/insights/hooks/useInsightsBookings.ts (1)
1-1: ⚡ Quick winImport
ColumnFilterTypefrom its source module, not the package barrel.Use the concrete module path to align with the no-barrel-import rule.
As per coding guidelines "Import directly from source files, not barrel files (e.g.,
@calcom/ui/components/buttonnot@calcom/ui)" and "Never use barrel imports from index.ts files".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useInsightsBookings.ts` at line 1, The file useInsightsBookings.ts currently imports ColumnFilterType via the package barrel ("`@calcom/features/data-table`"); update the import to reference the concrete source module that actually exports ColumnFilterType (the specific module inside the data-table package where ColumnFilterType is defined) instead of the barrel export, by replacing the import statement for ColumnFilterType with a direct import from that source module so it follows the no-barrel-import rule.apps/web/modules/insights/components/routing/RoutingKPICards.tsx (1)
3-3: ⚡ Quick winAvoid barrel import for
valueFormatter.Import this utility from its source file instead of
@calcom/features/insights/libto keep dependency boundaries explicit.Proposed change
-import { valueFormatter } from "`@calcom/features/insights/lib`"; +import { valueFormatter } from "`@calcom/features/insights/lib/valueFormatter`";As per coding guidelines "Import directly from source files, not barrel files (e.g.,
@calcom/ui/components/buttonnot@calcom/ui)" and "Never use barrel imports from index.ts files".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/RoutingKPICards.tsx` at line 3, The file RoutingKPICards.tsx currently imports valueFormatter from the barrel module "`@calcom/features/insights/lib`"; change that to import valueFormatter directly from its defining source file (the module that actually exports the utility, e.g., the specific utils/ or valueFormatter file inside the features/insights package) to avoid barrel imports—update the import statement in RoutingKPICards.tsx to point to the concrete source module exporting valueFormatter.apps/web/modules/insights/views/insights-routing-view.tsx (1)
6-11: ⚡ Quick winAvoid barrel import for routing components.
Lines 6-11 import from a barrel path; switch to direct source-file imports for each component.
As per coding guidelines: "Import directly from source files, not barrel files" and "Never use barrel imports from index.ts files".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/views/insights-routing-view.tsx` around lines 6 - 11, Replace the barrel import that pulls FailedBookingsByField, RoutedToPerPeriod, RoutingFormResponsesTable, and RoutingFunnel from "`@calcom/web/modules/insights/components/routing`" with direct imports from each component's source file; update the import statement(s) so each symbol (FailedBookingsByField, RoutedToPerPeriod, RoutingFormResponsesTable, RoutingFunnel) is imported from its own module file (e.g., the component's .tsx/.ts source) instead of the index barrel.apps/web/modules/insights/views/insights-view.tsx (1)
9-28: ⚡ Quick winReplace booking barrel import with direct file imports.
Lines 9-28 currently pull from a barrel; import each booking component from its concrete source file instead.
As per coding guidelines: "Import directly from source files, not barrel files" and "Never use barrel imports from index.ts files".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/views/insights-view.tsx` around lines 9 - 28, The current import statement pulls many booking components from a barrel; replace that single barrel import with direct imports from each component's concrete source file by importing AverageEventDurationChart, BookingKPICards, BookingsByHourChart, CSATOverTimeChart, EventTrendsChart, HighestNoShowHostTable, HighestRatedMembersTable, LeastBookedTeamMembersTable, LeastCompletedTeamMembersTable, LowestRatedMembersTable, MostBookedTeamMembersTable, MostCancelledBookingsTables, MostCompletedTeamMembersTable, NoShowHostsOverTimeChart, PopularEventsTable, RecentFeedbackTable, RecentNoShowGuestsChart, and TimezoneBadge from their individual module files (the actual component source files) instead of the barrel index.apps/web/modules/insights/hooks/useInsightsRoutingParameters.ts (1)
1-3: ⚡ Quick winReplace Day.js here with native
Dateordate-fns.Line 1 and Lines 12-22 only need day-boundary normalization + ISO conversion, so
dayjsis unnecessary in this hook.Suggested change
-import dayjs from "`@calcom/dayjs`"; import { useColumnFilters, useFilterValue, ZDateRangeFilterValue } from "`@calcom/features/data-table`"; import { getDefaultEndDate, getDefaultStartDate } from "`@calcom/features/data-table/lib/dateRange`"; import { useMemo } from "react"; import { useInsightsOrgTeams } from "./useInsightsOrgTeams"; @@ const startDate = useMemo(() => { - return dayjs(createdAtRange?.startDate ?? getDefaultStartDate().toISOString()) - .startOf("day") - .toISOString(); + const d = createdAtRange?.startDate ? new Date(createdAtRange.startDate) : getDefaultStartDate(); + d.setHours(0, 0, 0, 0); + return d.toISOString(); }, [createdAtRange?.startDate]); @@ const endDate = useMemo(() => { - return dayjs(createdAtRange?.endDate ?? getDefaultEndDate().toISOString()) - .endOf("day") - .toISOString(); + const d = createdAtRange?.endDate ? new Date(createdAtRange.endDate) : getDefaultEndDate(); + d.setHours(23, 59, 59, 999); + return d.toISOString(); }, [createdAtRange?.endDate]);As per coding guidelines: "Use
date-fnsor nativeDateinstead of Day.js when timezone awareness isn't needed".Also applies to: 12-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/hooks/useInsightsRoutingParameters.ts` around lines 1 - 3, Remove the dayjs import and replace its usage in useInsightsRoutingParameters with native Date or date-fns utilities: for any normalization currently done with dayjs(...).startOf('day').toISOString() or .endOf('day').toISOString(), use date-fns startOfDay/endOfDay + toISOString (or set hours/minutes/seconds on a native Date and call toISOString()). Update the logic in the useInsightsRoutingParameters hook (references: variables/params named start, end and any helper normalization code around lines 12-22) to call getDefaultStartDate/getDefaultEndDate as before but normalize using startOfDay(endOfDay) or equivalent native Date adjustments and return ISO strings; remove the dayjs import line.packages/features/insights/server/events.ts (1)
15-22: ⚡ Quick winMake reducer increments resilient to unseen status keys.
aggregate[item.timeStatus] += ...can produceNaNfor any status not preinitialized in the seed object. Initialize on first use before incrementing.Proposed fix
- if (typeof item.timeStatus === "string" && item) { - aggregate[item.timeStatus] += item?._count?._all ?? 0; - aggregate["_all"] += item?._count?._all ?? 0; + if (typeof item.timeStatus === "string") { + const count = item?._count?._all ?? 0; + aggregate[item.timeStatus] = (aggregate[item.timeStatus] ?? 0) + count; + aggregate["_all"] += count; if (item.noShowHost) { - aggregate["noShowHost"] += item?._count?._all ?? 0; + aggregate["noShowHost"] += count; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/insights/server/events.ts` around lines 15 - 22, The reducer in events.ts uses aggregate[item.timeStatus] without ensuring the key exists, causing NaN for unseen statuses; update the reducer (the function with parameters aggregate and item that references item.timeStatus and fields like item._count._all, aggregate["_all"], aggregate["noShowHost"]) to lazily initialize aggregate[item.timeStatus] = 0 (and aggregate["noShowHost"] where applicable) before any += operation so increments are safe for new status keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 710b2798-0bb3-4ba3-9793-8d07046ba4e1
📒 Files selected for processing (94)
apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsxapps/web/app/(use-page-wrapper)/insights/call-history/page.tsxapps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.tsapps/web/app/(use-page-wrapper)/insights/layout.tsxapps/web/app/(use-page-wrapper)/insights/page.tsxapps/web/app/(use-page-wrapper)/insights/router-position/page.tsxapps/web/app/(use-page-wrapper)/insights/routing/page.tsxapps/web/modules/insights/components/BookedByCell.tsxapps/web/modules/insights/components/BookingAtCell.tsxapps/web/modules/insights/components/BookingStatusBadge.tsxapps/web/modules/insights/components/CellWithOverflowX.tsxapps/web/modules/insights/components/ChartCard.tsxapps/web/modules/insights/components/KPICard.tsxapps/web/modules/insights/components/ResponseValueCell.tsxapps/web/modules/insights/components/UserStatsTable.tsxapps/web/modules/insights/components/booking/AverageEventDurationChart.tsxapps/web/modules/insights/components/booking/BookingKPICards.tsxapps/web/modules/insights/components/booking/BookingsByHourChart.tsxapps/web/modules/insights/components/booking/CSATOverTimeChart.tsxapps/web/modules/insights/components/booking/EventTrendsChart.tsxapps/web/modules/insights/components/booking/HighestNoShowHostTable.tsxapps/web/modules/insights/components/booking/HighestRatedMembersTable.tsxapps/web/modules/insights/components/booking/LeastBookedTeamMembersTable.tsxapps/web/modules/insights/components/booking/LeastCompletedBookings.tsxapps/web/modules/insights/components/booking/LowestRatedMembersTable.tsxapps/web/modules/insights/components/booking/MostBookedTeamMembersTable.tsxapps/web/modules/insights/components/booking/MostCancelledBookingsTables.tsxapps/web/modules/insights/components/booking/MostCompletedBookings.tsxapps/web/modules/insights/components/booking/NoShowHostsOverTimeChart.tsxapps/web/modules/insights/components/booking/PopularEventsTable.tsxapps/web/modules/insights/components/booking/RecentFeedbackTable.tsxapps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsxapps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsxapps/web/modules/insights/components/booking/TimezoneBadge.tsxapps/web/modules/insights/components/booking/index.tsapps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsxapps/web/modules/insights/components/filters/DateTargetSelector.tsxapps/web/modules/insights/components/filters/Download/Download.tsxapps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsxapps/web/modules/insights/components/filters/Download/index.tsxapps/web/modules/insights/components/filters/OrgTeamsFilter.tsxapps/web/modules/insights/components/filters/index.tsxapps/web/modules/insights/components/index.tsapps/web/modules/insights/components/routing/FailedBookingsByField.tsxapps/web/modules/insights/components/routing/RoutedToPerPeriod.tsxapps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsxapps/web/modules/insights/components/routing/RoutingFunnel.tsxapps/web/modules/insights/components/routing/RoutingFunnelContent.tsxapps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsxapps/web/modules/insights/components/routing/RoutingKPICards.tsxapps/web/modules/insights/components/routing/index.tsapps/web/modules/insights/hooks/useDefaultRoutingForm.tsapps/web/modules/insights/hooks/useInsightsBookingFacetedUniqueValues.tsapps/web/modules/insights/hooks/useInsightsBookingParameters.tsapps/web/modules/insights/hooks/useInsightsBookings.tsapps/web/modules/insights/hooks/useInsightsColumns.tsxapps/web/modules/insights/hooks/useInsightsOrgTeams.tsapps/web/modules/insights/hooks/useInsightsRoutingFacetedUniqueValues.tsapps/web/modules/insights/hooks/useInsightsRoutingParameters.tsapps/web/modules/insights/hooks/useToggleableLegend.tsapps/web/modules/insights/views/insights-call-history-view.tsxapps/web/modules/insights/views/insights-routing-view.tsxapps/web/modules/insights/views/insights-view.tsxapps/web/modules/insights/views/insights-virtual-queues-view.tsxapps/web/modules/shell/navigation/Navigation.tsxapps/web/pages/api/trpc/insights/[trpc].tspackages/features/di/containers/InsightsBooking.tspackages/features/di/containers/InsightsRouting.tspackages/features/di/modules/InsightsBooking.tspackages/features/di/modules/InsightsRouting.tspackages/features/di/tokens.tspackages/features/insights/lib/bookingStatusToText.tspackages/features/insights/lib/bookingUtils.tspackages/features/insights/lib/calculateDeltaType.tspackages/features/insights/lib/index.tspackages/features/insights/lib/objectToCsv.tspackages/features/insights/lib/repositories/TeamRepository.tspackages/features/insights/lib/types.tspackages/features/insights/lib/valueFormatter.tspackages/features/insights/server/buildBaseWhereCondition.tspackages/features/insights/server/events.tspackages/features/insights/server/insightsDateUtils.tspackages/features/insights/server/raw-data.schema.tspackages/features/insights/server/routing-events.tspackages/features/insights/server/virtual-queues.tspackages/features/insights/services/InsightsBookingBaseService.tspackages/features/insights/services/InsightsBookingDIService.tspackages/features/insights/services/InsightsRoutingBaseService.tspackages/features/insights/services/InsightsRoutingDIService.tspackages/features/insights/services/csvDataTransformer.tspackages/trpc/react/shared.tspackages/trpc/server/routers/viewer/_router.tsxpackages/trpc/server/routers/viewer/insights/_router.tspackages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
regisstedile
force-pushed
the
feat/insights
branch
from
51a19c5 to
e7c4a0c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx (1)
510-513:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the badge label.
"New"is hardcoded, so this badge ignores the active locale while the rest of the sidebar usest(...).Proposed fix
{child.isBadged && ( <Badge variant="blue" className="mt-0.5 text-xs"> - New + {t("new")} </Badge> )}As per coding guidelines,
Add translations to packages/i18n/locales/en/common.json for all UI strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx around lines 510 - 513, Replace the hardcoded "New" badge label with a localized string: use the existing translation helper (t) where the Badge is rendered inside SettingsLayoutAppDirClient.tsx (the block that checks child.isBadged and renders <Badge>), and add the corresponding key (e.g. "sidebar.badge.new" or similar) to packages/i18n/locales/en/common.json so the label respects the active locale; ensure you import/use the same t instance used elsewhere in this component so the badge text matches other sidebar translations.
🟡 Minor comments (10)
apps/web/modules/auth/saml-idp/saml-idp-view.tsx-7-10 (1)
7-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle the
signInpromise to avoid unhandled rejections.Line [8] fires
signInwithout handling rejection. Wrapping withvoid+catchkeeps this effect safe.Suggested change
useEffect(() => { - signIn("saml-idp", { callbackUrl: "/", code }); + void signIn("saml-idp", { callbackUrl: "/", code }).catch(() => { + // optionally report auth bootstrap failure + }); // eslint-disable-next-line react-hooks/exhaustive-deps }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/auth/saml-idp/saml-idp-view.tsx` around lines 7 - 10, The effect calls signIn("saml-idp", { callbackUrl: "/", code }) without handling its returned promise which can cause unhandled rejections; update the useEffect in saml-idp-view.tsx to handle the promise (e.g., use void signIn(...).catch(err => /* log or ignore */) or run an async IIFE and try/catch) so any rejection from signIn is caught; keep the call site as the existing useEffect and signIn invocation but add a .catch handler (or try/catch in an async function) to handle errors for the code variable path.apps/web/app/(use-page-wrapper)/auth/saml-idp/page.tsx-8-13 (1)
8-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject empty
codevalues in query validation.Line [8] currently allows
?code=(empty string), which still reaches the client sign-in flow. Tightening this to non-empty avoids invalid auth attempts.Suggested change
-const querySchema = z.object({ code: z.string() }); +const querySchema = z.object({ code: z.string().min(1) });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/auth/saml-idp/page.tsx around lines 8 - 13, The query validation currently allows an empty code value; update the Zod schema used in SamlIdpPage so the code parameter rejects empty strings (e.g., change querySchema from z.object({ code: z.string() }) to require non-empty strings via z.string().min(1) or z.string().nonempty()), then keep the existing safeParse and notFound logic so empty or missing codes do not render SamlIdpClient.apps/web/app/api/auth/saml/userinfo/route.ts-17-19 (1)
17-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce Bearer scheme for
authorizationparsing.Line [18] accepts any two-part header (not only
Bearer), which makes auth parsing too permissive.Suggested change
- const parts = (authHeader || "").split(" "); - if (parts.length > 1) return parts[1]; + const parts = (authHeader || "").split(" "); + if (parts.length === 2 && /^Bearer$/i.test(parts[0]) && parts[1]) return parts[1];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/api/auth/saml/userinfo/route.ts` around lines 17 - 19, The authorization parsing is too permissive because the current split logic returns any two-part header; update the extraction to enforce the Bearer scheme by verifying the first token equals "Bearer" (case-insensitive) before returning the token. Locate the authHeader handling in route.ts (the parts = (authHeader || "").split(" ") logic) and change it to check parts[0].toLowerCase() === "bearer" (or equivalent) and only then return parts[1]; otherwise return null/undefined or handle as an invalid authorization header.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/general/page.tsx-143-143 (1)
143-143:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate the permission notice string.
This user-facing message should be routed through
t(...)and backed by locale entries.As per coding guidelines:
**/*.{ts,tsx,jsx}: Add translations topackages/i18n/locales/en/common.jsonfor all UI strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/general/page.tsx at line 143, Wrap the user-facing permission notice string "Only organization owners and admins can update these settings." with the i18n translator call t(...), e.g. t('orgs.permissionNotice'), and replace the hard-coded paragraph accordingly in page.tsx; then add a matching key "orgs.permissionNotice": "Only organization owners and admins can update these settings." to packages/i18n/locales/en/common.json so the string is available in English. Ensure you import/use the same t function already used in this file (or from the i18n hook/util) so the translated text is rendered.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/invites/page.tsx-22-23 (1)
22-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize hardcoded UI/toast strings.
These strings should use translation keys (
t(...)) instead of inline English literals.As per coding guidelines:
**/*.{ts,tsx,jsx}: Add translations topackages/i18n/locales/en/common.jsonfor all UI strings.Also applies to: 31-31, 41-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/invites/page.tsx around lines 22 - 23, Hardcoded UI/toast strings (e.g., the message passed to showToast and any labels/strings around router.push in this component) must be replaced with translation keys via the t(...) function and corresponding entries added to packages/i18n/locales/en/common.json; locate usages such as showToast("You have joined the organization.", "success"), and any similar strings at the other noted locations (lines ~31 and ~41-49), replace the literal strings with t('settings.organizations.joined') or appropriately named keys, and add those keys and English values to common.json so the component imports/uses the i18n t function consistently.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/sso/page.tsx-15-18 (1)
15-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle clipboard-write failures explicitly.
If
navigator.clipboard.writeTextfails (permissions/insecure context), the promise rejects and users get no actionable feedback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/sso/page.tsx around lines 15 - 18, The copyToClipboard function currently awaits navigator.clipboard.writeText without handling rejection; wrap the writeText call in a try/catch inside copyToClipboard, and on error call showToast with an error variant and include the error message to give user feedback; optionally attempt a fallback copy using a temporary textarea + document.execCommand('copy') before failing, and ensure you still call showToast on success (using the existing success message) or on final failure (with the error text) so users know what happened.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/profile/page.tsx-22-23 (1)
22-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize hard-coded page text.
Line 22 and Lines 55-56 use hard-coded English strings, so this page will not fully localize.
As per coding guidelines
**/*.{ts,tsx,jsx}: Add translations to packages/i18n/locales/en/common.json for all UI strings.Also applies to: 55-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/profile/page.tsx around lines 22 - 23, Replace hard-coded UI strings with localized keys: add "organization_profile_title" (value "Organization profile") and any other literal strings used at lines 55-56 to packages/i18n/locales/en/common.json, then update the JSX in page.tsx to call the i18n translator (t("organization_profile_title") and t(...) for the other strings) instead of the hard-coded English text; ensure keys are unique and follow existing naming (e.g., profile_org_description already used) so the page is fully localized.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/sso/page.tsx-52-53 (1)
52-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace hard-coded UI copy with translation keys.
These user-facing strings are currently hard-coded, which breaks localization coverage on this page.
As per coding guidelines
**/*.{ts,tsx,jsx}: Add translations to packages/i18n/locales/en/common.json for all UI strings.Also applies to: 58-59, 65-67, 74-74, 83-83, 88-88, 105-105, 112-112, 124-125, 133-133, 145-145, 153-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/sso/page.tsx around lines 52 - 53, Replace all hard-coded user-facing strings in this file with i18n keys by using the translation hook (e.g., call useTranslations('common') and replace title="Single sign-on" and description="Configure SAML SSO for your organization." as well as the other literal strings referenced (lines with headings/buttons/text) with t('sso.title'), t('sso.description'), etc.; then add matching keys and English values in packages/i18n/locales/en/common.json under an "sso" namespace (e.g., "sso.title", "sso.description", and entries for the strings at the other listed locations) so the UI reads from translations instead of hard-coded copy.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/members/page.tsx-21-23 (1)
21-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize hard-coded strings in members UI.
This page still has multiple hard-coded English strings (including toasts and role labels), so localization is incomplete.
As per coding guidelines
**/*.{ts,tsx,jsx}: Add translations to packages/i18n/locales/en/common.json for all UI strings.Also applies to: 35-35, 47-47, 71-71, 74-74, 78-78, 80-80, 111-111, 122-122, 132-133, 137-137, 150-150
packages/features/insights/services/InsightsBookingBaseService.ts-31-35 (1)
31-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard null usernames before building the fallback avatar URL.
When
usernameisnull, this produces"/null/avatar.png", so stats that rely on this map render a broken avatar URL instead of a usable fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/insights/services/InsightsBookingBaseService.ts` around lines 31 - 35, When building the avatarUrl inside the usersFromTeam.forEach block (the call that populates userHashMap via userHashMap.set), guard against null/undefined user.username before constructing the fallback string; if user.avatarUrl is falsy then use a safe fallback (e.g. AVATAR_FALLBACK or an empty/default URL) instead of `/${user.username}/avatar.png` so a null username does not produce "/null/avatar.png". Update the avatarUrl assignment in that usersFromTeam.forEach/userHashMap.set site to choose between user.avatarUrl, a username-based path only when user.username is truthy, and a final constant fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/api/auth/saml/authorize/route.ts`:
- Around line 15-19: The code currently casts req.nextUrl.searchParams into
OAuthReq and passes it to oauthController.authorize (via the oAuthReq variable),
which trusts unvalidated client input; update the route handler in route.ts to
parse and validate the authorize query parameters before calling
oauthController.authorize (use a runtime validator such as a Zod/schema check or
explicit checks for required fields and types that match the OAuthReq shape),
reject or return a 400 on invalid/missing params, and only construct/passthrough
a validated OAuthReq to oauthController.authorize so NextResponse.redirect is
only invoked with verified input.
In `@apps/web/app/api/auth/saml/callback/route.ts`:
- Around line 17-20: The request payload parsing with parseRequestData is done
outside the try block so parse failures bypass the route's trace-aware error
handling; move the call to parseRequestData inside the same try that calls
oauthController.samlResponse (and similarly for the other occurrence around
lines 30-33) so both parsing and oauthController.samlResponse are executed
within the guarded error path, ensuring parse errors are caught and handled by
the route's error handler.
- Around line 27-28: The log statement in route.ts is printing the raw
user-supplied RelayState (requestData?.RelayState) which can leak sensitive
redirect/query data; update the log.error call that currently references
requestData?.RelayState to either omit the raw value or log a non-sensitive
representation (e.g., a boolean indicating presence, a truncated/redacted
string, or a deterministic hash like SHA‑256 of RelayState) and include context
(e.g., the uid/trace) instead; change the code around the log.error that
precedes the NextResponse.json return so it no longer writes the full RelayState
to logs but still provides enough info for debugging (use
requestData?.RelayState => "REDACTED" or hashed/flagged form).
In `@apps/web/app/api/auth/saml/token/route.ts`:
- Line 29: Replace the plain throw new Error in the SAML token route (the throw
using `uid`: `Error getting auth token. trace: ${uid}`) with an ErrorWithCode
instance: import ErrorWithCode where other non-tRPC modules use it, construct
the ErrorWithCode with an appropriate machine-readable code and the same message
including `uid`, and throw that instead to maintain typed/consistent error
handling in the route (ensure the import for ErrorWithCode is added if missing).
In `@apps/web/app/api/auth/saml/userinfo/route.ts`:
- Around line 40-43: Replace the plain Error thrown in the userinfo route with
an ErrorWithCode instance: locate the throw at the end of the catch block where
uid is generated and log.error(`trace: ${uid} Error getting user info:
${error}`) is called, import ErrorWithCode, and throw a new ErrorWithCode that
preserves the message and trace id (e.g. `Error getting user info. trace:
${uid}`) and sets an appropriate code (e.g. 'INTERNAL_ERROR' or similar per code
conventions) so structured error semantics are maintained in non-tRPC layers.
In `@packages/features/auth/lib/next-auth-options.ts`:
- Around line 434-436: Guard against a missing email before calling toLowerCase
by checking userInfo.email (e.g., if (!userInfo?.email) ...) and route the flow
to the controlled auth error path instead of invoking .toLowerCase(); update the
block that destructures { id, firstName, lastName, requested } and the
subsequent email = userInfo.email.toLowerCase() to first validate presence of
userInfo.email, log or surface a clear auth error, and return/throw the
project's standardized authentication failure (use the existing auth error
handling used elsewhere in next-auth-options.ts) so missing IdP emails fail
safely.
In `@packages/features/ee/sso/lib/saml.ts`:
- Line 7: Replace the insecure default for clientSecretVerifier so code fails
fast when SAML client secret is not provided: remove the fallback string "dummy"
and instead read process.env.SAML_CLIENT_SECRET_VERIFIER and throw a clear Error
(or return undefined and ensure SAML is disabled upstream) when it's missing;
update the exported symbol clientSecretVerifier (and any SAML initialization
logic that consumes it) to rely on this check so the app will not start or
enable SAML with a predictable secret.
In `@packages/features/insights/services/InsightsBookingBaseService.ts`:
- Around line 411-416: The membership lookup is skipped when teamsFromOrg is
empty, causing org-level members (whose id is already in teamIds) to be missed;
change the condition to base the query on teamIds (or always call
MembershipRepository.findAllByTeamIds when teamIds.length > 0) so userIdsFromOrg
is populated from MembershipRepository.findAllByTeamIds({ teamIds, select: {
userId: true } }) regardless of teamsFromOrg, then map to m.userId as before
(preserving the existing variable userIdsFromOrg and usage).
- Around line 204-214: The select handling in InsightsBookingBaseService is
honoring keys set to false because it uses Object.keys(select) unfiltered;
update the logic in the block that builds selectFields (the code that references
select, keys, bookingDataKeys, selectFields and uses Prisma.sql/Prisma.raw) to
first filter keys to only those with a truthy value (e.g.,
Object.entries(select).filter(([,v])=>v) or similar), validate those filtered
keys against bookingDataKeys, and then compose the SQL projection only from the
filtered keys so fields explicitly set to false are omitted, preserving the
SelectedFields<TSelect> contract.
- Around line 1187-1201: In calculatePreviousPeriodDates(), fix the off-by-one
by computing periodLength = differenceInCalendarDays(endDate, startDate) + 1
(use date-fns differenceInCalendarDays or native Date math) and then compute
lastPeriodStartDate = subDays(startDate, periodLength) and lastPeriodEndDate =
subDays(endDate, periodLength) (or equivalent native Date subtract), replacing
Day.js usage with date-fns (differenceInCalendarDays, subDays, format) or native
Date methods; return ISO strings and "YYYY-MM-DD" formatted strings (use format
from date-fns or toISOString/zero-pad for native) for startDate/ endDate/
formattedStartDate/ formattedEndDate.
In `@packages/trpc/server/routers/viewer/organizations/acceptInvite.handler.ts`:
- Around line 19-21: The membership lookup in prisma.membership.findUnique
currently allows any team invite and then uses the team's organizationId to set
user.organizationId; update the query predicate to require the team be an
organization by adding team: { isOrganization: true } (i.e., include team: {
isOrganization: true } inside the where clause for prisma.membership.findUnique)
so only organization memberships are accepted, and make the same change to the
other membership lookup later in this handler that performs an equivalent
membership check.
In `@packages/trpc/server/routers/viewer/organizations/create.handler.ts`:
- Around line 41-61: The conflict checks (the user.organizationId check, the
prisma.membership.findFirst call, and the slug availability check via
assertOrganizationSlugAvailable) must be made atomic with the organization
creation to prevent race conditions; move these validations and the subsequent
create/update DB operations into a single transactional block (e.g.,
prisma.$transaction) or rely on a DB-unique constraint on organization.slug and
catch unique-constraint errors, then throw TRPCError({ code: "CONFLICT", ... })
accordingly; ensure the code paths that call normalizeOrganizationSlug and
assertOrganizationSlugAvailable are executed inside that transaction or replaced
by a transactional uniqueness check so two concurrent requests cannot both
succeed.
In `@packages/trpc/server/routers/viewer/organizations/declineInvite.handler.ts`:
- Around line 24-30: The current check-then-delete has a race; replace the
separate existence check and prisma.membership.delete call with an atomic
conditional delete (e.g., use prisma.membership.deleteMany with where: { userId:
ctx.user.id, teamId: input.teamId, accepted: false }) and then assert the
returned count is 1; if count is 0 throw the same TRPCError ("Cannot decline an
already accepted invite." or "Invite not found") so the decline is only
performed when accepted is false in a single database operation (update the code
around the membership check and the prisma.membership.delete call in
declineInvite.handler).
In `@packages/trpc/server/routers/viewer/organizations/inviteMember.handler.ts`:
- Around line 34-44: The current check uses invitee.organizationId which can be
stale; instead query memberships to detect an accepted membership for the user
and reject the invite if one exists. Replace or augment the
invitee.organizationId check with a prisma query (e.g.,
prisma.membership.findFirst or findUnique variant) that searches for a
membership where userId === invitee.id and accepted === true (across teams), and
throw the same TRPCError if such a record is found; continue using the existing
membership.team.id logic only for invite-specific lookups (the existing
prisma.membership.findUnique for userId_teamId is fine for duplicate invites).
---
Outside diff comments:
In
`@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx:
- Around line 510-513: Replace the hardcoded "New" badge label with a localized
string: use the existing translation helper (t) where the Badge is rendered
inside SettingsLayoutAppDirClient.tsx (the block that checks child.isBadged and
renders <Badge>), and add the corresponding key (e.g. "sidebar.badge.new" or
similar) to packages/i18n/locales/en/common.json so the label respects the
active locale; ensure you import/use the same t instance used elsewhere in this
component so the badge text matches other sidebar translations.
---
Minor comments:
In `@apps/web/app/`(use-page-wrapper)/auth/saml-idp/page.tsx:
- Around line 8-13: The query validation currently allows an empty code value;
update the Zod schema used in SamlIdpPage so the code parameter rejects empty
strings (e.g., change querySchema from z.object({ code: z.string() }) to require
non-empty strings via z.string().min(1) or z.string().nonempty()), then keep the
existing safeParse and notFound logic so empty or missing codes do not render
SamlIdpClient.
In
`@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/general/page.tsx:
- Line 143: Wrap the user-facing permission notice string "Only organization
owners and admins can update these settings." with the i18n translator call
t(...), e.g. t('orgs.permissionNotice'), and replace the hard-coded paragraph
accordingly in page.tsx; then add a matching key "orgs.permissionNotice": "Only
organization owners and admins can update these settings." to
packages/i18n/locales/en/common.json so the string is available in English.
Ensure you import/use the same t function already used in this file (or from the
i18n hook/util) so the translated text is rendered.
In
`@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/invites/page.tsx:
- Around line 22-23: Hardcoded UI/toast strings (e.g., the message passed to
showToast and any labels/strings around router.push in this component) must be
replaced with translation keys via the t(...) function and corresponding entries
added to packages/i18n/locales/en/common.json; locate usages such as
showToast("You have joined the organization.", "success"), and any similar
strings at the other noted locations (lines ~31 and ~41-49), replace the literal
strings with t('settings.organizations.joined') or appropriately named keys, and
add those keys and English values to common.json so the component imports/uses
the i18n t function consistently.
In
`@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/profile/page.tsx:
- Around line 22-23: Replace hard-coded UI strings with localized keys: add
"organization_profile_title" (value "Organization profile") and any other
literal strings used at lines 55-56 to packages/i18n/locales/en/common.json,
then update the JSX in page.tsx to call the i18n translator
(t("organization_profile_title") and t(...) for the other strings) instead of
the hard-coded English text; ensure keys are unique and follow existing naming
(e.g., profile_org_description already used) so the page is fully localized.
In
`@apps/web/app/`(use-page-wrapper)/settings/(settings-layout)/organizations/sso/page.tsx:
- Around line 15-18: The copyToClipboard function currently awaits
navigator.clipboard.writeText without handling rejection; wrap the writeText
call in a try/catch inside copyToClipboard, and on error call showToast with an
error variant and include the error message to give user feedback; optionally
attempt a fallback copy using a temporary textarea +
document.execCommand('copy') before failing, and ensure you still call showToast
on success (using the existing success message) or on final failure (with the
error text) so users know what happened.
- Around line 52-53: Replace all hard-coded user-facing strings in this file
with i18n keys by using the translation hook (e.g., call
useTranslations('common') and replace title="Single sign-on" and
description="Configure SAML SSO for your organization." as well as the other
literal strings referenced (lines with headings/buttons/text) with
t('sso.title'), t('sso.description'), etc.; then add matching keys and English
values in packages/i18n/locales/en/common.json under an "sso" namespace (e.g.,
"sso.title", "sso.description", and entries for the strings at the other listed
locations) so the UI reads from translations instead of hard-coded copy.
In `@apps/web/app/api/auth/saml/userinfo/route.ts`:
- Around line 17-19: The authorization parsing is too permissive because the
current split logic returns any two-part header; update the extraction to
enforce the Bearer scheme by verifying the first token equals "Bearer"
(case-insensitive) before returning the token. Locate the authHeader handling in
route.ts (the parts = (authHeader || "").split(" ") logic) and change it to
check parts[0].toLowerCase() === "bearer" (or equivalent) and only then return
parts[1]; otherwise return null/undefined or handle as an invalid authorization
header.
In `@apps/web/modules/auth/saml-idp/saml-idp-view.tsx`:
- Around line 7-10: The effect calls signIn("saml-idp", { callbackUrl: "/", code
}) without handling its returned promise which can cause unhandled rejections;
update the useEffect in saml-idp-view.tsx to handle the promise (e.g., use void
signIn(...).catch(err => /* log or ignore */) or run an async IIFE and
try/catch) so any rejection from signIn is caught; keep the call site as the
existing useEffect and signIn invocation but add a .catch handler (or try/catch
in an async function) to handle errors for the code variable path.
In `@packages/features/insights/services/InsightsBookingBaseService.ts`:
- Around line 31-35: When building the avatarUrl inside the
usersFromTeam.forEach block (the call that populates userHashMap via
userHashMap.set), guard against null/undefined user.username before constructing
the fallback string; if user.avatarUrl is falsy then use a safe fallback (e.g.
AVATAR_FALLBACK or an empty/default URL) instead of
`/${user.username}/avatar.png` so a null username does not produce
"/null/avatar.png". Update the avatarUrl assignment in that
usersFromTeam.forEach/userHashMap.set site to choose between user.avatarUrl, a
username-based path only when user.username is truthy, and a final constant
fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53424c94-2bab-4ff8-b70b-f214825cda91
📒 Files selected for processing (43)
apps/web/app/(use-page-wrapper)/auth/saml-idp/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/general/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/invites/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/members/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/profile/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/sso/page.tsxapps/web/app/api/auth/saml/authorize/route.tsapps/web/app/api/auth/saml/callback/route.tsapps/web/app/api/auth/saml/token/route.tsapps/web/app/api/auth/saml/userinfo/route.tsapps/web/modules/auth/saml-idp/saml-idp-view.tsxapps/web/modules/insights/components/BookingAtCell.tsxapps/web/modules/insights/components/booking/BookingKPICards.tsxapps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsxapps/web/modules/insights/components/filters/Download/Download.tsxapps/web/next.config.tsapps/web/pages/api/trpc/organizations/[trpc].tsapps/web/playwright/settings/organizations.e2e.tspackages/features/auth/lib/next-auth-options.tspackages/features/ee/sso/lib/jackson.tspackages/features/ee/sso/lib/saml.tspackages/features/insights/server/insightsDateUtils.tspackages/features/insights/services/InsightsBookingBaseService.tspackages/trpc/react/shared.tspackages/trpc/server/routers/viewer/_router.tsxpackages/trpc/server/routers/viewer/insights/_router.tspackages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.tspackages/trpc/server/routers/viewer/organizations/_router.tsxpackages/trpc/server/routers/viewer/organizations/acceptInvite.handler.tspackages/trpc/server/routers/viewer/organizations/create.handler.tspackages/trpc/server/routers/viewer/organizations/declineInvite.handler.tspackages/trpc/server/routers/viewer/organizations/getCurrent.handler.tspackages/trpc/server/routers/viewer/organizations/inviteMember.handler.tspackages/trpc/server/routers/viewer/organizations/listMembers.handler.tspackages/trpc/server/routers/viewer/organizations/listPendingInvites.handler.tspackages/trpc/server/routers/viewer/organizations/organizationUtils.tspackages/trpc/server/routers/viewer/organizations/removeMember.handler.tspackages/trpc/server/routers/viewer/organizations/saml.handler.tspackages/trpc/server/routers/viewer/organizations/schema.tspackages/trpc/server/routers/viewer/organizations/update.handler.tspackages/trpc/server/routers/viewer/organizations/updateMemberRole.handler.ts
💤 Files with no reviewable changes (1)
- apps/web/next.config.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/page.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/modules/insights/components/BookingAtCell.tsx
- packages/trpc/server/routers/viewer/_router.tsx
- apps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsx
- packages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
- apps/web/modules/insights/components/filters/Download/Download.tsx
- packages/features/insights/server/insightsDateUtils.ts
- packages/trpc/server/routers/viewer/insights/_router.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx`:
- Around line 25-29: The legend currently uses hardcoded English labels and the
toggle logic relies on those labels; change the exported legend array (legend)
to use stable keys (e.g., key: "totalSubmissions", "successfulRoutings",
"acceptedBookings") paired with the existing color values (COLOR.TOTAL,
COLOR.SUCCESFUL, COLOR.ACCEPTED), then update any area visibility checks (the
code that checks labels to hide/show areas) to match these keys instead of label
text; at render time map legend entries to translated strings using the i18n
lookup (e.g., t("common.totalSubmissions")) so UI text is pulled from
packages/i18n/locales/en/common.json where you must add the three new
translation keys, and remove any direct string comparisons against English
labels elsewhere in the component.
In `@apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx`:
- Around line 37-38: In RoutingFunnelSkeleton, remove Math.random() from inline
JSX style (used on the height at the shown lines and also at 41-42, 45-46) and
instead precompute deterministic heights once per component render: inside the
RoutingFunnelSkeleton component create a fixed array or a useMemo hook (e.g.,
heights = useMemo(() => [...], [])) or a seeded pseudo-random generator to
produce the same heights on every render, then reference heights[index] in the
style properties; this ensures stable/hydratable skeleton geometry without
changing the JSX structure.
In `@apps/web/modules/insights/hooks/useInsightsColumns.tsx`:
- Around line 34-35: The memoized column definitions inside useInsightsColumns
call t(...) but t is not included in the useMemo dependency arrays, causing
stale localized labels after locale changes; update the dependency arrays for
the useMemo that returns columns (the block that checks isHeadersSuccess) to
include t, and also add t to the other useMemo that builds columns later in the
same file so both memoized column definitions re-compute when the localization
function changes.
- Line 278: The UTM column headers in useInsightsColumns.tsx are hardcoded
strings ("utm_medium", "utm_term", "utm_content", "utm_campaign"); replace those
literal header values with calls to the i18n translation function (e.g.,
t('insights.columns.utm_medium')) wherever the headers are set (look for the
header properties in the columns returned by useInsightsColumns) and add
corresponding keys and English values to packages/i18n/locales/en/common.json
(e.g., "insights.columns.utm_medium": "UTM Medium", etc.) so the UI uses
localized strings.
- Line 1: Remove the Day.js dependency and replace its usage with native Date
formatting in useInsightsColumns: delete the import "dayjs" and change the
expression dayjs(info.getValue()).format("MMM D, YYYY HH:mm") to create a Date
from info.getValue() and format it using Intl.DateTimeFormat (or manual
pad/locale logic) to produce "MMM D, YYYY HH:mm" output; update any tests/uses
of the getValue formatting in the same file (e.g., inside the cell renderer
where info.getValue() is formatted) to use the new Date-based formatter.
In `@apps/web/modules/insights/views/insights-view.tsx`:
- Around line 10-28: The grouped import that pulls components from the booking
barrel index (the import listing AverageEventDurationChart, BookingKPICards,
BookingsByHourChart, CSATOverTimeChart, EventTrendsChart,
HighestNoShowHostTable, HighestRatedMembersTable, LeastBookedTeamMembersTable,
LeastCompletedTeamMembersTable, LowestRatedMembersTable,
MostBookedTeamMembersTable, MostCancelledBookingsTables,
MostCompletedTeamMembersTable, NoShowHostsOverTimeChart, PopularEventsTable,
RecentFeedbackTable, RecentNoShowGuestsChart, TimezoneBadge) should be replaced
with direct imports from each component’s source file; locate the single import
statement referencing the booking barrel and replace it with individual imports
for each symbol (e.g., import AverageEventDurationChart from its component file,
import BookingKPICards from its component file, etc.), ensuring each component
name above is imported from its own source module rather than the barrel index.
In `@packages/features/insights/server/events.ts`:
- Around line 16-18: The dynamic status bucket increment can operate on
undefined keys (aggregate[item.timeStatus]) causing NaN; before adding counts in
the block that checks typeof item.timeStatus === "string" (and the similar block
around lines 27-32), ensure the targeted aggregate bucket is initialized to a
numeric zero if missing (e.g., set aggregate[item.timeStatus] = 0 when
undefined) and then add item._count._all (with fallback 0). Update the
increments for both the specific bucket and aggregate["_all"] to use these
guarded/initialized values so unseeded statuses don't corrupt totals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cbb3798-8307-46e9-8624-d34ed77db5ee
📒 Files selected for processing (94)
apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsxapps/web/app/(use-page-wrapper)/insights/call-history/page.tsxapps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.tsapps/web/app/(use-page-wrapper)/insights/layout.tsxapps/web/app/(use-page-wrapper)/insights/page.tsxapps/web/app/(use-page-wrapper)/insights/router-position/page.tsxapps/web/app/(use-page-wrapper)/insights/routing/page.tsxapps/web/modules/insights/components/BookedByCell.tsxapps/web/modules/insights/components/BookingAtCell.tsxapps/web/modules/insights/components/BookingStatusBadge.tsxapps/web/modules/insights/components/CellWithOverflowX.tsxapps/web/modules/insights/components/ChartCard.tsxapps/web/modules/insights/components/KPICard.tsxapps/web/modules/insights/components/ResponseValueCell.tsxapps/web/modules/insights/components/UserStatsTable.tsxapps/web/modules/insights/components/booking/AverageEventDurationChart.tsxapps/web/modules/insights/components/booking/BookingKPICards.tsxapps/web/modules/insights/components/booking/BookingsByHourChart.tsxapps/web/modules/insights/components/booking/CSATOverTimeChart.tsxapps/web/modules/insights/components/booking/EventTrendsChart.tsxapps/web/modules/insights/components/booking/HighestNoShowHostTable.tsxapps/web/modules/insights/components/booking/HighestRatedMembersTable.tsxapps/web/modules/insights/components/booking/LeastBookedTeamMembersTable.tsxapps/web/modules/insights/components/booking/LeastCompletedBookings.tsxapps/web/modules/insights/components/booking/LowestRatedMembersTable.tsxapps/web/modules/insights/components/booking/MostBookedTeamMembersTable.tsxapps/web/modules/insights/components/booking/MostCancelledBookingsTables.tsxapps/web/modules/insights/components/booking/MostCompletedBookings.tsxapps/web/modules/insights/components/booking/NoShowHostsOverTimeChart.tsxapps/web/modules/insights/components/booking/PopularEventsTable.tsxapps/web/modules/insights/components/booking/RecentFeedbackTable.tsxapps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsxapps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsxapps/web/modules/insights/components/booking/TimezoneBadge.tsxapps/web/modules/insights/components/booking/index.tsapps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsxapps/web/modules/insights/components/filters/DateTargetSelector.tsxapps/web/modules/insights/components/filters/Download/Download.tsxapps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsxapps/web/modules/insights/components/filters/Download/index.tsxapps/web/modules/insights/components/filters/OrgTeamsFilter.tsxapps/web/modules/insights/components/filters/index.tsxapps/web/modules/insights/components/index.tsapps/web/modules/insights/components/routing/FailedBookingsByField.tsxapps/web/modules/insights/components/routing/RoutedToPerPeriod.tsxapps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsxapps/web/modules/insights/components/routing/RoutingFunnel.tsxapps/web/modules/insights/components/routing/RoutingFunnelContent.tsxapps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsxapps/web/modules/insights/components/routing/RoutingKPICards.tsxapps/web/modules/insights/components/routing/index.tsapps/web/modules/insights/hooks/useDefaultRoutingForm.tsapps/web/modules/insights/hooks/useInsightsBookingFacetedUniqueValues.tsapps/web/modules/insights/hooks/useInsightsBookingParameters.tsapps/web/modules/insights/hooks/useInsightsBookings.tsapps/web/modules/insights/hooks/useInsightsColumns.tsxapps/web/modules/insights/hooks/useInsightsOrgTeams.tsapps/web/modules/insights/hooks/useInsightsRoutingFacetedUniqueValues.tsapps/web/modules/insights/hooks/useInsightsRoutingParameters.tsapps/web/modules/insights/hooks/useToggleableLegend.tsapps/web/modules/insights/views/insights-call-history-view.tsxapps/web/modules/insights/views/insights-routing-view.tsxapps/web/modules/insights/views/insights-view.tsxapps/web/modules/insights/views/insights-virtual-queues-view.tsxapps/web/modules/shell/navigation/Navigation.tsxapps/web/pages/api/trpc/insights/[trpc].tspackages/features/di/containers/InsightsBooking.tspackages/features/di/containers/InsightsRouting.tspackages/features/di/modules/InsightsBooking.tspackages/features/di/modules/InsightsRouting.tspackages/features/di/tokens.tspackages/features/insights/lib/bookingStatusToText.tspackages/features/insights/lib/bookingUtils.tspackages/features/insights/lib/calculateDeltaType.tspackages/features/insights/lib/index.tspackages/features/insights/lib/objectToCsv.tspackages/features/insights/lib/repositories/TeamRepository.tspackages/features/insights/lib/types.tspackages/features/insights/lib/valueFormatter.tspackages/features/insights/server/buildBaseWhereCondition.tspackages/features/insights/server/events.tspackages/features/insights/server/insightsDateUtils.tspackages/features/insights/server/raw-data.schema.tspackages/features/insights/server/routing-events.tspackages/features/insights/server/virtual-queues.tspackages/features/insights/services/InsightsBookingBaseService.tspackages/features/insights/services/InsightsBookingDIService.tspackages/features/insights/services/InsightsRoutingBaseService.tspackages/features/insights/services/InsightsRoutingDIService.tspackages/features/insights/services/csvDataTransformer.tspackages/trpc/react/shared.tspackages/trpc/server/routers/viewer/_router.tsxpackages/trpc/server/routers/viewer/insights/_router.tspackages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
✅ Files skipped from review due to trivial changes (3)
- apps/web/modules/insights/components/booking/index.ts
- apps/web/modules/insights/components/filters/Download/index.tsx
- packages/features/insights/lib/index.ts
🚧 Files skipped from review as they are similar to previous changes (77)
- apps/web/app/(use-page-wrapper)/insights/routing/page.tsx
- apps/web/modules/insights/components/index.ts
- packages/features/insights/lib/valueFormatter.ts
- packages/features/di/containers/InsightsBooking.ts
- packages/features/insights/lib/calculateDeltaType.ts
- packages/features/insights/server/routing-events.ts
- packages/features/insights/services/InsightsRoutingDIService.ts
- apps/web/modules/insights/components/BookedByCell.tsx
- packages/features/di/containers/InsightsRouting.ts
- packages/trpc/server/routers/viewer/_router.tsx
- packages/features/di/modules/InsightsBooking.ts
- apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts
- packages/features/insights/services/InsightsBookingDIService.ts
- apps/web/modules/insights/components/UserStatsTable.tsx
- apps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsx
- apps/web/modules/insights/components/routing/RoutingFunnel.tsx
- apps/web/modules/insights/components/CellWithOverflowX.tsx
- apps/web/app/(use-page-wrapper)/insights/call-history/page.tsx
- apps/web/modules/insights/components/routing/index.ts
- apps/web/modules/insights/components/booking/LeastBookedTeamMembersTable.tsx
- packages/features/insights/lib/types.ts
- apps/web/modules/insights/hooks/useInsightsRoutingParameters.ts
- apps/web/pages/api/trpc/insights/[trpc].ts
- apps/web/modules/insights/components/booking/MostBookedTeamMembersTable.tsx
- apps/web/modules/insights/components/booking/MostCancelledBookingsTables.tsx
- apps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsx
- apps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsx
- apps/web/modules/insights/components/booking/LowestRatedMembersTable.tsx
- apps/web/modules/insights/hooks/useInsightsBookingParameters.ts
- packages/features/di/modules/InsightsRouting.ts
- packages/features/insights/lib/bookingStatusToText.ts
- apps/web/modules/insights/components/booking/HighestRatedMembersTable.tsx
- apps/web/modules/insights/components/booking/RecentFeedbackTable.tsx
- apps/web/app/(use-page-wrapper)/insights/page.tsx
- apps/web/modules/insights/hooks/useInsightsBookings.ts
- apps/web/app/(use-page-wrapper)/insights/layout.tsx
- apps/web/modules/insights/components/routing/RoutingKPICards.tsx
- apps/web/modules/insights/components/booking/BookingKPICards.tsx
- apps/web/modules/insights/components/filters/Download/Download.tsx
- packages/features/insights/lib/objectToCsv.ts
- apps/web/modules/insights/components/booking/TimezoneBadge.tsx
- packages/features/di/tokens.ts
- apps/web/modules/insights/components/booking/AverageEventDurationChart.tsx
- apps/web/modules/insights/components/booking/HighestNoShowHostTable.tsx
- apps/web/app/(use-page-wrapper)/insights/router-position/page.tsx
- packages/features/insights/server/buildBaseWhereCondition.ts
- apps/web/modules/insights/views/insights-call-history-view.tsx
- apps/web/modules/insights/components/booking/MostCompletedBookings.tsx
- apps/web/modules/insights/components/BookingAtCell.tsx
- apps/web/modules/insights/components/booking/EventTrendsChart.tsx
- apps/web/modules/insights/views/insights-routing-view.tsx
- apps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsx
- apps/web/modules/insights/components/filters/DateTargetSelector.tsx
- apps/web/modules/insights/components/KPICard.tsx
- apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx
- apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsx
- packages/features/insights/server/raw-data.schema.ts
- apps/web/modules/insights/components/BookingStatusBadge.tsx
- apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx
- apps/web/modules/insights/hooks/useToggleableLegend.ts
- packages/features/insights/lib/bookingUtils.ts
- apps/web/modules/insights/hooks/useInsightsOrgTeams.ts
- apps/web/modules/insights/hooks/useInsightsBookingFacetedUniqueValues.ts
- packages/features/insights/services/InsightsRoutingBaseService.ts
- apps/web/modules/insights/hooks/useDefaultRoutingForm.ts
- packages/features/insights/services/csvDataTransformer.ts
- apps/web/modules/insights/components/booking/CSATOverTimeChart.tsx
- packages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
- packages/features/insights/server/insightsDateUtils.ts
- apps/web/modules/insights/components/routing/RoutedToPerPeriod.tsx
- apps/web/modules/insights/components/booking/PopularEventsTable.tsx
- apps/web/modules/insights/components/booking/BookingsByHourChart.tsx
- apps/web/modules/insights/components/ResponseValueCell.tsx
- apps/web/modules/insights/components/routing/FailedBookingsByField.tsx
- packages/trpc/server/routers/viewer/insights/_router.ts
- packages/features/insights/services/InsightsBookingBaseService.ts
- apps/web/modules/insights/components/booking/LeastCompletedBookings.tsx
Ports the Insights feature from upstream, replacing EE/routing-forms dependencies that don't exist in this fork with stubs and direct Prisma membership checks. Covers full booking analytics (KPIs, trends, charts, raw data export) with empty stubs for routing analytics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- fix(security): validate selectedTeamId in userBelongsToTeam middleware to prevent cross-team read bypass - fix(ui): render KPI cards in error state instead of returning null - fix(ui): fix orgTeamsType race condition with useEffect after session hydration - fix(ui): replace Day.js with date-fns in BookingAtCell, add aria-label, localize "Status:" - fix(download): propagate fetch errors and guard against empty-page infinite loop - fix(analytics): use completed_bookings for completed.deltaPrevious KPI - fix(query): replace include with select in membership queries - fix(error): replace plain Error with ErrorWithCode in InsightsBookingBaseService - fix(date): correct week boundary calculation for non-Sunday weekStart Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix KPICard delta calculation (was incorrectly subtracting count) - Use stable React key (emailMd5) instead of Math.random() in UserStatsTable - Fix setState-in-render anti-pattern in virtual queues view → useEffect - Fix pagination offset logic in RoutingFormResponsesDownload - Replace generic Error with typed ErrorWithCode in bookingUtils and useInsightsOrgTeams - Add CSV injection protection in objectToCsv (neutralize =+-@ prefixes) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix RoutedToPerPeriod: setIsDownloading(true) before fetch - Fix RoutingKPICards: import valueFormatter directly from source - Fix virtual queues view: localize Select placeholder with t() - Remove barrel export insights/lib/index.ts — import from source files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
regisstedile
force-pushed
the
feat/insights
branch
from
e7c4a0c to
2920a24
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts (1)
46-75:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd a final unauthorized guard when neither team membership nor org-admin access is present.
Currently, users without
organizationIdand without team membership can still pass through this middleware. Add an explicit final check beforenext(...).Suggested fix
if ((parse.data.isAll && ctx.user.organizationId) || (!membership && ctx.user.organizationId)) { @@ isOwnerAdminOfParentTeam = true; } + if (!membership && !isOwnerAdminOfParentTeam) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + } + return next({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts` around lines 46 - 75, Add an explicit final guard before calling next(...) that throws TRPCError({ code: "UNAUTHORIZED" }) when the requestor has neither team membership nor organization-level access; specifically, after computing membership, evaluating parse.data.isAll, running the org/parent-team checks and setting isOwnerAdminOfParentTeam, add a check like: if (!membership && !ctx.user.organizationId && !isOwnerAdminOfParentTeam) throw new TRPCError({ code: "UNAUTHORIZED" }); this ensures functions referenced here (membership, parse.data.isAll, ctx.user.organizationId, effectiveTeamId, hasInsightsAccess, isOwnerAdminOfParentTeam, next) will not allow users without any membership or org-admin rights to proceed.apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx (1)
62-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not silently succeed with partial exports on pagination stalls.
If a middle page is unexpectedly empty before
totalRecordsis reached, the loop exits and still emits a “100%” successful CSV with missing rows. Fail fast (or surface a warning) instead of breaking silently.Suggested fix
while (allData.length < totalRecords) { const result = await fetchBatch(offset); - if (result.data.length === 0) break; + if (result.data.length === 0) { + throw new Error("Pagination stalled before all records were fetched."); + } allData = [...allData, ...result.data]; offset += BATCH_SIZE;Also applies to: 72-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx` around lines 62 - 66, The loop that paginates using fetchBatch (inside RoutingFormResponsesDownload) currently breaks on an empty page and continues, which can produce a 100% CSV with missing rows; change the logic to treat an empty result.data before reaching totalRecords as an error (or surface a warning) instead of silently breaking: when result.data.length === 0 and allData.length < totalRecords, throw or return a failure/validation indication including offset/BATCH_SIZE and totalRecords so the caller can surface the problem; keep the normal completion path when allData.length >= totalRecords and retain use of fetchBatch, offset and BATCH_SIZE to compute which page failed.
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/insights/_router.ts (1)
1-1: ⚡ Quick winPrefer
date-fns/native Date for non-timezone date-only formatting.This filename generation only needs calendar dates and can follow the repo’s standard without Day.js.
As per coding guidelines:
Use date-fns or native Date instead of Day.js when timezone awareness isn't needed.Also applies to: 614-616
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trpc/server/routers/viewer/insights/_router.ts` at line 1, The file imports Day.js but the filename generation only needs calendar dates; replace the Day.js import (import dayjs from "`@calcom/dayjs`") and all uses of dayjs() for building filenames with date-fns or native Date functions — e.g., replace dayjs().format("YYYY-MM-DD") calls used in the filename generation logic with date-fns/format (format(new Date(), "yyyy-MM-dd")) or new Date().toISOString().slice(0,10); update any related helpers in this module that reference dayjs so they use the chosen date-fns/native API and remove the Day.js import.apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx (1)
1-1: ⚡ Quick winUse
date-fns(or native Date) for filename date formatting.This formatting does not require timezone-aware Day.js behavior, and the repo guideline prefers
date-fns/native Date here.As per coding guidelines:
Use date-fns or native Date instead of Day.js when timezone awareness isn't needed.Also applies to: 73-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx` at line 1, Replace the Day.js import and its filename-formatting usage with date-fns (or native Date) since timezone-aware Day.js isn't needed: remove the import line "import dayjs from '`@calcom/dayjs`'" and update the filename construction in the RoutingFormResponsesDownload component (the code around the current dayjs usage at lines ~73-75) to use date-fns' format(new Date(), 'yyyy-MM-dd') or new Date().toISOString()/toLocaleDateString() as appropriate; ensure any variable names involved in filename creation are kept the same so references in the component (e.g., the filename string builder) still work and update imports to include format from 'date-fns' if you choose date-fns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/modules/insights/components/CellWithOverflowX.tsx`:
- Line 28: The gradient overlay div in CellWithOverflowX.tsx (the element with
className "from-default absolute right-0 top-0 hidden h-full w-8 bg-linear-to-l
to-transparent") is intercepting pointer events and blocking interactions;
update that element's className to include pointer-events-none so the overlay
does not capture clicks/scroll/selection while preserving the visual gradient.
In `@apps/web/modules/insights/components/ChartCard.tsx`:
- Line 139: The aria-label for the toggle is hardcoded ("Toggle ..."); update
ChartCard's accessible label to use the i18n translation system instead of a
literal string: import/use the translation function (e.g., t) in ChartCard.tsx
and replace aria-label={`Toggle ${item.label}`} with a translated key (e.g.,
t('toggle', { label: item.label }) or a more specific key like t('chart.toggle',
{ label: item.label })), then add the corresponding entry to
packages/i18n/locales/en/common.json (and other locales) so the UI string is
translatable; alternatively, consider using aria-labelledby to reference visible
text if that fits the UI.
---
Duplicate comments:
In
`@apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx`:
- Around line 62-66: The loop that paginates using fetchBatch (inside
RoutingFormResponsesDownload) currently breaks on an empty page and continues,
which can produce a 100% CSV with missing rows; change the logic to treat an
empty result.data before reaching totalRecords as an error (or surface a
warning) instead of silently breaking: when result.data.length === 0 and
allData.length < totalRecords, throw or return a failure/validation indication
including offset/BATCH_SIZE and totalRecords so the caller can surface the
problem; keep the normal completion path when allData.length >= totalRecords and
retain use of fetchBatch, offset and BATCH_SIZE to compute which page failed.
In
`@packages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts`:
- Around line 46-75: Add an explicit final guard before calling next(...) that
throws TRPCError({ code: "UNAUTHORIZED" }) when the requestor has neither team
membership nor organization-level access; specifically, after computing
membership, evaluating parse.data.isAll, running the org/parent-team checks and
setting isOwnerAdminOfParentTeam, add a check like: if (!membership &&
!ctx.user.organizationId && !isOwnerAdminOfParentTeam) throw new TRPCError({
code: "UNAUTHORIZED" }); this ensures functions referenced here (membership,
parse.data.isAll, ctx.user.organizationId, effectiveTeamId, hasInsightsAccess,
isOwnerAdminOfParentTeam, next) will not allow users without any membership or
org-admin rights to proceed.
---
Nitpick comments:
In
`@apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx`:
- Line 1: Replace the Day.js import and its filename-formatting usage with
date-fns (or native Date) since timezone-aware Day.js isn't needed: remove the
import line "import dayjs from '`@calcom/dayjs`'" and update the filename
construction in the RoutingFormResponsesDownload component (the code around the
current dayjs usage at lines ~73-75) to use date-fns' format(new Date(),
'yyyy-MM-dd') or new Date().toISOString()/toLocaleDateString() as appropriate;
ensure any variable names involved in filename creation are kept the same so
references in the component (e.g., the filename string builder) still work and
update imports to include format from 'date-fns' if you choose date-fns.
In `@packages/trpc/server/routers/viewer/insights/_router.ts`:
- Line 1: The file imports Day.js but the filename generation only needs
calendar dates; replace the Day.js import (import dayjs from "`@calcom/dayjs`")
and all uses of dayjs() for building filenames with date-fns or native Date
functions — e.g., replace dayjs().format("YYYY-MM-DD") calls used in the
filename generation logic with date-fns/format (format(new Date(),
"yyyy-MM-dd")) or new Date().toISOString().slice(0,10); update any related
helpers in this module that reference dayjs so they use the chosen
date-fns/native API and remove the Day.js import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f78e15c-a927-48ca-b463-4a5a0fd0009b
📒 Files selected for processing (93)
apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsxapps/web/app/(use-page-wrapper)/insights/call-history/page.tsxapps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.tsapps/web/app/(use-page-wrapper)/insights/layout.tsxapps/web/app/(use-page-wrapper)/insights/page.tsxapps/web/app/(use-page-wrapper)/insights/router-position/page.tsxapps/web/app/(use-page-wrapper)/insights/routing/page.tsxapps/web/modules/insights/components/BookedByCell.tsxapps/web/modules/insights/components/BookingAtCell.tsxapps/web/modules/insights/components/BookingStatusBadge.tsxapps/web/modules/insights/components/CellWithOverflowX.tsxapps/web/modules/insights/components/ChartCard.tsxapps/web/modules/insights/components/KPICard.tsxapps/web/modules/insights/components/ResponseValueCell.tsxapps/web/modules/insights/components/UserStatsTable.tsxapps/web/modules/insights/components/booking/AverageEventDurationChart.tsxapps/web/modules/insights/components/booking/BookingKPICards.tsxapps/web/modules/insights/components/booking/BookingsByHourChart.tsxapps/web/modules/insights/components/booking/CSATOverTimeChart.tsxapps/web/modules/insights/components/booking/EventTrendsChart.tsxapps/web/modules/insights/components/booking/HighestNoShowHostTable.tsxapps/web/modules/insights/components/booking/HighestRatedMembersTable.tsxapps/web/modules/insights/components/booking/LeastBookedTeamMembersTable.tsxapps/web/modules/insights/components/booking/LeastCompletedBookings.tsxapps/web/modules/insights/components/booking/LowestRatedMembersTable.tsxapps/web/modules/insights/components/booking/MostBookedTeamMembersTable.tsxapps/web/modules/insights/components/booking/MostCancelledBookingsTables.tsxapps/web/modules/insights/components/booking/MostCompletedBookings.tsxapps/web/modules/insights/components/booking/NoShowHostsOverTimeChart.tsxapps/web/modules/insights/components/booking/PopularEventsTable.tsxapps/web/modules/insights/components/booking/RecentFeedbackTable.tsxapps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsxapps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsxapps/web/modules/insights/components/booking/TimezoneBadge.tsxapps/web/modules/insights/components/booking/index.tsapps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsxapps/web/modules/insights/components/filters/DateTargetSelector.tsxapps/web/modules/insights/components/filters/Download/Download.tsxapps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsxapps/web/modules/insights/components/filters/Download/index.tsxapps/web/modules/insights/components/filters/OrgTeamsFilter.tsxapps/web/modules/insights/components/filters/index.tsxapps/web/modules/insights/components/index.tsapps/web/modules/insights/components/routing/FailedBookingsByField.tsxapps/web/modules/insights/components/routing/RoutedToPerPeriod.tsxapps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsxapps/web/modules/insights/components/routing/RoutingFunnel.tsxapps/web/modules/insights/components/routing/RoutingFunnelContent.tsxapps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsxapps/web/modules/insights/components/routing/RoutingKPICards.tsxapps/web/modules/insights/components/routing/index.tsapps/web/modules/insights/hooks/useDefaultRoutingForm.tsapps/web/modules/insights/hooks/useInsightsBookingFacetedUniqueValues.tsapps/web/modules/insights/hooks/useInsightsBookingParameters.tsapps/web/modules/insights/hooks/useInsightsBookings.tsapps/web/modules/insights/hooks/useInsightsColumns.tsxapps/web/modules/insights/hooks/useInsightsOrgTeams.tsapps/web/modules/insights/hooks/useInsightsRoutingFacetedUniqueValues.tsapps/web/modules/insights/hooks/useInsightsRoutingParameters.tsapps/web/modules/insights/hooks/useToggleableLegend.tsapps/web/modules/insights/views/insights-call-history-view.tsxapps/web/modules/insights/views/insights-routing-view.tsxapps/web/modules/insights/views/insights-view.tsxapps/web/modules/insights/views/insights-virtual-queues-view.tsxapps/web/modules/shell/navigation/Navigation.tsxapps/web/pages/api/trpc/insights/[trpc].tspackages/features/di/containers/InsightsBooking.tspackages/features/di/containers/InsightsRouting.tspackages/features/di/modules/InsightsBooking.tspackages/features/di/modules/InsightsRouting.tspackages/features/di/tokens.tspackages/features/insights/lib/bookingStatusToText.tspackages/features/insights/lib/bookingUtils.tspackages/features/insights/lib/calculateDeltaType.tspackages/features/insights/lib/objectToCsv.tspackages/features/insights/lib/repositories/TeamRepository.tspackages/features/insights/lib/types.tspackages/features/insights/lib/valueFormatter.tspackages/features/insights/server/buildBaseWhereCondition.tspackages/features/insights/server/events.tspackages/features/insights/server/insightsDateUtils.tspackages/features/insights/server/raw-data.schema.tspackages/features/insights/server/routing-events.tspackages/features/insights/server/virtual-queues.tspackages/features/insights/services/InsightsBookingBaseService.tspackages/features/insights/services/InsightsBookingDIService.tspackages/features/insights/services/InsightsRoutingBaseService.tspackages/features/insights/services/InsightsRoutingDIService.tspackages/features/insights/services/csvDataTransformer.tspackages/trpc/react/shared.tspackages/trpc/server/routers/viewer/_router.tsxpackages/trpc/server/routers/viewer/insights/_router.tspackages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
✅ Files skipped from review due to trivial changes (2)
- packages/features/insights/lib/calculateDeltaType.ts
- apps/web/modules/insights/components/filters/Download/index.tsx
🚧 Files skipped from review as they are similar to previous changes (82)
- packages/trpc/react/shared.ts
- apps/web/modules/insights/components/booking/LeastBookedTeamMembersTable.tsx
- apps/web/modules/insights/components/booking/MostBookedTeamMembersTable.tsx
- apps/web/modules/insights/components/routing/index.ts
- apps/web/modules/insights/views/insights-routing-view.tsx
- packages/features/insights/services/InsightsBookingDIService.ts
- packages/features/di/tokens.ts
- packages/features/di/containers/InsightsBooking.ts
- apps/web/modules/insights/components/index.ts
- packages/features/insights/server/routing-events.ts
- apps/web/pages/api/trpc/insights/[trpc].ts
- packages/features/di/modules/InsightsRouting.ts
- apps/web/modules/insights/components/booking/RecentFeedbackTable.tsx
- packages/features/insights/lib/bookingStatusToText.ts
- apps/web/modules/insights/components/booking/HighestRatedMembersTable.tsx
- apps/web/modules/insights/components/UserStatsTable.tsx
- apps/web/app/(use-page-wrapper)/insights/router-position/page.tsx
- apps/web/modules/insights/components/routing/FailedBookingsByField.tsx
- packages/features/di/modules/InsightsBooking.ts
- apps/web/modules/insights/components/booking/MostCancelledBookingsTables.tsx
- packages/features/insights/lib/valueFormatter.ts
- apps/web/modules/insights/components/booking/HighestNoShowHostTable.tsx
- apps/web/modules/insights/components/BookingStatusBadge.tsx
- apps/web/modules/insights/components/booking/MostCompletedBookings.tsx
- packages/features/insights/lib/bookingUtils.ts
- packages/features/di/containers/InsightsRouting.ts
- apps/web/modules/insights/hooks/useInsightsRoutingParameters.ts
- apps/web/modules/shell/navigation/Navigation.tsx
- apps/web/modules/insights/components/BookedByCell.tsx
- apps/web/app/(use-page-wrapper)/insights/call-history/page.tsx
- apps/web/modules/insights/views/insights-virtual-queues-view.tsx
- packages/features/insights/lib/objectToCsv.ts
- apps/web/modules/insights/hooks/useInsightsOrgTeams.ts
- apps/web/modules/insights/components/booking/LeastCompletedBookings.tsx
- apps/web/modules/insights/components/filters/DateTargetSelector.tsx
- packages/trpc/server/routers/viewer/_router.tsx
- apps/web/modules/insights/components/booking/BookingsByHourChart.tsx
- apps/web/modules/insights/components/ResponseValueCell.tsx
- apps/web/modules/insights/components/routing/RoutingFunnel.tsx
- apps/web/modules/insights/hooks/useInsightsRoutingFacetedUniqueValues.ts
- apps/web/app/(use-page-wrapper)/insights/page.tsx
- apps/web/modules/insights/components/booking/TimezoneBadge.tsx
- packages/features/insights/services/InsightsRoutingBaseService.ts
- apps/web/modules/insights/components/booking/AverageEventDurationChart.tsx
- apps/web/modules/insights/hooks/useInsightsBookings.ts
- apps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsx
- apps/web/modules/insights/components/booking/CSATOverTimeChart.tsx
- apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsx
- apps/web/modules/insights/components/KPICard.tsx
- packages/features/insights/lib/repositories/TeamRepository.ts
- apps/web/modules/insights/components/context/InsightsOrgTeamsProvider.tsx
- apps/web/modules/insights/hooks/useToggleableLegend.ts
- packages/features/insights/server/raw-data.schema.ts
- apps/web/modules/insights/components/booking/NoShowHostsOverTimeChart.tsx
- packages/features/insights/services/csvDataTransformer.ts
- apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx
- packages/features/insights/server/events.ts
- apps/web/modules/insights/components/booking/index.ts
- apps/web/modules/insights/components/booking/EventTrendsChart.tsx
- apps/web/modules/insights/components/routing/RoutingFormResponsesTable.tsx
- packages/features/insights/server/buildBaseWhereCondition.ts
- apps/web/modules/insights/components/booking/BookingKPICards.tsx
- apps/web/modules/insights/components/booking/PopularEventsTable.tsx
- apps/web/modules/insights/components/BookingAtCell.tsx
- apps/web/modules/insights/views/insights-view.tsx
- apps/web/modules/insights/components/filters/Download/Download.tsx
- packages/features/insights/services/InsightsRoutingDIService.ts
- apps/web/modules/insights/hooks/useInsightsBookingFacetedUniqueValues.ts
- apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx
- apps/web/app/(use-page-wrapper)/insights/layout.tsx
- apps/web/modules/insights/components/routing/RoutedToPerPeriod.tsx
- apps/web/app/(use-page-wrapper)/insights/routing/page.tsx
- packages/features/insights/lib/types.ts
- apps/web/modules/insights/components/booking/RecentNoShowGuestsChart.tsx
- apps/web/modules/insights/components/booking/LowestRatedMembersTable.tsx
- apps/web/modules/insights/components/routing/RoutingKPICards.tsx
- apps/web/modules/insights/hooks/useInsightsColumns.tsx
- apps/web/modules/insights/hooks/useInsightsBookingParameters.ts
- apps/web/modules/insights/hooks/useDefaultRoutingForm.ts
- apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx
- packages/features/insights/services/InsightsBookingBaseService.ts
- packages/features/insights/server/insightsDateUtils.ts
- Replace Math.random() in RoutingFunnelSkeleton with deterministic heights - Replace dayjs with date-fns in useInsightsColumns createdAt cell - Add t() to useMemo deps in useInsightsColumns - Wrap UTM column headers with t() and add i18n keys to en/common.json - Replace barrel import in insights-view.tsx with direct per-component imports - Fix NaN corruption in events.ts aggregate accumulation - Fix calculatePreviousPeriodDates off-by-one and remove dayjs from InsightsBookingBaseService Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx (1)
62-62: ⚡ Quick winDrop the structural “Custom Tooltip component” comment.
This is a “what” comment and can be removed to align with the repo’s comment policy.
As per coding guidelines: “Only add code comments that explain why, not what … Never add comments that simply restate what the code does.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx` at line 62, Remove the redundant "Custom Tooltip component" comment that simply restates what the code does; locate the comment above the Tooltip-related component/function (e.g., the CustomTooltip declaration or the JSX return inside RoutingFunnelContent) and delete that single-line comment so the file only contains comments that explain why, not what.apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx (1)
18-23: ⚡ Quick winRemove JSX “what” comments that restate layout structure.
These comments describe what the markup already makes obvious. Please remove them (or keep only comments that explain non-obvious why decisions).
As per coding guidelines: “Only add code comments that explain why, not what … Never add comments that simply restate what the code does.”
Also applies to: 31-34, 40-41, 57-57, 65-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx` around lines 18 - 23, In the RoutingFunnelSkeleton component remove the redundant JSX "what" comments that merely restate the DOM (e.g., comments above the div.h-full.w-full.p-4, the div.relative.h-[220px], and the div.text-muted-foreground.absolute... plus the other noted comment locations), leaving only comments that explain non-obvious "why" decisions; search for comment text like "Chart container", "Chart area", "Y-axis" (and similar comments referenced) and delete them so only purposeful explanatory comments remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx`:
- Around line 82-93: The percentage calculation breaks when the
"totalSubmissions" series is hidden because totalSubmissions is computed by
searching the tooltip payload array (which omits hidden series) — update
RoutingFunnelContent to read the canonical total submissions from the full row
in the tooltip entry (use payload[0].payload.totalSubmissions || 0) instead of
payload.find(...). This ensures totalSubmissions uses the underlying record for
that date when computing percentages for successfulRoutings and
acceptedBookings.
In `@apps/web/modules/insights/views/insights-view.tsx`:
- Line 148: The mailto href in the InsightsView component currently hardcodes
English subject/body; replace that hardcoded string with localized values (use
the project's translation hook/function used elsewhere in insights-view.tsx) and
construct the mailto using those translated subject/body strings. Add the new
translation keys (e.g., "insights.contactEmailSubject" and
"insights.contactEmailBody") to packages/i18n/locales/en/common.json and use
those keys in the component when building the href instead of the hardcoded
text.
---
Nitpick comments:
In `@apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx`:
- Line 62: Remove the redundant "Custom Tooltip component" comment that simply
restates what the code does; locate the comment above the Tooltip-related
component/function (e.g., the CustomTooltip declaration or the JSX return inside
RoutingFunnelContent) and delete that single-line comment so the file only
contains comments that explain why, not what.
In `@apps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsx`:
- Around line 18-23: In the RoutingFunnelSkeleton component remove the redundant
JSX "what" comments that merely restate the DOM (e.g., comments above the
div.h-full.w-full.p-4, the div.relative.h-[220px], and the
div.text-muted-foreground.absolute... plus the other noted comment locations),
leaving only comments that explain non-obvious "why" decisions; search for
comment text like "Chart container", "Chart area", "Y-axis" (and similar
comments referenced) and delete them so only purposeful explanatory comments
remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b80fafa-acd8-4d27-8dee-bb5dc56a8cab
📒 Files selected for processing (7)
apps/web/modules/insights/components/routing/RoutingFunnelContent.tsxapps/web/modules/insights/components/routing/RoutingFunnelSkeleton.tsxapps/web/modules/insights/hooks/useInsightsColumns.tsxapps/web/modules/insights/views/insights-view.tsxpackages/features/insights/server/events.tspackages/features/insights/services/InsightsBookingBaseService.tspackages/i18n/locales/en/common.json
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/locales/en/common.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/features/insights/server/events.ts
- apps/web/modules/insights/hooks/useInsightsColumns.tsx
- packages/features/insights/services/InsightsBookingBaseService.ts
- Add pointer-events-none to gradient overlay in CellWithOverflowX - Translate aria-label in ChartCard Legend using t() with toggle_series key Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix tooltip percentage calculation: read totalSubmissions from raw data point instead of visible payload to avoid broken percentages when the totalSubmissions series is toggled off - Translate mailto subject/body in insights-view using t() + encodeURIComponent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
- Add DataTableProvider context + useDataTable/useColumnFilters/useSegments hooks - Add FilterCheckboxField/FilterCheckboxFieldsContainer stubs (TeamsFilter) - Inline RoutingFormFieldType enum (routing-forms package not in fork) - Simplify virtual-queues view to empty state (no TestForm dependency) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Export `legend` alias from RoutingFunnelContent (was LEGEND_KEYS, consumed as legend) - Add useFilterValue hook stub for data-table EE replacement - Re-export hooks (useDataTable, useColumnFilters, useFilterValue, useSegments) from data-table index - Export lib/dateRange from data-table index (getDefaultStartDate/End needed by routing params hook) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TypeScript build was failing because "insights" was not a member of the AppFlags union type used by checkIfFeatureIsEnabledGlobally(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add bookingUserId/Name/Email/AvatarUrl to InsightsRoutingTableItem so
they're properly typed instead of falling through to [key:string]:unknown
- Fix getRoutedToPerPeriodData stub shape to match RoutedToPerPeriod component
(needs { users: { data, total }, periodStats: { data } })
- Fix getRoutingFormPaginatedResponsesForDownload stub to return { data, total }
instead of { data, headers } as expected by RoutingFormResponsesDownload
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add displayLabel support to ChartCard Legend component - Add missing UNAUTHORIZED check in userBelongsToTeam procedure - Minor UI cleanups in booking chart components Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add rel="noopener noreferrer" to external link in UpgradeTipWrapper - Remove unused dayjs import in Download components, use native Date instead Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/modules/insights/components/filters/Download/Download.tsx (1)
58-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when pagination ends before reaching
totalRecords.Line 60 currently breaks on empty pages and still proceeds to a 100% success toast and CSV export, which can silently produce partial data. Treat this as an error when
allData.length < totalRecords.💡 Suggested fix
while (allData.length < totalRecords) { const result = await fetchBatch(offset); - if (result.data.length === 0) break; + if (result.data.length === 0) { + throw new Error("Pagination ended early before all records were fetched."); + } allData = [...allData, ...result.data]; offset += BATCH_SIZE; @@ showProgressToast(100); + if (allData.length < totalRecords) { + throw new Error("Download incomplete: fetched fewer records than expected."); + } const toDateStr = (d: string) => new Date(d).toISOString().slice(0, 10);Also applies to: 68-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/modules/insights/components/filters/Download/Download.tsx` around lines 58 - 61, The loop that paginates using fetchBatch (see the while loop using allData, offset, and totalRecords) currently breaks on an empty result and continues to show a 100% success toast and export a CSV even if allData.length < totalRecords; change the behavior so that when fetchBatch returns an empty page before allData.length >= totalRecords you treat it as a failure: stop pagination, surface an error (throw or return a rejected promise / set an error state), log/report the short-fetch condition, and ensure the success toast and CSV export logic (the downstream code that runs after the loop) are skipped and a failure toast is shown instead; apply the same change to the other pagination block that repeats the pattern (lines around the second loop using fetchBatch and allData).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/modules/insights/components/filters/OrgTeamsFilter.tsx`:
- Around line 45-46: The code in OrgTeamsFilter.tsx relies on data[0] for org
detection and avatar selection (used when building the "all" option and later
around the orgTeamsType branches), which breaks if the array order changes;
replace direct index access with a lookup that finds the correct org entry from
data (e.g., data.find(...)) based on a stable identifier or predicate (the
property that identifies the "all" org or current org) and use that found entry
to set imageUrl and to decide visibility for the "all" option; update any
occurrences where data[0] is used (refer to the orgTeamsType variable and the
imageUrl assignment in the return object) so they use the foundOrg variable
instead.
- Around line 76-77: The avatar alt text in OrgTeamsFilter.tsx is hardcoded and
can produce "undefined logo"; update the alt generation to use the i18n
translation helper (e.g., t) and a safe fallback instead of concatenating raw
placeholder. Specifically, in the OrgTeamsFilter component where alt uses
`${placeholder} logo`, replace with a localized string (use a key like
common.teamAvatar or common.avatarLabel with optional interpolation for the
placeholder) and ensure you fall back to a generic translated label when
placeholder is falsy; apply the same change for the other occurrence around
getPlaceholderAvatar/imageSrc. Add the corresponding keys and English text to
packages/i18n/locales/en/common.json (e.g., "teamAvatar": "{{name}} logo" and
"avatar": "logo" or similar).
---
Duplicate comments:
In `@apps/web/modules/insights/components/filters/Download/Download.tsx`:
- Around line 58-61: The loop that paginates using fetchBatch (see the while
loop using allData, offset, and totalRecords) currently breaks on an empty
result and continues to show a 100% success toast and export a CSV even if
allData.length < totalRecords; change the behavior so that when fetchBatch
returns an empty page before allData.length >= totalRecords you treat it as a
failure: stop pagination, surface an error (throw or return a rejected promise /
set an error state), log/report the short-fetch condition, and ensure the
success toast and CSV export logic (the downstream code that runs after the
loop) are skipped and a failure toast is shown instead; apply the same change to
the other pagination block that repeats the pattern (lines around the second
loop using fetchBatch and allData).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30515ff1-9738-4e11-a378-9f4b734affca
📒 Files selected for processing (26)
apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsxapps/web/modules/insights/components/ChartCard.tsxapps/web/modules/insights/components/booking/BookingKPICards.tsxapps/web/modules/insights/components/booking/BookingsByHourChart.tsxapps/web/modules/insights/components/booking/CSATOverTimeChart.tsxapps/web/modules/insights/components/booking/EventTrendsChart.tsxapps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsxapps/web/modules/insights/components/filters/Download/Download.tsxapps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsxapps/web/modules/insights/components/filters/OrgTeamsFilter.tsxapps/web/modules/insights/components/routing/RoutingFunnelContent.tsxapps/web/modules/insights/hooks/useDefaultRoutingForm.tsapps/web/modules/insights/hooks/useInsightsColumns.tsxapps/web/modules/insights/views/insights-virtual-queues-view.tsxpackages/features/data-table/DataTableProvider.tsxpackages/features/data-table/hooks/useColumnFilters.tspackages/features/data-table/hooks/useDataTable.tspackages/features/data-table/hooks/useFilterValue.tspackages/features/data-table/hooks/useSegments.tspackages/features/data-table/index.tspackages/features/filters/components/TeamsFilter.tsxpackages/features/flags/config.tspackages/features/insights/server/routing-events.tspackages/features/insights/services/InsightsRoutingBaseService.tspackages/i18n/locales/en/common.jsonpackages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
✅ Files skipped from review due to trivial changes (3)
- packages/features/data-table/hooks/useSegments.ts
- packages/features/data-table/hooks/useColumnFilters.ts
- packages/features/data-table/index.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/features/insights/server/routing-events.ts
- packages/i18n/locales/en/common.json
- apps/web/app/(use-page-wrapper)/insights/UpgradeTipWrapper.tsx
- apps/web/modules/insights/hooks/useDefaultRoutingForm.ts
- apps/web/modules/insights/components/booking/BookingsByHourChart.tsx
- apps/web/modules/insights/components/routing/RoutingFunnelContent.tsx
- packages/features/insights/services/InsightsRoutingBaseService.ts
- packages/trpc/server/routers/viewer/insights/procedures/userBelongsToTeam.ts
- apps/web/modules/insights/components/ChartCard.tsx
- apps/web/modules/insights/components/booking/RecentFeedbackTableContent.tsx
- apps/web/modules/insights/components/booking/CSATOverTimeChart.tsx
- apps/web/modules/insights/hooks/useInsightsColumns.tsx
- apps/web/modules/insights/components/filters/Download/RoutingFormResponsesDownload.tsx
- OrgTeamsFilter: find org entry by id instead of assuming data[0] is always the org - OrgTeamsFilter: localize avatar alt text, avoid "undefined logo" output Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Author
|
Closing to reorganize: the branch accidentally includes commits for Organizations, SAML, and auth that belong in separate PRs. Will reopen a clean insights-only PR shortly. |
Three root causes:
1. `getRoutingFormHeaders` returned untyped `[]` → `HeaderRow = never` → cascade
Fix: explicit `Promise<RoutingFormHeaderRow[]>` return type on stub
2. `InsightsRoutingTableItem` missing explicit fields (bookingAssignmentReason,
utm_*, createdAt, fields) → index signature `[key]:unknown` hit → `unknown`
Fix: add all accessed properties with proper types
3. `useColumnFilters` in packages/features accepted no args, but routing hooks
call it with `{ exclude: [...] }`
Fix: add optional `exclude` parameter with filter logic
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Closing — will use locally only. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Ports the Insights feature from upstream cal.com to cal.diy.
The upstream Insights module depends on EE-only packages and routing-forms internals absent from this MIT fork. This PR replaces them with:
What's included
Booking analytics (fully functional)
Routing analytics (stub)
Security notes
How should this be tested?
Env vars needed: standard cal.diy setup only (`DATABASE_URL`, `NEXTAUTH_SECRET`, `CALENDSO_ENCRYPTION_KEY`)
Mandatory Tasks