perf: caching + modularize data fetching for team booking pages #22172
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
|
✅ No security or compliance issues detected. Reviewed everything up to de6966b. Security Overview
Detected Code Changes
Reply to this PR with |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/01/25)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
cubic found 5 issues across 9 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx
Outdated
Show resolved
Hide resolved
apps/web/lib/org/[orgSlug]/[user]/[type]/getServerSideProps.tsx
Outdated
Show resolved
Hide resolved
| teamSlug: string; | ||
| orgSlug: string | null; | ||
| }) { | ||
| revalidateTag(`team:${teamSlug}${orgSlug ? `:${orgSlug}` : ""}`); |
There was a problem hiding this comment.
This feels a little odd to me to have the team first and then the org since teams are nested inside of orgs
| meetingSlug: string; | ||
| orgSlug: string | null; | ||
| }) { | ||
| revalidateTag(`event-type:${teamSlug}:${meetingSlug}${orgSlug ? `:${orgSlug}` : ""}`); |
| }); | ||
| } | ||
|
|
||
| async function _getTeamEventType(teamSlug: string, meetingSlug: string, orgSlug: string | null) { |
There was a problem hiding this comment.
We should move these to repositories. Prisma queries shouldn't be directly in the web app like this
There was a problem hiding this comment.
I will move it to packages/features/! We use some helpers from packages/features inside so we can't use repositories in packages/lib.
There was a problem hiding this comment.
Moved
getTeamEventTypetopackages/features/eventtypes/lib/getTeamEventType.tsgetTeamDatatopackages/features/ee/teams/lib/getTeamData.ts.
Let me know if that's wrong!
WalkthroughThis update introduces a comprehensive caching mechanism for team booking pages and event types within a Next.js application. New server-side modules provide functions to fetch, cache, and revalidate team and event type data using Next.js cache APIs, with cache tags constructed from team and organization slugs. Feature flag support is added for conditionally enabling the cached booking page, with fallback to legacy rendering. Several React components and server modules are updated or added to utilize cached data and ensure cache consistency after mutations. Supporting utilities, type exports, and constants for cache TTL are also introduced. Cache revalidation is triggered in relevant mutation success handlers to maintain up-to-date data. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/pageWithCachedData.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/features/ee/organizations/pages/settings/other-team-profile-view.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/platform/atoms/booker/BookerWebWrapper.tsx (1)
37-48: Stubbed query object is incomplete – potential runtime breakageWhen
eventDatais provided you manually craft an object that only containsdata,isSuccess,isError, andisPending.
BookerComponent(and other hooks) may depend on additional fields returned byuseEventsuch asrefetch,isFetching, etc. Passing an object that doesn’t fully satisfy that shape can lead to undefined-access errors.-const event = props.eventData - ? { - data: props.eventData, - isSuccess: true, - isError: false, - isPending: false, - } - : clientFetchedEvent; +const event = props.eventData + ? { + ...clientFetchedEvent, // preserves refetch, status flags, etc. + data: props.eventData, + isSuccess: true, + isError: false, + isPending: false, + } + : clientFetchedEvent;Alternatively, keep
useEventenabled and passinitialDatato it; that guarantees the returned object is 100 % compatible with the hook’s contract.
♻️ Duplicate comments (10)
packages/lib/constants.ts (1)
231-231: Expose TTL via env-var to avoid hard-coding
Same feedback as in the previous review: consider making the value configurable to accommodate different cache horizons across environments.-export const NEXTJS_CACHE_TTL = 3600; // 1 hour +export const NEXTJS_CACHE_TTL = parseInt(process.env.NEXTJS_CACHE_TTL ?? "3600", 10); // 1 hour defaultpackages/features/ee/teams/pages/team-profile-view.tsx (1)
45-45: Client component importing server-only module will cause build failure.This violates Next.js architecture - client components cannot import modules that use "use server". Move the cache revalidation behind an API route/server action or perform it in a wrapper server component.
packages/features/ee/teams/pages/team-appearance-view.tsx (1)
21-21: Client component importing server-only module will cause build failure.This violates Next.js architecture - client components cannot import modules that use "use server". Move the cache revalidation behind an API route/server action or perform it in a wrapper server component.
packages/features/ee/teams/pages/team-settings-view.tsx (1)
24-24: Client component importing server-only module will cause build failure.This violates Next.js architecture - client components cannot import modules that use "use server". Move the cache revalidation behind an API route/server action or perform it in a wrapper server component.
packages/features/ee/organizations/pages/settings/other-team-profile-view.tsx (1)
34-34: Client component importing server-only module will cause build failure.This violates Next.js architecture - client components cannot import modules that use "use server". Move the cache revalidation behind an API route/server action or perform it in a wrapper server component.
packages/features/ee/teams/components/RoundRobinSettings.tsx (1)
14-14: Client component importing server-only module will cause build failure.This violates Next.js architecture - client components cannot import modules that use "use server". Move the cache revalidation behind an API route/server action or perform it in a wrapper server component.
packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx (3)
23-23: Server action import in client component will cause build errors.This import brings a server-only function into a client component, which will fail at build time with "Server-only module cannot be imported from a Client Component" error.
The cache invalidation should be handled server-side, either by:
- Moving the mutation logic to a server action
- Calling the cache invalidation through an API route
- Using the mutation's
onSettledto trigger a separate server action call-import { revalidateTeamEventTypeCache } from "@calcom/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions";
142-150: Move cache invalidation to server-side logic.The cache invalidation call in the client component will fail due to server/client boundary violations.
324-332: Move cache invalidation to server-side logic.Same issue as in the update handler - this client-side cache invalidation call will fail.
packages/features/eventtypes/lib/getPublicEvent.ts (1)
565-853: Remove unusedgetPublicEventRefactoredfunctionThis function is never exported or referenced anywhere in the codebase, creating unnecessary dead code that increases bundle size and maintenance overhead.
🧹 Nitpick comments (4)
packages/platform/atoms/booker/BookerWebWrapper.tsx (1)
29-33: Nit: clarify prop name
eventDatarepresents pre-fetched event information. Using a more explicit name such asinitialEventDatawould communicate intent and avoid confusion with data that might change at runtime.packages/features/ee/teams/components/RoundRobinSettings.tsx (1)
43-43: Fix typo in comment."Rounb robin" should be "Round robin".
- // Rounb robin reset interval / basis governs host selection logic on the booking page. + // Round robin reset interval / basis governs host selection logic on the booking page.apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/pageWithCachedData.tsx (1)
80-81: Consider returning default metadata instead of empty objectWhen
teamDataorenrichedEventTypeis missing, returning an empty object might hurt SEO. Consider returning a default metadata object with appropriate error messaging or 404 status.Example improvement:
- if (!teamData) return {}; // should never happen + if (!teamData) return { + title: "Team Not Found", + description: "The requested team could not be found.", + robots: { index: false, follow: false } + };Also applies to: 88-89
packages/features/eventtypes/lib/getPublicEvent.ts (1)
246-249: Consider clarifying the hosts return structureThe function returns both
subsetOfHosts(always populated) andhosts(conditionally populated based onfetchAllUsers). This dual structure might be confusing. Consider adding a comment explaining why both are needed for backward compatibility.Add clarifying comment:
return { + // subsetOfHosts: Always returns enriched hosts (limited to 3 unless fetchAllUsers is true) subsetOfHosts: enrichedHosts, + // hosts: Only populated when fetchAllUsers is true, for backward compatibility hosts: fetchAllUsers ? enrichedHosts : undefined, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts(1 hunks)apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/page.tsx(4 hunks)apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/pageWithCachedData.tsx(1 hunks)apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/queries.ts(1 hunks)apps/web/modules/team/type-view-cached.tsx(1 hunks)packages/features/ee/organizations/pages/settings/other-team-profile-view.tsx(2 hunks)packages/features/ee/teams/components/RoundRobinSettings.tsx(2 hunks)packages/features/ee/teams/lib/getTeamData.ts(1 hunks)packages/features/ee/teams/pages/team-appearance-view.tsx(2 hunks)packages/features/ee/teams/pages/team-profile-view.tsx(2 hunks)packages/features/ee/teams/pages/team-settings-view.tsx(2 hunks)packages/features/eventtypes/lib/getPublicEvent.ts(6 hunks)packages/features/eventtypes/lib/getTeamEventType.ts(1 hunks)packages/features/flags/config.ts(1 hunks)packages/features/flags/hooks/index.ts(1 hunks)packages/lib/constants.ts(1 hunks)packages/platform/atoms/booker/BookerWebWrapper.tsx(1 hunks)packages/platform/atoms/booker/index.ts(1 hunks)packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (8)
packages/features/flags/hooks/index.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/platform/atoms/booker/index.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/platform/atoms/booker/BookerWebWrapper.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
apps/web/modules/team/type-view-cached.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/page.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/pageWithCachedData.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/queries.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/features/eventtypes/lib/getPublicEvent.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
🧬 Code Graph Analysis (9)
packages/features/ee/teams/pages/team-profile-view.tsx (1)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
revalidateTeamDataCache(6-14)
packages/features/ee/teams/pages/team-settings-view.tsx (1)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
revalidateTeamDataCache(6-14)
packages/platform/atoms/booker/BookerWebWrapper.tsx (1)
packages/features/bookings/Booker/types.ts (1)
BookerProps(19-107)
packages/features/ee/organizations/pages/settings/other-team-profile-view.tsx (1)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
revalidateTeamDataCache(6-14)
packages/features/ee/teams/pages/team-appearance-view.tsx (1)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
revalidateTeamDataCache(6-14)
packages/features/ee/teams/components/RoundRobinSettings.tsx (1)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
revalidateTeamDataCache(6-14)
packages/platform/atoms/event-types/wrappers/EventTypeWebWrapper.tsx (1)
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
revalidateTeamEventTypeCache(17-27)
packages/features/ee/teams/lib/getTeamData.ts (1)
packages/features/ee/organizations/lib/orgDomains.ts (1)
getSlugOrRequestedSlug(160-173)
packages/features/eventtypes/lib/getTeamEventType.ts (2)
packages/features/ee/organizations/lib/orgDomains.ts (1)
getSlugOrRequestedSlug(160-173)packages/features/eventtypes/lib/getPublicEvent.ts (1)
getPublicEventSelect(53-156)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Tests / E2E Embed Core
- GitHub Check: Tests / E2E App Store
- GitHub Check: Tests / E2E Embed React
- GitHub Check: Tests / E2E API v2
- GitHub Check: Analyze Build / analyze
- GitHub Check: Tests / E2E (3/4)
- GitHub Check: Tests / E2E (4/4)
- GitHub Check: Tests / E2E (2/4)
- GitHub Check: Tests / Integration
- GitHub Check: Tests / E2E (1/4)
- GitHub Check: Security Check
🔇 Additional comments (15)
packages/features/flags/config.ts (1)
26-26: Flag addition looks goodThe new
"team-booking-page-cache"key is consistent with existing boolean flags and will automatically be covered byTeamFeatures. No further action needed.packages/features/flags/hooks/index.ts (1)
25-26: Default flag wired up correctly
initialDatanow includes"team-booking-page-cache", keeping the runtime flag object in sync with the type definition.packages/platform/atoms/booker/index.ts (1)
2-2: Nice quality-of-life re-exportRe-exporting the props type from the package root simplifies imports for downstream consumers.
apps/web/modules/team/type-view-cached.tsx (1)
1-19: Clean and well-structured component implementation.The component follows React best practices with proper type safety, error boundary usage, and clean prop spreading. The type definition correctly makes
eventDatarequired while maintaining flexibility for other props.packages/features/eventtypes/lib/getTeamEventType.ts (1)
6-22: Well-implemented database query with comprehensive field selection.The function correctly handles flexible slug matching using
getSlugOrRequestedSlugand includes proper filtering for both team and organization hierarchies. The use ofgetPublicEventSelect(false)ensures all necessary public event fields are included for the booking page.packages/features/ee/teams/lib/getTeamData.ts (1)
15-47: Comprehensive team data selection with proper nesting.The select statement correctly includes all necessary team and organization fields, with proper nested selection for parent organization data and settings.
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/actions.ts (1)
1-27: Well-implemented server actions for cache invalidation.The functions properly use Next.js
revalidateTagAPI with consistent cache tag construction. The parameter structure is clean and the server action directives are correctly applied.apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/page.tsx (1)
19-25: Feature flag implementation looks good!The conditional rendering based on the global feature flag and search parameter is implemented correctly. The async handling of
searchParamsis proper, and the delegation to cached or legacy implementations is clean.Also applies to: 27-30, 69-72
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/pageWithCachedData.tsx (2)
44-64: Excellent input validation and error handling!Good use of Zod schema for parameter validation with slug normalization. The early return with
notFound()for invalid params is appropriate.
125-216: Well-structured server component with comprehensive error handling!The component properly handles:
- Organization redirects
- Data validation with 404 responses
- Reschedule logic with appropriate status checks
- Internationalization support
- Multiple duration configuration
The implementation is thorough and follows best practices.
apps/web/app/(booking-page-wrapper)/team/[slug]/[type]/queries.ts (3)
21-26: Proper caching implementation with well-structured cache tags!The cache tag format
team:${orgSlug}:${teamSlug}enables granular cache invalidation. Good use of the centralized TTL constant.
50-53: Efficient parallel data fetching!Good use of
Promise.allto fetch team and event type data concurrently, reducing latency.
132-148: Good fallback mechanism for CRM data!The dynamic import fallback ensures CRM data is properly resolved even when not provided in query parameters. This maintains backward compatibility.
packages/features/eventtypes/lib/getPublicEvent.ts (2)
53-156: Good modularization of the Prisma select query!Exporting the select object enables reuse across different queries and maintains consistency in the data structure.
994-1028: Excellent extraction of shared event data processing!This function successfully consolidates common event data transformations, improving code reusability and maintainability.
| orderBy: { | ||
| slug: { sort: "asc", nulls: "last" }, | ||
| }, |
There was a problem hiding this comment.
Fix orderBy syntax for Prisma.
The orderBy syntax is incorrect. Prisma doesn't support the { sort: "asc", nulls: "last" } format.
Use the correct Prisma syntax:
- orderBy: {
- slug: { sort: "asc", nulls: "last" },
- },
+ orderBy: {
+ slug: "asc",
+ },If you need null handling, use:
orderBy: [
{ slug: { sort: "asc", nulls: "last" } }
]🤖 Prompt for AI Agents
In packages/features/ee/teams/lib/getTeamData.ts around lines 12 to 14, the
orderBy syntax is incorrect for Prisma as it does not support the object format
with sort and nulls directly. To fix this, change the orderBy property to be an
array containing an object with slug and its sorting options, like orderBy: [{
slug: { sort: "asc", nulls: "last" } }], ensuring proper Prisma syntax and null
handling.
What does this PR do?
This PR does 3 things to team booking pages:
(1) Replace client-side event-data fetching (/api/trpc/public/event -> this uses getPublicEvent under the hood) with server-side fetching
(2) Cache this ^server-side fetching and invalidate it when event-type is updated
(3) Cache the team data fetching (was already a server-side fetch)
Benefit of this is obviously (1) offloading pressures on our server by defending spamming towards /api/trpc/public/event and team data fetching (2) faster page initial load
Code-wise, This PR
pageWithCachedData.tsxRSC alongsidepage.tsxRSC. We renderpageWithCachedDataif global feature flag forteam-booking-page-cacheis enabled &&experimentalTeamBookingPageCachesearch param is"true". Otherwise, we renderpage.tsxas usual.pageWithCachedDatais stable, we can get rid ofpage.tsx,getServerSideProps.tsxandtype-view.tsxentirely.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Manual tests
/api/trpc/public/eventis no longer used461833540-19fb3788-c34a-406c-93c0-00a74124ec75.mov
revalidateTeamBookingPage, you will see that cache invalidation doesn't work (this confirms that (1) page uses cached data (2) invalidation logic works)Automated tests
trueinisCachedTeamBookingEnabledand waited for e2e tests)